| Summary: | NEW_SUGGESTION is sent for every modification to the suggestion | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Caroline Cyr La Rose <caroline.cyr-la-rose> |
| Component: | Acquisitions | Assignee: | Matthias Le Gac <matthias.le-gac> |
| Status: | Pushed to oldstable --- | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
| Severity: | minor | ||
| Priority: | P5 - low | CC: | aleisha, azmikhalib, dch, dcook, fridolin.somers, klas.blomberg, koha, lucas, marie-luce.laflamme, matthias.le-gac, mirjam.vantieghem, patrick.robitaille, philippe.blouin |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35854 | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
24.05.00,23.11.06,23.05.12
|
|
| Circulation function: | |||
| Bug Depends on: | 33236 | ||
| Bug Blocks: | 36820 | ||
| Attachments: |
Bug 36122: NEW_SUGGESTION is sent for every modification to the suggestion
Bug 36122: add unit test Bug 36122: modif test Bug 36122: NEW_SUGGESTION is sent for every modification to the suggestion Bug 36122: add unit test Bug 36122: modif test Bug 36122: NEW_SUGGESTION is sent for every modification to the suggestion Bug 36122: Add unit test Bug 36122: Update unit test |
||
|
Description
Caroline Cyr La Rose
2024-02-16 19:47:44 UTC
Might be the same as bug 35854, but I'm not 100% sure. I forgot the patron's email address in the test plan. I don't think notices are generated if the patron doesn't have an email address. To recreate: 1. Enable EmailPurchaseSuggestions 2. Go to a patron's account and add an email address, a different one than the one used for EmailPurchaseSuggestions 3. Create a suggestion from the OPAC 3.1. Go to the OPAC and log in as a patron 3.2. Go to Your account > Purchase suggestions 3.3. Click New purchase suggestion 3.4. Enter a title 3.5. Click Submit your suggestion 4. Check the message_queue, there should be one NEW_SUGGESTION message sent to whichever address you chose in EmailPurchaseSuggestions (in a fresh db, there should be 1 entry in message_queue) 5. Update the suggestion to accepted 5.1. In the staff interface, go to Acquisitions > Suggestions 5.2. In the suggestions list, check the box next to the suggestion 5.3. At the bottom of the screen, choose Mark selected as: Accepted 5.4. Click Submit 6. Check the message_queue again, there should be one ACCEPTED message sent to the patron and a new NEW_SUGGESTION message sent to whichever address you chose in EmailPurchaseSuggestions (in a fresh db, there should now be 3 entries in message_queue) 7. Order the suggestion 7.1. Go to Acquisitions 7.2. Search for a vendor 7.3. Click New > Basket 7.4. Enter a basket name (Optional: for testing I like to change Create items when: cataloging the record) 7.5. Click Save 7.6. Click Add to basket 7.7. Click From a suggestion 7.8. Click Order next to the suggestion 7.9. Enter order details 7.10. Click Save 7.11. Click Close basket 7.12. Click Yes, close 8. Check the message_queue again, there should be one ORDERED message sent to the patron and a new NEW_SUGGESTION message sent to whichever address you chose in EmailPurchaseSuggestions (in a fresh db, there should now be 5 entries in message_queue) +1 +1 Created attachment 162962 [details] [review] Bug 36122: NEW_SUGGESTION is sent for every modification to the suggestion Test plan : To recreate: 1. Enable EmailPurchaseSuggestions 2. Go to a patron's account and add an email address, a different one than the one used for EmailPurchaseSuggestions 3. Create a suggestion from the OPAC 3.1. Go to the OPAC and log in as a patron 3.2. Go to Your account > Purchase suggestions 3.3. Click New purchase suggestion 3.4. Enter a title 3.5. Click Submit your suggestion 4. Check the message_queue, there should be one NEW_SUGGESTION message sent to whichever address you chose in EmailPurchaseSuggestions (in a fresh db, there should be 1 entry in message_queue) 5. Update the suggestion to accepted 5.1. In the staff interface, go to Acquisitions > Suggestions 5.2. In the suggestions list, check the box next to the suggestion 5.3. At the bottom of the screen, choose Mark selected as: Accepted 5.4. Click Submit 6. Check the message_queue again, there should be one ACCEPTED message sent to the patron and a new NEW_SUGGESTION message sent to whichever address you chose in EmailPurchaseSuggestions (in a fresh db, there should now be 3 entries in message_queue) 7. Order the suggestion 7.1. Go to Acquisitions 7.2. Search for a vendor 7.3. Click New > Basket 7.4. Enter a basket name (Optional: for testing I like to change Create items when: cataloging the record) 7.5. Click Save 7.6. Click Add to basket 7.7. Click From a suggestion 7.8. Click Order next to the suggestion 7.9. Enter order details 7.10. Click Save 7.11. Click Close basket 7.12. Click Yes, close 8. Check the message_queue again, there should be one ORDERED message sent to the patron and a new NEW_SUGGESTION message sent to whichever address you chose in EmailPurchaseSuggestions (in a fresh db, there should now be 5 entries in message_queue) 9. Apply the patch 10. now repeat steps 3 to 8 but : step 4 you have one NEW_SUGGESTION message in message_queue step 6 you have only one ACCEPTED message in message_queue step 8 you have only one ORDERED message in message_queue The NEW_SUGGESTION are now sent only on the submit of the suggestion and not on each steps. Can we have a Unit test for this please. Created attachment 162989 [details] [review] Bug 36122: add unit test Test plan : To recreate: 1. Enable EmailPurchaseSuggestions 2. Go to a patron's account and add an email address, a different one than the one used for EmailPurchaseSuggestions 3. Create a suggestion from the OPAC 3.1. Go to the OPAC and log in as a patron 3.2. Go to Your account > Purchase suggestions 3.3. Click New purchase suggestion 3.4. Enter a title 3.5. Click Submit your suggestion 4. Check the message_queue, there should be one NEW_SUGGESTION message sent to whichever address you chose in EmailPurchaseSuggestions (in a fresh db, there should be 1 entry in message_queue) 5. Update the suggestion to accepted 5.1. In the staff interface, go to Acquisitions > Suggestions 5.2. In the suggestions list, check the box next to the suggestion 5.3. At the bottom of the screen, choose Mark selected as: Accepted 5.4. Click Submit 6. Check the message_queue again, there should be one ACCEPTED message sent to the patron and a new NEW_SUGGESTION message sent to whichever address you chose in EmailPurchaseSuggestions (in a fresh db, there should now be 3 entries in message_queue) 7. Order the suggestion 7.1. Go to Acquisitions 7.2. Search for a vendor 7.3. Click New > Basket 7.4. Enter a basket name (Optional: for testing I like to change Create items when: cataloging the record) 7.5. Click Save 7.6. Click Add to basket 7.7. Click From a suggestion 7.8. Click Order next to the suggestion 7.9. Enter order details 7.10. Click Save 7.11. Click Close basket 7.12. Click Yes, close 8. Check the message_queue again, there should be one ORDERED message sent to the patron and a new NEW_SUGGESTION message sent to whichever address you chose in EmailPurchaseSuggestions (in a fresh db, there should now be 5 entries in message_queue) 9. Apply the patch 10. now repeat steps 3 to 8 but : step 4 you have one NEW_SUGGESTION message in message_queue step 6 you have only one ACCEPTED message in message_queue step 8 you have only one ORDERED message in message_queue The NEW_SUGGESTION are now sent only on the submit of the suggestion and not on each steps. this is the first time I do test for Koha, so let me know if there's anything to improve. Mattais,
Can you review these tests? Something seems not quite right:
# Subtest: store
1..8
ok 1 - If suggesteddate not passed in, it will default to today
ok 2 - If suggesteddate passed in, it should be taken into account
ok 3 - If suggestion id modified, suggesteddate should not be modified
ok 4 - If EmailPurchaseSuggestions is not enabled, a message should not be sent
not ok 5 - No message was sent
# Failed test 'No message was sent'
# at /kohadevbox/koha/t/db_dependent/Koha/Suggestions.t line 89.
ok 6 - If the status of suggestion is not set to ASKED, a message should not be sent
ok 7 - store raises an exception on invalid STATUS
ok 8 - UNKNOWN status stored
# Looks like you failed 1 test of 8.
not ok 1 - store
Thanks!
(In reply to Lucas Gass from comment #10) > Mattais, > > Can you review these tests? Something seems not quite right: > > # Subtest: store > 1..8 > ok 1 - If suggesteddate not passed in, it will default to today > ok 2 - If suggesteddate passed in, it should be taken into account > ok 3 - If suggestion id modified, suggesteddate should not be modified > ok 4 - If EmailPurchaseSuggestions is not enabled, a message should not > be sent > not ok 5 - No message was sent > > # Failed test 'No message was sent' > # at /kohadevbox/koha/t/db_dependent/Koha/Suggestions.t line 89. > ok 6 - If the status of suggestion is not set to ASKED, a message should > not be sent > ok 7 - store raises an exception on invalid STATUS > ok 8 - UNKNOWN status stored > # Looks like you failed 1 test of 8. > not ok 1 - store > > > Thanks! I can't reproduce the problem, I've tried different situations but test 5 passes every time. Can you add these print "print STDERR "syspref: " . $syspref->variable . " value :" . $syspref->value . "\n";" to line 85 and 86 (before and after the syspref store line) and tell me what you get? To be sure that the changes made to the preferences have been good. Created attachment 164481 [details] [review] Bug 36122: modif test Current status: Needs Signoff I have modified how I change the value of the 'EmailPurchaseSuggestions' syspref during testing. Let me know if it works better now Created attachment 165577 [details] [review] Bug 36122: NEW_SUGGESTION is sent for every modification to the suggestion Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 165578 [details] [review] Bug 36122: add unit test Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 165579 [details] [review] Bug 36122: modif test Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 166051 [details] [review] Bug 36122: NEW_SUGGESTION is sent for every modification to the suggestion Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166052 [details] [review] Bug 36122: Add unit test Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166053 [details] [review] Bug 36122: Update unit test Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Great improvement and bugfix, thanks! QA scripts are happy, the Unit tests cover the change well. Passing QA Pushed for 24.05! Well done everyone, thank you! Well done everyone. I have just gotten a report of this happening to someone, so it's great to see it fixed already upstream! That said... looking at this again and I'm not sure that this is 100% solved yet. Technically, if you moved the status of a suggestion back to "Pending", it would trigger another email. The solution for this probably would've been to use !$self->in_storage before $self->SUPER::store() to set a flag for email sending. (In reply to David Cook from comment #23) > That said... looking at this again and I'm not sure that this is 100% solved > yet. > > Technically, if you moved the status of a suggestion back to "Pending", it > would trigger another email. > > The solution for this probably would've been to use !$self->in_storage > before $self->SUPER::store() to set a flag for email sending. Another issue is if you have a suggestion as "Pending" and a staff user edits it without changing the status, emails will continue to be sent out. Both those raised in bug 36820 Pushed to 23.11.x for 23.11.06 Backported to 23.05.x for upcoming 23.05.12 We have applied this patch, and there is one curiosity: Before this patch there were sent two identical emails using NEW_SUGGESTION - one was sent to the borrower, one was sent to the library After this patch is one email sent, that to the library. No mail is sent to the borrower, although it says so on the message-tab in the borrowerrecord. *** Bug 35816 has been marked as a duplicate of this bug. *** |