Bug 10509 - Using i18n function for translation in webinstaller mysql files
Summary: Using i18n function for translation in webinstaller mysql files
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (web-based installer) (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 10520 10521 10526
  Show dependency treegraph
 
Reported: 2013-06-26 11:27 UTC by Marcel de Rooy
Modified: 2015-12-11 20:19 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10509: Using updates instead of inserts in translated mysql files (81.77 KB, patch)
2013-06-26 12:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10509: Introducing a global folder next to the English folder (56.77 KB, patch)
2013-06-26 12:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10509: Using i18n function for translation of MySQL web installer files (9.61 KB, patch)
2013-07-01 10:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10509: Followup for load_sql_in_order in unit tests (2.91 KB, patch)
2013-07-01 10:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10509: Using i18n function for translation of MySQL web installer files (9.62 KB, patch)
2013-08-02 17:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10509: Followup for load_sql_in_order in unit tests (2.93 KB, patch)
2013-08-02 17:57 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2013-06-26 11:27:36 UTC
Currently, all languages in installer/data/mysql contain a few subfolders with .sql files mostly containing insert statements. There is a lot of redundancy and maintaining this construction is more work.

I propose to use en (English) as a global folder, or even a separate folder global (with English just being one of the languages). The global folder should contain inserts; these sql files should be run first. The translation folder should update only the descriptions etc.

This is based on a recent discussion on the dev list.
Comment 1 Marcel de Rooy 2013-06-26 11:35:50 UTC
I am attaching two example patches (they are not completely ready for signoff yet, but they already work).
The first patch uses en as global folder and updates the de-DE/mandatory sql files as an example.
The second patch uses a new global folder next to the en folder.
I will ask for some feedback on the dev list about both approaches.

The second approach has a maintenance drawback: we need to maintain updates in the en folder, but the advantage is: it is simpler to add a new language. And it is easier to do specific 'English language' stuff without disturbing the global approach. (Stopwords is obsolete?, but could be an example of it.)

The work is done in such a way that we can progressively update all files in the language folders without breaking the current installation process.
Comment 2 Marcel de Rooy 2013-06-26 12:36:38 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2013-06-26 12:36:42 UTC Comment hidden (obsolete)
Comment 4 Zeno Tajoli 2013-06-26 14:40:17 UTC
Hi to all,

after thinking about, I prefer the second approch.
But in the language-specific files you can't have only update.
We need to permitt also delete and insert.

For example in italian sql setup we don't want those two classification
('sudocs', 'SuDoc Classification (U.S. GPO)', 0, 'generic')
('anscr', 'ANSCR (Sound Recordings)', 0, 'generic')
And we also will add this one
('indire', 'Classificazione INDIRE', 0, 'generic')

But I think it is still possible with this patch
Comment 5 Marcel de Rooy 2013-06-26 14:49:16 UTC
Zeno, it will still be possible to do that (with both methods).

jcamins mentioned on irc that we have also 'en' in the templates, why create global here?
Comment 6 Marcel de Rooy 2013-06-26 14:56:47 UTC
Other points from irc discussion:

Why not use SOURCE in the sql files to refer to english?
Or use a file like 'usesupdates'  in the lang folder to tell the installer to run english first?

-- idea borrowed from Evergreen
CREATE FUNCTION i18n(str TEXT) RETURNS TEXT DETERMINISTIC
RETURN str;
-- then mark up insert statements with the strings we want to translate
INSERT INTO `userflags` VALUES(9,'editcatalogue',i18n('Edit Catalog (Modify bibliographic/holdings data)'),0);
-- the i18n() bit is just syntactical sugar to make it easy to extract strings for a POT file
-- then code (to be written) would create the language-specific inserts from the PO files
INSERT INTO `userflags` VALUES(9,'editcatalogue','Katalogdaten ändern (Titel- und Bestandsdaten ändern)',0);
Comment 7 Zeno Tajoli 2013-06-26 15:10:05 UTC
Well, theoreticaly I prefer a neutral 'global' dir with all standard
setup and many different dirs (en, en-US, it-IT, etc) with oly translation and little specific changes.
But it si only a preference, I can use every solution.
Comment 8 Bernardo Gonzalez Kriegel 2013-06-27 02:20:51 UTC
I was working on another sql file and was tempted to introduce i18n() function to enable translation (in the near future I hope), but as I don't know when or where in the install process this function will be defined, do you think that as an interim solution it's possible to write something like

drop function if exists i18n;
create function i18n( str TEXT) returns text deterministic return str;

in the file?

BTW, I think that 'en' works as our global, so no need to add a new dir and less things to change.
Comment 9 Marcel de Rooy 2013-06-27 06:41:40 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #8)
> I was working on another sql file and was tempted to introduce i18n()
> function to enable translation (in the near future I hope), but as I don't
> know when or where in the install process this function will be defined, do
> you think that as an interim solution it's possible to write something like
> drop function if exists i18n;
> create function i18n( str TEXT) returns text deterministic return str;
> in the file?

I would propose to not combine these things. As the outcome of this discussion has not been crystalized(?) yet, it could only complicate things. Your patch (I suspect for authorities) should just go first.

> BTW, I think that 'en' works as our global, so no need to add a new dir and
> less things to change.

I tend to go in that direction too now, although some have expressed interest in the global direction..
Comment 10 Marcel de Rooy 2013-06-27 13:42:11 UTC
Just thinking about this, we could do something else too :)
What do you think about running the English file with i18n calls while defining i18n for the German translation in this case with something as:

