In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Érythrée" (correct one) and "Allemagne".
Created attachment 58390 [details] [review] Bug 17809 - ER country twice in fr-FR database files In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Erythrée" (correct one) and "Allemagne". Test plan : - Create a new database using fr-FR or import the sql file - Look at authorised values COUNTRY => without patch you have ER / Allemange => with patch you have ER / ErythrÃe
Looking closer, please check: CC, ER, GQ, and PK as well.
And confirm qualif 370 values.
Going to sign off and put the issues raised in comment #2 into bug 17837. Comment #3 may not have any relevance, as the two french words clearly refer to different things and very well may be valid. I'll leave that to a native French speaker to determine.
Created attachment 58577 [details] [review] Bug 17809 - ER country twice in fr-FR database files In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Erythrée" (correct one) and "Allemagne". Test plan : - Create a new database using fr-FR or import the sql file - Look at authorised values COUNTRY => without patch you have ER / Allemange => with patch you have ER / ErythrÃe NOTE: Allemange occurs twice as well, correctly with DE. This patch correctly removes the ER one. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Frido, I think it would be great to have a update DB entry to check if this value exists. If it does, I do not think we should remove it, but at least warn the administrator that there is something wrong and something should be fixed.
(In reply to Jonathan Druart from comment #6) > Frido, I think it would be great to have a update DB entry to check if this > value exists. > If it does, I do not think we should remove it, but at least warn the > administrator that there is something wrong and something should be fixed. If I create a db update, simply replace in COUNTRY authorized value the description for "ER" into "Erythrée" if it is "Allemagne" ?
(In reply to Fridolin SOMERS from comment #7) > (In reply to Jonathan Druart from comment #6) > > Frido, I think it would be great to have a update DB entry to check if this > > value exists. > > If it does, I do not think we should remove it, but at least warn the > > administrator that there is something wrong and something should be fixed. > > If I create a db update, simply replace in COUNTRY authorized value the > description for "ER" into "Erythrée" if it is "Allemagne" ? Nope, I'd suggest just to display a warning message if (ER, Allemagne) exists.
(In reply to Jonathan Druart from comment #8) > Nope, I'd suggest just to display a warning message if (ER, Allemagne) > exists. So, don't correct it. Just give pretty red upgrade messages?
yep
Created attachment 59259 [details] [review] Bug 17809: Add upgrade warning messages TEST PLAN --------- 0) backup db 1) drop db and recreate empty 2) run fr-FR web install 3) update systempreferences set Value='16.00003' where variable='Version'; -- it is currently 16.00004 as of this writing 4) run fr-FR web install -- bottom of red warnings should be the check messages. 5) perlcritic -1 installer/data/mysql/atomic_update/bug_17809_pretty_red.perl -- $VERSION message, which we don't care about -- maybe a message about return value of print ignored.
Created attachment 59261 [details] [review] Bug 17809 correct authorisation values for fr-FR This corrects: CC -- duplicate, but name not full. GQ -- duplicate, but only one name correct. PK -- Poland was mislabelled. This removes and edits duplicates and corrects PK to PL. Signed-off-by: Luke Honiss <luke.honiss@gmail.com>
Created attachment 59262 [details] [review] Bug 17809 - ER country twice in fr-FR database files In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Erythrée" (correct one) and "Allemagne". Test plan : - Create a new database using fr-FR or import the sql file - Look at authorised values COUNTRY => without patch you have ER / Allemange => with patch you have ER / ErythrÃe NOTE: Allemange occurs twice as well, correctly with DE. This patch correctly removes the ER one. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
*** Bug 17837 has been marked as a duplicate of this bug. ***
The first patch written by me needs signing off. The second patch written by me was signed off by Luke Honiss. The third patch written by Fridolin SOMERS was signed off by me. In short, I am setting this to needs sign off, because of the first patch.
I am worried about the atomicupdate - it looks like only checks for the authorised value code, but not for the category, so this might lead to wrong warning messages. Some of those codes look like classifications we might have as collections in our databases for example. + my $number_cc = $schema->resultset('AuthorisedValue')->search( + { + authorised_value => 'CC' + } + )->count; + if ( $number_cc > 1 ) {
(In reply to Katrin Fischer from comment #16) > I am worried about the atomicupdate - it looks like only checks for the > authorised value code, but not for the category, so this might lead to wrong > warning messages. Some of those codes look like classifications we might > have as collections in our databases for example. > > > + my $number_cc = $schema->resultset('AuthorisedValue')->search( > + { > + authorised_value => 'CC' > + } > + )->count; > + if ( $number_cc > 1 ) { Ideed. M. Tompsett can you work on it ? I do it if you want.
(In reply to Fridolin SOMERS from comment #17) > (In reply to Katrin Fischer from comment #16) > > I am worried about [SNIP] > Indeed. > M. Tompsett can you work on it ? > I do it if you want. I'm not sure how to correct it. So if it is clear to you. You go ahead.
Created attachment 72099 [details] [review] Bug 17809: (follow-up) search in COUNTRY category
Someone has to sign-off the last 2 patches.
Created attachment 72842 [details] [review] Bug 17809: Add upgrade warning messages TEST PLAN --------- 0) backup db 1) drop db and recreate empty 2) run fr-FR web install 3) update systempreferences set Value='16.00003' where variable='Version'; -- it is currently 16.00004 as of this writing 4) run fr-FR web install -- bottom of red warnings should be the check messages. 5) perlcritic -1 installer/data/mysql/atomic_update/bug_17809_pretty_red.perl -- $VERSION message, which we don't care about -- maybe a message about return value of print ignored. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 72843 [details] [review] Bug 17809 correct authorisation values for fr-FR This corrects: CC -- duplicate, but name not full. GQ -- duplicate, but only one name correct. PK -- Poland was mislabelled. This removes and edits duplicates and corrects PK to PL. Signed-off-by: Luke Honiss <luke.honiss@gmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 72844 [details] [review] Bug 17809 - ER country twice in fr-FR database files In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Erythrée" (correct one) and "Allemagne". Test plan : - Create a new database using fr-FR or import the sql file - Look at authorised values COUNTRY => without patch you have ER / Allemange => with patch you have ER / ErythrÃe NOTE: Allemange occurs twice as well, correctly with DE. This patch correctly removes the ER one. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 72845 [details] [review] Bug 17809: (follow-up) search in COUNTRY category Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Hi Frido, please make sure to write your patch subjects according to the guidelines: Bug XXXXX: ...
(In reply to Katrin Fischer from comment #25) > Hi Frido, please make sure to write your patch subjects according to the > guidelines: Bug XXXXX: ... Oups yes I'm aware of that now ;)
Created attachment 73107 [details] [review] Bug 17809: Add upgrade warning messages TEST PLAN --------- 0) backup db 1) drop db and recreate empty 2) run fr-FR web install 3) update systempreferences set Value='16.00003' where variable='Version'; -- it is currently 16.00004 as of this writing 4) run fr-FR web install -- bottom of red warnings should be the check messages. 5) perlcritic -1 installer/data/mysql/atomic_update/bug_17809_pretty_red.perl -- $VERSION message, which we don't care about -- maybe a message about return value of print ignored. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 73108 [details] [review] Bug 17809: Correct authorised values for fr-FR This corrects: CC -- duplicate, but name not full. GQ -- duplicate, but only one name correct. PK -- Poland was mislabelled. This removes and edits duplicates and corrects PK to PL. Signed-off-by: Luke Honiss <luke.honiss@gmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 73109 [details] [review] Bug 17809: Fix ER country appearing twice in fr-FR database files In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Erythrée" (correct one) and "Allemagne". Test plan : - Create a new database using fr-FR or import the sql file - Look at authorised values COUNTRY => without patch you have ER / Allemange => with patch you have ER / ErythrÃe NOTE: Allemange occurs twice as well, correctly with DE. This patch correctly removes the ER one. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 73110 [details] [review] Bug 17809: (follow-up) search in COUNTRY category Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
$schema must not be used in updatedatabase.pl, use SQL queries instead Use warn instead of carp (like other entries).
Should we remove the draft status from here? https://wiki.koha-community.org/wiki/Database_updates#DBIx::Class_.28DRAFT.29
(In reply to Katrin Fischer from comment #32) > Should we remove the draft status from here? > https://wiki.koha-community.org/wiki/Database_updates#DBIx::Class_.28DRAFT.29 Done
What next ?
Waiting for concerns on comment 31 to be addressed.
(In reply to Jonathan Druart from comment #35) > Waiting for concerns on comment 31 to be addressed. Oh indeed. @Mark Tompsett tell me I you can't work on rewrite.
Authorised value CC for Coco/Keeling was fixed by Bug 22887
Ah but now during upgrade 19.12.00.071 Bug 22887, duplicates are detected. So we may only correct the sql file ?
Frido, can you deal with this please?
Created attachment 113304 [details] [review] Bug 17809: Correct authorised values for fr-FR This corrects: CC -- duplicate, but name not full. GQ -- duplicate, but only one name correct. PK -- Poland was mislabelled. This removes and edits duplicates and corrects PK to PL. Signed-off-by: Luke Honiss <luke.honiss@gmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 113305 [details] [review] Bug 17809: Fix ER country appearing twice in fr-FR database files In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Erythrée" (correct one) and "Allemagne". Test plan : - Create a new database using fr-FR or import the sql file - Look at authorised values COUNTRY => without patch you have ER / Allemange => with patch you have ER / ErythrÃe NOTE: Allemange occurs twice as well, correctly with DE. This patch correctly removes the ER one. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
(In reply to Fridolin SOMERS from comment #38) > Ah but now during upgrade 19.12.00.071 Bug 22887, duplicates are detected. > So we may only correct the sql file ? In order to move forward with this bug, it only correct the sql files.
root@kohadevbox:koha(BZ17809|BISECTING)$ koha-mysql kohadev < installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql ERROR 1062 (23000) at line 613: Duplicate entry 'qualif-370' for key 'authorised_values.av_uniq' Still not working.
Created attachment 114713 [details] [review] Bug 17809: FR installer: fix duplicate authorised value qualif 370 Monteur vs Réalisateur
With those patches, reset_all_unimarc finishes without errors in koha-testing-docker :D
Created attachment 114758 [details] [review] Bug 17809: Correct authorised values for fr-FR This corrects: CC -- duplicate, but name not full. GQ -- duplicate, but only one name correct. PK -- Poland was mislabelled. This removes and edits duplicates and corrects PK to PL. Signed-off-by: Luke Honiss <luke.honiss@gmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 114759 [details] [review] Bug 17809: Fix ER country appearing twice in fr-FR database files In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Erythrée" (correct one) and "Allemagne". Test plan : - Create a new database using fr-FR or import the sql file - Look at authorised values COUNTRY => without patch you have ER / Allemange => with patch you have ER / ErythrÃe NOTE: Allemange occurs twice as well, correctly with DE. This patch correctly removes the ER one. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 114760 [details] [review] Bug 17809: FR installer: fix duplicate authorised value qualif 370 Monteur vs Réalisateur Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Thanks a lot Victor ;) Hope it will soon be QA
Created attachment 114792 [details] [review] Bug 17809: Correct authorised values for fr-FR This corrects: CC -- duplicate, but name not full. GQ -- duplicate, but only one name correct. PK -- Poland was mislabelled. This removes and edits duplicates and corrects PK to PL. Signed-off-by: Luke Honiss <luke.honiss@gmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 114795 [details] [review] Bug 17809: Correct authorised values for fr-FR This corrects: CC -- duplicate, but name not full. GQ -- duplicate, but only one name correct. PK -- Poland was mislabelled. This removes and edits duplicates and corrects PK to PL. Signed-off-by: Luke Honiss <luke.honiss@gmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 114796 [details] [review] Bug 17809: Fix ER country appearing twice in fr-FR database files In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Erythrée" (correct one) and "Allemagne". Test plan : - Create a new database using fr-FR or import the sql file - Look at authorised values COUNTRY => without patch you have ER / Allemange => with patch you have ER / ErythrÃe NOTE: Allemange occurs twice as well, correctly with DE. This patch correctly removes the ER one. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 114797 [details] [review] Bug 17809: FR installer: fix duplicate authorised value qualif 370 Monteur vs Réalisateur Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.01
Missing dependencies, not backported to 20.05
There is no dependency on bug 27331 right? It's related but the two bugs address different issues.
(In reply to Victor Grousset/tuxayo from comment #57) > There is no dependency on bug 27331 right? It's related but the two bugs > address different issues. To test this on master and 20.11 you need it. For other branches you don't.