-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
feat: adds search query for /keys #263
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #263 +/- ##
==========================================
- Coverage 95.06% 94.61% -0.45%
==========================================
Files 5 5
Lines 324 390 +66
==========================================
+ Hits 308 369 +61
- Misses 16 21 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
At least you should also update tests in test_main.py: I think you can create a new one. |
folksonomy/api.py
Outdated
""" | ||
query_params = [owner] | ||
|
||
if q: |
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.
Can't you make it more readable like, this for example:
folksonomy_api/folksonomy/api.py
Line 355 in c3bc052
{f"AND k IN ({placeholders})" if keys_list else ""} |
Having the all the query in one block can be easier to read and debug I guess.
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.
I know I have also done the same as you here:
folksonomy_api/folksonomy/api.py
Line 673 in c3bc052
if q: |
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.
made it more readable and added tests too
folksonomy/api.py
Outdated
'values',count(distinct(v)) | ||
) as j | ||
'k', k, | ||
'count', count(*), |
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.
'count', count(*), | |
'count', COUNT(*), |
folksonomy/api.py
Outdated
) as j | ||
'k', k, | ||
'count', count(*), | ||
'values', count(distinct v) |
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.
'values', count(distinct v) | |
'values', COUNT(distinct v) |
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.
made the change
What
Adds search query
/?q=col
for searching is/keys
endpoint.Screenshot
Fixes bug(s)
#19