ethanheyrman / restaurant-roulette

0 stars 1 forks source link

Django filter logic #35

Closed ethanheyrman closed 4 years ago

ethanheyrman commented 4 years ago

Adds a Django view and associated endpoint accessible at /restaurant/filtered/. In order to reach the endpoint, supply query parameters, prepended with a question mark.

i.e. localhost:8000/restaurant/filtered/?category=French&price=3

This view can definitely be more intelligent and should be improved in coming iterations. Currently, however, it returns a list of length 5 containing filtered restaurants and potentially filler restaurants if iterative filtering resulted in fewer than 5 restaurants.

Several other QOL issues were addressed in this PR as well. Should be comprehensive enough to close #10.

ethanheyrman commented 4 years ago

Also, pinging @NiharikaTomar and @jkuzminski7 if y'all wanna see the PR for the filtered restaurants page

BijanT commented 4 years ago

It looks like the gitignore was deleted here

BijanT commented 4 years ago

The following query gives me an error when using Shane's input set for the DB: http://localhost:8000/restaurant/filtered/?category=American&price=1

supple2 commented 4 years ago

The following query gives me an error when using Shane's input set for the DB: http://localhost:8000/restaurant/filtered/?category=American&price=1

I think the reason for this is that there is no category called American, there is American (New) and American (Traditional). Also, there are sometimes multiple categories that a restaurant falls under, and those are separated by a ";" character

BijanT commented 4 years ago

I thought that at first too, but it works fine when the price is equal to 2, 3, or 4

BijanT commented 4 years ago

Also, how are we supposed to query for multiple users' preferences?

supple2 commented 4 years ago

Hmm, so when you use a price of 4, it should return nothing correct? Since there are American restaurants in the list with 2 and 3 for price but none with 1 or 4

BijanT commented 4 years ago

Sorry, I was wrong. When price is 4, it also crashes with the same error.

BijanT commented 4 years ago

I should probably actually include the error message:

TypeError at /restaurant/filtered/ Object of type Q is not JSON serializable

ethanheyrman commented 4 years ago

also to address a few comments, there isn't really a universally accepted way to construct URL query strings when multiple values pertain to the same query argument. the way that I have decided to do this is by capturing the list of parameters from the URL. for the sake of example, if two people only enter price preferences in the form, the URL to the server would take the form localhost:8000/restaurant/filtered/?price=2&price=3, and the price list is parsed out such that price = [ '2', '3' ].

also, we should also have a broader discussion in iteration 2 on refining filters for conflicting preferences. this filter, for example, immediately creates a set of size 0 because a price of 2 and a price of 3 have nothing in common. just to get the ball rolling we could clarify the price filter to serve as max price. django's filtering allows more complex(?) clauses such as less than or equal to

jkuzminski7 commented 4 years ago

So are Niha and I able to use this to query the database yet? Or does it need to get merged before we can use it?

ethanheyrman commented 4 years ago

if this passes review by both Shane and Bijan, i'll accept the pull request and then you should be able to make future branches off of develop

jkuzminski7 commented 4 years ago

Ok. So just to be clear we can't do the api work for filter until this gets merged correct?

ethanheyrman commented 4 years ago

technically if you want the most seamless means of integrating these changes into your feature branch, it would be easiest to wait until this is accepted. to answer your question, you cannot use this api from develop until this branch gets merged into it. if you're feeling ambitious, however, you could create a feature branch off of develop (or use an existing feature branch and pull the most recent develop into it) then pull this branch into the feature branch. its up to you really