-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add AAQ v2 faq search endpoint #613
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments.
The commits and PR could be better named, ie "Add AAQ v2 faq search endpoint"
aaq/views.py
Outdated
query_metadata = serializer.validated_data["query_metadata"] | ||
url = urllib.parse.urljoin(settings.AAQ_V2_API_URL, "search") | ||
payload = { | ||
"query_text": f"{query_text}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These probably don't have to be f-strings
|
||
response = requests.request("POST", url, json=payload, headers=headers) | ||
|
||
json_msg = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like its just recreating the dict exactly how we got it from AAQ and returning it?
Shouldn't we build a message here like AAQ v1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Hlamallama Have have you seen this feedback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DevChima yes, i've implemeted the feedback from Eric
aaq/views.py
Outdated
"state": response.json()["state"], | ||
} | ||
|
||
return_data = json_msg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extra variable isn't really required
No description provided.