Skip to content

Pluralize remaining_in_stock translation key#14397

Open
David-OFN-CA wants to merge 2 commits into
openfoodfoundation:masterfrom
David-OFN-CA:14389-pluralize-remaining-in-stock
Open

Pluralize remaining_in_stock translation key#14397
David-OFN-CA wants to merge 2 commits into
openfoodfoundation:masterfrom
David-OFN-CA:14389-pluralize-remaining-in-stock

Conversation

@David-OFN-CA

@David-OFN-CA David-OFN-CA commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

What? Why?

Closes #14389

The `js.shopfront.variant.remaining_in_stock` key was a flat string `"Only %{quantity} left"`, which gave translators only one form. French (and other languages) need separate singular and plural forms — e.g. `"Seulement 1 produit restant"` vs `"Seulement 2 produits restants"` — but there was no way to provide them.

Changes:

  • Add `one:` / `other:` sub-keys to `remaining_in_stock` in `en.yml` (both identical in English)
  • Update the AngularJS shopfront template to pass `count:` instead of `quantity:`
  • Update the server-side product preview template likewise
  • Migrate all 30 non-English locale files from `%{quantity}` to `%{count}` — required because the templates no longer pass `quantity:`, so any locale still using `%{quantity}` would render a broken raw string for real users
  • Add proper `one:`/`other:` plural forms to all four French locales (`fr`, `fr_CA`, `fr_CH`, `fr_BE`) as a concrete demonstration of the feature
  • Expand the locale spec to verify French singular and plural forms via `I18n.t(..., locale: :fr)`

What should we test?

  1. Visit a shop with a product that has `on_hand: 1` or `2` and `low_stock_display` enabled
  2. Confirm "Only 1 left" / "Only 2 left" still appears in English
  3. With a French locale, confirm "Seulement 1 produit restant" (singular) and "Seulement 2 produits restants" (plural) appear correctly

Screenshots (French locale)

image

Notes for maintainers

Transifex: The next automated Transifex sync may re-flatten the French `one:`/`other:` keys if Transifex's source still has the old flat string format. The `en.yml` change (from flat string to `one:`/`other:`) should signal to Transifex that this is now a pluralized key — but please verify the Transifex source key is updated so the French forms are not overwritten on next sync.

Other languages: Languages with more complex plural rules (Russian, Ukrainian, Arabic, Welsh, Serbian) have been migrated to `%{count}` flat strings for now. Their translators can upgrade to full plural forms (`one:`, `few:`, `many:`, `other:` etc.) via Transifex once the source key structure is recognised.

Changelog Category

  • User facing changes

Change `js.shopfront.variant.remaining_in_stock` from a plain string
to `one:`/`other:` sub-keys, and update both call sites to pass
`count:` so that translators can provide separate singular and plural
forms (e.g. French: "produit restant" vs "produits restants").

Closes openfoodfoundation#14389

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lural forms

Migrate all 30 non-English locale files from %{quantity} to %{count} so
existing translations don't break now that templates pass count: instead
of quantity:. Add one:/other: plural forms to fr, fr_CA, fr_CH, fr_BE to
demonstrate the feature. Expand locale spec to verify French singular and
plural forms via I18n.t(..., locale: :fr).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@David-OFN-CA David-OFN-CA marked this pull request as ready for review June 12, 2026 06:31
@sigmundpetersen sigmundpetersen moved this from All the things 💤 to Code review 🔎 in OFN Delivery board Jun 12, 2026
@rioug rioug added the user facing changes Thes pull requests affect the user experience label Jun 15, 2026

@rioug rioug left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @David-OFN-CA

We don't usually want to update the translation other than the english one, but for this scenario I think It makes sense. I am wondering that might get overridden by transifex ? @mkllnk , @dacook what do you think ?

Comment on lines +8 to +16
RSpec.describe "js.shopfront.variant.remaining_in_stock i18n key" do
it "returns the correct string when count is 1 (singular)" do
expect(I18n.t("js.shopfront.variant.remaining_in_stock", count: 1)).to eq("Only 1 left")
end

it "returns the correct string when count is 2 (plural)" do
expect(I18n.t("js.shopfront.variant.remaining_in_stock", count: 2)).to eq("Only 2 left")
end
end

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't really tell us much but it will be improved in the next commit with the addition of the French language.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I would have added this test; it's testing the I18n feature, and config, but not application code. But it does verify the change 👍

- if !variant.on_demand && variant.on_hand <= 3
.variant-remaining-stock
= t("js.shopfront.variant.remaining_in_stock", quantity: variant.on_hand)
= t("js.shopfront.variant.remaining_in_stock", count: variant.on_hand)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though maybe we don't need to change the variable, but TIL count as a special role for pluralization : https://guides.rubyonrails.org/i18n.html#pluralization.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
= t("js.shopfront.variant.remaining_in_stock", count: variant.on_hand)
= t("js.shopfront.variant.remaining_in_stock", quantity: variant.on_hand, count: variant.on_hand)

