Bug 17809 - Correct some authorised values in fr-FR
Summary: Correct some authorised values in fr-FR
Status: ASSIGNED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin SOMERS
QA Contact: Testopia
URL:
Keywords:
: 17837 (view as bug list)
Depends on: 22887
Blocks: 17837
  Show dependency treegraph
 
Reported: 2016-12-22 15:02 UTC by Fridolin SOMERS
Modified: 2020-06-30 08:27 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 17809 - ER country twice in fr-FR database files (1.83 KB, patch)
2016-12-22 15:07 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 17809 - ER country twice in fr-FR database files (1.99 KB, patch)
2017-01-03 18:32 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 17809: Add upgrade warning messages (2.53 KB, patch)
2017-01-19 14:46 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 17809 correct authorisation values for fr-FR (3.98 KB, patch)
2017-01-19 14:46 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 17809 - ER country twice in fr-FR database files (1.99 KB, patch)
2017-01-19 14:46 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 17809: (follow-up) search in COUNTRY category (1.58 KB, patch)
2018-02-23 07:50 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 17809: Add upgrade warning messages (2.59 KB, patch)
2018-03-14 12:01 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17809 correct authorisation values for fr-FR (4.04 KB, patch)
2018-03-14 12:01 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17809 - ER country twice in fr-FR database files (2.05 KB, patch)
2018-03-14 12:01 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17809: (follow-up) search in COUNTRY category (1.65 KB, patch)
2018-03-14 12:01 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 17809: Add upgrade warning messages (2.65 KB, patch)
2018-03-20 20:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17809: Correct authorised values for fr-FR (4.10 KB, patch)
2018-03-20 20:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17809: Fix ER country appearing twice in fr-FR database files (2.13 KB, patch)
2018-03-20 20:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17809: (follow-up) search in COUNTRY category (1.71 KB, patch)
2018-03-20 20:34 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin SOMERS 2016-12-22 15:02:37 UTC
In installer/data/mysql/fr-FR/1-Obligatoire/authorised_values.sql, ER is defined twice, for "Érythrée" (correct one) and "Allemagne".
Comment 1 Fridolin SOMERS 2016-12-22 15:07:22 UTC Comment hidden (obsolete)
Comment 2 M. Tompsett 2016-12-22 19:02:46 UTC
Looking closer, please check: CC, ER, GQ, and PK as well.
Comment 3 M. Tompsett 2016-12-22 19:09:13 UTC
And confirm qualif 370 values.
Comment 4 M. Tompsett 2017-01-03 18:27:00 UTC
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.
Comment 5 M. Tompsett 2017-01-03 18:32:02 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2017-01-11 08:55:29 UTC
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.
Comment 7 Fridolin SOMERS 2017-01-11 12:47:44 UTC
(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" ?
Comment 8 Jonathan Druart 2017-01-11 13:30:47 UTC
(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.
Comment 9 M. Tompsett 2017-01-11 19:22:48 UTC
(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?
Comment 10 Jonathan Druart 2017-01-11 19:45:57 UTC
yep
Comment 11 M. Tompsett 2017-01-19 14:46:08 UTC
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.
Comment 12 M. Tompsett 2017-01-19 14:46:18 UTC
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>
Comment 13 M. Tompsett 2017-01-19 14:46:35 UTC
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>
Comment 14 M. Tompsett 2017-01-19 14:47:29 UTC
*** Bug 17837 has been marked as a duplicate of this bug. ***
Comment 15 M. Tompsett 2017-01-19 14:49:35 UTC
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.
Comment 16 Katrin Fischer 2017-05-12 05:41:24 UTC
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 ) {
Comment 17 Fridolin SOMERS 2018-01-31 12:40:32 UTC
(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.
Comment 18 M. Tompsett 2018-02-01 03:06:07 UTC
(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.
Comment 19 Fridolin SOMERS 2018-02-23 07:50:20 UTC
Created attachment 72099 [details] [review]
Bug 17809: (follow-up) search in COUNTRY category
Comment 20 Fridolin SOMERS 2018-02-23 07:53:18 UTC
Someone has to sign-off the last 2 patches.
Comment 21 Brendan Gallagher 2018-03-14 12:01:37 UTC
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>
Comment 22 Brendan Gallagher 2018-03-14 12:01:50 UTC
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>
Comment 23 Brendan Gallagher 2018-03-14 12:01:54 UTC
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>
Comment 24 Brendan Gallagher 2018-03-14 12:01:58 UTC
Created attachment 72845 [details] [review]
Bug 17809: (follow-up) search in COUNTRY category

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 25 Katrin Fischer 2018-03-18 22:03:52 UTC
Hi Frido, please make sure to write your patch subjects according to the guidelines: Bug XXXXX: ...
Comment 26 Fridolin SOMERS 2018-03-19 13:09:09 UTC
(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 ;)
Comment 27 Katrin Fischer 2018-03-20 20:34:24 UTC
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>
Comment 28 Katrin Fischer 2018-03-20 20:34:28 UTC
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>
Comment 29 Katrin Fischer 2018-03-20 20:34:31 UTC
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>
Comment 30 Katrin Fischer 2018-03-20 20:34:35 UTC
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>
Comment 31 Jonathan Druart 2018-03-26 18:10:15 UTC
$schema must not be used in updatedatabase.pl, use SQL queries instead

Use warn instead of carp (like other entries).
Comment 32 Katrin Fischer 2018-03-27 20:06:41 UTC
Should we remove the draft status from here?
https://wiki.koha-community.org/wiki/Database_updates#DBIx::Class_.28DRAFT.29
Comment 33 Jonathan Druart 2018-03-27 20:22:50 UTC
(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
Comment 34 Fridolin SOMERS 2018-06-12 12:16:30 UTC
What next ?
Comment 35 Jonathan Druart 2018-06-12 12:55:53 UTC
Waiting for concerns on comment 31 to be addressed.
Comment 36 Fridolin SOMERS 2018-06-18 13:52:12 UTC
(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.
Comment 37 Fridolin SOMERS 2020-06-30 08:24:59 UTC
Authorised value CC for Coco/Keeling was fixed by Bug 22887
Comment 38 Fridolin SOMERS 2020-06-30 08:27:32 UTC
Ah but now during upgrade 19.12.00.071 Bug 22887, duplicates are detected.
So we may only correct the sql file ?