DELIMITER //
DROP FUNCTION IF EXISTS i18n//
CREATE FUNCTION i18n (d text) RETURNS text DETERMINISTIC
BEGIN
RETURN CASE d
WHEN 'Remaining circulation permissions'
'Übrige Ausleihberechtigungen'
WHEN 'Override blocked renewals'
'Gesperrte Verlängerungen überschreiben'
WHEN 'Execute overdue items report'
'Überfälligkeiten-Report ausführen'
[ etc. etc.]
WHEN 'Configure plugins'
'Plugins konfigurieren'
ELSE d
END;
END //

This file could also be very easily created from a POD file..
Comment 11 Bernardo Gonzalez Kriegel 2013-06-27 14:30:45 UTC
In that case you will need to call German (or any lang) file first so i18n() function it is correctly defined.

That's not reversing the order?
Comment 12 Marcel de Rooy 2013-06-27 16:41:53 UTC
> In that case you will need to call German (or any lang) file first so i18n()
> function it is correctly defined.

Yes and no :) Actually when running the English sql file that contains the i18n calls, I first pass the German i18n definition to mysql. This will make the English install insert German strings. But you could say that I only have one sql file to run, and I 'source' (read: run) the i18n definition first.

For English, I would have a fake i18n that returns the same string, so no translation.
Comment 13 Marcel de Rooy 2013-07-01 10:29:37 UTC Comment hidden (obsolete)
Comment 14 Marcel de Rooy 2013-07-01 10:30:07 UTC Comment hidden (obsolete)
Comment 15 Marcel de Rooy 2013-07-01 10:36:43 UTC
As we discussed this bug, the solution went in another direction than initially foreseen.
I have renamed the bug therefore.
This patch provides the foundation for converting the language folders one-by-one to a new style.
I will open new reports for the language patches. Plan to first send English and German within some days.
You are more than welcome to test this patch and sign it off :)
Comment 16 Kyle M Hall 2013-08-02 17:56:55 UTC
Created attachment 20076 [details] [review]
Bug 10509: Using i18n function for translation of MySQL web installer files

This patch makes the changes needed to progressively use a i18n function to
translate strings in the web installer's MySQL files. The English scripts may
contain fields like i18n(columnvalue); for English i18n(a) is a, but for the
other languages i18n(a) is the translation of a.
The corresponding script in the language folder contains the i18n definition.
The work is done in such a way that current installations are not affected.

Subsequent patches will make the changes in the .sql files themselves. This can
be done per folder, per language. Using the extension .i18n in the last
subfolder name triggers the new translation mechanism. (See also test plan.)

