Bug 17469

Summary: fr-CA web installer is missing some sample notices
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: Installation and upgrade (web-based installer)Assignee: Caitlin Goodger <caitlingoodger.student>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: caitlingoodger.student, chris, francois.charbonnier, gmcharlt, jonathan.druart, katrin.fischer, kyle, mtj, mtompset
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17577
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 14622    
Bug Blocks:    
Attachments: Bug 17469: Add missing sample notices to fr-CA web installer
Bug 17469: Add missing sample notices to fr-CA web installer
Bug 17469: Add missing sample notices to fr-CA web installer
Bug 17469: Add missing sample notices to fr-CA web installer
Bug 17469: Add missing sample notices to fr-CA web installer
Bug 17469: Add missing sample notices to fr-CA web installer
Bug 17469: Add missing sample notices fr-CA test
Bug 17469: Add missing sample notices to fr-CA web installer
Bug 17469: Add missing sample notices fr-CA test
Bug 17469: Add missing sample notices to fr-CA web installer
Bug 17469: Follow-up adding hold print notice
Bug 17469: Follow-up adding hold print notice
Bug 17469: Add missing sample notices fr-CA test
[SIGNED OFF] Bug 17469: Add missing sample notices to fr-CA web installer
[SIGNED OFF] Bug 17469: Follow-up adding hold print notice
Bug 17469: Add missing sample notices fr-CA test
Bug 17469: Add missing sample notices to fr-CA web installer
Bug 17469: Follow-up adding hold print notice
Bug 17469: Fix number of values

Description Katrin Fischer 2016-10-19 21:57:46 UTC
xt/sample_notice.t fails when you add fr-CA to it, because the file is missing some of the sample notices: 

Sample notices to add in fr-CA/obligatoire/sample_notices.sql: HOLD_SLIP, MEMBERSHIP_EXPIRY, PASSWORD_RESET

Sample notices to delete in fr-CA/obligatoire/sample_notices.sql: RESERVESLIP
(was renamed to be HOLD_SLIP!)

The file and the test should be fixed!
Comment 1 Katrin Fischer 2016-10-19 22:16:49 UTC
Can't provide a French translation, but especially the missing HOLD_SLIP could be annoying. Proposing a fix in English for now.
Comment 2 Katrin Fischer 2016-10-19 22:17:39 UTC
Created attachment 56681 [details] [review]
Bug 17469: Add missing sample notices to fr-CA web installer

Adds missing notices to the fr-CA web installer:
- MEMBERSHIP_EXPIRY
- PASSWORD_RESET
Renames RESERVESLIP to HOLD_SLIP

Add fr-CA installer to xt/sample_letter.t

To test:
- Verify sample_notices.sql is correct SQL and loads cleanly
- Verify xt/sample_letter.t test checks fr-CA now and
  runs all tests successfully

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 3 Mark Tompsett 2017-01-16 15:42:37 UTC
#!/bin/bash

mysql -u root -p < fr-FR.sql > letter-FR.txt
mysql -u root -p < fr-CA.sql > letter-CA.txt
diff letter-FR.txt letter-CA.txt

use koha_library;
delete from letter;
source /home/mtompset/kohaclone/installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql;
select module,code,branchcode,is_html,message_transport_type from letter;

use koha_library;
delete from letter;
source /home/mtompset/kohaclone/installer/data/mysql/fr-CA/obligatoire/sample_notices.sql;
select module,code,branchcode,is_html,message_transport_type from letter;

mtompset@debian:~/kohaclone$ source check_letters.sh
Enter password:
Enter password:
12d11
< circulation   HOLD_SLIP               1       email
19a19
> circulation   RESERVESLIP             1       email

-- This is the rename Katrin noticed.

24d23
< members       MEMBERSHIP_EXPIRY               0       email
26d24
< members       PASSWORD_RESET          1       email

-- These are the two Katrin noticed missing.

31d28
< reserves      HOLD            0       print

-- There is one more you may wish to include, Katrin.
Comment 4 Mark Tompsett 2017-01-16 16:05:55 UTC
Bug 17577 seems to have triggered a conflict.
Comment 5 Mark Tompsett 2017-01-16 16:10:02 UTC
Created attachment 59019 [details] [review]
Bug 17469: Add missing sample notices to fr-CA web installer

Adds missing notices to the fr-CA web installer:
- MEMBERSHIP_EXPIRY
- PASSWORD_RESET
Renames RESERVESLIP to HOLD_SLIP

Add fr-CA installer to xt/sample_letter.t

To test:
- Verify sample_notices.sql is correct SQL and loads cleanly
- Verify xt/sample_letter.t test checks fr-CA now and
  runs all tests successfully

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Mark Tompsett 2017-01-16 16:12:08 UTC
Created attachment 59020 [details] [review]
Bug 17469: Add missing sample notices to fr-CA web installer

