Replace plural categories as field name with singular category

[obulat]

Fixes

Fixes @openverse-frontend#1169

Description

Note: this does not really use category when filtering the search result, hence Draft status.
But setting category to singular is correct because all other parameters use singular words.
category field was using plural and singular forms interchangeably in the codebase. This caused errors when filtering by category:

• for audio it always returned 0 results
• for images, it seems to have returned correctly filtered results, but with category field sent to the front end being set to null.

To ensure that this change doesn't break anyone's code that was using categories as an image filter, I added it to the list of deprecated parameters. I removed it from the deprecated parameters, and added a duplicate filter to the search controller to make sure that both parameters work

Testing Instructions

Run just up, just init, and try searching for something, filtering it by category.
http://localhost:8000/v1/audio/?q=look%20back&category=music should return 1 result

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[sarayourfriend]

This would require a major version change in it's current state.

If we don't want to create a new API version and we want to have category as singular then we need to support both category and categories.

[zackkrida]

I think I'd prefer to support both, with a note that in the next major version one will be removed. Ideally by that time we could email our API consumers or something like that, too. Edit: Forgot the entire point of version numbers for a second 🤦

[sarayourfriend]

I think I'd prefer to support both, with a note that in the next major version one will be removed. Ideally by that time we could email our API consumers or something like that, too.

I would be included in the release notes of the differences between v1 and v2, no?

[obulat]

I didn't understand what deprecated parameter means, to be honest. I thought it works like the deprecation notices in some python packages: when something is deprecated, it still works, but displays a warning. Apparently, it doesn't work at all now.

[krysal]

Works great! The category filter returns exactly the same count from the database 💯

http://localhost:8000/v1/audio/?q=&category=music

{
	"result_count": 180,
	...
}

SELECT COUNT(id) FROM audio WHERE category='music';

count
180

We might need to update the test data for audio, it's difficult to get results for anything without looking at the DB because the majority of titles are in German 😅 and there are only two categories: music and pronunciation.

I added a small test for this in #584, let me know if you want to merge it here. Thanks for finding and addressing this so quickly 👏 It's a relief that we don't have to do migrations for this.

[obulat]

I think I'd prefer to support both, with a note that in the next major version one will be removed. Ideally by that time we could email our API consumers or something like that, too. Edit: Forgot the entire point of version numbers for a second 🤦

Should I open an issue for allowing for this pattern of deprecation (use both options in current version and display a deprecation warning, and treat it as an error in the future version, as is done now with deprecations)? Or do we already have it available in the API, and I haven't found how to use it? @dhruvkb , what do you think?

[dhruvkb]

I don't think we have a full-fledged deprecation pattern yet but ee do have a rudimentary system here:

openverse-api/api/catalog/api/serializers/media_serializers.py

Line 149 in a5f8b48

DeprecatedParam = namedtuple("DeprecatedParam", ["original", "successor"])

I'd love to have a documented deprecation system (even semver) for the future as we move to newer API versions but at this stage, I'm not sure if that's something to invest effort in. We only have a handful of consumers that we can directly notify of breaking changes before we deploy.

[sarayourfriend]

Why have a version prefix if we're going to deploy breaking changes?

Unless we're prepared to hold off deploying breaking changes until our small number of API consumers have confirmed that they're ready to handle it, then we're really misleading people by having a version prefix in the API routes.

Unless something is a security risk then breaking changes on a single API version doesn't make sense to me. If we need to release new versions of the API with breaking changes then we do need to spend the time to actually version the API (as Dhruv suggested, this isn't trivial work) but until then we'll just have to live with creating a messy v1 API (which is fine IMO, who ever created a clean v1 API anyway?)

[dhruvkb]

The v1 prefix is kind of misleading anyway. Technically, with all the changes we've made so far, we've already broken backward compatibility with the CC Search's v1 API. So any client relying on https://api.creativecommons.engineering/v1/ has been broken since the redirect.

[zackkrida]

I do think healthy versioning practices and not breaking things is like...the way to keep API consumers happy, and we have 172 registered apps currently (some duplicates due to registration emails still not working).

[obulat]

Just to clarify, the change in this PR is not breaking: categories for images will still work, and categories for audio will still not work start filtering by category, but that shouldn't be a breaking change. And we can convert it to a breaking change (adding categories to deprecated parameters) when we do decide to bump the API version.

One thing that I'd like to check is whether categories was only used in the DRF serializer side, and not on the database side. I recreated the API using the main branch, and I see that the db column name is fine, category singular, but I'd still like to be extra cautious and confirm :)

@dhruvkb ? @sarayourfriend ? @AetherUnbound ? Sorry for mentioning all of you here...

[AetherUnbound]

This looks great! I can't comment too much on the API versioning, as that's not something I have too much experience with.

The database is congruent with category as a singular word, so from that perspective I don't foresee any issues!