From bug 18811 comment40: It sounds like there is something else to fix: On a new DB: MariaDB [koha_kohadev]> select tagfield, tagsubfield, hidden from auth_subfield_structure where tagfield=551 and tagsubfield=9 and authtypecode="GEOGR_NAME"\G *************************** 1. row *************************** tagfield: 551 tagsubfield: 9 hidden: 1 Edit the subfields for GEOGR_NAME /admin/auth_subfields_structure.pl?op=add_form&tagfield=551&authtypecode=GEOGR_NAME Possible value for "Select to display or not"/hidden are -5 and 0, so "Show all" is displayed. Save without modifying anything: hidden is now '0'.
Created attachment 70842 [details] [review] Bug 20074: Fix hidden value in auth_subfields_structure The template included an else branch where a value not equal to 0 or -5 was interpreted as 0 (Show all). This effectively converted hidden values 1 and 8 to 0. This patch removes that else branch. Test plan: Toggle a authority field between Hide all and Show all. Check what happens. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 70843 [details] [review] Bug 20074: Database revision to convert hidden to -5 The hidden value is used as a boolean in authority frameworks (in contrast with biblio framework). It is consistent to apply the same value (-5) to all non-zero values. Test plan: Check if you a have 0, 1 and 8 value in hidden somewhere. Run the dbrev. Verify that 1 and 8 are converted to -5. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Another improvement would be to correct the sql installer files. See bug 20075
Created attachment 70847 [details] [review] Bug 20074: (Follow-up) Remove another remainder of biblio hidden logic Script authorities.pl still contains a module 2 calculation on the hidden attribute, although it is used as a boolean. Since -5 mod 2 == 1, it does no harm, but we better remove it. Test plan: Edit and save an authority. Verify that everything still works as expected.
Created attachment 71926 [details] [review] Bug 20074: Fix hidden value in auth_subfields_structure The template included an else branch where a value not equal to 0 or -5 was interpreted as 0 (Show all). This effectively converted hidden values 1 and 8 to 0. This patch removes that else branch. Test plan: Toggle a authority field between Hide all and Show all. Check what happens. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 71927 [details] [review] Bug 20074: Database revision to convert hidden to -5 The hidden value is used as a boolean in authority frameworks (in contrast with biblio framework). It is consistent to apply the same value (-5) to all non-zero values. Test plan: Check if you a have 0, 1 and 8 value in hidden somewhere. Run the dbrev. Verify that 1 and 8 are converted to -5. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 71928 [details] [review] Bug 20074: (Follow-up) Remove another remainder of biblio hidden logic Script authorities.pl still contains a module 2 calculation on the hidden attribute, although it is used as a boolean. Since -5 mod 2 == 1, it does no harm, but we better remove it. Test plan: Edit and save an authority. Verify that everything still works as expected. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 72191 [details] [review] Bug 20074: Fix hidden value in auth_subfields_structure The template included an else branch where a value not equal to 0 or -5 was interpreted as 0 (Show all). This effectively converted hidden values 1 and 8 to 0. This patch removes that else branch. Test plan: Toggle a authority field between Hide all and Show all. Check what happens. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 72192 [details] [review] Bug 20074: Database revision to convert hidden to -5 The hidden value is used as a boolean in authority frameworks (in contrast with biblio framework). It is consistent to apply the same value (-5) to all non-zero values. Test plan: Check if you a have 0, 1 and 8 value in hidden somewhere. Run the dbrev. Verify that 1 and 8 are converted to -5. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 72193 [details] [review] Bug 20074: (Follow-up) Remove another remainder of biblio hidden logic Script authorities.pl still contains a module 2 calculation on the hidden attribute, although it is used as a boolean. Since -5 mod 2 == 1, it does no harm, but we better remove it. Test plan: Edit and save an authority. Verify that everything still works as expected. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
The database update will take care of the problem for existing installations, but we will reintroduce the problem in every new installation as long as bug 20075 is not fixed. Maybe we should hold off the database update here or perform it a second time with the fix for the frameworks?
(In reply to Katrin Fischer from comment #11) > The database update will take care of the problem for existing > installations, but we will reintroduce the problem in every new installation > as long as bug 20075 is not fixed. Maybe we should hold off the database > update here or perform it a second time with the fix for the frameworks? Thx Katrin. The problem we solve here, is not reintroduced since we improved the logic. I guess it is still possible to get 20075 also into 18.05, but we could repeat the db update on 20075 too altough is has only theoretical value for purists.
(In reply to Marcel de Rooy from comment #12) > (In reply to Katrin Fischer from comment #11) > > The database update will take care of the problem for existing > > installations, but we will reintroduce the problem in every new installation > > as long as bug 20075 is not fixed. Maybe we should hold off the database > > update here or perform it a second time with the fix for the frameworks? > > Thx Katrin. > The problem we solve here, is not reintroduced since we improved the logic. > I guess it is still possible to get 20075 also into 18.05, but we could > repeat the db update on 20075 too altough is has only theoretical value for > purists. Poor choice of words - "reintroduce unclean data" would have been better :)
(In reply to Katrin Fischer from comment #13) > (In reply to Marcel de Rooy from comment #12) > > (In reply to Katrin Fischer from comment #11) > > > The database update will take care of the problem for existing > > > installations, but we will reintroduce the problem in every new installation > > > as long as bug 20075 is not fixed. Maybe we should hold off the database > > > update here or perform it a second time with the fix for the frameworks? > > > > Thx Katrin. > > The problem we solve here, is not reintroduced since we improved the logic. > > I guess it is still possible to get 20075 also into 18.05, but we could > > repeat the db update on 20075 too altough is has only theoretical value for > > purists. > > Poor choice of words - "reintroduce unclean data" would have been better :) OK. I submitted a few patches on 20075 to obtain clean data.
Created attachment 72209 [details] [review] Bug 20074: (Follow-up) Replace hidden value -5 by 1 in data and interface [ Moved from 20075 to 20074 ] Since hidden is used as a boolean, it makes more sense to save the values 0 and 1 instead of 0 and -5. Test plan: Test toggling between Show all and Hide all in the auth frameworks. Run the db rev. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
If you agree, Jonathan, take the last patch along coming from 20075. It fits better here..
Pushed to master for 18.05, thanks to everybody involved!
Created attachment 128400 [details] Add syspref to deal with overdues notification I took the same test plan as victor but I added the system preference to manage the case more easily, especially for users who do not have access to the koha server. Test plan 1. Check the size of the message queue With the following SQL query (using an SQL report if you want) SELECT COUNT(*) FROM message_queue; 2. Run misc/cronjobs/overdue_notices.pl 3. Check the size of the message queue To ensure that no other overdues will create noise in this test plan. Or you can take them into account. 4. Choose a patron with no email address 5. Create an overdue (checkout an item and unfold "Checkout settings" and set a date in the past which is compatible with what you find in staff:/cgi-bin/koha/tools/overduerules.pl 6. Run misc/cronjobs/overdue_notices.pl 7. Check that you have two new messages in the queue 8. Inspect these two messages SELECT * FROM message_queue ORDER BY time_queued DESC LIMIT 2 \G 1. One has the type "print" and the borrowernumber matching the patron. 2. The other has subject: Overdue Notices borrowernumber: NULL message_transport_type: email and contains "These messages were not sent directly to the patrons." This is the one we don't want anymore. Because it's now obsolete due to the first message. 9. Apply this patch 10. Change syspref 'EmailOverduesNoEmail' to "Don't send" 11. Delete data from message_queue (if you have access) for a cleaner view 12. Run again misc/cronjobs/overdue_notices.pl 13. Check that only the print message is now generated and not the "Overdue Notices" one.
Sorry for disturbing I miss my last bug attachement with git bz :/