Bug 28153 - Add 'Hold reminder' messaging preference
Summary: Add 'Hold reminder' messaging preference
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 15986
Blocks: 29194 29586
  Show dependency treegraph
 
Reported: 2021-04-15 13:09 UTC by Nick Clemens
Modified: 2022-04-22 11:18 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement allows staff/patrons to control individual preferences for holds reminder noticess in the patron's messaging preferences area.
Version(s) released in:
21.11.00,21.11.01


Attachments
Bug 28153: Add 'Hold reminder' messaging preference (6.13 KB, patch)
2021-04-15 13:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28153: Add 'Hold reminder' messaging preference (6.19 KB, patch)
2021-08-02 12:58 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28153: Add 'Hold reminder' messaging preference (6.28 KB, patch)
2021-10-11 13:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28153: (follow-up) Add installer files and update atomicupdate (5.44 KB, patch)
2021-10-15 15:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28153: (follow-up) Ensure update is idempotent (5.04 KB, patch)
2021-10-19 11:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28153: (follow-up) Ensure update is idempotent (5.09 KB, patch)
2021-10-19 13:19 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-04-15 13:09:52 UTC

    
Comment 1 Nick Clemens 2021-04-15 13:35:38 UTC
Created attachment 119626 [details] [review]
Bug 28153: Add 'Hold reminder' messaging preference

This adds a new preference for patrons to choose how they wish to receive
'Hold reminder' notices.

The notice is always digested per branch

To test:
 1 - Apply patches
 2 - Update database
 3 - Ensure EnhancedMessagingPreferences and EnhancedMessagingPreferencesOPAC are enabled
 4 - View a patron and note new messaging preference
 5 - Confirm same on the opac for a patron account
 6 - All transports should be disabled by default
 7 - Place a hold for the patron and check it in to confirm
 8 - Run hold reminder script
    perl misc/cronjobs/holds_reminder.pl -v -c
 9 - No message should be queued for patron
10 - Enable the message in a transport for which 'HOLD_REMINDER' notice has content for the patron
11 - Run the script
12 - Patron should have a message queued
13 - Ensure a different transport has content for the notice
14 - Run the script forcing a transport
   perl misc/cronjobs/holds_reminder.pl -v -c -mtt=print
15 - The patron should have a message queued in the forced transport
Comment 2 Andrew Fuerste-Henry 2021-08-02 12:58:55 UTC
Created attachment 123374 [details] [review]
Bug 28153: Add 'Hold reminder' messaging preference

This adds a new preference for patrons to choose how they wish to receive
'Hold reminder' notices.

The notice is always digested per branch

To test:
 1 - Apply patches
 2 - Update database
 3 - Ensure EnhancedMessagingPreferences and EnhancedMessagingPreferencesOPAC are enabled
 4 - View a patron and note new messaging preference
 5 - Confirm same on the opac for a patron account
 6 - All transports should be disabled by default
 7 - Place a hold for the patron and check it in to confirm
 8 - Run hold reminder script
    perl misc/cronjobs/holds_reminder.pl -v -c
 9 - No message should be queued for patron
10 - Enable the message in a transport for which 'HOLD_REMINDER' notice has content for the patron
11 - Run the script
12 - Patron should have a message queued
13 - Ensure a different transport has content for the notice
14 - Run the script forcing a transport
   perl misc/cronjobs/holds_reminder.pl -v -c -mtt=print
15 - The patron should have a message queued in the forced transport

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 3 Marcel de Rooy 2021-10-01 09:50:13 UTC
I would recommend to use this opportunity to sort the various message settings, at least on the OPAC side.
It is a bit strange to see Hold reminder now below under Item checkout.
But this definitely has a multi language aspect !
Comment 4 Nick Clemens 2021-10-08 12:45:56 UTC
(In reply to Marcel de Rooy from comment #3)
> I would recommend to use this opportunity to sort the various message
> settings, at least on the OPAC side.
> It is a bit strange to see Hold reminder now below under Item checkout.
> But this definitely has a multi language aspect !

I think this should be its own bug, filed as bug 29194
Comment 5 Marcel de Rooy 2021-10-11 13:04:09 UTC
Created attachment 126043 [details] [review]
Bug 28153: Add 'Hold reminder' messaging preference

This adds a new preference for patrons to choose how they wish to receive
'Hold reminder' notices.

The notice is always digested per branch

To test:
 1 - Apply patches
 2 - Update database
 3 - Ensure EnhancedMessagingPreferences and EnhancedMessagingPreferencesOPAC are enabled
 4 - View a patron and note new messaging preference
 5 - Confirm same on the opac for a patron account
 6 - All transports should be disabled by default
 7 - Place a hold for the patron and check it in to confirm
 8 - Run hold reminder script
    perl misc/cronjobs/holds_reminder.pl -v -c
 9 - No message should be queued for patron
10 - Enable the message in a transport for which 'HOLD_REMINDER' notice has content for the patron
11 - Run the script
12 - Patron should have a message queued
13 - Ensure a different transport has content for the notice
14 - Run the script forcing a transport
   perl misc/cronjobs/holds_reminder.pl -v -c -mtt=print
15 - The patron should have a message queued in the forced transport

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Jonathan Druart 2021-10-15 08:52:33 UTC
Hum? There is a lot missing here.

1. You forgot installer files

2. message_attribute_id is hardcoded, people may have custom stuffs there. Insert then retrieve the id before inserting into message_transports.

3. Shouldn't Users with Hold_Filled have Hold_Reminder?

4. There is not way to turn the feature off: if the cron is not running the users won't get the notices.
Existing behaviour, true for other notices, not blocker.
Comment 7 Nick Clemens 2021-10-15 15:03:07 UTC
Created attachment 126349 [details] [review]
Bug 28153: (follow-up) Add installer files and update atomicupdate
Comment 8 Nick Clemens 2021-10-15 15:04:24 UTC
(In reply to Jonathan Druart from comment #6) 
> 1. You forgot installer files 
 Updated 
> 2. message_attribute_id is hardcoded, people may have custom stuffs there.
> Insert then retrieve the id before inserting into message_transports.
 All previous updates have been hardcoded, I don't think we support users adding their own values here
> 3. Shouldn't Users with Hold_Filled have Hold_Reminder?
 Added to update
Comment 9 Jonathan Druart 2021-10-18 09:33:49 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 10 Jonathan Druart 2021-10-18 09:36:55 UTC
(In reply to Nick Clemens from comment #8)
> (In reply to Jonathan Druart from comment #6) 
> > 3. Shouldn't Users with Hold_Filled have Hold_Reminder?
>  Added to update

A quick note: Maybe we should add it only if Hold_Reminder does not exist yet. If the dbrev is backported and ran twice, a patron who removed it will have it back.

Not considering blocker, follow-up if you think it's relevant enough.
Comment 11 Nick Clemens 2021-10-19 11:29:02 UTC
Created attachment 126483 [details] [review]
Bug 28153: (follow-up) Ensure update is idempotent
Comment 12 Owen Leonard 2021-10-19 13:19:03 UTC
Created attachment 126499 [details] [review]
Bug 28153: (follow-up) Ensure update is idempotent

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 13 Jonathan Druart 2021-10-19 13:31:36 UTC
Patch pushed to master, thanks!
Comment 14 Kyle M Hall 2021-12-17 13:54:23 UTC
Pushed to 21.11.x for 21.11.01