[Bug 11742] New: Overdue notice/status triggers displaying the wrong notice for default
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Bug ID: 11742 Summary: Overdue notice/status triggers displaying the wrong notice for default Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Tools Assignee: gmcharlt@gmail.com Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org If you copy the notice set for an overdue trigger, overduerules.pl will display the description for the copied notice instead of the actual notice! This is because the copied notice shares the same letter code as the notice that was copied. Video example: http://screencast.com/t/WeEG3qlw -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m Assignee|gmcharlt@gmail.com |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 25892 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25892&action=edit Bug 11742: A letter code should be unique. This patch is a dirty way to fix a design issue on notices. Currently the code assumes that a letter code is unique. Which is wrong, the primary key is module, code, branchcode. But the C4::Letters::GetLetters routine returns a hashref with letter code in keys => only 1 per letter code is returned. I tried to fix the API but it is more complicated than I thought. Test plan: Try to duplicate a letter code using edit, add and copy actions. If you manage to do it, please describe how you did. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- I am not really fond of this patch (especially as it will break the bug 9016 integration). Feel free to provide something else. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |gmcharlt@gmail.com --- Comment #3 from Galen Charlton <gmcharlt@gmail.com> --- Now that the patch for bug 9016 has been pushed, this needs a fresh look. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 28020 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28020&action=edit Bug 11742: Change prototype for GetLetters The GetLetters subroutine should return an arrayref with different letters for a module. Test plan: 0/ Delete your notices with module=claimacquisition, claimissues, serial 1/ Go on the late orders page (acqui/lateorders.pl) and verify you cannot choose a notice for claiming 2/ Create a notice with module=claimacquisition 3/ Go on the late orders page (acqui/lateorders.pl) and verify you can choose the notice for claiming 4/ Go on the Claim serials page (serials/claims.pl) and repeat the same thing with the a "claimissues" notice 5/ Create a new subscription (serials/subscription-add.pl) and verify you cannot choose a notification for patrons. 6/ Create a notice with module "serial" and verify you can. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25892|0 |1 is obsolete| | --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 28021 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28021&action=edit Bug 11742: A letter code should be unique. This patch is a dirty way to fix a design issue on notices. Currently the code assumes that a letter code is unique. Which is wrong, the primary key is module, code, branchcode. Maybe we should add a primary key (id) for the letter table in order to pass the id to the template and correctly manage the letter code duplication. Test plan: Try to duplicate a letter code using edit, add and copy actions. If you manage to do it, please describe how you did. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Heather Braum <hbraum@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hbraum@nekls.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28020|0 |1 is obsolete| | --- Comment #6 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 28687 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28687&action=edit Bug 11742: Change return type for GetLetters The GetLetters subroutine should return an arrayref with different letters for a module. Test plan: 0/ Delete your notices with module=claimacquisition, claimissues, serial 1/ Go on the late orders page (acqui/lateorders.pl) and verify you cannot choose a notice for claiming 2/ Create a notice with module=claimacquisition 3/ Go on the late orders page (acqui/lateorders.pl) and verify you can choose the notice for claiming 4/ Go on the Claim serials page (serials/claims.pl) and repeat the same thing with the a "claimissues" notice 5/ Create a new subscription (serials/subscription-add.pl) and verify you cannot choose a notification for patrons. 6/ Create a notice with module "serial" and verify you can. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28021|0 |1 is obsolete| | --- Comment #7 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 28688 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28688&action=edit Bug 11742: A letter code should be unique. This patch is a dirty way to fix a design issue on notices. Currently the code assumes that a letter code is unique. Which is wrong, the primary key is module, code, branchcode. Maybe we should add a primary key (id) for the letter table in order to pass the id to the template and correctly manage the letter code duplication. Test plan: Try to duplicate a letter code using edit, add and copy actions. If you manage to do it, please describe how you did. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Problems: 1) If you create a new notice with the same module/code/branch as an existing letter it will overwrite the existing letter without notice 2) If you choose to edit a notice, and change it's module/code/branch to match another letter, it will overwrite the other letter, while leaving the one you edited unmodified! I think letters is in serious need of an id column. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 28729 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28729&action=edit Bug 11742: FIX the code parameter was not take into account In C4::Letters::GetLetters, the code filter was not used as a query parameter. Moreover, the JS code was buggy. We only need to check the letter code, except if it is an edit and the letter code has not been changed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Kyle M Hall from comment #8)
Problems:
Good catch Kyle, thanks!
1) If you create a new notice with the same module/code/branch as an existing letter it will overwrite the existing letter without notice
Should be fixed.
2) If you choose to edit a notice, and change it's module/code/branch to match another letter, it will overwrite the other letter, while leaving the one you edited unmodified!
Should be fixed too.
I think letters is in serious need of an id column.
Yes definitely, but it is a big change! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28729|0 |1 is obsolete| | --- Comment #11 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 28956 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28956&action=edit Bug 11742: FIX the code parameter was not take into account In C4::Letters::GetLetters, the code filter was not used as a query parameter. Moreover, the JS code was buggy. We only need to check the letter code, except if it is an edit and the letter code has not been changed. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28687|0 |1 is obsolete| | Attachment #28688|0 |1 is obsolete| | Attachment #28956|0 |1 is obsolete| | --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 29049 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29049&action=edit [PASSED QA] Bug 11742: Change return type for GetLetters The GetLetters subroutine should return an arrayref with different letters for a module. Test plan: 0/ Delete your notices with module=claimacquisition, claimissues, serial 1/ Go on the late orders page (acqui/lateorders.pl) and verify you cannot choose a notice for claiming 2/ Create a notice with module=claimacquisition 3/ Go on the late orders page (acqui/lateorders.pl) and verify you can choose the notice for claiming 4/ Go on the Claim serials page (serials/claims.pl) and repeat the same thing with the a "claimissues" notice 5/ Create a new subscription (serials/subscription-add.pl) and verify you cannot choose a notification for patrons. 6/ Create a notice with module "serial" and verify you can. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. Additional tests done: - copy notice ODUE, on saving you are now prompted to choose a new CODE for the notice - edit new notice, try to set code back to ODUE. You are prompted that the code is already in use. This will prevent people from accidentally overwriting a letter with the same letter code. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 29050 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29050&action=edit [PASSED QA] Bug 11742: A letter code should be unique. This patch is a dirty way to fix a design issue on notices. Currently the code assumes that a letter code is unique. Which is wrong, the primary key is module, code, branchcode. Maybe we should add a primary key (id) for the letter table in order to pass the id to the template and correctly manage the letter code duplication. Test plan: Try to duplicate a letter code using edit, add and copy actions. If you manage to do it, please describe how you did. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 29051 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29051&action=edit [PASSED QA] Bug 11742: FIX the code parameter was not take into account In C4::Letters::GetLetters, the code filter was not used as a query parameter. Moreover, the JS code was buggy. We only need to check the letter code, except if it is an edit and the letter code has not been changed. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master (along with a trivial followup fixing the tests count). Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 29180 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29180&action=edit Bug 11742: (QA followup) wrong number of tests The number of tests was wrong. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I feel like we made a mistake here. Now it's not possible to add notices that have a hardcoded notice code like 'HOLD' for multiple branches. Maybe I am wrong...? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 --- Comment #18 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #17)
I feel like we made a mistake here. Now it's not possible to add notices that have a hardcoded notice code like 'HOLD' for multiple branches.
Maybe I am wrong...?
Good catch. This behavior will be problematic quickly and should be fix before the release. I am going to submit a patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11742 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |13215 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org