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

Basic content for I18n on Admin panel #2809

Conversation

aitbw
Copy link
Collaborator

@aitbw aitbw commented Jul 27, 2018

References

This is a work started by @iagirre on AyuntamientoMadrid#1509 —the final work was continued by @raul-fuentes and finished by yours truly (alongside @voodoorai2000) on AyuntamientoMadrid#1580

Objectives

  • Allow CONSUL installations to customize basic I18n texts through the Admin panel, so there's no need to edit config/locales/ files manually

We didn't backport the original PR created by @iagirre because, even though it did work, it was extremely verbose in the sense that we needed to populate the DB (for development) with the I18n keys mentioned in #2625, which were too many and there was the need to create a new Rake task to generate said seeds as well

Moreover, the original PR defined a new g helper method that replaced t, meaning all views were to be modified in order to reflect the changes the feature is supposed to introduce —despite this being the more obvious and simple approach, it wasn't easy to review and merge conflicts were just around the corner, especially for those forks with heavily-modified views

We ultimately decided to rework the original PR, focusing on monkey patching the t helper method to respond to the translations accordingly and reducing @iagirre's work verbosity

Visual Changes

Notes

  • You'll need to migrate your current DB schema to create the newly added i18n_contents and i18n_content_translations tables that power this feature

  • ⚠️ If you include a new language key under app/models/i18n_content, please make sure it's also available on your config/application.rb file ⚠️

  • Further visual improvements will be introduced on a different PR by @decabeza, stay tuned! 🎨

  • We're currently working on a new PR to refactor this feature, especially on the backend side of things, to make it more easier to read and maintain

raul-fuentes and others added 30 commits July 26, 2018 19:05
In the admin section of the application, a new page
has been added so that the admins are able to manage
the selected texts for translate.

The texts have been divided in different "sections",
depending on the nature of themselves (budgets, polls,
proposals, management, etc.). Each section has become a tab
with a form associated to edit all the texts for her.

When a language is added, it's added for ALL the texts in the
application. That means that, if an admin adds french for debates,
the french form will appear for the rest of the texts. That doesn't
mean that they need to fill all the texts, only that the languages
work for all of them instead of individually.
Refactoring and making similar specs to the milestones globalization specs
We were getting a 500 error because the translations had not been initialized
https://stackoverflow.com/a/4054527
When visiting, for example, /admin/site_customization/information_texts?locale=fr
we were getting an `UncaughtThrowError: uncaught throw :exception`
With the following payload
```
File "/aytomad/app/participa/participacion/releases/20180726231929/app/views/admin/site_customization/information_texts/_form_field.html.erb" line 5

File "/aytomad/app/participa/participacion/releases/20180726231929/app/helpers/globalize_helper.rb" line 35 in block in globalize
```

Substituting this line seems to solve it

Note that we had to remove the portuguese local too, as it was giving a different
exception due to this change. This problem, has been solved in the original
globalization PR
voodoorai2000 and others added 3 commits July 26, 2018 21:55
After creating a translation in spanish, it was also displaying it when selecting
the english locale.

This was due to the code picking the first translation available

With this commit, we are checking for an existing translation in the current locale
and displaying it if it exists
When visiting http://localhost:3000/admin/site_customization/information_texts?locale=fr
and creating a translation, other languages where storing translations in db with
the following values:
"<span class=\"translation_missing\" title=\"translation missing: es.debates.index.search_results.one\">One</span>">

With this commit we are not storing this translations

Note that this only happened when using a param[:locale] in the url at least for french
Copy link
Member

@voodoorai2000 voodoorai2000 left a comment

Choose a reason for hiding this comment

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

🎉

@voodoorai2000 voodoorai2000 merged commit 2c79197 into consuldemocracy:master Jul 27, 2018
@aitbw aitbw deleted the backport/translatable-content-for-admin branch July 27, 2018 16:01
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.

3 participants