Bug 36122 - NEW_SUGGESTION is sent for every modification to the suggestion
Summary: NEW_SUGGESTION is sent for every modification to the suggestion
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor (vote)
Assignee: Matthias Le Gac
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 33236
Blocks:
  Show dependency treegraph
 
Reported: 2024-02-16 19:47 UTC by Caroline Cyr La Rose
Modified: 2024-05-02 13:28 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00


Attachments
Bug 36122: NEW_SUGGESTION is sent for every modification to the suggestion (828 bytes, patch)
2024-03-08 14:01 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 36122: add unit test (2.95 KB, patch)
2024-03-08 21:53 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 36122: modif test (1.88 KB, patch)
2024-04-05 14:21 UTC, Matthias Le Gac
Details | Diff | Splinter Review
Bug 36122: NEW_SUGGESTION is sent for every modification to the suggestion (880 bytes, patch)
2024-04-25 18:52 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 36122: add unit test (3.00 KB, patch)
2024-04-25 18:52 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 36122: modif test (1.93 KB, patch)
2024-04-25 18:52 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 36122: NEW_SUGGESTION is sent for every modification to the suggestion (947 bytes, patch)
2024-05-02 10:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36122: Add unit test (3.07 KB, patch)
2024-05-02 10:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36122: Update unit test (2.00 KB, patch)
2024-05-02 10:18 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Caroline Cyr La Rose 2024-02-16 19:47:44 UTC
If EmailPurchaseSuggestions is enabled, a new NEW_SUGGESTION notice is sent to the staff every time a modification is made to the suggestion.

Create the suggestion --> NEW_SUGGESTION is sent to staff

Accept the suggestion --> ACCEPTED is sent to patron; NEW_SUGGESTION is sent to staff

Order the suggestion --> ORDERED is sent to patron; NEW_SUGGESTION is sent to staff

To recreate:
1. Enable EmailPurchaseSuggestions

2. Create a suggestion from the OPAC
   2.1. Go to the OPAC and log in as a patron
   2.2. Go to Your account > Purchase suggestions
   2.3. Click New purchase suggestion
   2.4. Enter a title
   2.5. Click Submit your suggestion

3. 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)

4. Update the suggestion to accepted 
   4.1. In the staff interface, go to Acquisitions > Suggestions
   4.2. In the suggestions list, check the box next to the suggestion
   4.3. At the bottom of the screen, choose Mark selected as: Accepted
   4.4. Click Submit

5. 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)

6. Order the suggestion
   3.1. Go to Acquisitions
   3.2. Search for a vendor
   3.3. Click New > Basket
   3.4. Enter a basket name
   (Optional: for testing I like to change Create items when: cataloging the record)
   3.5. Click Save
   3.6. Click Add to basket
   3.7. Click From a suggestion
   3.8. Click Order next to the suggestion
   3.9. Enter order details
   3.10. Click Save
   3.11. Click Close basket
   3.12. Click Yes, close

7. 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)
Comment 1 Caroline Cyr La Rose 2024-02-16 19:50:05 UTC
Might be the same as bug 35854, but I'm not 100% sure.
Comment 2 Caroline Cyr La Rose 2024-02-16 20:05:47 UTC
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)
Comment 3 Caroline Cyr La Rose 2024-02-16 20:58:59 UTC
Possibly linked to bug 33236
Comment 4 Mirjam Vantieghem 2024-02-20 15:08:59 UTC
+1
Comment 5 Marie-Luce Laflamme 2024-03-05 19:18:50 UTC
+1
Comment 6 Matthias Le Gac 2024-03-08 14:01:03 UTC
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.
Comment 7 Martin Renvoize 2024-03-08 14:05:02 UTC
Can we have a Unit test for this please.
Comment 8 Matthias Le Gac 2024-03-08 21:53:45 UTC
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.
Comment 9 Matthias Le Gac 2024-03-08 21:55:11 UTC
this is the first time I do test for Koha, so let me know if there's anything to improve.
Comment 10 Lucas Gass 2024-03-09 17:39:02 UTC
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!
Comment 11 Matthias Le Gac 2024-03-11 14:10:55 UTC
(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.
Comment 12 Matthias Le Gac 2024-04-05 14:21:39 UTC
Created attachment 164481 [details] [review]
Bug 36122: modif test

Current status: Needs Signoff
Comment 13 Matthias Le Gac 2024-04-05 14:30:30 UTC
I have modified how I change the value of the 'EmailPurchaseSuggestions' syspref during testing. Let me know if it works better now
Comment 14 Roman Dolny 2024-04-25 18:52:14 UTC
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>
Comment 15 Roman Dolny 2024-04-25 18:52:17 UTC
Created attachment 165578 [details] [review]
Bug 36122: add unit test

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 16 Roman Dolny 2024-04-25 18:52:20 UTC
Created attachment 165579 [details] [review]
Bug 36122: modif test

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 17 Martin Renvoize 2024-05-02 10:18:40 UTC
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>
Comment 18 Martin Renvoize 2024-05-02 10:18:43 UTC
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>
Comment 19 Martin Renvoize 2024-05-02 10:18:46 UTC
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>
Comment 20 Martin Renvoize 2024-05-02 10:19:56 UTC
Great improvement and bugfix, thanks!

QA scripts are happy, the Unit tests cover the change well.

Passing QA
Comment 21 Katrin Fischer 2024-05-02 13:28:51 UTC
Pushed for 24.05!

Well done everyone, thank you!