[Koha-bugs] [Bug 24594] Rewrite MARC21 mandatory data to YAML

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24594

--- Comment #6 from Bernardo Gonzalez Kriegel <bgkriegel at gmail.com> ---
Hi Jonathan, my reply

(In reply to Jonathan Druart from comment #5)
> 1. There are some QA issues:
>  FAIL  
> installer/data/mysql/en/marcflavour/marc21/mandatory/
> authorities_normal_marc21.yml                                               
> 
>    FAIL   yaml_valid                                                        
> YAML Error: Inconsistent indentation level                                  

Yes, I was aware of this, I left notes on the commit messages.
Current QA test uses YAML.pm, it considers as valid this:
---
data:
  - "a b c d"
...
but not this:
---
data:
  - "a b 
     c d"
...
however it is valid :(

The problem arises in some multiline sql_statements. The solution is to put
them as long single lines. It makes readability difficult, but I can do it.
Using YAML::Sick or YAML::XS gives no error, but implies changes in QA tools.
As I said, I can change them if you prefer.


> 
> 2. I did an install 'en' before and after the patchset, I mysqldump --tab to
> generate 2 files per table, then compared the 2 generated directories with
> `diff -r --brief` (and also a `perl -p -i -e "s/.*Dump completed.*//" **/*`
> to remove timestamp of the dump).
> Most of them are wrong diff because of different timestamps, but some of
> them are either weird, or caught issues:
> 
> a) When a row is on several lines, I get, for instance with letter
> "TO_PROCESS":
> 652,657c652,657
> \ suggestions	TO_PROCESS		Notify fund owner	0	A suggestion is ready to be
> processed	Dear <<borrowers.firstname>> <<borrowers.surname>>,
> \ 
> \ A new suggestion is ready to be processed: <<suggestions.title>> by
> <<suggestions.author>>.
> \ 
> \ Thank you,
> \ 
> ---
> > suggestions	TO_PROCESS		Notify fund owner	0	A suggestion is ready to be processed	Dear <<borrowers.firstname>> <<borrowers.surname>>,\
> > \
> > A new suggestion is ready to be processed: <<suggestions.title>> by <<suggestions.author>>.\
> > \
> > Thank you,\
> > \
> 
> Without the patch:
> 656 Thank you,\
> With the patch:
> 656 Thank you,^M\
> 
> I am not sure how relevant is this, the UI looks correct.

Some of the sample notices where clearly generated in Windows, I assume this
because they have '\r\n' for newlines. Some others not.
So I decided to join all multiline fields with both chars. I left a comment on
the last patch of Bug 13897
"'\r\n' is used for correct display in Windows clients."
in the code 
"? join "\r\n", @{$row->{$col}}                # join multiline values"

The diff you see comes from those notices that originally have only '\n' for
newlines.

> 
> b) matcher_matchpoints is different:
> without:
> 1 1
> 2 2
> 3 3
> with:
> 3 1
> 3 2
> 3 3

Yes, my bad. The sql_statements make use of MAX() to insert values, then the
result depends on the order of insertions.
I will add a followup to Bug 24593 to fix the problem.

> 
> d) subscription_numberpatterns (to investigate)
> 
> 1c1
> < 1	Number	1	Simple Numbering method	No. {X}	Number	1	1	99999	1	\N		\N	\N	\N
> \N	\N		\N	\N	\N	\N	\N
> ---
> > 1	Number	1	Simple Numbering method	No. {X}	Number	1	1	99999	1	\N	\N	\N	\N	\N	\N	\N	\N	\N	\N	\N	\N	\N
...
> 

The difference in this case comes from the values of label2 & label3 fields.
In the SQL files some of these fields are NULL, in YAML they can't because they
are translated, and the translate script will fail, so I put on them an empty
string "". I think that this change does not affect the numbered patterns.
If a NULL value is needed, then I must change the translate script
(LangInstaller.pm)


> e) systempreferences (to ignore)
> There is an obvious change here on "FrameworksLoaded", the list of sql files
> is replaced with a list of yml files.
> I did not know this pref existed, it seems to be used to make sure we are
> not going to load twice the same files.
> We can ignore this one.

Ok!

Summary:
1) Can be rewritten to single lines if needed
2a) I think is not a problem
2b) Followup uploaded to Bug 24593
2d) Waiting your opinion

Thanks!

-- 
You are receiving this mail because:
You are watching all bug changes.