Skip to content
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

react/CardStats: update stat strings to include plural versions fixes… #5407

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

philli-m
Copy link
Contributor

@philli-m philli-m commented Jan 22, 2024

#5370 and fix tests

Describe your changes
Briefly explain what you did and provide context for a clearer understanding.

Tasks

  • PR name contains story or task reference
  • Steps to recreate and test the changes
  • Documentation (docs and inline)
  • Tests (including n+1 and django_assert_num_queries where applicable)
  • Changelog

fix can be seen on project detail http://localhost:8003/projekte/module/participatory-budgeting-3-phase-3/?mode=list if you have a idea added to the module and are logged in as admin

updated card to ensure stats have correct pluralisation fixes #5370

expected outcome:
screenshot(30)

@philli-m philli-m requested a review from hom3mad3 January 22, 2024 10:49
@hom3mad3
Copy link
Contributor

@philli-m test fixes look ok, however there is no change in the actual component when i have more than 1 comment, for example? 1 Kommentar 2 Kommentar

@philli-m
Copy link
Contributor Author

eeek, just checked and looks like the django pluralisation is broken in comments contribution string too, will have a look, might have been django update :(

@hom3mad3 hom3mad3 self-requested a review January 23, 2024 11:48
@philli-m
Copy link
Contributor Author

@hom3mad3 I checked and it's weird translation issue, because we have a non pluralised version of 'Comment' in a4, the translations get confused, you can only see the error when you deleted the locale files and re-make po. Luckily we don't use that string it seems so I have removed it in this PR, once thats merged and we use the new a4 the pluralisation should work.
Contributions plurals in comments is a different issue but it does work after page reload, not ideal but not terrible

@philli-m philli-m requested review from goapunk and m4ra January 25, 2024 08:40
@philli-m
Copy link
Contributor Author

NOTE: didn't update the a4 hash as this will be done with rebase but after the comment string will be fixed

@philli-m philli-m force-pushed the pm-2023-12-card-strings branch from 70ad56a to 481fd85 Compare January 30, 2024 13:58
@philli-m philli-m force-pushed the pm-2023-12-card-strings branch from 481fd85 to bef508e Compare January 30, 2024 15:00
@philli-m
Copy link
Contributor Author

@hom3mad3 this should now be fully working with updated a4

@hom3mad3 hom3mad3 merged commit 1a25a88 into dev Jan 30, 2024
3 checks passed
@hom3mad3 hom3mad3 deleted the pm-2023-12-card-strings branch January 30, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants