Bug 17499 adds Koha-objects for messaging preferences.
Created attachment 63435 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t
Created attachment 63436 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences.
Created attachment 63437 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences.
Created attachment 63438 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass
Created attachment 63439 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences Unit tests: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Run t/db_dependent/Koha/Patron/Categories.t
Created attachment 63440 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference Move default values for days in advance and wants_digest outside set method, because otherwise it would conflict with the standard way of how set works in Koha-objects (one may accidentally reset those parameters because they don't pass them and excpect them to remain unchanged).
Created attachment 63441 [details] [review] Bug 18595: Remove references to C4::Members::Messaging
I attached the patches that I was working on last year. I would like to point out that the work was left unfinished. Especially the patches without a proper test plan are probably somewhat untested and need more testing. There could be new occurrences to C4::Members::Messaging that also need to be handled by these patches. Please keep this in mind if testing. I remember there was some bug related to category messaging preference view and I'm not sure if that got fixed. I was hoping someone in the community finds this work useful and wants to take it to the finish line. Feel free to steal this Bug or help fixing issues by follow-ups.
Created attachment 63794 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t
Created attachment 63795 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences.
Created attachment 63796 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences.
Created attachment 63797 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass
Created attachment 63798 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences This patch moves C4::Members::Messaging::GetMessagingPreferences to Koha::Patron::Message::Preference(s) The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingPreferences" (grep -r 'GetMessagingPreferences'). 3. Observe no results in source code. ---- INTRANET ---- 1. todo ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t
Created attachment 63799 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- INTRANET ---- 1. todo ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t
Created attachment 63800 [details] [review] Bug 18595: Remove references to C4::Members::Messaging $ grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. To test: 1. Check the pages touched by this commit 2. Try setting and viewing patron and patron category messaging preferences Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t
(In reply to Lari Taskula from comment #8) > I remember there was some bug related to category messaging > preference view and I'm not sure if that got fixed. Couldn't reproduce issues regarding patron category messaging preferences anymore. Switched status to Needs Signoff, but please see my thoughts in comment #8
(In reply to Lari Taskula from comment #4) > Created attachment 63438 [details] [review] [review] > Bug 18595: Unit tests preparing for GetMessagingPreferences move > > This patch prepares removing GetMessagingPreferences by adding some useful > unit > tests for subroutines that the move targets. > > Reserves.t already covers print notice, but this patch adds tests for a set > transport type. > > To test: > 1. Run t/db_dependent/Circulation.t > 2. Run t/db_dependent/Reserves.t > 3. Observe test pass prove -v t/db_dependent/Circulation.t does not pass I get: # Subtest: SendCirculationAlert test 1..4 # No tests run! not ok 96 - No tests run for subtest "SendCirculationAlert test" t/db_dependent/Reserves.t returns green
Created attachment 63813 [details] [review] Bug 18595: (follow-up) Fix failing test in Circulation.t To test: 1. prove t/db_dependent/Circulation.t
(In reply to Lari Taskula from comment #18) > Created attachment 63813 [details] [review] [review] > Bug 18595: (follow-up) Fix failing test in Circulation.t > > To test: > 1. prove t/db_dependent/Circulation.t Test passes now...
Went through all patches / test plans, no issues detected after the followup-fix Will sign off
Created attachment 63815 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63816 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63817 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63818 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass Works together with followup patch Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63819 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences This patch moves C4::Members::Messaging::GetMessagingPreferences to Koha::Patron::Message::Preference(s) The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingPreferences" (grep -r 'GetMessagingPreferences'). 3. Observe no results in source code. ---- INTRANET ---- 1. todo ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63820 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- INTRANET ---- 1. todo ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63821 [details] [review] Bug 18595: Remove references to C4::Members::Messaging $ grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. To test: 1. Check the pages touched by this commit 2. Try setting and viewing patron and patron category messaging preferences Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63822 [details] [review] Bug 18595: (follow-up) Fix failing test in Circulation.t To test: 1. prove t/db_dependent/Circulation.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63823 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- INTRANET ---- 1. todo ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63824 [details] [review] Bug 18595: Remove references to C4::Members::Messaging $ grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. To test: 1. Check the pages touched by this commit 2. Try setting and viewing patron and patron category messaging preferences Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63825 [details] [review] Bug 18595: (follow-up) Fix failing test in Circulation.t To test: 1. prove t/db_dependent/Circulation.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63826 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- INTRANET ---- 1. todo ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch> QA tools pass now OK Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63827 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- INTRANET ---- 1. todo ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch> QA tools pass now OK Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63828 [details] [review] Bug 18595: Remove references to C4::Members::Messaging $ grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. To test: 1. Check the pages touched by this commit 2. Try setting and viewing patron and patron category messaging preferences Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 63829 [details] [review] Bug 18595: (follow-up) Fix failing test in Circulation.t To test: 1. prove t/db_dependent/Circulation.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69441 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69442 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69443 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69444 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69445 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69446 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69447 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69448 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69449 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass Works together with followup patch Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69450 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences This patch moves C4::Members::Messaging::GetMessagingPreferences to Koha::Patron::Message::Preference(s) The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingPreferences" (grep -r 'GetMessagingPreferences'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69451 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch> QA tools pass now OK Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69452 [details] [review] Bug 18595: Remove references to C4::Members::Messaging To test: 1. grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. 2. Check the pages touched by this commit 3. Try setting and viewing patron and patron category messaging preferences As this completes the move from C4::Members::Messaging to new Koha-objects for messaging preferences, everything should be ready for proper testing. Make sure you have EnhancedMessagingPreferences system preference enabled. Then, try creating new categories and setting default messaging preferences, as well as editing the messaging preferences of the already existing categories. Try adding new patrons and make sure the default patron messaging preferences are by default the same that you set for that category. Edit already exisiting patrons' messaging preferences and make sure changes are applied. Observe no crash at any point, otherwise fail this patch. Don't forget to test both intranet and OPAC. Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 69453 [details] [review] Bug 18595: Disable digest checkbox when forced on To test: 1. Force a digest on via message_transports table (delete rows where message_attribute is 1 and is_digest is 0) 2. Go to either OPAC or staff client messaging preferences 3. Observe digest checkbox is disabled for message attribute 1 (normally Item due row)
Rebased on updated Bug 17499 and added one more patch.
Created attachment 69667 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 69668 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 69669 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 69670 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass Works together with followup patch Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 69671 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences This patch moves C4::Members::Messaging::GetMessagingPreferences to Koha::Patron::Message::Preference(s) The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingPreferences" (grep -r 'GetMessagingPreferences'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 69672 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch> QA tools pass now OK Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 69673 [details] [review] Bug 18595: Remove references to C4::Members::Messaging To test: 1. grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. 2. Check the pages touched by this commit 3. Try setting and viewing patron and patron category messaging preferences As this completes the move from C4::Members::Messaging to new Koha-objects for messaging preferences, everything should be ready for proper testing. Make sure you have EnhancedMessagingPreferences system preference enabled. Then, try creating new categories and setting default messaging preferences, as well as editing the messaging preferences of the already existing categories. Try adding new patrons and make sure the default patron messaging preferences are by default the same that you set for that category. Edit already exisiting patrons' messaging preferences and make sure changes are applied. Observe no crash at any point, otherwise fail this patch. Don't forget to test both intranet and OPAC. Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 69674 [details] [review] Bug 18595: Disable digest checkbox when forced on To test: 1. Force a digest on via message_transports table (delete rows where message_attribute is 1 and is_digest is 0) 2. Go to either OPAC or staff client messaging preferences 3. Observe digest checkbox is disabled for message attribute 1 (normally Item due row) Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Starting QA now...
Hello Lari, Koha::Patron::Message::Preferences package is missing Koha::Patron::set_default_messaging_preferences method is missing You use Koha::Patron::Message::Attributes but only in unit tests. And package is missing. Also, don't forget to provide unit tests for all new subroutine created.
Conflicts in tools/import_borrowers.pl
Created attachment 93873 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 93874 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 93875 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 93876 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass Works together with followup patch Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 93877 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences This patch moves C4::Members::Messaging::GetMessagingPreferences to Koha::Patron::Message::Preference(s) The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingPreferences" (grep -r 'GetMessagingPreferences'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 93878 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch> QA tools pass now OK Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 93879 [details] [review] Bug 18595: Remove references to C4::Members::Messaging To test: 1. grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. 2. Check the pages touched by this commit 3. Try setting and viewing patron and patron category messaging preferences As this completes the move from C4::Members::Messaging to new Koha-objects for messaging preferences, everything should be ready for proper testing. Make sure you have EnhancedMessagingPreferences system preference enabled. Then, try creating new categories and setting default messaging preferences, as well as editing the messaging preferences of the already existing categories. Try adding new patrons and make sure the default patron messaging preferences are by default the same that you set for that category. Edit already exisiting patrons' messaging preferences and make sure changes are applied. Observe no crash at any point, otherwise fail this patch. Don't forget to test both intranet and OPAC. Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 93880 [details] [review] Bug 18595: Disable digest checkbox when forced on To test: 1. Force a digest on via message_transports table (delete rows where message_attribute is 1 and is_digest is 0) SQL: delete from message_transports where message_attribute_id=1 and is_digest=0; 2. Go to either OPAC or staff client messaging preferences 3. Observe digest checkbox is disabled for message attribute 1 (normally Item due row) Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Resolved a bunch of conflicts, need sign-offs.
Created attachment 93908 [details] [review] Bug 18595: Validate days_in_advance
Created attachment 94753 [details] [review] Bug 18595: Disable digest checkbox when forced on To test: 1. Force a digest on via message_transports table (delete rows where message_attribute is 1 and is_digest is 0) SQL: delete from message_transports where message_attribute_id=1 and is_digest=0; 2. Go to either OPAC or staff client messaging preferences 3. Observe digest checkbox is disabled for message attribute 1 (normally Item due row) Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 94754 [details] [review] Bug 18595: Validate days_in_advance
I've tried to rebase the patch but ended up with error: sha1 information is lacking or useless (C4/Auth_with_ldap.pm).
Created attachment 103444 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 103445 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 103446 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 103447 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass Works together with followup patch Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 103448 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences This patch moves C4::Members::Messaging::GetMessagingPreferences to Koha::Patron::Message::Preference(s) The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingPreferences" (grep -r 'GetMessagingPreferences'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 103449 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch> QA tools pass now OK Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 103450 [details] [review] Bug 18595: Remove references to C4::Members::Messaging To test: 1. grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. 2. Check the pages touched by this commit 3. Try setting and viewing patron and patron category messaging preferences As this completes the move from C4::Members::Messaging to new Koha-objects for messaging preferences, everything should be ready for proper testing. Make sure you have EnhancedMessagingPreferences system preference enabled. Then, try creating new categories and setting default messaging preferences, as well as editing the messaging preferences of the already existing categories. Try adding new patrons and make sure the default patron messaging preferences are by default the same that you set for that category. Edit already exisiting patrons' messaging preferences and make sure changes are applied. Observe no crash at any point, otherwise fail this patch. Don't forget to test both intranet and OPAC. Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 103451 [details] [review] Bug 18595: Disable digest checkbox when forced on To test: 1. Force a digest on via message_transports table (delete rows where message_attribute is 1 and is_digest is 0) SQL: delete from message_transports where message_attribute_id=1 and is_digest=0; 2. Go to either OPAC or staff client messaging preferences 3. Observe digest checkbox is disabled for message attribute 1 (normally Item due row) Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 103452 [details] [review] Bug 18595: Validate days_in_advance
The dependent patch needs a little bit of touching up - moving this to BLOCKED until we have taken care of it.
These have a ton of conflicts with the rebased patches for bug 17499 I was just looking it. I assume it would make sense to test them together - could these be rebased?
(In reply to Katrin Fischer from comment #84) > These have a ton of conflicts with the rebased patches for bug 17499 I was > just looking it. I assume it would make sense to test them together - could > these be rebased? The dependent bug is PQA now and Joubu has looked over it... would be a great moment to get the ball rolling here again as well :)
Nice to see these moving on. Unfortunately I am currently unavailable to contribute further and have removed myself as assignee. If someone wants to finish the work here, it would be greatly appreciated.
Created attachment 110151 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t
Created attachment 110152 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 110153 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 110154 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass Works together with followup patch Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 110155 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences This patch moves C4::Members::Messaging::GetMessagingPreferences to Koha::Patron::Message::Preference(s) The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingPreferences" (grep -r 'GetMessagingPreferences'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 110156 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch> QA tools pass now OK Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 110157 [details] [review] Bug 18595: Remove references to C4::Members::Messaging To test: 1. grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. 2. Check the pages touched by this commit 3. Try setting and viewing patron and patron category messaging preferences As this completes the move from C4::Members::Messaging to new Koha-objects for messaging preferences, everything should be ready for proper testing. Make sure you have EnhancedMessagingPreferences system preference enabled. Then, try creating new categories and setting default messaging preferences, as well as editing the messaging preferences of the already existing categories. Try adding new patrons and make sure the default patron messaging preferences are by default the same that you set for that category. Edit already exisiting patrons' messaging preferences and make sure changes are applied. Observe no crash at any point, otherwise fail this patch. Don't forget to test both intranet and OPAC. Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 110158 [details] [review] Bug 18595: Disable digest checkbox when forced on To test: 1. Force a digest on via message_transports table (delete rows where message_attribute is 1 and is_digest is 0) SQL: delete from message_transports where message_attribute_id=1 and is_digest=0; 2. Go to either OPAC or staff client messaging preferences 3. Observe digest checkbox is disabled for message attribute 1 (normally Item due row) Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Created attachment 110159 [details] [review] Bug 18595: Validate days_in_advance
Rebased and tested against current master and bug 17499.
Hi Emmi, thx for adopting this one!
Hi Emmi, thanks for the rebases. I did code review and everything is looking really good! I have some trouble setting up my koha-testing-docker right now so I still need to go over the test plan for these patches. Could you meanwhile a) Add your Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com> git trailer so that there will be a "paper trail" of you doing changes, now it basically reads that Lari did everything and also that the people signing-off here would have signed-off these changes but in actuality they have not. Please Add the Rebased-by trailer after the signed-off-by lines so that we know they were tested in a earlier revision and give credit for the people doing that. b) Rebase one more time. There was minor merge conflict with one of the patches.
Created attachment 112147 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 112148 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::Message::Preferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 112149 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 112150 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass Works together with followup patch Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 112151 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences This patch moves C4::Members::Messaging::GetMessagingPreferences to Koha::Patron::Message::Preference(s) The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingPreferences" (grep -r 'GetMessagingPreferences'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 112152 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::Message::Preference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch> QA tools pass now OK Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 112153 [details] [review] Bug 18595: Remove references to C4::Members::Messaging To test: 1. grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. 2. Check the pages touched by this commit 3. Try setting and viewing patron and patron category messaging preferences As this completes the move from C4::Members::Messaging to new Koha-objects for messaging preferences, everything should be ready for proper testing. Make sure you have EnhancedMessagingPreferences system preference enabled. Then, try creating new categories and setting default messaging preferences, as well as editing the messaging preferences of the already existing categories. Try adding new patrons and make sure the default patron messaging preferences are by default the same that you set for that category. Edit already exisiting patrons' messaging preferences and make sure changes are applied. Observe no crash at any point, otherwise fail this patch. Don't forget to test both intranet and OPAC. Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 112154 [details] [review] Bug 18595: Disable digest checkbox when forced on To test: 1. Force a digest on via message_transports table (delete rows where message_attribute is 1 and is_digest is 0) SQL: delete from message_transports where message_attribute_id=1 and is_digest=0; 2. Go to either OPAC or staff client messaging preferences 3. Observe digest checkbox is disabled for message attribute 1 (normally Item due row) Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 112155 [details] [review] Bug 18595: Validate days_in_advance Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
(In reply to Joonas Kylmälä from comment #98) > Hi Emmi, > > thanks for the rebases. I did code review and everything is looking really > good! I have some trouble setting up my koha-testing-docker right now so I > still need to go over the test plan for these patches. Could you meanwhile > > a) Add your Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com> git > trailer so that there will be a "paper trail" of you doing changes, now it > basically reads that Lari did everything and also that the people > signing-off here would have signed-off these changes but in actuality they > have not. Please Add the Rebased-by trailer after the signed-off-by lines so > that we know they were tested in a earlier revision and give credit for the > people doing that. > > b) Rebase one more time. There was minor merge conflict with one of the > patches. Rebased and fixed minor merge conflict with tests. Added git trailer after sign-offs (followed instruction here: https://wiki.koha-community.org/wiki/Commit_messages)
Some things need a bit more work: If no patron email we get error =============================== 1. Add patron without email 2. Go to cgi-bin/koha/opac-messaging.pl and select email checkbox for every messaging setting 3. Observe following error: > Patron has not set email address, cannot use email as message transport at /usr/share/perl5/Exception/Class/Base.pm line 88 > 1. in Exception::Class::Base::throw at /usr/share/perl5/Exception/Class/Base.pm line 88 > 2. in Koha::Patron::Message::Preference::_set_message_transport_types at /kohadevbox/koha/Koha/Patron/Message/Preference.pm line 409 > 3. in Koha::Patron::Message::Preference::message_transport_types at /kohadevbox/koha/Koha/Patron/Message/Preference.pm line 224 > 4. in Koha::Patron::Message::Preference::set at /kohadevbox/koha/Koha/Patron/Message/Preference.pm line 247 > 5. in C4::Form::MessagingPreferences::handle_form_action at /kohadevbox/koha/C4/Form/MessagingPreferences.pm line 110 > 6. in (eval) at /kohadevbox/koha/opac/opac-messaging.pl line 67 After fixing this it should be made sure also that removing email address in a patron which had the messaging preferences removes the messaging preferences (or handles it some proper way). More robust input handling in OPAC needed ========================================= In the opac messaging preferences change with the HTML dev console the value of "Days in advance" to be 50 and submit the request, you will then get following error: > days_in_advance has to be a value between 0-30 for Advance_Notice. at /usr/share/perl5/Exception/Class/Base.pm line 88 Expected outcome: a nicer result is shown to the user about invalid parameters, we should handle all errors in the code. What do the last two commits do =============================== Bug 18595: Disable digest checkbox when forced on: - This one maybe I don't understand because of the first error caused by missing email. Specifically I don't understand in which situation the digest checkbox should be disabled. Bug 18595: Validate days_in_advance - What does this do exactly? I see it change days_in_advance=NULL but what is the reason behind that? QA tool errors ============== $ koha-qa.pl -v 2 -c 9 FAIL C4/Reserves.pm FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). OK forbidden patterns OK git manipulation OK pod OK pod coverage OK spelling OK valid FAIL circ/returns.pl FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). OK forbidden patterns OK git manipulation OK pod OK spelling OK valid FAIL koha-tmpl/intranet-tmpl/prog/en/includes/messaging-preference-form.inc OK filters OK forbidden patterns OK git manipulation OK js_in_body OK spelling FAIL tt_valid lines 163, 169 OK valid_template FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt OK filters OK forbidden patterns OK git manipulation OK js_in_body OK spelling FAIL tt_valid lines 131, 133 OK valid_template For the critic qa errors see the details with for example using $ perlcritic circ/returns.pl $ perlcritic C4/Reserves.pm Not quite sure how to fix the template issues. That's all the issues I spotted. The code review I did didn't find any issues and all the test mentioned in the test plan work.
(In reply to Joonas Kylmälä from comment #109) > What do the last two commits do > =============================== > > Bug 18595: Disable digest checkbox when forced on: > - This one maybe I don't understand because of the first error caused by > missing email. Specifically I don't understand in which situation the digest > checkbox should be disabled. There are libraries that wish to only send notifications as digests. IIRC, the only way to achieve this was to configure it directly in the database via "message_transports" table by removing rows where "is_digest" is 0. This patch disables digest checkbox elements in such cases. > Bug 18595: Validate days_in_advance > - What does this do exactly? I see it change days_in_advance=NULL but what > is the reason behind that? It's been 3 years since I provided these patches so I can't exactly remember why and where but some patron messaging preferences had invalid values for borrower_message_preferences.days_in_advance column and it caused plenty of "Internal Server Error" messages in production. No idea whether it came from Koha bugs at some point in the history, or from sysadmins themselves, but I suppose it's safer to do a little database cleanup and run such query because it simply nulls days_in_advance for every option that does not allow it.
(In reply to Joonas Kylmälä from comment #109) > Some things need a bit more work: > > If no patron email we get error > =============================== > > 1. Add patron without email > 2. Go to cgi-bin/koha/opac-messaging.pl and select email checkbox for every > messaging setting > 3. Observe following error: > > > Patron has not set email address, cannot use email as message transport at /usr/share/perl5/Exception/Class/Base.pm line 88 > > 1. in Exception::Class::Base::throw at /usr/share/perl5/Exception/Class/Base.pm line 88 > > 2. in Koha::Patron::Message::Preference::_set_message_transport_types at /kohadevbox/koha/Koha/Patron/Message/Preference.pm line 409 > > 3. in Koha::Patron::Message::Preference::message_transport_types at /kohadevbox/koha/Koha/Patron/Message/Preference.pm line 224 > > 4. in Koha::Patron::Message::Preference::set at /kohadevbox/koha/Koha/Patron/Message/Preference.pm line 247 > > 5. in C4::Form::MessagingPreferences::handle_form_action at /kohadevbox/koha/C4/Form/MessagingPreferences.pm line 110 > > 6. in (eval) at /kohadevbox/koha/opac/opac-messaging.pl line 67 > > After fixing this it should be made sure also that removing email address in > a patron which had the messaging preferences removes the messaging > preferences (or handles it some proper way). > Same error occurs in the staff client if we have email as message transport type for patron (I'm guessing that error will also be raised if sms number isn't set but is chosen as message preference). However patron is still saved. I'm not sure if automatically removing message preference is a good approach. Maybe preventing form from sending and error message like 'Patron has email as message transport type but not email. Either provide valid email or change patrons messaging preferences.' is enough. Hmm, or error message next to input field. > More robust input handling in OPAC needed > ========================================= > > In the opac messaging preferences change with the HTML dev console the value > of "Days in advance" to be 50 and submit the request, you will then get > following error: > > > days_in_advance has to be a value between 0-30 for Advance_Notice. at /usr/share/perl5/Exception/Class/Base.pm line 88 > > Expected outcome: a nicer result is shown to the user about invalid > parameters, we should handle all errors in the code. > Again same occurs in the staff client. All in all we need better error handling with these changes. > What do the last two commits do > =============================== > > Bug 18595: Disable digest checkbox when forced on: > - This one maybe I don't understand because of the first error caused by > missing email. Specifically I don't understand in which situation the digest > checkbox should be disabled. > > > Bug 18595: Validate days_in_advance > - What does this do exactly? I see it change days_in_advance=NULL but what > is the reason behind that? > Lari already commented these. With last one I agree some cleanup might be needed. First one I understand but I think it's worth checking other options... Could we e.g. disable checkbox with jQuery? > QA tool errors > ============== > > $ koha-qa.pl -v 2 -c 9 > > FAIL C4/Reserves.pm > FAIL critic > # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). > OK forbidden patterns > OK git manipulation > OK pod > OK pod coverage > OK spelling > OK valid > > FAIL circ/returns.pl > FAIL critic > # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). > OK forbidden patterns > OK git manipulation > OK pod > OK spelling > OK valid > > FAIL koha-tmpl/intranet-tmpl/prog/en/includes/messaging-preference-form.inc > OK filters > OK forbidden patterns > OK git manipulation > OK js_in_body > OK spelling > FAIL tt_valid > lines 163, 169 > OK valid_template > > FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt > OK filters > OK forbidden patterns > OK git manipulation > OK js_in_body > OK spelling > FAIL tt_valid > lines 131, 133 > OK valid_template > > For the critic qa errors see the details with for example using > > $ perlcritic circ/returns.pl > $ perlcritic C4/Reserves.pm > > Not quite sure how to fix the template issues. > My guess is that if statement isn't allowed to be used inside input tag.
After discussing with Emmi, I will be taking over this Bug once again and fix the issues mentioned above.
Created attachment 112690 [details] [review] Bug 18595: (follow-up) Disable transports if missing contact information To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC - SMSSendDriver ---- GENERAL ---- 1. Apply patch ---- PREREQUISITES ---- 1. Select a patron to test this patch with 2. Remove patron's primary email 3. Remove patron's SMS number ---- OPAC ---- 1. Login to OPAC with your test patron. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe two warnings "Your user account is missing email address" and "Your user account is missing SMS number" 4. See that the SMS and Email checkboxes are disabled in the messaging preferences table. 5. At the bottom of the page, set SMS number 6. Refresh page, observe only one warning "Your user account is missing email address" and observe the SMS checkboxes are now clickable. 7. Give your test patron a primary email address 8. Refresh OPAC "your messaging" page 9. Observe all warnings are now gone and both SMS number and Email checkboxes in the messaging preferences table are clickable. 10. Set patron some messaging preferences and make sure your change applies. ---- INTRANET ---- 1. Repeat "OPAC" steps but instead of "your messaging" page, use the "Modify patron" view.
(In reply to Joonas Kylmälä from comment #109) > More robust input handling in OPAC needed > ========================================= > > In the opac messaging preferences change with the HTML dev console the value > of "Days in advance" to be 50 and submit the request, you will then get > following error: > > > days_in_advance has to be a value between 0-30 for Advance_Notice. at /usr/share/perl5/Exception/Class/Base.pm line 88 > > Expected outcome: a nicer result is shown to the user about invalid > parameters, we should handle all errors in the code. This requires us to revisit Bug 17499. We need more detailed Koha::Exceptions to have translatable error messages in the GUI. Because now we throw multiple Koha::Exceptions::BadParameter and specifying the problem in its 'error' field. While we could parse the error message itself, I don't think that's a good idea. Instead we would throw something like Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange with 'min' and 'max' parameters. The more formal the exceptions the better.
Created attachment 113126 [details] [review] Bug 18595: (follow-up) Catch message preference exceptions in OPAC This patch handles message preference exceptions in OPAC messaging template instead of the usual white "Internal Server Error" screen. To test: 0. This test plan assumes you have a default Koha installation that adds message attributes and transports from installer/data/mysql/mandatory/sample_notices_message_attributes.sql and installer/data/mysql/mandatory/sample_notices_message_transports.sql 1. Open developer tools, select console and run the following command: $('#email1').val('non-existing message transport type').prop('checked', true); $('form[name=opacmessaging]').submit(); Observe error: Message transport type non-existing message transport type does not exist. 2. Open developer tools, select console and run the following command: $($('select[name="2-DAYS"] option')[0]).attr('value', 31); $('form[name=opacmessaging]').submit(); Observe error: Days in advance selection is out of range. The value must be between 0 and 30. 3. Run SQL command: delete from message_transports where message_attribute_id=1 and is_digest=0 \ and message_transport_type='email'; Open developer tools, select console and run the following command: $('#digest1').prop('checked', false); $('#forcedigest1').remove(); $('form[name=opacmessaging]').submit(); Observe error: Digest must be selected for Item_Due. Recover your test database by running the following SQL command: insert into message_transports (message_attribute_id, message_transport_type, \ is_digest, letter_module, letter_code) values (1, 'email', 0, 'circulation', \ 'DUE'); 4. Remove your email address, open developer tools, select console and run the following command: $('#email1').removeAttr('disabled'); $('#email1').prop('checked', true); $('form[name=opacmessaging]').submit(); Observe error: You cannot select email notifications because you haven't set an email address. 5. Remove your SMS number, open developer tools, select console and run the following command: $('#sms1').removeAttr('disabled'); $('#sms1').prop('checked', true); $('form[name=opacmessaging]').submit(); Observe error: You cannot select SMS notifications because you haven't set an SMS number address. 6. The rest of the template error messages are currently inaccessible and therefore untestable via GUI, but check the changes this patch introduces manually. These exceptions are: Koha::Exceptions::Patron::Message::Preference::AttributeNotFound Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable You can test them programmatically by searching opac-messaging.pl for the following: C4::Form::MessagingPreferences::handle_form_action( ... and throwing each exception before it by following code: Koha::Exceptions::Patron::Message::Preference::AttributeNotFound->throw(); Sponsored-by: The National Library of Finland
Created attachment 113192 [details] [review] Bug 18595: (follow-up) Disable transports if missing contact information To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC - SMSSendDriver ---- GENERAL ---- 1. Apply patch ---- PREREQUISITES ---- 1. Select a patron to test this patch with 2. Remove patron's primary email 3. Remove patron's SMS number ---- OPAC ---- 1. Login to OPAC with your test patron. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe two warnings "Your user account is missing email address" and "Your user account is missing SMS number" 4. See that the SMS and Email checkboxes are disabled in the messaging preferences table. 5. At the bottom of the page, set SMS number 6. Refresh page, observe only one warning "Your user account is missing email address" and observe the SMS checkboxes are now clickable. 7. Give your test patron a primary email address 8. Refresh OPAC "your messaging" page 9. Observe all warnings are now gone and both SMS number and Email checkboxes in the messaging preferences table are clickable. 10. Set patron some messaging preferences and make sure your change applies. ---- INTRANET ---- 1. Repeat "OPAC" steps but instead of "your messaging" page, use the "Modify patron" view. Sponsored-by: The National Library of Finland
Created attachment 113221 [details] [review] Bug 18595: (follow-up) Disable transports if missing contact information To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC - SMSSendDriver ---- GENERAL ---- 1. Apply patch ---- PREREQUISITES ---- 1. Select a patron to test this patch with 2. Remove patron's primary email 3. Remove patron's SMS number ---- OPAC ---- 1. Login to OPAC with your test patron. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe two warnings "Cannot select email notifications" and "Cannot select SMS notifications" 4. See that the SMS and Email checkboxes are disabled in the messaging preferences table. 5. At the bottom of the page, set SMS number 6. Refresh page, observe only one warning "Cannot select email notifications" and observe the SMS checkboxes are now clickable. 7. Give your test patron a primary email address 8. Refresh OPAC "your messaging" page 9. Observe all warnings are now gone and both SMS number and Email checkboxes in the messaging preferences table are clickable. 10. Set patron some messaging preferences and make sure your change applies. ---- INTRANET ---- 1. Repeat "OPAC" steps but instead of "your messaging" page, use the "Modify patron" view. Sponsored-by: The National Library of Finland
Created attachment 113222 [details] [review] Bug 18595: (follow-up) Catch message preference exceptions in OPAC This patch handles message preference exceptions in OPAC messaging template instead of the usual white "Internal Server Error" screen. To test: 0. This test plan assumes you have a default Koha installation that adds message attributes and transports from installer/data/mysql/mandatory/sample_notices_message_attributes.sql and installer/data/mysql/mandatory/sample_notices_message_transports.sql 1. Open developer tools, select console and run the following command: $('#email1').val('non-existing message transport type').prop('checked', true); $('form[name=opacmessaging]').submit(); Observe error: Message transport type non-existing message transport type does not exist. 2. Open developer tools, select console and run the following command: $($('select[name="2-DAYS"] option')[0]).attr('value', 31); $('form[name=opacmessaging]').submit(); Observe error: Days in advance selection is out of range. The value must be between 0 and 30. 3. Run SQL command: delete from message_transports where message_attribute_id=1 and is_digest=0 \ and message_transport_type='email'; Open developer tools, select console and run the following command: $('#digest1').prop('checked', false); $('#forcedigest1').remove(); $('form[name=opacmessaging]').submit(); Observe error: Digest must be selected for Item_Due. Recover your test database by running the following SQL command: insert into message_transports (message_attribute_id, message_transport_type, \ is_digest, letter_module, letter_code) values (1, 'email', 0, 'circulation', \ 'DUE'); 4. Remove your email address, open developer tools, select console and run the following command: $('#email1').removeAttr('disabled'); $('#email1').prop('checked', true); $('form[name=opacmessaging]').submit(); Observe error: You cannot select email notifications because you haven't set an email address. 5. Remove your SMS number, open developer tools, select console and run the following command: $('#sms1').removeAttr('disabled'); $('#sms1').prop('checked', true); $('form[name=opacmessaging]').submit(); Observe error: You cannot select SMS notifications because you haven't set an SMS number address. 6. The rest of the template error messages are currently inaccessible and therefore untestable via GUI, but check the changes this patch introduces manually. These exceptions are: Koha::Exceptions::Patron::Message::Preference::AttributeNotFound Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable You can test them programmatically by searching opac-messaging.pl for the following: C4::Form::MessagingPreferences::handle_form_action( ... and throwing each exception before it by following code: Koha::Exceptions::Patron::Message::Preference::AttributeNotFound->throw(); Sponsored-by: The National Library of Finland
Created attachment 113769 [details] [review] Bug 18595: (follow-up) Remove duplicated logic Sponsored-by: The National Library of Finland
Lari, will you continue working on this? Currently the patches don't apply.
(In reply to Joonas Kylmälä from comment #120) > Lari, will you continue working on this? Currently the patches don't apply. Yes! Thanks for bringing it up. I will do a rebase in the following days.
Created attachment 146541 [details] [review] Bug 18595: Unit tests for Koha::Patron::Categories->default_messaging This patch adds unit tests for Koha::Patron::Categories->default_messaging. Tests will help preparing for moving C4::Members::Messaging to Koha-objects. To test: 1. Run t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 146542 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingOptions This patch moves C4::Members::GetMessagingOptions to Koha::Patron::MessagePreferences->get_options. The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! This patch also removes previously unused GetMessagingOptions in opac-messaging.pl To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingOptions" (grep -r 'GetMessagingOptions'). 3. Observe no results in source code. ---- OPAC ---- 1. Login to OPAC. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. ---- INTRANET ---- 1. Login to intranet. 2. Go to edit patron's details and scroll down to their messaging preferences. 3. Observe the preferences looking normal and as they should be. 4. Set any combination of messaging preferences. 5. Observe them apply as expected, repeat step 4-5 as much as you like. 6. Go to Administration -> Patron categories -> Edit (for any category) 7. Scroll down to observe default messaging preferences for this patron category 8. Observe the preferences looking normal and as they should be. 9. Set any combination of messaging preferences. 10. Observe them apply as expected, repeat step 9-10 as much as you like. 11. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 12. Fill in required data, do not modify messaging preferences, click Save. 13. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 146543 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreferencesFromDefaults This patch moves C4::Members::SetMessagingPreferencesFromDefaults to Koha::Patron->set_default_messaging_preferences The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreferencesFromDefaults" (grep -r 'SetMessagingPreferencesFromDefaults'). 3. Observe no results in source code. ---- INTRANET ---- 1. Login to intranet. 2. Go to Administration -> Patron categories -> Edit (for any category) 3. Scroll down to observe default messaging preferences for this patron category 4. Observe the preferences, remember them for the next step. 5. Go to cgi-bin/koha/members/memberentry.pl?op=add&categorycode=XXX, replace XXX with the categorycode you were just editing. 6. Look at messaging preferences. They should be same as in step 4. 7. Fill in required data, do not modify messaging preferences, click Save. 8. Observe a new patron with default messaging preferences. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 146544 [details] [review] Bug 18595: Unit tests preparing for GetMessagingPreferences move This patch prepares removing GetMessagingPreferences by adding some useful unit tests for subroutines that the move targets. Reserves.t already covers print notice, but this patch adds tests for a set transport type. To test: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Observe test pass Works together with followup patch Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 146545 [details] [review] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences This patch moves C4::Members::Messaging::GetMessagingPreferences to Koha::Patron::MessagePreference(s) The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "GetMessagingPreferences" (grep -r 'GetMessagingPreferences'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t 4. prove t/db_dependent/api/v1/patrons.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 146546 [details] [review] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::MessagePreference The patch makes a heavy touch on core messaging preference features, but should not introduce any visible changes for end user. Keep this in mind if testing. If you notice any changes in Koha, something is probably wrong! To test: Make sure you have enabled the following system preferences: - EnhancedMessagingPreferences ---- GENERAL ---- 1. Apply patch 2. Search Koha source code for references of "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 3. Observe no results in source code. ---- UNIT TESTS ---- 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t 4. prove t/db_dependent/api/v1/patrons.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron <veron@veron.ch> QA tools pass now OK Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 146547 [details] [review] Bug 18595: Remove references to C4::Members::Messaging To test: 1. grep -r 'C4::Members::Messaging' This command should not produce any results in Koha source code after this patch. 2. Check the pages touched by this commit 3. Try setting and viewing patron and patron category messaging preferences As this completes the move from C4::Members::Messaging to new Koha-objects for messaging preferences, everything should be ready for proper testing. Make sure you have EnhancedMessagingPreferences system preference enabled. Then, try creating new categories and setting default messaging preferences, as well as editing the messaging preferences of the already existing categories. Try adding new patrons and make sure the default patron messaging preferences are by default the same that you set for that category. Edit already exisiting patrons' messaging preferences and make sure changes are applied. Observe no crash at any point, otherwise fail this patch. Don't forget to test both intranet and OPAC. Finally, run unit tests: 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Reserves.t 3. prove t/db_dependent/Koha/Patron/Categories.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 146548 [details] [review] Bug 18595: Disable digest checkbox when forced on To test: 1. Force a digest on via message_transports table (delete rows where message_attribute is 1 and is_digest is 0) SQL: delete from message_transports where message_attribute_id=1 and is_digest=0; 2. Go to either OPAC or staff client messaging preferences 3. Observe digest checkbox is disabled for message attribute 1 (normally Item due row) Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 146549 [details] [review] Bug 18595: Validate days_in_advance Rebased-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Created attachment 146550 [details] [review] Bug 18595: (follow-up) Disable transports if missing contact information To test: Make sure you have enabled the following system preferences: - opacuserlogin - EnhancedMessagingPreferences - EnhancedMessagingPreferencesOPAC - SMSSendDriver ---- GENERAL ---- 1. Apply patch ---- PREREQUISITES ---- 1. Select a patron to test this patch with 2. Remove patron's primary email 3. Remove patron's SMS number ---- OPAC ---- 1. Login to OPAC with your test patron. 2. Go to "your messaging" (cgi-bin/koha/opac-messaging.pl). 3. Observe two warnings "Cannot select email notifications" and "Cannot select SMS notifications" 4. See that the SMS and Email checkboxes are disabled in the messaging preferences table. 5. At the bottom of the page, set SMS number 6. Observe "Cannot select SMS notifications" gone and SMS checkboxes clickable 7. Give your test patron a primary email address 8. Refresh OPAC "your messaging" page 9. Observe all warnings are now gone and both SMS number and Email checkboxes in the messaging preferences table are clickable. 10. Set patron some messaging preferences and make sure your change applies. ---- INTRANET ---- 1. Repeat "OPAC" steps but instead of "your messaging" page, use the "Modify patron" view. Sponsored-by: The National Library of Finland
Created attachment 146551 [details] [review] Bug 18595: (follow-up) Catch message preference exceptions in OPAC This patch handles message preference exceptions in OPAC messaging template instead of the usual white "Internal Server Error" screen. To test: 0. This test plan assumes you have a default Koha installation that adds message attributes and transports from installer/data/mysql/mandatory/sample_notices_message_attributes.sql and installer/data/mysql/mandatory/sample_notices_message_transports.sql 1. Open developer tools, select console and run the following command: $('#email1').val('non-existing message transport type').prop('checked', true); $('form[name=opacmessaging]').submit(); Observe error: Message transport type non-existing message transport type does not exist. 2. Open developer tools, select console and run the following command: $($('select[name="2-DAYS"] option')[0]).attr('value', 31); $('form[name=opacmessaging]').submit(); Observe error: Days in advance selection is out of range. The value must be between 0 and 30. 3. Run SQL command: delete from message_transports where message_attribute_id=1 and is_digest=0 \ and message_transport_type='email'; Open developer tools, select console and run the following command: $('#digest1').prop('checked', false); $('#forcedigest1').remove(); $('form[name=opacmessaging]').submit(); Observe error: Digest must be selected for Item_Due. Recover your test database by running the following SQL command: insert into message_transports (message_attribute_id, message_transport_type, \ is_digest, letter_module, letter_code) values (1, 'email', 0, 'circulation', \ 'DUE'); 4. Remove your email address, open developer tools, select console and run the following command: $('#email1').removeAttr('disabled'); $('#email1').prop('checked', true); $('form[name=opacmessaging]').submit(); Observe error: You cannot select email notifications because you haven't set an email address. 5. Remove your SMS number, open developer tools, select console and run the following command: $('#sms1').removeAttr('disabled'); $('#sms1').prop('checked', true); $('form[name=opacmessaging]').submit(); Observe error: You cannot select SMS notifications because you haven't set an SMS number address. 6. The rest of the template error messages are currently inaccessible and therefore untestable via GUI, but check the changes this patch introduces manually. These exceptions are: Koha::Exceptions::Patron::MessagePreference::AttributeNotFound Koha::Exceptions::Patron::MessagePreference::DaysInAdvanceNotAvailable Koha::Exceptions::Patron::MessagePreference::DigestNotAvailable You can test them programmatically by searching opac-messaging.pl for the following: C4::Form::MessagingPreferences::handle_form_action( ... and throwing each exception before it by following code: Koha::Exceptions::Patron::MessagePreference::AttributeNotFound->throw(); Sponsored-by: The National Library of Finland
Created attachment 146552 [details] [review] Bug 18595: (follow-up) Remove duplicated logic Sponsored-by: The National Library of Finland
Do you have anyone lined up for testing here.. I'd love to see it progressed but I think we need to get some alignment for testing and qa before we attempt another rebase.