| Summary: | NEW_SUGGESTION is still sent for some modifications to the suggestion | ||
|---|---|---|---|
| Product: | Koha | Reporter: | David Cook <dcook> |
| Component: | Acquisitions | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | Failed QA --- | QA Contact: | Testopia <testopia> |
| Severity: | minor | ||
| Priority: | P5 - low | CC: | david, jonathan.druart, klas.blomberg, lucas, marion.durand, mirjam.vantieghem |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 36122 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 36820: Adjust and add tests
Bug 36820: Do not generate NEW_SUGGESTION if status has not changed |
||
|
Description
David Cook
2024-05-09 06:21:24 UTC
Created attachment 167209 [details] [review] Bug 36820: Adjust and add tests Created attachment 167210 [details] [review] Bug 36820: Do not generate NEW_SUGGESTION if status has not changed If a suggestion is updated but its status has not been modified we should not send a NEW_SUGGESTION notice. I think this patch would cover the first scenario, but not the second scenario. With the patch, if you moved it from "Accepted" back to "Pending", you'd still get a NEW_SUGGESTION email, which I think is probably still not desired? Can confirm David Cook's comment - if status is changed back to pending, an email is sent. Not sure what the correct behavour should be.
Tests fail (so have changed to Failed QA):
prove t/db_dependent/Koha/Suggestions.t
t/db_dependent/Koha/Suggestions.t ..
# Failed test 'If EmailPurchaseSuggestions is not enabled, a message should not be sent'
# at t/db_dependent/Koha/Suggestions.t line 75.
# got: '0'
# expected: '1'
# Failed test 'NEW_SUGGESTION should not be sent again if status has not changed'
# at t/db_dependent/Koha/Suggestions.t line 94.
# got: '0'
# expected: '1'
# Looks like you planned 9 tests but ran 10.
# Looks like you failed 2 tests of 10 run.
t/db_dependent/Koha/Suggestions.t .. 1/11
# Failed test 'store'
# at t/db_dependent/Koha/Suggestions.t line 119.
t/db_dependent/Koha/Suggestions.t .. 11/11 # Looks like you failed 1 test of 11.
t/db_dependent/Koha/Suggestions.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/11 subtests
Test Summary Report
-------------------
t/db_dependent/Koha/Suggestions.t (Wstat: 256 (exited 1) Tests: 11 Failed: 1)
Failed test: 1
Non-zero exit status: 1
Files=1, Tests=11, 8 wallclock secs ( 0.02 usr 0.01 sys + 4.55 cusr 2.98 csys = 7.56 CPU)
Result: FAIL
Testing notes (using KTD):
1. For EmailPurchaseSuggestions/EmailAddressForSuggestions set to 'EmailAddressForSuggestions' and add an email address, such as me@example.com
2. Create a suggestion in the staff interface (Acquisitions > Suggestions > + New purchase suggestion)
3. Check that the message queue for a pending notice:
3.1 koha-mysql kohadev
3.2 select * from message_queue;
4. Edit the suggestion and add some text to the notes field, but don't change the status
5. Repeat step 3 - another NEW_SUGGESTION notice is queued.
6. Change the status of the suggestion from Pending to Accepted.
7. Repeat step 3 - the ACCEPTED notice is queued.
8. Change the status of the suggestion from Accepted to Pending.
9. Repeat step 3 - another NEW_SUGGESTION notice is queued.
10. Apply the patch and restart_all.
11. Repeat steps 2-3 (add a new suggestion).
==> Result: a NEW_SUGGESTION notice is queued for the new suggestion
12. Repeat step 4 (edit a suggestion but don't change the status).
==> Result: NO NEW_SUGGESTION notice is queued
13. Repeat step 6 (change status from Pending to Accepted).
==> Result: the ACCEPTED notice is queued.
14. Repeat step 8 (change status from Accepted to Pending)
==> Result: another NEW_SEUGGESTION notice is queued (not sure whether this is the correct behavour or not - see comment#3)
15. Run the tests, these should pass: prove t/db_dependent/Koha/Suggestions.t
We also get a NEW_SUGGESTION-mail when we change manager. In that case there shold only be a mail NOTIFY_MANAGER Could this patch just be:
- if ( $emailpurchasesuggestions && $self->STATUS eq 'ASKED' ) {
+ if ( $emailpurchasesuggestions && $new_suggestion ) {
?
(In reply to Lucas Gass (lukeg) from comment #6) > Could this patch just be: > > - if ( $emailpurchasesuggestions && $self->STATUS eq 'ASKED' ) { > + if ( $emailpurchasesuggestions && $new_suggestion ) { > > > > ? I don't think so. $new_suggestion is set if the object is not in storage (ie. the object does not exist yet). Here we want to test in an edition context. use t::lib::TestBuilder; my $builder = t::lib::TestBuilder->new; my $suggestion = $builder->build_object({class => 'Koha::Suggestions'}); say $suggestion->in_storage; say $suggestion->quantity; $suggestion->quantity(42); say $suggestion->in_storage; say $suggestion->quantity; say join ", ", $suggestion->_result->get_dirty_columns; will output: 1 17450 1 42 quantity, 42 |