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

Prevent Faker Address country_code from raising RetryLimitExceeded #9036

Merged

Conversation

Quentinchampenois
Copy link
Contributor

🎩 What? Why?

Seeds are failing when executing with another locale than default (:en).
It seems that Faker i18n translations are different between english and other languages, see english failing key country_code and french for example.

In this PR, country code is no longer generated using unique of Faker but with the current index when creating Scope.

📌 Related Issues

Testing

Step to reproduce are well explained in the related issue :

  1. In a Decidim application (like development_app)
  2. Update the Decidim's locale in config/initializers/decidim.rb :
[...]
  config.available_locales = %w(fr)
  # organization from the System area, system admins will be able to overwrite
  config.default_locale = :fr
[...]
  1. Then clear the database and execute seeds : bundle exec rake db:setup
  2. Seeds should run without failure.

You can also try with an other default locale like :ca, :es, etc...

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

♥️ Thank you!

@Quentinchampenois Quentinchampenois marked this pull request as ready for review March 18, 2022 08:21
Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

Tried it locally, it works, thanks for the fix! Funnily, with :ca it didn't have any issue because there isn't a Faker country_code and it seems like it's fall backing to the defined on :en 🤷🏽‍♂️
Anyhow, this for sure is an improvement, fixing the seeds for all locales I think.

@andreslucena andreslucena added module: dev type: fix PRs that implement a fix for a bug module: core labels Mar 18, 2022
@andreslucena andreslucena merged commit 0a1c0d8 into decidim:develop Mar 18, 2022
@andreslucena
Copy link
Member

Can you please backport to v0.26-stable 🙏🏽? Thanks

@andreslucena andreslucena added this to Pending review from Product in Maintainers via automation Mar 18, 2022
@andreslucena andreslucena moved this from Pending review from Product to Backport pending in Maintainers Mar 18, 2022
@andreslucena andreslucena added target: developer-experience and removed type: fix PRs that implement a fix for a bug labels Mar 18, 2022
@andreslucena andreslucena moved this from Backport pending to Done in Maintainers Mar 21, 2022
tramuntanal pushed a commit to CodiTramuntana/decidim that referenced this pull request Apr 5, 2022
tramuntanal added a commit to CodiTramuntana/decidim that referenced this pull request Apr 5, 2022
…ecidim#9036) (#73)

Co-authored-by: Quentin Champ <26109239+Quentinchampenois@users.noreply.github.com>
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Cannot db:seed when changing default locale (Faker issue)
3 participants