|
Description
Nicole C. Engard
2011-02-16 21:02:09 UTC
Very very needed! As the patron does not know which librarian will manage its suggestion, new suggestions notices could be sent either to the Acq manager for the library system, or to the Acq manager for each branch of the system, if a branch is specified by the patron. M. Saby *** Bug 7322 has been marked as a duplicate of this bug. *** Seeking co-sponsors: http://devs.bywatersolutions.com/projects/new-suggestions-sent-via-email/ *** Bug 19459 has been marked as a duplicate of this bug. *** Created attachment 83384 [details] [review] Bug 5770: New system pref for new suggestion email Add two new system preferences for new suggestion email. Created attachment 83385 [details] [review] Bug 5770: Notify for new suggestion Test plan: 1 - Make sure create a new notice suggestions > NEW_SUGGESTION 2 - Change EmailPurchaseSuggestions and EmailAddressForSuggestions system preferences. 3 - Create a suggestion 4 - Verify the notice is generated in the message_queue table. 6 - Verify that the e-mail will be sent to the address specified in the EmailPurchaseSuggestions system preferences. 7 - And then return to 2. 8 - Repeat steps. Created attachment 83386 [details] [review] Bug 5770: Notify for new suggestion - sample notices This patch adds the new notice for all languages. Created attachment 83387 [details] [review] Bug 5770: (follow-up) New system pref for new suggestion email Created attachment 83388 [details] [review] Bug 5770: Email librarian when purchase suggestion made Test plan: 1 - Make sure create a new notice suggestions > NEW_SUGGESTION 2 - Change EmailPurchaseSuggestions and EmailAddressForSuggestions system preferences. 3 - Create a suggestion 4 - Verify the notice is generated in the message_queue table. 6 - Verify that the e-mail will be sent to the address specified in the EmailPurchaseSuggestions system preferences. 7 - And then return to 2. 8 - Repeat steps. Created attachment 83390 [details] [review] Bug 5770: New system pref and notice for new suggestion email Hi, please don't forget to assign the bug to yourself when providing a patch! Created attachment 83615 [details] [review] Bug 5770: Notify for new suggestion - sample notices Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 83616 [details] [review] Bug 5770: Email librarian when purchase suggestion made Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 83617 [details] [review] Bug 5770: New system pref for new suggestion email Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Works as advertised, a few notes: 1 - I encourage everyone to use skeleton.perl as a base for update statements :-) - perl ones are easier to push 2 - I would like to see the new notice use TT syntax instead of our legacy syntax Created attachment 84463 [details] [review] Bug 5770: (QA follow-up) Minor formatting fixes - Rewords system preference slightly - Minor changes to the sample template like " : " to ": " - Alphabetizes sysprefs.sql I'd like TT syntax, but I think we could deal with it separately as not all notices are TT yet. If we do TT we might want to do not just a conversion, but also do a nice one, using [% IF (....) %] to avoid empty labels and the like. I've added a follow-up doing some minor changes - plz have a look at those. Failing for missing tests for the change on NewSuggestion. Please add tests! Created attachment 84812 [details] [review] Bug 5770: Unit test and skeleton perl Hi Devinim, most of the time if something is failed by the QA team it's ok to switch directly back "Signed off". Tests are such a case :) Created attachment 89139 [details] [review] Bug 5770: Email librarian when purchase suggestion made Created attachment 89140 [details] [review] Bug 5770: New system pref for new suggestion email Created attachment 89141 [details] [review] Bug 5770: Unit test Created attachment 89142 [details] [review] Bug 5770: Notify for new suggestion - sample notices Created attachment 89144 [details] [review] Bug 5770: Email librarian when purchase suggestion made Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 89145 [details] [review] Bug 5770: New system pref for new suggestion email Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 89146 [details] [review] Bug 5770: Unit test Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 89147 [details] [review] Bug 5770: Notify for new suggestion - sample notices Signed-off-by: Liz Rea <wizzyrea@gmail.com> Couple of QA points here... not sure if Josef Oops.. lets start that again. 1) I think we should allow for the feature to be disabled and so I'd like to see the syspref allow for that option - I'll work on a QA followup for that 2) We should perhaps take account of the reply-to handling built into branch addresses and admin email address - I can also look at doing a followup 3) I'm not a fan of us introducing the letter_code option to C4::Letter::GetMessagesQueue just for use in the tests - I'll do a followup for that too. Otherwise, I really like the feature so please bear with me whilst I code up the followups for QA Created attachment 89188 [details] [review] Bug 5770: Email librarian when purchase suggestion made Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 89189 [details] [review] Bug 5770: New system pref for new suggestion email Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 89190 [details] [review] Bug 5770: Unit test Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 89191 [details] [review] Bug 5770: Notify for new suggestion - sample notices Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 89192 [details] [review] Bug 5770: (QA follow-up) Allow feature to be disable This patch adds 'None' to the available options for the feature so it may be disabled entirely (and sets that as default) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 89193 [details] [review] Bug 5770: (QA follow-up) Revert change to GetQueuedMessages Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 89194 [details] [review] Bug 5770: (QA follow-up) Move system preference to appropriate section The new sysprefs wher in the general admin section. I believe they are better placed within the acquisitions area. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 89195 [details] [review] Bug 5770: (QA follow-up) Adapt for replyto handling The ReplyTo preferences allwo for libraries to setup various combinations of addresses for email headers to ensure correct mail delivery. This patch accounts for their configuration and use in the suggestions notifications Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Right, I'm not confident in this bug.. Passing QA no(In reply to Martin Renvoize from comment #38) > Right, I'm not confident in this bug.. Passing QA not = now? Grr, silly typo.. Now! :) Awesome work all! Pushed to master for 19.05 When approving a pending suggestion, nothing happens. In the logs there are two errors: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`suggestions`, CONSTRAINT `suggestions_ibfk_branchcode` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE) [for Statement "UPDATE `suggestions` SET `STATUS` = ?, `acceptedby` = ?, `accepteddate` = ?, `branchcode` = ?, `managedby` = ?, `manageddate` = ? WHERE ( `suggestionid` = ? )" with ParamValues: 0="ACCEPTED", 1="51", 2='2019-05-14T15:34:15', 3="__ANY__", 4="51", 5='2019-05-14T15:34:15', 6="3"] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. No suggestions ASKED letter transported by email at /home/vagrant/kohaclone/C4/Letters.pm line 608. Note the branchcode=__ANY__ Hum, maybe not related to this patchset actually. (In reply to Jonathan Druart from comment #42) > When approving a pending suggestion, nothing happens. In the logs there are > two errors: > > DBD::mysql::st execute failed: Cannot add or update a child row: a foreign > key constraint fails (`koha_kohadev`.`suggestions`, CONSTRAINT > `suggestions_ibfk_branchcode` FOREIGN KEY (`branchcode`) REFERENCES > `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE) [for > Statement "UPDATE `suggestions` SET `STATUS` = ?, `acceptedby` = ?, > `accepteddate` = ?, `branchcode` = ?, `managedby` = ?, `manageddate` = ? > WHERE ( `suggestionid` = ? )" with ParamValues: 0="ACCEPTED", 1="51", > 2='2019-05-14T15:34:15', 3="__ANY__", 4="51", 5='2019-05-14T15:34:15', > 6="3"] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. See bug 22905, not related to this patchset. > No suggestions ASKED letter transported by email at > /home/vagrant/kohaclone/C4/Letters.pm line 608. Not sure what is that (need to do something?), but IIRC it already happened before this patchset as well. New feature will not be backported to 18.11.x series. |