Adds missing notices to the fr-CA web installer:
- MEMBERSHIP_EXPIRY
- PASSWORD_RESET
Renames RESERVESLIP to HOLD_SLIP

Add fr-CA installer to xt/sample_letter.t

To test:
- Verify sample_notices.sql is correct SQL and loads cleanly
- Verify xt/sample_letter.t test checks fr-CA now and
  runs all tests successfully

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Mark Tompsett 2017-01-16 16:15:59 UTC
Created attachment 59021 [details] [review]
Bug 17469: Add missing sample notices to fr-CA web installer

Adds missing notices to the fr-CA web installer:
- MEMBERSHIP_EXPIRY
- PASSWORD_RESET
Renames RESERVESLIP to HOLD_SLIP

Add fr-CA installer to xt/sample_letter.t

To test:
- Verify sample_notices.sql is correct SQL and loads cleanly
- Verify xt/sample_letter.t test checks fr-CA now and
  runs all tests successfully

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Mark Tompsett 2017-01-16 16:22:18 UTC
Created attachment 59022 [details] [review]
Bug 17469: Add missing sample notices to fr-CA web installer

Add fr-CA installer to xt/sample_letter.t

To test:
- Verify xt/sample_letter.t test checks fr-CA now and
  fails

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 9 Mark Tompsett 2017-01-16 16:22:29 UTC
Created attachment 59023 [details] [review]
Bug 17469: Add missing sample notices to fr-CA web installer

Adds missing notices to the fr-CA web installer:
- MEMBERSHIP_EXPIRY
- PASSWORD_RESET
Renames RESERVESLIP to HOLD_SLIP

TEST PLAN
---------
1) Apply the first patch
2) prove xt/sample_notices.t
   -- it should fail.
3) Apply the second patch
4) prove xt/sample_notices.t
   -- it should pass.
5) run all tests (prove t; prove xt)
   -- they should generally pass
6) run koha qa test tools

NOTE: Split test patch from fix patch, so could prove problem
      exists easily.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 10 Mark Tompsett 2017-01-16 16:26:16 UTC
Created attachment 59024 [details] [review]
Bug 17469: Add missing sample notices fr-CA test

Add fr-CA installer to xt/sample_letter.t

To test:
- Verify xt/sample_letter.t test checks fr-CA now and
  fails for just fr-CA.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 11 Mark Tompsett 2017-01-16 16:26:28 UTC
Created attachment 59025 [details] [review]
Bug 17469: Add missing sample notices to fr-CA web installer

Adds missing notices to the fr-CA web installer:
- MEMBERSHIP_EXPIRY
- PASSWORD_RESET
Renames RESERVESLIP to HOLD_SLIP

TEST PLAN
---------
1) Apply the first patch
2) prove xt/sample_notices.t
   -- it should fail for fr-CA.
3) Apply the second patch
4) prove xt/sample_notices.t
   -- it should pass.
5) run all tests (prove t; prove xt)
   -- they should generally pass
6) run koha qa test tools

NOTE: Split test patch from fix patch, so could prove problem
      exists easily.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 12 Mark Tompsett 2017-01-16 16:29:23 UTC
Sorry for the excessive messages. I messed up splitting things. Just one this to clean up.

As per comment #3:
31d28
< reserves      HOLD            0       print

-- There is one more you may wish to include, Katrin.
Comment 13 Caitlin Goodger 2017-01-16 22:39:52 UTC
Created attachment 59033 [details] [review]
Bug 17469: Add missing sample notices fr-CA test

Add fr-CA installer to xt/sample_letter.t

To test:
- Verify xt/sample_letter.t test checks fr-CA now and
  fails for just fr-CA.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Caitlin Goodger <caitlingoodger.student@wegc.school.nz>
Comment 14 Caitlin Goodger 2017-01-16 22:40:05 UTC
Created attachment 59034 [details] [review]
Bug 17469: Add missing sample notices to fr-CA web installer

Adds missing notices to the fr-CA web installer:
- MEMBERSHIP_EXPIRY
- PASSWORD_RESET
Renames RESERVESLIP to HOLD_SLIP

TEST PLAN
---------
1) Apply the first patch
2) prove xt/sample_notices.t
   -- it should fail for fr-CA.
3) Apply the second patch
4) prove xt/sample_notices.t
   -- it should pass.
5) run all tests (prove t; prove xt)
   -- they should generally pass
6) run koha qa test tools

NOTE: Split test patch from fix patch, so could prove problem
      exists easily.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Caitlin Goodger <caitlingoodger.student@wegc.school.nz>
