add Spanish to config.i18n_locale

Bug #1618183 reported by Ben Shum
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Medium
Unassigned
2.10
Fix Released
Medium
Unassigned

Bug Description

Evergreen master

We need to add Spanish to the list of languages in config.i18n_locale. When testing the setup for Spanish in the catalog, if we are missing an entry in that table, it will never give the option for Spanish no matter how you configure eg_vhost.conf.

Simple little patch forthcoming.

Revision history for this message
Ben Shum (bshum) wrote :
Changed in evergreen:
milestone: none → 2.11-rc
importance: Undecided → Medium
status: New → Triaged
tags: added: i18n
tags: added: pullrequest
Dan Wells (dbw2)
Changed in evergreen:
assignee: nobody → Dan Wells (dbw2)
Revision history for this message
Dan Wells (dbw2) wrote :

Pushed to master, thanks Ben! Do we want to backport to 2.10? I think so, given the translation updates we did for that release, but want to make sure there aren't gotchas involved.

Changed in evergreen:
milestone: 2.11-rc → 2.11-beta
assignee: Dan Wells (dbw2) → nobody
status: Triaged → Fix Committed
Revision history for this message
Ben Shum (bshum) wrote :

I'll defer to gmcharlt on backport opinions since he's the 2.10 maintainer, but personally I'd be a little wary of adding this patch to the 2.10 series without some explanation for users. The potential scenario I envision is one where someone upgrades to 2.10.x (with this INSERT to the table), and then also upgrades later to 2.11.0 with the same INSERT failing and rolling back their entire upgrade script to 2.11 due to a collision where the entry already exists.

We'd have to creatively work around the problem for both 2.11 and 2.10 if we wanted to include the fix in both series.

My opinion is to focus on the future only with the fix in master for 2.11 and leave 2.10 alone. If some expert system admins really want the Spanish translation in their catalog, they can add the entry (and deal with the full ramifications of that later on)

Revision history for this message
Thomas Berezansky (tsbere) wrote :

If the upgrade script was formatted like this it would only insert if it isn't already there:

INSERT INTO config.i18n_locale (code,marc_code,name,description)
    SELECT 'es-ES', 'spa', oils_i18n_gettext('es-ES', 'Spanish', 'i18n_l', 'name'),
        oils_i18n_gettext('es-ES', 'Spanish', 'i18n_l', 'description')
  WHERE NOT EXISTS (SELECT 1 FROM config.i18n_locale WHERE code = 'es-ES');

Not going to argue either way for backporting, though. Just providing a possible solution to the "you already have that row!" problem.

Revision history for this message
Dan Wells (dbw2) wrote :

The main reason I would want to backport would be the effort we just put into getting the Spanish translations into 2.10. Also, the potential for INSERT collision is real, but IMHO not a big concern. The final upgrade script for 2.11.0 will go from the latest 2.10.x, so if this is in 2.10.7, it won't be repeated in the 2.10.7->2.11.0 upgrade script anyway. As a general rule of thumb, I do see the issue, though (i.e. if 2.11.0 was already out, since we generally don't make new cross-version upgrades for point releases).

I am also in favor of Thomas's recipe as a belt+suspenders solution.

In the end, it is certainly Galen's call to make.

Changed in evergreen:
status: Fix Committed → Fix Released
Revision history for this message
Galen Charlton (gmc) wrote :

I've backported to rel_2_10 along with some scaffolding to reduce the changes that somebody upgrading from 2.10.8 or later to 2.11.x will see the 2.10.7-2.11.0 upgrade step roll back.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.