In Installer.pm the all_languages parameter is removed from load_sql_in_order.
This code was not used. This change also affects the call in install.pl.
The most important change is the use of a new subroutine _run_mysql_file in
load_sql. This new routine contains the above logic.
Although I also tested changing marc_framework_sql_list and
sample_data_sql_list, it proved imo to be easier (and more maintainable) to do
this via load_sql.

Test plan:
Since no .sql files are changed in this patch, please test if you can install
English and e.g. German (or another one) in the web installer without any
changes.

Bonus points for the following additional tests :)
They serve to illustrate the i18n concept more clearly.
1) Place in en/mandatory/userflags.sql around a few strings i18n('...').
Run English install again and check the userflags table. No changes expected.
2) Place in de-DE/mandatory/userflags.sql i18n around a few strings too. Also
precede the insert statements with this definition:
DROP FUNCTION IF EXISTS i18n;
CREATE FUNCTION i18n (d text) RETURNS text DETERMINISTIC RETURN concat(d,'xx');
Now run German install again; some of the userflag strings (with i18n) should
now end with xx.
3) Before retesting German install, make some changes:
Edit en/optional/sample_holidays.sql. Add i18n calls around strings.
Create an additional folder optional.i18n in de-DE.
Copy sample_holidays.sql and sample_holidays.txt from de-DE/optional.
Edit de-DE/optional.i18n/sample_holidays.sql. Remove all text! Add:
DROP FUNCTION IF EXISTS i18n;
CREATE FUNCTION i18n (d text) RETURNS text DETERMINISTIC
RETURN CASE d
WHEN 'Sundays' THEN 'Sonntag'
ELSE d END;
Now run German install again; in the optional part of the install mark
sample holidays (in the last section standing separately; do not confuse with
 the other one). Check the repeatable_holidays table afterwards. The string
above should have been translated only, the others are still English.
Undo the changes in .sql files; remove optional.i18n folder.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 17 Kyle M Hall 2013-08-02 17:57:33 UTC Comment hidden (obsolete)
Comment 18 Chris Cormack 2013-09-12 08:11:54 UTC
This no longer applies because the tests (that weren't being used) have been removed. It would be good to add a test for the 2 new subroutines added.
Comment 19 Marcel de Rooy 2013-09-16 08:52:35 UTC
Comment on attachment 20077 [details] [review]
Bug 10509: Followup for load_sql_in_order in unit tests

The second patch is obsolete with the removal of the lib/KohaTest suite.
Comment 20 Marcel de Rooy 2013-09-16 08:54:56 UTC
The first patch still applies. Since the second patch only referred to obsolete test units, I reset the status to Signed off. The signoff on the first patch is (imo) not impacted by the removal of the second patch.
Comment 21 Marcel de Rooy 2013-09-16 09:36:40 UTC
(In reply to Chris Cormack from comment #18)
> This no longer applies because the tests (that weren't being used) have been
> removed. It would be good to add a test for the 2 new subroutines added.

Chris, if you refer to the two subroutines _run_mysql_file and  _tr_sql_fname, I just consider them as internal 'private' routines (with an underscore) for load_sql_in_order. That would not (always/explicitly) require unit tests.

Note that the changes would certainly justify changes in the unit tests for load_sql_in_order, but unfortunately we do not even have a unit test for this routine or its module. Should that block this change?
Comment 22 Marcel de Rooy 2013-10-28 10:41:05 UTC
Changing status to Blocked.
Subsequent patches are already in Not apply-status. Apparently, this line of development is not interesting enough to continue.
Comment 23 Zeno Tajoli 2013-10-28 11:16:51 UTC
What is it better to do for develop this patch ?
Comment 24 Marcel de Rooy 2013-10-28 13:07:31 UTC
(In reply to Zeno Tajoli from comment #23)
> What is it better to do for develop this patch ?

The patches were there, but nothing happened. They do no longer apply (quite fast actually with other changes), etc.
Comment 25 Magnus Enger 2015-12-11 20:19:34 UTC
Looks like this depends on bug 10520 which is CLOSED WONTFIX.