-
Notifications
You must be signed in to change notification settings - Fork 63
Conversation
Noting here that I've marked this PR as low priority because without the deployment of API with the category fix, this would not fix anything, as the singular |
Should we mark it back to low? |
Honestly, I marked it as critical to highlight the dependency on the API and push the fix there 😆 Wouldn't it be better to use the singular version that we know is not deprecated? or is the intention to keep this open until we deprecate the plural field? 🤔 |
Great, I understand that it's actually a close to critical issue across the stack. The way the currently deployed API works, using singular After the category fix in the API is deployed (I've merged it just now), both singular and plural versions will work, so then this fix can be applied and deployed. When we finally change the version from |
That makes sense. BTW, everything looks good here, I reviewed using the local API with the fix but was almost impossible to find a term with mixed categories results so that could make time to try updating the API sample data too. |
@obulat is this blocked until the API is deployed? |
@zackkrida , yes, it's waiting for the API deployment. |
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 should be good to go now that WordPress/openverse-api#583 has landed 🚀
1d17f21
to
7f90f1f
Compare
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.
LGTM!
Fixes
Fixes #1169 by @krysal
Description
This PR replaces plural API parameter
categories
with singularcategory
, and updates some documentation.This probably shouldn't be merged until the API fix is deployed because it will break the image category filtering.
Testing Instructions
If you run this as is, filtering by category will return un-filtered result because the production API discards filters that don't exist. If you run it with API from WordPress/openverse-api#583 running locally, you should get filtered results for audio and images.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin