Description
Nicole C. Engard
2013-02-04 20:02:32 UTC
Still valid? Yes. Created attachment 47305 [details] [review] Bug 9543 - Show patrons messaging subscription on holds notification Display an information about a patron's messaging preferences if an item is checke in where the patron has put a hold. To test: - Apply patch - Make sure that SMS driver and TalkingTech sysprefs are defined to enable SMS and Phone preferences for users (SMS driver can be a dummy value) - Place a hold for a patron - Set patron's messaging preferences to different choices (including none) - Check in the item that is on hold (it has not to be checked out for the test - Verify that below the message "Hold found (item is already waiting") an information appears about the patron's messaging preferences. I apologise if this is a silly question, but why 'Hold_Filled' message? I'd expect 'Item_Check_in'. Please disregard, previous message, error in reading Created attachment 49533 [details] [review] Bug 9543 - Show patrons messaging subscription on holds notification Display an information about a patron's messaging preferences if an item is checked in where the patron has put a hold. To test: - Apply patch - Make sure that SMS driver and TalkingTech sysprefs are defined to enable SMS and Phone preferences for users (SMS driver can be a dummy value) - Place a hold for a patron - Set patron's messaging preferences to different choices (including none) - Check in the item that is on hold (it has not to be checked out for the test - Verify that below the message "Hold found (item is already waiting") an information appears about the patron's messaging preferences. Created attachment 49939 [details] [review] Bug 9543 - Show patrons messaging subscription on holds notification Display an information about a patron's messaging preferences if an item is checked in where the patron has put a hold. To test: - Apply patch - Make sure that SMS driver and TalkingTech sysprefs are defined to enable SMS and Phone preferences for users (SMS driver can be a dummy value) - Place a hold for a patron - Set patron's messaging preferences to different choices (including none) - Check in the item that is on hold (it has not to be checked out for the test - Verify that below the message "Hold found (item is already waiting") an information appears about the patron's messaging preferences. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 49940 [details] [review] Bug 9543 - Show patrons messaging subscription on holds notification not already waiting Same plan as previous patch, but this one should show the message for a hold not already waiting but not requiring a transfer (hold-found2 alert box) I added a follow up to display the information on the message box before a hold is confirmed, but only if no transfer is required Still applies on current master. 1st patch has signoff by Nick Clemens 2nd patch (follow-up by Nick Clemens) needs a signoff. I have applied the patches. However, I am seeing a "Please transfer item to" message when I check in a book where patron home library and checkin library are the same. -Applied patches - Made sure that SMS driver and TalkingTech sysprefs are defined to enable SMS and Phone preferences for users (SMS driver can be a dummy value). - Placed a hold for a patron - Set patron's messaging preferences to different choices (including none) - Check in the item that is on hold (it has not to be checked out for the test - Verified that below the message "Hold found (item is already waiting") an information appears about the patron's messaging preferences. --Message (notification preference) only appears where a transfer is not required. Created attachment 51346 [details] [review] Bug 16390: Accounts.t does not need MPL This test fails if you do not have a MPL branch, but a closer look shows that we only need to replace one occurrence. Test plan: Run Accounts.t Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Hi Marc and Nick, After signing off on the two patches (and obsoleting yours) I tried to attach my commit, but obviously I screwed up the commit. I'll try to straighten this out, but if you have suggestions let me know. (In reply to Chris Kirby from comment #14) Hi Chris, It looks like maybe you did: git bz attach 9543 HEAD~2 Commits are counted from head, so the last patches are: HEAD HEAD~1 (HEAD~2) (HEAD~3) Two attach the two latest you want to do: git bz attach 9543 HEAD~1^.. I un-obsoleted the original patches and obsoleted the extraneous one. I am leaving as signed off for now Thanks, Nick. Created attachment 51515 [details] [review] Bug 9543 - Show patrons messaging subscription on holds notification Display an information about a patron's messaging preferences if an item is checked in where the patron has put a hold. To test: - Apply patch - Make sure that SMS driver and TalkingTech sysprefs are defined to enable SMS and Phone preferences for users (SMS driver can be a dummy value) - Place a hold for a patron - Set patron's messaging preferences to different choices (including none) - Check in the item that is on hold (it has not to be checked out for the test - Verify that below the message "Hold found (item is already waiting") an information appears about the patron's messaging preferences. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Chris Kirby <christopherlawrencekirby@gmail.com> Created attachment 51516 [details] [review] Bug 9543 - Show patrons messaging subscription on holds notification not already waiting Same plan as previous patch, but this one should show the message for a hold not already waiting but not requiring a transfer (hold-found2 alert box) Signed-off-by: Chris Kirby <christopherlawrencekirby@gmail.com> QA comments: - Avoid c/p of code (use BLOCK instead) - Correctly name the variables: key is a message_transport_type, it would make sense to describe it better (mtt?) - If several MTT are selected, the display is not nice: "Patron is notified: SMS. Email. Phone." Created attachment 51641 [details] [review] Bug 9543 - (Follow-up) Show patrons messaging subscription on holds notification As of comment #19 - Code de-duplicated to a BLOCK statement - Variable renamed - Wording slightly changed. I like it that way, but feel free to change it in a follow-up Not sure if it can be switched back to signed-off or if it needs an other sign-off. Thanks Marc. What about [%~ UNLESS loop.last %], [% ELSE %].[% END ~%] To separate the mtt with a comma and end them with a dot? I can provide a follow-up if you think it's better. (In reply to Jonathan Druart from comment #22) > Thanks Marc. > What about > [%~ UNLESS loop.last %], [% ELSE %].[% END ~%] > To separate the mtt with a comma and end them with a dot? > I can provide a follow-up if you think it's better. Hi Jonathan, that would be nicer indeed, and yes please for the follow up :-) Created attachment 51660 [details] [review] Bug 9543 - Show patrons messaging subscription on holds notification Display an information about a patron's messaging preferences if an item is checked in where the patron has put a hold. To test: - Apply patch - Make sure that SMS driver and TalkingTech sysprefs are defined to enable SMS and Phone preferences for users (SMS driver can be a dummy value) - Place a hold for a patron - Set patron's messaging preferences to different choices (including none) - Check in the item that is on hold (it has not to be checked out for the test - Verify that below the message "Hold found (item is already waiting") an information appears about the patron's messaging preferences. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Chris Kirby <christopherlawrencekirby@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 51661 [details] [review] Bug 9543 - Show patrons messaging subscription on holds notification not already waiting Same plan as previous patch, but this one should show the message for a hold not already waiting but not requiring a transfer (hold-found2 alert box) Signed-off-by: Chris Kirby <christopherlawrencekirby@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 51662 [details] [review] Bug 9543 - (Follow-up) Show patrons messaging subscription on holds notification As of comment #19 - Code de-duplicated to a BLOCK statement - Variable renamed - Wording slightly changed. I like it that way, but feel free to change it in a follow-up Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 51663 [details] [review] Bug 9543: Separate MTT with commas Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to Master - Should be in the November 2016 Release. Thanks I'm undecided whether to push this enhancement to 16.05 branch. CONS: As a enhancement, it should be reserved to November release. PRO: It's not that big. It impacts portions of code (returns.tt) that may have to be modified later by bugfixes that would be difficult to apply if master diverges too much from stable. Any advice? Why do you want to backport it? (In reply to Jonathan Druart from comment #30) > Why do you want to backport it? Ok, misread the PRO. I'd say don't backport enhancements, in general. |