Comment 15 Mark Tompsett 2017-01-17 00:48:42 UTC
The reason I did not sign off was because of comment #3 which was purposefully repeated the outstanding part in comment #12.

Sorry but I'm unsigning it. There is still just one this one to clean up.

As per comment #3:
31d28
< reserves      HOLD            0       print

-- There is one more you may wish to include, Katrin.

It can be signed off, if there is a good reason to exclude this. However, that wasn't given.
Comment 16 Chris Cormack 2017-01-17 01:53:07 UTC
It's much nicer to not leave it as needs signoff if its not ready for needs signoff. The students and I will try to fix
Comment 17 Caitlin Goodger 2017-01-17 02:14:25 UTC
Created attachment 59048 [details] [review]
Bug 17469: Follow-up adding hold print notice
Comment 18 Mark Tompsett 2017-01-17 03:15:02 UTC
Comment on attachment 59048 [details] [review]
Bug 17469: Follow-up adding hold print notice

Review of attachment 59048 [details] [review]:
-----------------------------------------------------------------

Not sure which solution is best and most likely to be accepted by the community.

::: installer/data/mysql/fr-CA/obligatoire/sample_notices.sql
@@ +15,4 @@
>  ('orderacquisition','ACQORDER','','Acquisition order',0,'Order','<<aqbooksellers.name>>\r\n<<aqbooksellers.address1>>\r\n<<aqbooksellers.address2>>\r\n<<aqbooksellers.address3>>\r\n<<aqbooksellers.address4>>\r\n<<aqbooksellers.phone>>\r\n\r\nPlease order for the library:\r\n\r\n<order>Ordernumber <<aqorders.ordernumber>> (<<biblio.title>>) (quantity: <<aqorders.quantity>>) ($<<aqorders.listprice>> each).</order>\r\n\r\nThank you,\n\n<<branches.branchname>>', 'email'),
>  ('members','ACCTDETAILS','','Modèle de message pour les détails d\'un compte lecteur - DEFAULT',0,'Votre compte Koha','Bonjour <<borrowers.firstname>> <<borrowers.surname>>,\r\n\r\nLes informations pour vous connecter à votre compte lecteur sont :\r\n\r\nIdentifiant :  <<borrowers.userid>>\r\nMot de passe: <<borrowers.password>>\r\n\r\nSi vous rencontrez des problèmes ou avez des questions concernant votre compte, veuillez contacter l\'administrateur de Koha.\r\n\r\nMerci, \r\nL\'administrateur de Koha\r\nkohaadmin@yoursite.org','email'),
>  ('reserves','HOLD','','Réservation en attente de retrait',0,'Réservation en attente de retrait à <<branches.branchname>>','Bonjour <<borrowers.firstname>> <<borrowers.surname>>,\r\n\r\nVotre réservation est disponible depuis le <<reserves.waitingdate>> :\r\n\r\nTitre : <<biblio.title>>\r\nAuteur : <<biblio.author>>\r\nCopie : <<items.copynumber>>\r\nLocalisation : <<branches.branchname>>\r\n<<branches.branchaddress1>>\r\n<<branches.branchaddress2>>\r\n<<branches.branchaddress3>>\r\n<<branches.branchcity>> <<branches.branchzip>>\r\n\r\nNous vous invitons à venir emprunter votre document rapidement.\r\n\r\nMerci.\r\n\r\n<<branches.branchname>>','email'),
> +('reserves', 'HOLD', 'Réservation disponible pour retrait', 'Réservation disponible à la banque de retrait du site <<branches.branchname>>', '<<branches.branchname>>\n<<branches.branchaddress1>>\n<<branches.branchaddress2>>\n\n\nLe service des réservations\n\n\n\n\n\n\n\n<<borrowers.firstname>> <<borrowers.surname>>\n<<borrowers.address>>\n<<borrowers.city>> <<borrowers.zipcode>>\n\n\n\n\n\n\n\n\n\n\n<<borrowers.firstname>> <<borrowers.surname>> <<borrowers.cardnumber>>\n\nVotre réservation est disponible pour retrait depuis le <<reserves.waitingdate>>:\r\n\r\nTitre: <<biblio.title>>\r\nAuteur: <<biblio.author>>\r\nNuméro de fascicule: <<items.copynumber>>\r\n', 'print'),

Good try, but '' and 0 are missing, as the field lists between the fr-FR and fr-CA differ. A separate INSERT statement would be better, if you are going to cut and paste. Otherwise, make sure the fields line up. Another cut-and-paste idea would be just to copy the email HOLD above, and change it to 'print' on the end. I am sure the folks pushing for this will submit a fix on another bug.
Comment 19 Caitlin Goodger 2017-01-17 20:39:17 UTC
Created attachment 59100 [details] [review]
Bug 17469: Follow-up adding hold print notice
Comment 20 Katrin Fischer 2017-01-17 21:50:08 UTC
Hi Caitlin, thx for rescuing this puppy (... or camel? :) )
Comment 21 Katrin Fischer 2017-01-18 00:29:25 UTC
Created attachment 59125 [details] [review]
Bug 17469: Add missing sample notices fr-CA test

Add fr-CA installer to xt/sample_letter.t

To test:
- Verify xt/sample_letter.t test checks fr-CA now and
  fails for just fr-CA.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Caitlin Goodger <caitlingoodger.student@wegc.school.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Katrin Fischer 2017-01-18 00:29:30 UTC
Created attachment 59126 [details] [review]
[SIGNED OFF] Bug 17469: Add missing sample notices to fr-CA web installer

Adds missing notices to the fr-CA web installer:
- MEMBERSHIP_EXPIRY
- PASSWORD_RESET
Renames RESERVESLIP to HOLD_SLIP

TEST PLAN
---------
1) Apply the first patch
2) prove xt/sample_notices.t
   -- it should fail for fr-CA.
3) Apply the second patch
4) prove xt/sample_notices.t
   -- it should pass.
5) run all tests (prove t; prove xt)
   -- they should generally pass
6) run koha qa test tools

NOTE: Split test patch from fix patch, so could prove problem
      exists easily.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Caitlin Goodger <caitlingoodger.student@wegc.school.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2017-01-18 00:29:34 UTC
Created attachment 59127 [details] [review]
[SIGNED OFF] Bug 17469: Follow-up adding hold print notice

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2017-01-18 00:32:17 UTC
Signed off on the follow-ups - thx!
Comment 25 Chris Cormack 2017-01-18 03:43:02 UTC
Well done Caitlin.
Comment 26 Jonathan Druart 2017-01-19 12:59:13 UTC
Does not work for me:

sudo koha-mysql kohadev < installer/data/mysql/fr-CA/obligatoire/sample_notices.sql
ERROR 1062 (23000) at line 1: Duplicate entry 'circulation-CHECKIN--email' for key 'PRIMARY'
Comment 27 Jonathan Druart 2017-01-19 13:33:51 UTC
Created attachment 59246 [details] [review]
Bug 17469: Add missing sample notices fr-CA test

Add fr-CA installer to xt/sample_letter.t

To test:
- Verify xt/sample_letter.t test checks fr-CA now and
  fails for just fr-CA.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Caitlin Goodger <caitlingoodger.student@wegc.school.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 28 Jonathan Druart 2017-01-19 13:34:05 UTC
Created attachment 59247 [details] [review]
Bug 17469: Add missing sample notices to fr-CA web installer

Adds missing notices to the fr-CA web installer:
- MEMBERSHIP_EXPIRY
- PASSWORD_RESET
Renames RESERVESLIP to HOLD_SLIP

TEST PLAN
---------
1) Apply the first patch
2) prove xt/sample_notices.t
   -- it should fail for fr-CA.
3) Apply the second patch
4) prove xt/sample_notices.t
   -- it should pass.
5) run all tests (prove t; prove xt)
   -- they should generally pass
6) run koha qa test tools

NOTE: Split test patch from fix patch, so could prove problem
      exists easily.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Caitlin Goodger <caitlingoodger.student@wegc.school.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 29 Jonathan Druart 2017-01-19 13:34:19 UTC
Created attachment 59248 [details] [review]
Bug 17469: Follow-up adding hold print notice

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 30 Jonathan Druart 2017-01-19 13:34:33 UTC
Created attachment 59249 [details] [review]
Bug 17469: Fix number of values

Will fix
  ERROR 1136 (21S01) at line 57: Column count doesn't match value count at row 2

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 31 Jonathan Druart 2017-01-19 13:35:11 UTC
(In reply to Jonathan Druart from comment #26)
> Does not work for me:
> 
> sudo koha-mysql kohadev <
> installer/data/mysql/fr-CA/obligatoire/sample_notices.sql
> ERROR 1062 (23000) at line 1: Duplicate entry 'circulation-CHECKIN--email'
> for key 'PRIMARY'

Fixed with the last patch.
Comment 32 Kyle M Hall 2017-01-19 13:38:45 UTC
Pushed to master for Koha 17.05, thanks Katrin, Caitlin, Jonathan!
Comment 33 Katrin Fischer 2017-01-22 19:22:43 UTC
These patches have been pushed to 16.11.x and will be in 16.11.02.
.
Comment 34 Mason James 2017-01-27 03:11:06 UTC
Pushed to 16.05.x, for 16.05.08 release