Installer data on main/marc21/unimarc SQL files is difficult to translate and is prone to get off-sync with english original As a way to overcome this problem, I propose to use XML files from which corresponding SQL and TXT files could be created. XML files can be translated in the usual way, and all installer data can be created/translated before install time. There will be no need to preserve language folders, except some particular cases (e.g. NORMARC) An agreement will be needed for a superset of the all sample data on different language folders
I'll make this enh dependent on 7939 10963 13322. Bug 7939 modifies LangInstaller.pm Bug 10963 rewrites MARC21 sample bib frameworks, no need to translate them Bug 13322 updates MARC21 default bib framework All signed ATM
Created attachment 37165 [details] Bug 13897 Patchset Test implementation patchset Only cover mysql mandatory/optional and marc21 mandatory, just as example
Bernardo, can you describe 'the usual way' to transalte XML files ? Do you plan to use a .po file ?
(In reply to Zeno Tajoli from comment #3) > Bernardo, can you describe 'the usual way' to transalte XML files ? > Do you plan to use a .po file ? Yes, sorry, the plan is to use .po files- In the patchset I introduce an XML file for each SQL one in common mandatory/optional and MARC21 mandatory, soon I'll provide for marc21 optional and hopefully UNIMARC. I add mysql folders as targets to translation script, so new .po files are build from XML files, and when we use the translation script it copy all sql folders and rebuild sql and txt files from translated xml files, to allow an install with translated values.
I suggest to put .po file in a different git server, to limit the growth of general git archive.
Created attachment 95884 [details] [review] Bug 13897: Use XML files for installer data This patch modifies C4/Installer.pm to add support for loading XML files into database (MySQL only). As an example of the functionality, optional auth_val.sql file is replaced by auth_val.xml The rationale behind this feature is to enable the translation of the data that is loaded into the database. That will be addressed in another bug. But taking into account that goal, non translatable values are put into XML attributes, translatable ones into elements which can already be captured by translate script. To test: 0) Do a clean install with all optional data, then dump authorised_values table, reserve. 1) Apply the patch 2) Do a clean install in English (marc21/unimarc) 3) On optional data check for description of auth_val "Some basic default authorised values for ..." 4) Select all optional data 5) Finish installation 6) Dump again authorised_values table and compare with that of point '0'. There must be no differences.
Hola Bernado, I like the idea, but not XML, neither the MySQL only limitation. Why not write our own stuff, yml-based?
This looks good. My only slight concern is whether it ties us more strictly to MySQL or in particular a particular MySQL version. (I'm not sure how widely supported the MySQL XML dump format is.. do they change the format when new feature are introduced for example and does MariaDB support it natively too). Perhaps we could use the well supported and maintained SQL::Translator to produce something similar in XML, YAML or JSON as our middle format ?
(In reply to Jonathan Druart from comment #9) > Hola Bernado, > I like the idea, but not XML, neither the MySQL only limitation. Salut Jonathan, I don't like XML either, nor the MySQL only. Just remember that I was thinking in translations, and xml support it's included right now. But yaml can be handled. > Why not write our own stuff, yml-based? No problem, I'll write a POC patch, but have a question: Do you think the sequence a) Read YAML > Write $tmp/out.sql > DBIx_::RunSQL->run_sql_file($tmp/out.sql) is a valid approach, or b) Read YAML > Insert into DB somehow it's better? I suppose b) is slower. (In reply to Martin Renvoize from comment #10) > This looks good. My only slight concern is whether it ties us more strictly > to MySQL or in particular a particular MySQL version. (I'm not sure how > widely supported the MySQL XML dump format is.. do they change the format > when new feature are introduced for example and does MariaDB support it > natively too). > > Perhaps we could use the well supported and maintained SQL::Translator to > produce something similar in XML, YAML or JSON as our middle format ? Hi Martin, SQL::Translator documentation states: "Presently only the definition parts of SQL are handled (CREATE, ALTER), not the manipulation of data (INSERT, UPDATE, DELETE)." And this files normally do INSERTs, don't know if it's useful in this case.
(In reply to Bernardo Gonzalez Kriegel from comment #11) > Do you think the sequence > a) Read YAML > Write $tmp/out.sql > > DBIx_::RunSQL->run_sql_file($tmp/out.sql) > is a valid approach, or > b) Read YAML > Insert into DB somehow > it's better? > I suppose b) is slower. > Since this is installer data, I don't think it necessarily has to be fast? I figure the most important part is to make it easy to read and update? Actually, I don't know if we necessarily need to change C4/Installer.pm? We could use YAML as a declarative format for the database data, and then the Release Manager/Release Maintainer could run a tool to compile it into a SQL script. If we only do that compilation at release time, it would also be easier to audit and find bugs in the final result.
(In reply to David Cook from comment #12) > We could use YAML as a declarative format for the database data, and then > the Release Manager/Release Maintainer could run a tool to compile it into a > SQL script. > This could also be used to add support for different databases, especially in terms of optimizing bulk data loading. https://dev.mysql.com/doc/refman/5.6/en/optimizing-innodb-bulk-data-loading.html https://www.postgresql.org/docs/9.1/ecpg-sql-set-autocommit.html
While we're on this topic, perhaps it's worthwhile talking about something like this: https://metacpan.org/pod/DBIx::Class::Schema::Versioned
I am not sure if XML is really easier to read here, but enabling translation would be great. :) I'd vote against 'compiling at release time'. If we still need to create SQL files and store them, we should do so continously like we do for CSS etc. as this needs to be testable with current data all the time. Please keep in mind when changing the web installer, that it's currently possible to add additional SQL files to the installer that will be picked up - it would be nice to keep that feature. There is also the option to have a syspref.sql for some additional settings (see de-DE installer) that not all languages have made use of.
(In reply to Bernardo Gonzalez Kriegel from comment #11) > (In reply to Jonathan Druart from comment #9) > > Hola Bernado, > > I like the idea, but not XML, neither the MySQL only limitation. > > Salut Jonathan, > I don't like XML either, nor the MySQL only. > Just remember that I was thinking in translations, and xml support it's > included right now. > But yaml can be handled. > > > Why not write our own stuff, yml-based? > > No problem, I'll write a POC patch, but have a question: > > Do you think the sequence > a) Read YAML > Write $tmp/out.sql > > DBIx_::RunSQL->run_sql_file($tmp/out.sql) > is a valid approach, or > b) Read YAML > Insert into DB somehow > it's better? > I suppose b) is slower. I would: - Retrieve and parse the whole YAML file - Generate the SQL queries (like 1 per table) - Execute the INSERT statements (using DBIx::RunSQL->run_sql)
Sorry Bernardo, I didn't mean to knock this one off the rails so much :(. Hmm, it's a shame SQL::Translator only handles definition... I must have misremembered... it's SQL::Abstract ( the the various DBIx::Class extensions) that does the rest. SQL::Abstract is Perl script however so may not be especially helpful here? As for DBIx::Class::Schema::Versioned, I'm really not sure I understand what that would win for us (especially as the general recommendation in DBIx::Class land now is not to use it, but rather use DeploymentHandler instead). Koha's installer is actually pretty powerful as it is in my opinion and this is just one minor area we can improve.. please highlight some problems you feel such a move would resolve? Going back to the actual problem this patchset is trying to resolve.. can you remind me what it is about the SQL format files in the first place means we can't easily support translating them without moving to some intermediary file format?
> Going back to the actual problem this patchset is trying to resolve.. can > you remind me what it is about the SQL format files in the first place means > we can't easily support translating them without moving to some intermediary > file format? I think the problem is that you can't 'mark' what needs translation. You only want to translate a small bit of the SQL files, the descriptions while preventing people to translate values that should not be changed.
(In reply to Martin Renvoize from comment #17) > Going back to the actual problem this patchset is trying to resolve.. can > you remind me what it is about the SQL format files in the first place means > we can't easily support translating them without moving to some intermediary > file format? .sql files are not following the usual translation workflow. There is 1 file per language (in the term of language in installer/data/mysql/, so not so much compared to the number of languages we really support (POs). If you want to translate a sql file, you need to open a bug report, patch it, wait for someone to SO/QA/push. Also it's painful to support (dev POV), as if you want to add an AV, you have to edit X .sql files, test them, (rebase), etc. With only 1 sql file, at the root of installer dir (installer/data/mysq) the translation will be done using the usual workflow (after a change to our translator tool as we still do not support YAML file, but we do support .pref which are YAML, so should not be too hard)
Sorry.. I don't think my question was clear.. I'm totally onboard with wanting to replace the workflow we have and having one update file which then followed the standard translations workflow we have elsewhere.. What I wasn't fully understanding is why we needed to move from a lots of SQL files to one YAML/XML/Whatever file instead of a single SQL file which gets translated via the tools. Sounds like SQL is just too hard to parse as is for the existing translation tools.. that's the answer.
Created attachment 96370 [details] [review] Bug 13897: Use YAML files for installer data This patch modifies C4/Installer.pm to add support for loading YAML files into database. As an example of the functionality, optional auth_val.sql file is replaced by auth_val.yml The rationale behind this feature is to enable the translation of the data that is loaded into the database. That will be addressed in another bug. But taking into account that goal, translatable values are declared in the YAML files, to ease identification by translate script. To test: 0) Do a clean install with all optional data, then dump authorised_values table, reserve. 1) Apply the patch 2) Do a clean install in English (marc21/unimarc) 3) On optional data check for description of auth_val "Some basic default authorised values for ..." 4) Select all optional data 5) Finish installation 6) Dump again authorised_values table and compare with that of point '0'. Expected differences: most rows now have 'lib_opac' field with the same value as 'lib'
Great to have so many comment :) (In reply to David Cook from comment #12) > > Since this is installer data, I don't think it necessarily has to be fast? I > figure the most important part is to make it easy to read and update? Agreed, I think it's worth it > Actually, I don't know if we necessarily need to change C4/Installer.pm? > > We could use YAML as a declarative format for the database data, and then > the Release Manager/Release Maintainer could run a tool to compile it into a > SQL script. I think, as Katrin and Jonathan, that it's better not to compile sql, just use the files, then a change in C4/Installer.pm is needed. (In reply to Jonathan Druart from comment #16) > > > Why not write our own stuff, yml-based? > ... > I would: > - Retrieve and parse the whole YAML file > - Generate the SQL queries (like 1 per table) > - Execute the INSERT statements (using DBIx::RunSQL->run_sql) In the new patch I'm inserting directly. We can try to measure the delay, if any (In reply to Katrin Fischer from comment #18) > > Going back to the actual problem this patchset is trying to resolve.. can > > you remind me what it is about the SQL format files in the first place means > > we can't easily support translating them without moving to some intermediary > > file format? > > I think the problem is that you can't 'mark' what needs translation. You > only want to translate a small bit of the SQL files, the descriptions while > preventing people to translate values that should not be changed. Katrin is right, I couldn't find a general way to parse sql files to extract/replace strings. There are too many variations. The only one I'd made is the one used to generate translated frameworks, in translation server, that has different functions to extract/replace strings with 1,2,6,15,16 commas (,); and that after massaging EN frameworks sql files so they are exactly as expected. (In reply to Martin Renvoize from comment #20) > What I wasn't fully understanding is why we needed to move from a lots of > SQL files to one YAML/XML/Whatever file instead of a single SQL file which > gets translated via the tools. > > Sounds like SQL is just too hard to parse as is for the existing translation > tools.. that's the answer. It is hard, at last for me :( Thank you all! If you can, please take a look at the second patch. It uses YAML instead of XML, it's easier to read. I tried it and works. Do not install the first patch.
Looks great to me!
OK, that's lovely.. super simple to understand and write.. nice work Bernardo!
That looks a lot nicer! Since you're going with this approach, perhaps it would be worth turning off AutoCommit, running the INSERTs, and then COMMITing? According to the MySQL documentation, that should be higher performance. Probably easier to handle exceptions that way too. I wonder if we should add more error-handling in the eval{} block as well. I suppose any errors will be caught during $loadyaml->execute(), so maybe that's unnecessary. (Due to my nitpicker nature, I'd suggest $loadyaml be $load_sth instead, but that's really just nitpicking, so feel free to ignore it.) +1 Bernardo
Bernardo, I am all good with it, we could improve (performance) later (like one INSERT per file) if needed. Another improvement would be to move the .txt file into the YAML. I would like to see the translator tool ready before we push this one. Did you investigate already the work that needs to be done? I think we should move the already translated strings into a new .po (one .po for all installer files?) I can have a look if you did not already, but prefer to avoid duplication of work :)
Created attachment 96397 [details] [review] Bug 13897: Catch the error and warn it
Jonathan, thanks to change Bug description, planned to do it today :) (In reply to Jonathan Druart from comment #26) > I am all good with it, we could improve (performance) later (like one INSERT > per file) if needed. Ok > Another improvement would be to move the .txt file into the YAML. I thought about that too, which would allow it to be translated as well. I will add the content in a description field. > I would like to see the translator tool ready before we push this one. Did > you investigate already the work that needs to be done? Yes... for xml :) I will work on this in Bug 24262 > I think we should move the already translated strings into a new .po (one > .po for all installer files?) I agree, perhaps 'xx-YY-installer.po' for common files, and append marcflavor content for MARC21/UNIMARC/NORMARC to existing files, or add new ones. > I can have a look if you did not already, but prefer to avoid duplication of > work :) Thanks! I'll try to solve this and let you know. Now some questions: 1) Bug 22824 - This patch can work indistinctly with YAML, YAML::Syck or YAML::XS, no problem with that. 2) YAML format: I'm using --- table: translatable: { A: 1 } rows: - A: 1 B: 2 - A: 3 B: 4 With this format we have only one table per file, and all rows must have the same number of values ( required by prepare($query) ) If we define things like --- - firstTable: translatable: { A: 1 } rows: - A1: 1 B1: 2 - A2: 1 B2: 2 - firstTable: translatable: { A: 1 } rows: - A: 1 B: 2 C: 3 - A: 4 B: 5 C: 6 - secondTable: translatable: { G: 1 } rows: - F: 1 G: 2 - F: 3 G: 4 we can have entries for the same table with different number of values, and also entries for different tables inserted in order. (BTW, of current installer sql files: 30 just one table, 9 more than one table; common+en files ) A change in format needs a simple revision of the code, but I would like some opinions about the format.
What about the following structure: table_name: translatable: [ A, B ] fields [ A, B, C, D ] rows: - A: 1 B: 2 - A: 1 C: 2 D: 4 - A: 5 - B: 6 - C: 7 - D: 8 I do not think we should force to have the same number of fields. What if you want the default value for instance? That's not a big deal but I would prefer to get rid of this limitation since the beginning :)
(In reply to Jonathan Druart from comment #29) > What about the following structure: > ... > > I do not think we should force to have the same number of fields. What if > you want the default value for instance? That's not a big deal but I would > prefer to get rid of this limitation since the beginning :) Maybe I'm wrong but to do that we'll need to do a prepare/execute for each row, because we won't know which columns are necessary; and this could be slower ( https://docstore.mik.ua/orelly/linux/dbi/ch05_05.htm ) Columns must be absent on insert statement for them to use default values. It'll be equivalent to a $dbh->do() for each row. Or we can fill missing values with empty/NULL my @values = map { $row->{$_} ? $row->{$_} : NULL } @columns; for example, if we are going to use all column names in one prepare(). (do not like this) What about entries for many tables in a file? Note: I like your idea, simply don't know how to solve it.
Couldn't we just use the DEFAULT keyword to specify a default value? (I haven't tried it so just a suggestion.)
Created attachment 96539 [details] [review] Bug 13897: Use YAML files for installer data This patch modifies C4/Installer.pm to add support for loading YAML files into database. As an example of the functionality, optional auth_val.sql file is replaced by auth_val.yml The rationale behind this feature is to enable the translation of the data that is loaded into the database. That will be addressed in another bug. But taking into account that goal, translatable values are declared in the YAML files, to ease identification by translate script. To test: 0) Do a clean install with all optional data, then dump authorised_values table, reserve. 1) Apply the patch 2) Do a clean install in English (marc21/unimarc) 3) On optional data check for description of auth_val "Some basic default authorised values for ..." 4) Select all optional data 5) Finish installation 6) Dump again authorised_values table and compare with that of point '0'. No differences should be found.
Hi, new patch version taking into account comments. I introduce a new format for YAML, it permits more than one table per file, or same table with different number of columns. The example file is an exact translation of the sql one, same order and column number, as a way to exemplify it's format I also added a patch in Bug 24262, with it, this and Bug 18688, it's possible to translate and install in new language using it's own installer files. A lot more work is needed to enable the complete translation of installer files, but hope this can be considered as first step in that direction.
Created attachment 96591 [details] [review] Bug 13897: Use YAML files for installer data This patch modifies C4/Installer.pm to add support for loading YAML files into database. As an example of the functionality, optional auth_val.sql file is replaced by auth_val.yml The rationale behind this feature is to enable the translation of the data that is loaded into the database. That will be addressed in another bug. But taking into account that goal, translatable values are declared in the YAML files, to ease identification by translate script. Also file description is moved into the yaml file. To test: 0) Do a clean install with all optional data, then dump authorised_values table, reserve. 1) Apply the patch 2) Do a clean install in English (marc21/unimarc) 3) On optional data check for description of auth_val "Some basic default authorised values for ..." 4) Select all optional data 5) Finish installation 6) Dump again authorised_values table and compare with that of point '0'. No differences should be found.
Created attachment 96592 [details] [review] Bug 13897: Catch the error and warn it
Created attachment 96754 [details] [review] Bug 13897: Use YAML files for installer data This patch modifies C4/Installer.pm to add support for loading YAML files into database. As an example of the functionality, optional auth_val.sql file is replaced by auth_val.yml The rationale behind this feature is to enable the translation of the data that is loaded into the database. That will be addressed in another bug. But taking into account that goal, translatable values are declared in the YAML files, to ease identification by translate script. Also file description is moved into the yaml file. To test: 0) Do a clean install with all optional data, then dump authorised_values table, reserve. 1) Apply the patch 2) Do a clean install in English (marc21/unimarc) 3) On optional data check for description of auth_val "Some basic default authorised values for ..." 4) Select all optional data 5) Finish installation 6) Dump again authorised_values table and compare with that of point '0'. No differences should be found. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 96755 [details] [review] Bug 13897: Catch the error and warn it Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I really like this approach, thanks for working on it Bernardo.. signing off
Bernardo, should not we move it out of the 'en' directory? I think we should move it to installer/data/mysql/optional, then remove the other auth_val.sql files.
(In reply to Jonathan Druart from comment #39) > Bernardo, should not we move it out of the 'en' directory? > > I think we should move it to installer/data/mysql/optional, then remove the > other auth_val.sql files. It would not be a problem, if there were no descriptions to be translated. Currently we have localized versions, all translated ./de-DE/optional/auth_val.sql ./es-ES/optional/auth_val.sql ./it-IT/necessari/auth_val.sql ./pl-PL/optional/auth_val.sql ./nb-NO/2-Valgfritt/auth_val.sql ./fr-CA/facultatif/auth_val.sql with the same content, sort of.
Created attachment 97068 [details] [review] Bug 13897: Use YAML files for installer data This patch modifies C4/Installer.pm to add support for loading YAML files into database. As an example of the functionality, optional auth_val.sql file is replaced by auth_val.yml The rationale behind this feature is to enable the translation of the data that is loaded into the database. That will be addressed in another bug. But taking into account that goal, translatable values are declared in the YAML files, to ease identification by translate script. Also file description is moved into the yaml file. To test: 0) Do a clean install with all optional data, then dump authorised_values table, reserve. 1) Apply the patch 2) Do a clean install in English (marc21/unimarc) 3) On optional data check for description of auth_val "Some basic default authorised values for ..." 4) Select all optional data 5) Finish installation 6) Dump again authorised_values table and compare with that of point '0'. No differences should be found. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 97069 [details] [review] Bug 13897: Catch the error and warn it Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Jonathan Druart from comment #39) > Bernardo, should not we move it out of the 'en' directory? > > I think we should move it to installer/data/mysql/optional, then remove the > other auth_val.sql files. Should not this be done before pushing it?
(In reply to Jonathan Druart from comment #43) > (In reply to Jonathan Druart from comment #39) > > Bernardo, should not we move it out of the 'en' directory? > > > > I think we should move it to installer/data/mysql/optional, then remove the > > other auth_val.sql files. > > Should not this be done before pushing it? - Installer works as expected and strings are shown as before - QA script is happy ¡Excelente trabajo Pichu, para variar! Feliz 2020!!
(In reply to Jonathan Druart from comment #43) > (In reply to Jonathan Druart from comment #39) > > Bernardo, should not we move it out of the 'en' directory? > > > > I think we should move it to installer/data/mysql/optional, then remove the > > other auth_val.sql files. > > Should not this be done before pushing it? There are translated files for this. Are you suggesting those are moved into .yml?
From the YAML file: +description: + - "Some basic default authorised values for library locations, item lost status, etc." + - "You can change these at any time after installation." This is what need to be used for translation.
I think Jonathan is already thinking about this, so setting back to SO so it is clear the QA step is not finished. From my POV, it looks like ready.
(In reply to Jonathan Druart from comment #46) > From the YAML file: > +description: > + - "Some basic default authorised values for library locations, item lost > status, etc." > + - "You can change these at any time after installation." > > This is what need to be used for translation. Yes, of course. In Bug 24262 I catch those strings so they can be translated. In Comment #26 Jonathan suggested that txt file content could be moved into the yaml files, that's what I did Then I changed Bug 24262 accordingly C4/Installer.pm it's changed to show the description from the yaml, current behaviour is preserved.
Does this get your final PQA stamp now Jonathan? It looks like between the two bugs we have all that is required to me.
Created attachment 98507 [details] [review] Bug 13897: Process description, multiline values and SQL statements in YAML files This patch adds 3 features: 1) Display description of YAML files at install time for frameworks, and fixes it's encoding. 2) Enable use of multiline values, field required 3) Process SQL statements declared in YAML files With this features we can process files with the following generic YAML strucure: --- description: - "File description" tables: - table_name: translatable: [ title, content ] multiline: [ content ] rows: - title: "Example title" content: - "Content:" - "" - "This is the content." id: 1 value: ~ sql_statements: - "UPDATE table_name SET value ='empty' WHERE value IS NULL" ... * file description is now inside the YAML, can have multiple lines. This attribute is expected in all YAML files. * translatable attribute in table declare fields that can be translated * multiline attribute in table declare fields that can have multiple lines of content, they are joined using '\r\n' before insert into database. This is useful to express fields like 'news' content, and to simplify it's translation. '\r\n' is used for correct display in Windows clients. * sql_statements allows to add multiple SQL sentences, not insertions normally, that are executed in order. This features are not needed for the example file of this patch, but will be used in new bugs. To test: 1) Use the same test plan of first patch.
New patch added, needs sign.
Created attachment 98741 [details] [review] Bug 13897: Surround columns with backticks We have some column's names that need to be surrounded by ` as they are protected keywords ('rows' for instance)
Support for YAML files in misc4dev is available in this merge request: https://gitlab.com/koha-community/koha-misc4dev/-/merge_requests/37
Created attachment 99392 [details] [review] Bug 13897: Use YAML files for installer data This patch modifies C4/Installer.pm to add support for loading YAML files into database. As an example of the functionality, optional auth_val.sql file is replaced by auth_val.yml The rationale behind this feature is to enable the translation of the data that is loaded into the database. That will be addressed in another bug. But taking into account that goal, translatable values are declared in the YAML files, to ease identification by translate script. Also file description is moved into the yaml file. To test: 0) Do a clean install with all optional data, then dump authorised_values table, reserve. 1) Apply the patch 2) Do a clean install in English (marc21/unimarc) 3) On optional data check for description of auth_val "Some basic default authorised values for ..." 4) Select all optional data 5) Finish installation 6) Dump again authorised_values table and compare with that of point '0'. No differences should be found. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 99393 [details] [review] Bug 13897: Catch the error and warn it Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 99394 [details] [review] Bug 13897: Process description, multiline values and SQL statements in YAML files This patch adds 3 features: 1) Display description of YAML files at install time for frameworks, and fixes it's encoding. 2) Enable use of multiline values, field required 3) Process SQL statements declared in YAML files With this features we can process files with the following generic YAML strucure: --- description: - "File description" tables: - table_name: translatable: [ title, content ] multiline: [ content ] rows: - title: "Example title" content: - "Content:" - "" - "This is the content." id: 1 value: ~ sql_statements: - "UPDATE table_name SET value ='empty' WHERE value IS NULL" ... * file description is now inside the YAML, can have multiple lines. This attribute is expected in all YAML files. * translatable attribute in table declare fields that can be translated * multiline attribute in table declare fields that can have multiple lines of content, they are joined using '\r\n' before insert into database. This is useful to express fields like 'news' content, and to simplify it's translation. '\r\n' is used for correct display in Windows clients. * sql_statements allows to add multiple SQL sentences, not insertions normally, that are executed in order. This features are not needed for the example file of this patch, but will be used in new bugs. To test: 1) Use the same test plan of first patch. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 99395 [details] [review] Bug 13897: Surround columns with backticks We have some column's names that need to be surrounded by ` as they are protected keywords ('rows' for instance) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Great work Bernardo, thanks for your efforts here.. All looking good and working as expected for me.. Signing off.
Created attachment 100340 [details] [review] Bug 13897: Use YAML files for installer data This patch modifies C4/Installer.pm to add support for loading YAML files into database. As an example of the functionality, optional auth_val.sql file is replaced by auth_val.yml The rationale behind this feature is to enable the translation of the data that is loaded into the database. That will be addressed in another bug. But taking into account that goal, translatable values are declared in the YAML files, to ease identification by translate script. Also file description is moved into the yaml file. To test: 0) Do a clean install with all optional data, then dump authorised_values table, reserve. 1) Apply the patch 2) Do a clean install in English (marc21/unimarc) 3) On optional data check for description of auth_val "Some basic default authorised values for ..." 4) Select all optional data 5) Finish installation 6) Dump again authorised_values table and compare with that of point '0'. No differences should be found. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 100341 [details] [review] Bug 13897: Catch the error and warn it Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 100342 [details] [review] Bug 13897: Process description, multiline values and SQL statements in YAML files This patch adds 3 features: 1) Display description of YAML files at install time for frameworks, and fixes it's encoding. 2) Enable use of multiline values, field required 3) Process SQL statements declared in YAML files With this features we can process files with the following generic YAML strucure: --- description: - "File description" tables: - table_name: translatable: [ title, content ] multiline: [ content ] rows: - title: "Example title" content: - "Content:" - "" - "This is the content." id: 1 value: ~ sql_statements: - "UPDATE table_name SET value ='empty' WHERE value IS NULL" ... * file description is now inside the YAML, can have multiple lines. This attribute is expected in all YAML files. * translatable attribute in table declare fields that can be translated * multiline attribute in table declare fields that can have multiple lines of content, they are joined using '\r\n' before insert into database. This is useful to express fields like 'news' content, and to simplify it's translation. '\r\n' is used for correct display in Windows clients. * sql_statements allows to add multiple SQL sentences, not insertions normally, that are executed in order. This features are not needed for the example file of this patch, but will be used in new bugs. To test: 1) Use the same test plan of first patch. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 100343 [details] [review] Bug 13897: Surround columns with backticks We have some column's names that need to be surrounded by ` as they are protected keywords ('rows' for instance) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
enhancement not backported to 19.11.x