@dacook dacook left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that these locale updates will get overridden when we prepare the release after merging. This happened to a recent PR that I had.

I think that these files are not the source of truth; it's the Transifex database that is source of truth. We can update the database with tx push, but would need to be careful not to override any recent updates in the DB. I'm not sure of the best process for doing that, given that this PR depends on the changes, but the changes need to be made externally.

Another strategy could be to use a find/replace tool within Transifex to make this update. Again, it would have to be timed carefully: after this PR is merged to master, and before the subsequent release.

🤔 Is it possible to make changes in a backwards-compatible way? We could provide both variable names in the I18n call, eg: t("js.shopfront.variant.remaining_in_stock", quantity: variant.on_hand, count: variant.on_hand). But I don't like leaving migration code inside the app code.
Or can we add some kind of temporary hook to I18n to gracefully handle count/quantity.

Or of course, we could create a new similar key, with the same content (eg js.shopfront.variant.remaining_in_stock2). Transifex should automatically translate the content because it's the same as before. I don't like it, but that's probably the easiest and safest transition.

I think we need to build our knowledge around handling translations with Transifex. Maybe there's a better way to handle it that I'm not aware of.
Maybe there could be a way to two-way sync the locales.
Maybe we should make the existing state clearer and move the locales to a separate read-only repo that is automatically generated from Transifex (I think we considered that in the past).

Comment on lines +8 to +16
RSpec.describe "js.shopfront.variant.remaining_in_stock i18n key" do
it "returns the correct string when count is 1 (singular)" do
expect(I18n.t("js.shopfront.variant.remaining_in_stock", count: 1)).to eq("Only 1 left")
end

it "returns the correct string when count is 2 (plural)" do
expect(I18n.t("js.shopfront.variant.remaining_in_stock", count: 2)).to eq("Only 2 left")
end
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I would have added this test; it's testing the I18n feature, and config, but not application code. But it does verify the change 👍

@dacook

dacook commented Jun 15, 2026

Copy link
Copy Markdown
Member

Transifex has a Find/Replace tool that appears able to do a global find/replace. But I'm not sure if you can replace variable names (these are treated as special). And there's probably other quantity variables that we don't want to replace, so the Find/Replace tool wouldn't be helpful.
https://help.transifex.com/en/articles/6318944-additional-tools-in-the-transifex-editor#h_877aac78b5
image

This is how variables appear in the editor:
image

@rioug

rioug commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

I am wondering if we should just update the English translation and flag it to instance manager and let the community handle it ?

@dacook

dacook commented Jun 15, 2026

Copy link
Copy Markdown
Member

Hmm that might be ok, but for any language that's not updated before the release, I think that page would be broken, which doesn't sound good to me.
I have to admit without testing it out, I'm not sure exactly what will happen.

Looking back over the options, I would probably choose to add both variables and a comment explaining why:

# quantity is provided for backwards compatibility until all translations are updated
t("js.shopfront.variant.remaining_in_stock", quantity: variant.on_hand, count: variant.on_hand)

What do you think of that?

@rioug

rioug commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Yeah that seems like the better option, we should make sure to create an issue to clean that up after this gets merged.

@David-OFN-CA

Copy link
Copy Markdown
Collaborator Author

hi @dacook @rioug - thanks for the feedback, just trying to confirm where any further steps are needed from me at this point?

@dacook dacook left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to adjust the code as suggested, in order to ensure a safe transition. @mkllnk can you please review the above comments and let us know if you agree with this approach?

If so, we can apply this suggestion and move to testing queue.

- if !variant.on_demand && variant.on_hand <= 3
.variant-remaining-stock
= t("js.shopfront.variant.remaining_in_stock", quantity: variant.on_hand)
= t("js.shopfront.variant.remaining_in_stock", count: variant.on_hand)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
= t("js.shopfront.variant.remaining_in_stock", count: variant.on_hand)
= t("js.shopfront.variant.remaining_in_stock", quantity: variant.on_hand, count: variant.on_hand)

.variant-remaining-stock{ "ng-if": "displayRemainingInStock()" }
{{ "js.shopfront.variant.remaining_in_stock" | t:{quantity: available()} }}
{{ "js.shopfront.variant.remaining_in_stock" | t:{count: available()} }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{ "js.shopfront.variant.remaining_in_stock" | t:{count: available()} }}
{{ "js.shopfront.variant.remaining_in_stock" | t:{quantity: available(), count: available()} }}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

user facing changes Thes pull requests affect the user experience

Projects

Status: Code review 🔎

Development

Successfully merging this pull request may close these issues.

Translation string to be pluralized

4 participants