fix: Fix invalid ngettext usage

[last-partizan]

Description

Format should be called after ngettext, not before.

See example: https://docs.djangoproject.com/en/5.1/topics/i18n/translation/#pluralization

Formatting changes is because i don't know how to properly format it with your previous style, and there's no included autoformat/or formatting linter. I just formatted it automatically with ruff-lsp. Feel free to revert fomatting changes to your style or explain me what to do.

[last-partizan]

That's very good question, because it got me thinking... And turns out, I missed important detail.

In our production code, we override this class with

            extra = ngettext(
                "Try again later in about {wait} second",
                "Try again later in about {wait} seconds",
                wait,
            ).format(wait=wait)

To fix, invalid usage of "second[s]" in languages with three plural forms (like Ukrainian or Russian). I don't see translations for this string in current DRF, and English has only two plural forms, so there would be no changes at all.

Proper way to fix it, would be the same, and i'm pushing updated changes.

Expected changes in Ukrainian, after this change lands, after updating .po with makemessages and after translations added:

-"Спробуйте ще раз через 5 секунди"
+"Спробуйте ще раз через 5 секунд"

Because we have three plural forms:

• (singular, 1) - "секунда", or in this case "секунду"
• (plural 2-4) - "секунди"
• (plural 5-9) - "секунд"

[last-partizan]

@auvipy please take a look at this again.

[last-partizan]

This code is covered by tests, and translation strings is unchanged, so everything should be fine.

We probably need to make sure changelog is clear about what's changed here. I have squashed commits and updated text, but if changelog is autogenerated from the first line of commit, you may need to move relevant instructions into the first line when Merging/Squashing.

Important part there: "If you have overridden extra_detail_singular or extra_detail_plural, you should now replace them with a single def extra_detail override.".

[auvipy]

we will need documentation update then? and it seems to be an API change as well?

[last-partizan]

This is an API change, which the project prefers not to do at this stage.

By "at this stage", you mean "project is mature enough to not do API changes"? or some sort of "no API changes for minor releases"?

It may be possible to juggle with some properties etc. to strictly extend the previous API without removing Throttled.extra_detail_* attributes, but I'm not sure that complexity would be a good idea.

Not sure how. Even if we can use something like ngettext(self.extra_detail_plural ..., it will not work, because for ngettext to work properly it needs strings right there.

And, I don't see any docs mentioning extra_detail_ on class Throttled, so maybe this is not a public API...

[peterthomassen]

By "at this stage", you mean "project is mature enough to not do API changes"? or some sort of "no API changes for minor releases"?

The former, see https://github.com/encode/django-rest-framework/blob/master/CONTRIBUTING.md and the note on https://www.django-rest-framework.org/community/contributing/.

And, I don't see any docs mentioning extra_detail_ on class Throttled, so maybe this is not a public API...

Many people consider all API public unless identifiers start with an underscore.

[last-partizan]

That's a tricky situation, because it's a bug that cannot be fixed without changing public API.

Sure, it's minor bug, but so are the consequences of breaking this part of API.

We can ... maybe, add some checks in __setattr__ that raise errors when users trying to set extra_detail_singular or extra_detail_plural. But that's overkill for my taste.

Hmmmm, maybe add a check to rest_framework/checks.py, that ensures no subclasses of Throttled are setting those props?

But, from my point of view, best course of action here: just add a note about change in the changelog/release notes, so the change would be visible.

[peterthomassen]

@tomchristie Thoughts?