Bug 19451

Summary: Let borrowers-force-messaging-defaults.pl optionally add preferences only when not already present
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Command-line UtilitiesAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, josef.moravec, nick, robin
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18197
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This report adds the command-line option 'no-overwrite' so that you can add preferences only when they are not yet present (in other words: skip patrons that already set their prefs).
Version(s) released in:
Bug Depends on: 19472    
Bug Blocks: 19452    
Attachments: Bug 19451: Add no_overwrite option to borrowers-force-messaging-defaults.pl
Bug 19451: Add no_overwrite option to borrowers-force-messaging-defaults.pl
Bug 19451: Add no_overwrite option to borrowers-force-messaging-defaults.pl
Bug 19451: Add no_overwrite option to borrowers-force-messaging-defaults.pl
Bug 19451: (QA follow-up) Change parameter name no_overwrite to no-overwrite for consistency
Bug 19451: (QA follow-up) Replace weird subquery

Description Marcel de Rooy 2017-10-12 10:15:54 UTC
A welcome addition to borrowers-force-messaging-defaults.pl would be to allow adding the messaging defaults only to patrons without such preferences.
This will be made possible with the empty_only option. It copies the prefs only when the borrower prefs are empty (read: are not present).
Comment 1 Marcel de Rooy 2017-10-12 10:40:16 UTC
Renaming empty_only to no_overwrite which might be better understood.
Comment 2 Marcel de Rooy 2017-10-12 10:49:55 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2017-10-17 12:21:08 UTC
Created attachment 68200 [details] [review]
Bug 19451: Add no_overwrite option to borrowers-force-messaging-defaults.pl

This option allows you to add preferences only when they are not yet
present. In other words: skip patrons that already set their prefs.

Test plan:
[1] Delete all borrower messaging prefs for a patron.
[2] Run borrowers-force-messaging-defaults.pl -no_overwrite -doit
    Verify that the patron now has default msg preferences.
[3] Change his settings and make them non-default.
    For instance, increase days in advance.
[4] Run borrowers-force-messaging-defaults.pl -no_overwrite -doit
    Verify that the patron still has the non-default settings.
[5] Run borrowers-force-messaging-defaults.pl -doit
    Verify that the patron msg prefs have been overwritten.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2017-10-17 12:21:37 UTC
Rebased
Comment 5 Owen Leonard 2017-10-30 12:55:57 UTC
Created attachment 68844 [details] [review]
Bug 19451: Add no_overwrite option to borrowers-force-messaging-defaults.pl

This option allows you to add preferences only when they are not yet
present. In other words: skip patrons that already set their prefs.

Test plan:
[1] Delete all borrower messaging prefs for a patron.
[2] Run borrowers-force-messaging-defaults.pl -no_overwrite -doit
    Verify that the patron now has default msg preferences.
[3] Change his settings and make them non-default.
    For instance, increase days in advance.
[4] Run borrowers-force-messaging-defaults.pl -no_overwrite -doit
    Verify that the patron still has the non-default settings.
[5] Run borrowers-force-messaging-defaults.pl -doit
    Verify that the patron msg prefs have been overwritten.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 6 Josef Moravec 2018-02-07 21:14:06 UTC
Created attachment 71327 [details] [review]
Bug 19451: Add no_overwrite option to borrowers-force-messaging-defaults.pl

This option allows you to add preferences only when they are not yet
present. In other words: skip patrons that already set their prefs.

Test plan:
[1] Delete all borrower messaging prefs for a patron.
[2] Run borrowers-force-messaging-defaults.pl -no-overwrite -doit
    Verify that the patron now has default msg preferences.
[3] Change his settings and make them non-default.
    For instance, increase days in advance.
[4] Run borrowers-force-messaging-defaults.pl -no-overwrite -doit
    Verify that the patron still has the non-default settings.
[5] Run borrowers-force-messaging-defaults.pl -doit
    Verify that the patron msg prefs have been overwritten.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 7 Josef Moravec 2018-02-07 21:14:12 UTC
Created attachment 71328 [details] [review]
Bug 19451: (QA follow-up) Change parameter name no_overwrite to no-overwrite for consistency

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 8 Jonathan Druart 2018-02-08 18:04:30 UTC
+    if( $no_overwrite ) {
+        $sql .= " AND (SELECT COUNT(*) FROM borrower_message_preferences mp WHERE mp.borrowernumber=bo.borrowernumber) = 0"
+    }

This is weird, I think you want to add a join here instead.
Comment 9 Marcel de Rooy 2018-02-09 11:04:55 UTC
Created attachment 71367 [details] [review]
Bug 19451: (QA follow-up) Replace weird subquery

As requested, we add a JOIN and make the SELECT distinct.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested that no-overwrite still works as expected.
Comment 10 Jonathan Druart 2018-02-12 21:08:46 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 11 Nick Clemens (kidclamp) 2018-02-16 01:51:19 UTC
Awesome work all! Enhancement, not backported to stable.