Search fixes#3193
Conversation
|
|
||
| DEBUG = True | ||
|
|
||
| DATABASES = { |
There was a problem hiding this comment.
since this feields are exact replica, could you create it as a variable then reuse it
| def get(self, request: Request): | ||
| search = request.GET.get("search") | ||
| limit = int(request.GET.get("limit", 0)) | ||
| fields = request.GET.get("fields", ["title", "slug"]) |
There was a problem hiding this comment.
my understanding is that fields would be returned as a string. i would recommend not setting the default as a list but rather splitting the .get value before then doing an if check.
atm it seems like we're trying to handle str and list
| search = request.GET.get("search") | ||
| limit = int(request.GET.get("limit", 0)) | ||
| fields = request.GET.get("fields", ["title", "slug"]) | ||
| meta = request.GET.get("meta", ["id"]) |
| # for i in range(2): | ||
| # TopicPageFactory.create( | ||
| # path=f"topic-rare-{i}", | ||
| # depth=1, |
There was a problem hiding this comment.
we can remove this if not needed
| # Given | ||
| search = "rare" # All topics have a title, only even ones have rare in it | ||
| query = f"search={search}" | ||
| expected_results = [t for t in topics if "rare" in t.title] |
There was a problem hiding this comment.
would it be best to use Django's __icontains filter here instead of a forloop?
| @@ -0,0 +1,34 @@ | |||
| POST http://localhost:8000/api/charts/v3 | |||
| Content-Type: application/json | |||
|
|
|||
There was a problem hiding this comment.
What are the purpose of the .http files?
| meta = request.GET.get("meta", ["id"]) | ||
| topic_results = TopicPage.objects.all().search(search) | ||
| if not limit or topic_results.count() < limit: | ||
| # TODO: go get more |
There was a problem hiding this comment.
we may not need the extra count() db query for every search. i think you can check if limit is set then do the limit
9591546 to
f71e309
Compare
f71e309 to
ef6d3e2
Compare
|


Fixes #CDD-<ISSUE_ID>
Type of change
Please select the options that are relevant.
Checklist: