Description
Jonathan Druart
2016-12-12 16:22:14 UTC
Created attachment 58133 [details] [review] Bug 17762: DB changes - Add new 'lang' columns to letter and borrowers tables Sponsored-by: Orex Digital Created attachment 58134 [details] [review] Bug 17762: DBIC Schema changes Sponsored-by: Orex Digital Created attachment 58135 [details] [review] Bug 17762: Add the new TranslateNotices syspref This whole patch set adds the ability to translate notices in different languages. That way, if a library has patrons from different countries, they will not have to send the notices in only one language but they will be able to translate them. Test plan: - Switch on TranslateNotices - Translate the interface in different languages - Edit patron's details => You should see a new "prefered language" entry That language will be used to send notices to this patron - Edit several notice templates => You should see different tabs, 1 tab per language translated at the opac (which sounds to make sense) - Now the notice generated for this patron should be the one translated in the language picked for him To confirm that, try to generate several notices for this patron. If the notice is not translated, the default one should be used. Sponsored-by: Orex Digital Created attachment 58136 [details] [review] Bug 17762: Add the lang attribute to the patron form Not the opac because we do not want the patron to modify it, they won't be necessary translated. Sponsored-by: Orex Digital Created attachment 58137 [details] [review] Bug 17762: Remove FK on message_transports I don't really understand where does come from this foreign key but at first glance it does not make sense. Note that it seems we could also remove message_transports.branchcode Sponsored-by: Orex Digital Created attachment 58138 [details] [review] Bug 17762: Add lang to the PK of letter We need to enlarge the PK of the letter table. Sponsored-by: Orex Digital Created attachment 58139 [details] [review] Bug 17762: Update the letter form interface If the pref is on, the notice template will be translatable in different languages Sponsored-by: Orex Digital Created attachment 58140 [details] [review] Bug 17762: Send lang to GetPreparedLetter This patch set the lang parameter when C4::Letters::GetPreparedLetter is called to generate the notice. Note that we do not need to pass it if want_librarian is set. TODO: I do not know what to do with TransferSlip Sponsored-by: Orex Digital Created attachment 58141 [details] [review] Bug 17762: Add the lang parameter to C4::Letters::getletter Sponsored-by: Orex Digital Hi After applying the patch I get an internal Server error and I can see this log i plack-error.log DBD::mysql::st execute failed: Unknown column 'me.lang' in 'field list' [for Statement "SELECT `me`.`borrowernumber`, `me`.`cardnumber`, `me`.`surname`, `me`.`firstname`, `me`.`title`, `me`.`othernames`, `me`.`initials`, `me`.`streetnumber`, `me`.`streettype`, `me`.`address`, `me`.`address2`, `me`.`city`, `me`.`state`, `me`.`zipcode`, `me`.`country`, `me`.`email`, `me`.`phone`, `me`.`mobile`, `me`.`fax`, `me`.`emailpro`, `me`.`phonepro`, `me`.`B_streetnumber`, `me`.`B_streettype`, `me`.`B_address`, `me`.`B_address2`, `me`.`B_city`, `me`.`B_state`, `me`.`B_zipcode`, `me`.`B_country`, `me`.`B_email`, `me`.`B_phone`, `me`.`dateofbirth`, `me`.`branchcode`, `me`.`categorycode`, `me`.`dateenrolled`, `me`.`dateexpiry`, `me`.`gonenoaddress`, `me`.`lost`, `me`.`debarred`, `me`.`debarredcomment`, `me`.`contactname`, `me`.`contactfirstname`, `me`.`contacttitle`, `me`.`guarantorid`, `me`.`borrowernotes`, `me`.`relationship`, `me`.`sex`, `me`.`password`, `me`.`flags`, `me`.`userid`, `me`.`opacnote`, `me`.`contactnote`, `me`.`sort1`, `me`.`sort2`, `me`.`altcontactfirstname`, `me`.`altcontactsurname`, `me`.`altcontactaddress1`, `me`.`altcontactaddress2`, `me`.`altcontactaddress3`, `me`.`altcontactstate`, `me`.`altcontactzipcode`, `me`.`altcontactcountry`, `me`.`altcontactphone`, `me`.`smsalertnumber`, `me`.`sms_provider_id`, `me`.`privacy`, `me`.`privacy_guarantor_checkouts`, `me`.`checkprevcheckout`, `me`.`updated_on`, `me`.`lastseen`, `me`.`lang` FROM `borrowers` `me` WHERE ( `userid` = ? )" with ParamValues: 0='Odigital'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. DBIx::Class::Storage::DBI::_dbh_execute(): Unknown column 'me.lang' in 'field list' at /home/vagrant/kohaclone/Koha/Objects.pm line 176 (In reply to Hugo Agud from comment #10) > After applying the patch I get an internal Server error and I can see this > log i plack-error.log Hi Hugo, You need to apply patches from bug 17234 (not the 3.14 patch) and bug 17666. Then you have to execute the updatedatabase script (`perl installer/data/mysql/updatedatabase.pl`) to update the DB structure. Then restart plack and everything should go fine. I have installed the patch and I have been able to test with PREDUE and ODUE and it almost works fine. I am able to receive the letter in the prefered language, BUT if I change the prefered language it has no effect on the letter language I have restarted memcached, plack and the kohadevbox with no effect on the language lettter (In reply to Hugo Agud from comment #12) Hi Hugo, thanks for testing! > I am able to receive the letter in the prefered language, BUT if I change > the prefered language it has no effect on the letter language Can you detail the different steps you made? You should not need to restart plack or memcached. Thanks to you.. I have applied your test plan and then change the language preference for notices and start again Test plan: - Switch on TranslateNotices - Translate the interface in different languages - Edit patron's details => You should see a new "prefered language" entry That language will be used to send notices to this patron - Edit several notice templates - Now the notice generated for this patron should be the one translated in the language picked for him To confirm that, try to generate several notices for this patron. If the notice is not translated, the default one should be used. Once I finished this with success, I decided to change the 'Preferred language for notices' to ca-ES and I have tested in different ways 1. Execute again the cron scripts for advance and overdue, it generate the letter in the firs language I seleted (es-ES) 2. I checked-in and checked-out again the books, and execute again the cron scripts overudue and advance notice and the letters still in (es-ES) instead of the new (ca-ES) I have restarted memcached and plack and the server just in case.. I hope it helps Created attachment 58525 [details] [review] Bug 17762: Fix sql fields insertion The "Insert ->" buttons used to copy the sql field to the textarea was based on the id of the fieldset. This id has to be unique to work as expected. It is now composed of mtt + lang instead of mtt only checked and working fine! Thanks Jonathan Jonathan, could you add followup due to bug 16239? Updated wiki page for reference: https://wiki.koha-community.org/wiki/Interface_patterns Thanks Ah, looks like it probably will need rebase Created attachment 59452 [details] [review] Bug 17762: DB changes - Add new 'lang' columns to letter and borrowers tables Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 59453 [details] [review] Bug 17762: DBIC Schema changes Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 59454 [details] [review] Bug 17762: Add the new TranslateNotices syspref This whole patch set adds the ability to translate notices in different languages. That way, if a library has patrons from different countries, they will not have to send the notices in only one language but they will be able to translate them. Test plan: - Switch on TranslateNotices - Translate the interface in different languages - Edit patron's details => You should see a new "prefered language" entry That language will be used to send notices to this patron - Edit several notice templates => You should see different tabs, 1 tab per language translated at the opac (which sounds to make sense) - Now the notice generated for this patron should be the one translated in the language picked for him To confirm that, try to generate several notices for this patron. If the notice is not translated, the default one should be used. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 59455 [details] [review] Bug 17762: Add the lang attribute to the patron form Not the opac because we do not want the patron to modify it, they won't be necessary translated. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 59456 [details] [review] Bug 17762: Remove FK on message_transports I don't really understand where does come from this foreign key but at first glance it does not make sense. Note that it seems we could also remove message_transports.branchcode Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 59457 [details] [review] Bug 17762: Add lang to the PK of letter We need to enlarge the PK of the letter table. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 59458 [details] [review] Bug 17762: Update the letter form interface If the pref is on, the notice template will be translatable in different languages Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 59459 [details] [review] Bug 17762: Send lang to GetPreparedLetter This patch set the lang parameter when C4::Letters::GetPreparedLetter is called to generate the notice. Note that we do not need to pass it if want_librarian is set. TODO: I do not know what to do with TransferSlip Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 59460 [details] [review] Bug 17762: Add the lang parameter to C4::Letters::getletter Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 59461 [details] [review] Bug 17762: Fix sql fields insertion The "Insert ->" buttons used to copy the sql field to the textarea was based on the id of the fieldset. This id has to be unique to work as expected. It is now composed of mtt + lang instead of mtt only Signed-off-by: Hugo Agud <hagud@orex.es> Jonathan, I like the idea and how it is implemented. My only doubt is about the existence of the 'default' tab. I expected 'default' to be a given language, not a different one than (say) Ensligh. It's default in the sense that it will fallback to this notice if the one we want is not defined. Created attachment 61614 [details] [review] Bug 17762: DB changes - Add new 'lang' columns to letter and borrowers tables Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 61615 [details] [review] Bug 17762: Add the new TranslateNotices syspref This whole patch set adds the ability to translate notices in different languages. That way, if a library has patrons from different countries, they will not have to send the notices in only one language but they will be able to translate them. Test plan: - Switch on TranslateNotices - Translate the interface in different languages - Edit patron's details => You should see a new "prefered language" entry That language will be used to send notices to this patron - Edit several notice templates => You should see different tabs, 1 tab per language translated at the opac (which sounds to make sense) - Now the notice generated for this patron should be the one translated in the language picked for him To confirm that, try to generate several notices for this patron. If the notice is not translated, the default one should be used. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 61616 [details] [review] Bug 17762: Add the lang attribute to the patron form Not the opac because we do not want the patron to modify it, they won't be necessary translated. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 61617 [details] [review] Bug 17762: Remove FK on message_transports I don't really understand where does come from this foreign key but at first glance it does not make sense. Note that it seems we could also remove message_transports.branchcode Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 61618 [details] [review] Bug 17762: Add lang to the PK of letter We need to enlarge the PK of the letter table. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 61619 [details] [review] Bug 17762: Update the letter form interface If the pref is on, the notice template will be translatable in different languages Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 61620 [details] [review] Bug 17762: Send lang to GetPreparedLetter This patch set the lang parameter when C4::Letters::GetPreparedLetter is called to generate the notice. Note that we do not need to pass it if want_librarian is set. TODO: I do not know what to do with TransferSlip Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 61621 [details] [review] Bug 17762: Add the lang parameter to C4::Letters::getletter Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 61622 [details] [review] Bug 17762: Fix sql fields insertion The "Insert ->" buttons used to copy the sql field to the textarea was based on the id of the fieldset. This id has to be unique to work as expected. It is now composed of mtt + lang instead of mtt only Signed-off-by: Hugo Agud <hagud@orex.es> Created attachment 61623 [details] [review] Bug 17762: Fix test for NewsChannels.t Created attachment 61624 [details] [review] Bug 17762: DBIC Schema changes Rebased and fixed a test. Created attachment 63199 [details] [review] Bug 17762: DB changes - Add new 'lang' columns to letter and borrowers tables Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63200 [details] [review] Bug 17762: Add the new TranslateNotices syspref This whole patch set adds the ability to translate notices in different languages. That way, if a library has patrons from different countries, they will not have to send the notices in only one language but they will be able to translate them. Test plan: - Switch on TranslateNotices - Translate the interface in different languages - Edit patron's details => You should see a new "prefered language" entry That language will be used to send notices to this patron - Edit several notice templates => You should see different tabs, 1 tab per language translated at the opac (which sounds to make sense) - Now the notice generated for this patron should be the one translated in the language picked for him To confirm that, try to generate several notices for this patron. If the notice is not translated, the default one should be used. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63201 [details] [review] Bug 17762: Add the lang attribute to the patron form Not the opac because we do not want the patron to modify it, they won't be necessary translated. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63202 [details] [review] Bug 17762: Remove FK on message_transports I don't really understand where does come from this foreign key but at first glance it does not make sense. Note that it seems we could also remove message_transports.branchcode Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63203 [details] [review] Bug 17762: Add lang to the PK of letter We need to enlarge the PK of the letter table. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63204 [details] [review] Bug 17762: Update the letter form interface If the pref is on, the notice template will be translatable in different languages Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63205 [details] [review] Bug 17762: Send lang to GetPreparedLetter This patch set the lang parameter when C4::Letters::GetPreparedLetter is called to generate the notice. Note that we do not need to pass it if want_librarian is set. TODO: I do not know what to do with TransferSlip Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63206 [details] [review] Bug 17762: Add the lang parameter to C4::Letters::getletter Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63207 [details] [review] Bug 17762: Fix sql fields insertion The "Insert ->" buttons used to copy the sql field to the textarea was based on the id of the fieldset. This id has to be unique to work as expected. It is now composed of mtt + lang instead of mtt only Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63208 [details] [review] Bug 17762: Fix test for NewsChannels.t Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63209 [details] [review] Bug 17762: DBIC Schema changes Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 63210 [details] [review] Bug 17762 (QA Followup) Use default lang if pref disabled Created attachment 63211 [details] [review] Bug 17762 (QA Followup) Fix Letters.t - add lang to fixtures All looks good, I added a followup so that we don't send translated notices if the pref is off. One question, you have: a/members/moremember.pl 335 # Display the language description instead of the code 336 # Note that this is certainly wrong It seems to work, why is it wrong? (In reply to Nick Clemens from comment #56) > All looks good, I added a followup so that we don't send translated notices > if the pref is off. > > One question, you have: > a/members/moremember.pl > 335 # Display the language description instead of the code > 336 # Note that this is certainly wrong > > It seems to work, why is it wrong? I think we should display the description in the translated language. For instance "Spanish" is always "Español", whatever the language of the interface (same as the language list in the footer). It sounds wrong to me. Pushed to master for 17.05, thanks Jonathan, Nick! Very much looking forward to this! This won't get ported back to 16.11.x as it is an enhancement. *** Bug 7224 has been marked as a duplicate of this bug. *** *** Bug 17160 has been marked as a duplicate of this bug. *** |