[Bug 11814] New: Serial statuses should be stored in constants
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Bug ID: 11814 Summary: Serial statuses should be stored in constants Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: jonathan.druart@biblibre.com QA Contact: testopia@bugs.koha-community.org See bug 11689 comment 3 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Depends on| |11689 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=11814 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 25541 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25541&action=edit Bug 11814 - Use constants to describe statuses Make the code a bit more understandable by using constants to define the status values and reference those. Adds self-documentation. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 25542 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25542&action=edit Bug 11814 - (follow-up) Use constants to describe statuses This patch deals with all hard-coded status codes in the C4::Serials module. Test plan: Test a complete workflow in the serial module (create, order, receive, generate next) trying to use all statuses. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #3 from David Cook <dcook@prosentient.com.au> --- I really like the idea of this patch! It seems like it would make it a lot easier to do development on serials this way! I'm curious about your follow-up. Are there are a few more places that the constants could be used in that SQL? "AND (serial.STATUS IN ($missing_statuses_string) OR ((planneddate < now() AND serial.STATUS =1) OR serial.STATUS = 3 OR serial.STATUS = 7))" Couldn't 1, 3, and 7 be exchanged a variable containing the value from the constant? I don't use serials very often, so I might let someone else with more experience with the workflow try this one out. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25542|0 |1 is obsolete| | --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 27605 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27605&action=edit Bug 11814 - (follow-up) Use constants to describe statuses This patch deals with all hard-coded status codes in the C4::Serials module. Test plan: Test a complete workflow in the serial module (create, order, receive, generate next) trying to use all statuses. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to David Cook from comment #3)
Couldn't 1, 3, and 7 be exchanged a variable containing the value from the constant?
Yes, they should. Done in the last patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |paola.rossi@cineca.it --- Comment #6 from Paola Rossi <paola.rossi@cineca.it> --- I've tried to apply against master 3.17.00.054 Applying: Bug 11814 - Use constants to describe statuses fatal: sha1 information is lacking or useless (C4/Serials.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 11814 - Use constants to describe statuses So I pass the patch to "Patch doesn't apply" status. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 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=11814 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25541|0 |1 is obsolete| | Attachment #27605|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33512 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33512&action=edit Bug 11814 - Use constants to describe statuses Make the code a bit more understandable by using constants to define the status values and reference those. Adds self-documentation. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33513 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33513&action=edit Bug 11814 - (follow-up) Use constants to describe statuses This patch deals with all hard-coded status codes in the C4::Serials module. Test plan: Test a complete workflow in the serial module (create, order, receive, generate next) trying to use all statuses. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33513|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33514 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33514&action=edit Bug 11814 - (follow-up) Use constants to describe statuses This patch deals with all hard-coded status codes in the C4::Serials module. Test plan: Test a complete workflow in the serial module (create, order, receive, generate next) trying to use all statuses. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #10 from Paola Rossi <paola.rossi@cineca.it> --- I've applied the patch against master 3.17.00.054 I've created a subscription [length > 1 issues]. Selecting "Serial collection" of the subscription, I checked the EXPECTED issue and selected "Edit serials". I changed the issue from the EXPECTED status to the ARRIVED status. A new issue in the EXPECTED status should have been created [the next issue], but it was not. So I passed the patch to "Failed QA" status. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33514|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33552 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33552&action=edit Bug 11814 - (follow-up) Use constants to describe statuses This patch deals with all hard-coded status codes in the C4::Serials module. Test plan: Test a complete workflow in the serial module (create, order, receive, generate next) trying to use all statuses. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Paola Rossi from comment #10)
I've applied the patch against master 3.17.00.054
I've created a subscription [length > 1 issues]. Selecting "Serial collection" of the subscription, I checked the EXPECTED issue and selected "Edit serials". I changed the issue from the EXPECTED status to the ARRIVED status. A new issue in the EXPECTED status should have been created [the next issue], but it was not.
Paola, Sorry for that, 1 replacement was wrong! I checked all of them and it should be good now. The new expected issue is now generated. Changes are: @@ -1245,7 +1245,7 @@ sub ModSerialStatus { } $missinglist .= "; $serialseq" - if ( ( grep { $_ == $status } ( MISSING_STATUSES, NOT_ISSUED ) ) && ( $missinglist !~/(^|;)\s*$serialseq(?=;|$)/ ) ); + if ( ( grep { $_ == $status } ( MISSING_STATUSES ) ) && ( $missinglist !~/(^|;)\s*$serialseq(?=;|$)/ ) ); $missinglist .= "; not issued $serialseq" if ( $status == NOT_ISSUED && $missinglist !~ /(^|;)\s*$serialseq(?=;|$)/ ); @@ -1258,7 +1258,7 @@ sub ModSerialStatus { } # create new waited entry if needed (ie : was a "waited" and has changed) - if ( $oldstatus == ARRIVED && $status != ARRIVED ) { + if ( $oldstatus == EXPECTED && $status != EXPECTED ) { my $subscription = GetSubscription($subscriptionid); my $pattern = C4::Serials::Numberpattern::GetSubscriptionNumberpattern($subscription->{numberpattern}); The first one is a bad merge conflict. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #13 from Paola Rossi <paola.rossi@cineca.it> --- Thanks, Jonathan. Today I've applied the patches against master 3.17.00.058 head 13297. I think that the status codes in claims.pl had to be still patched. On koha/serials/claims.pl: Home › Serials › Claims I've tried to claim 2 issues which were in late for a certain subscription. The mail was received correctly, but: a) the "Claims counts" remained 0 for one of the issues (whose serial.serialid was X): ERROR b) the Claim date was not set for the same issue (whose serial.serialid was X): ERROR c) the status of the other issue, whose Claim count and Claim date were set rightly, didn't become "Claimed" 7 (ERROR) [but X, I think]. So I pass the patch to "Failed QA" status. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33890 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33890&action=edit Bug 11814: Fix sql parameters order Oops, bad paramaters passed on executing the sql query produced unexpected behaviors. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #15 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Paola Rossi from comment #13)
Thanks, Jonathan.
Today I've applied the patches against master 3.17.00.058 head 13297.
I think that the status codes in claims.pl had to be still patched.
They are, but a sql query was not executed correctly. The 3 issues you raised seem to be fixed now. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Paola Rossi <paola.rossi@cineca.it> 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=11814 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33512|0 |1 is obsolete| | --- Comment #16 from Paola Rossi <paola.rossi@cineca.it> --- Created attachment 33990 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33990&action=edit Bug 11814 - Use constants to describe statuses I've applied the patches against master 3.17.00.060. Everything is OK [about statuses VS create, receive, generate next, routing list, and subscribers]. I pass the patch to "Signed Off" status. NB1. On Serials, on koha/serials/serials-search.pl, in output, this is the written link for the "Serial receive" Action of an open X subscription: .../koha/serials/serials-edit.pl?subscriptionid=X&serstatus=1,3,7 NB2. misc/cronjobs/serialsUpdate.pl manages the status 3 (LATE). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33552|0 |1 is obsolete| | --- Comment #17 from Paola Rossi <paola.rossi@cineca.it> --- Created attachment 33991 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33991&action=edit Bug 11814 - (follow-up) Use constants to describe statuses -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33890|0 |1 is obsolete| | --- Comment #18 from Paola Rossi <paola.rossi@cineca.it> --- Created attachment 33992 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33992&action=edit Bug 11814: Fix sql parameters order -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |kyle@bywatersolutions.com --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Seems to be causing Serials.t to fail. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #20 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 34550 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34550&action=edit Bug 11814: Fix existing tests -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33990|0 |1 is obsolete| | Attachment #33991|0 |1 is obsolete| | Attachment #33992|0 |1 is obsolete| | Attachment #34550|0 |1 is obsolete| | --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 34563 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34563&action=edit Bug 11814 - Use constants to describe statuses Make the code a bit more understandable by using constants to define the status values and reference those. Adds self-documentation. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 34564 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34564&action=edit Bug 11814 - (follow-up) Use constants to describe statuses This patch deals with all hard-coded status codes in the C4::Serials module. Test plan: Test a complete workflow in the serial module (create, order, receive, generate next) trying to use all statuses. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #23 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 34565 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34565&action=edit Bug 11814: Fix sql parameters order Oops, bad paramaters passed on executing the sql query produced unexpected behaviors. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #24 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 34566 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34566&action=edit Bug 11814: Fix existing tests -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34563|0 |1 is obsolete| | Attachment #34564|0 |1 is obsolete| | Attachment #34565|0 |1 is obsolete| | Attachment #34566|0 |1 is obsolete| | --- Comment #25 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 34567 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34567&action=edit [PASSED QA] Bug 11814 - Use constants to describe statuses Make the code a bit more understandable by using constants to define the status values and reference those. Adds self-documentation. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #26 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 34568 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34568&action=edit [PASSED QA] Bug 11814 - (follow-up) Use constants to describe statuses This patch deals with all hard-coded status codes in the C4::Serials module. Test plan: Test a complete workflow in the serial module (create, order, receive, generate next) trying to use all statuses. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #27 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 34569 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34569&action=edit [PASSED QA] Bug 11814: Fix sql parameters order Oops, bad paramaters passed on executing the sql query produced unexpected behaviors. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 --- Comment #28 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 34570 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34570&action=edit [PASSED QA] Bug 11814: Fix existing tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11814 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Passed QA |Pushed to Master --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Kyle and Jonathan! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org