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

Make dev seeds independent on available locales #4733

Merged
merged 7 commits into from
Dec 14, 2021

Conversation

javierm
Copy link
Member

@javierm javierm commented Nov 10, 2021

References

Objectives

  • Make it possible to run the db:dev_seed task on CONSUL installations using languages which don't include English and Spanish
  • Use default budget phases names in dev seeds

@javierm javierm self-assigned this Nov 10, 2021
@javierm javierm added this to Reviewing in Consul Democracy via automation Nov 10, 2021
@taitus taitus self-assigned this Dec 13, 2021
We're using Faker during this task, and it depends on English being
available as a fallback for methods like the Lorem ipsum ones.
These locales are officially maintained by CONSUL developers, so we're
using them when available.

This way we'll be able to use `random_locales` in places where we're
manually using English and Spanish, giving support to other locales
while maintaining compatibility with the current version.
So it's easier to differentiate between them during development.
Now changing the code dealing with locales is going to be easier.
@javierm javierm force-pushed the dev_seeds_without_english branch 4 times, most recently from aff00c6 to 8f48d81 Compare December 14, 2021 12:43
Consul Democracy automation moved this from Reviewing to Testing Dec 14, 2021
Some developers work on CONSUL installations where Spanish and/or
English aren't part of the available locales. In those cases, the
`dev_seed` task was crashing because we were using attributes like
`name_en` and `name_es`.

So we're using attributes for random locales instead.

We're using a proc so we don't have code like this all over the place:

random_locales.map do |locale|
  I18n.with_locale(locale) do
    phase.name = I18n.t("budgets.phase.#{phase.kind}")
    phase.save!
  end
end

This would make the code harder to read and would execute a `save!` once
per locale, which would make the task much slower.

We could also avoid the procs writing something like:

def random_locales_attributes(**attribute_names_with_values)
  random_locales.each_with_object({}) do |locale, attributes|
    I18n.with_locale(locale) do
      attribute_names_with_values.each do |attribute_name, (i18n_key, i18n_args)|
        value = I18n.t(i18n_key, (i18n_args || {}).merge(language: I18n.t("i18n.language.name")))
        attributes["#{attribute_name}_#{locale.to_s.underscore}"] = value
      end
    end
  end
end

And calling the method with with:

random_locales_attributes(name: ["seeds.budgets.name", year: Date.current.year - 1])

However, this code would also be different that what we usually do, we'd
have to apply some magic to pass the `language:` parameter, and the
strings wouldn't be recognized by i18n-tasks, so we aren't sure we're
really gaining anything.
We were making some typos during development in the name of the keys and
tests were still passing.

We're also removing some texts that were never used.
@javierm javierm merged commit 031ec8f into master Dec 14, 2021
Consul Democracy automation moved this from Testing to Release 1.5.0 Dec 14, 2021
@javierm javierm deleted the dev_seeds_without_english branch December 14, 2021 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dev seeds fail if Spanish is not enabled
2 participants