[Bug 36137] New: update_totalissues.pl should always skip_holds_queue
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Bug ID: 36137 Summary: update_totalissues.pl should always skip_holds_queue Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: lucas@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com update_totalissues.pl calls UpdateTotalIssues but does not pass a parameter in to skip the building of the holds queue. update_totalissues.pl should never update holdability. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 --- Comment #1 from Lucas Gass <lucas@bywatersolutions.com> --- Looking closer here I see a few more things: I don't see why an if/else is needed here. We can just pass the value $incremental to UpdateTotalIssues - my $ret; - if ( $incremental && $totalissues > 0 ) { - $ret = UpdateTotalIssues( $biblionumber, $totalissues ); - } - else { - $ret = UpdateTotalIssues( $biblionumber, 0, $totalissues ); - } Instead this should work just as good: + my $ret = UpdateTotalIssues( $biblionumber, $incremental, $totalissues, 1 ); Also the POD for skip_holds_queue is missing from C4/Biblio -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 --- Comment #2 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 162301 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162301&action=edit Bug 36137: Make update_totalissues cron always skip the holds queue To test: 1. Run the update_totalissues cron, the holds queue is updated. 2. APPLY PATCH, restart services 3. Run update_totalissues cron again, this time the holds queue should always be skipped. NOTE: This patch also eliminates what looks to me to be an unessesary if/else case. This patch also adds POD for the skips_hold_queue variable in C4/Biblio -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Assignee|koha-bugs@lists.koha-commun |lucas@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162301|0 |1 is obsolete| | --- Comment #3 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 163766 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163766&action=edit Bug 36137: Make update_totalissues cron always skip the holds queue To test: 1. Run the update_totalissues cron, the holds queue is updated. 2. APPLY PATCH, restart services 3. Run update_totalissues cron again, this time the holds queue should always be skipped. NOTE: This patch also eliminates what looks to me to be an unessesary if/else case. This patch also adds POD for the skips_hold_queue variable in C4/Biblio Signed-off-by: Frédéric Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov QA Contact|testopia@bugs.koha-communit |emily.lamancusa@montgomeryc |y.org |ountymd.gov Status|Signed Off |Failed QA --- Comment #4 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- (In reply to Lucas Gass from comment #1)
I don't see why an if/else is needed here. We can just pass the value $incremental to UpdateTotalIssues
- my $ret; - if ( $incremental && $totalissues > 0 ) { - $ret = UpdateTotalIssues( $biblionumber, $totalissues ); - } - else { - $ret = UpdateTotalIssues( $biblionumber, 0, $totalissues ); - }
Instead this should work just as good:
+ my $ret = UpdateTotalIssues( $biblionumber, $incremental, $totalissues, 1 );
The if/else is necessary. In the if branch, $totalissues is being passed as the $increase parameter (i.e. the amount to increment the existing value by). In the else branch, $totalissues is being passed as the $value parameter (i.e. the value to overwrite the existing value). Providing a defined $value parameter causes UpdateTotalIssues to ignore the $increment parameter, so if the --incremental flag is set, the third parameter needs to be undef. Otherwise UpdateTotalIssues will overwrite the existing value instead of incrementing it as desired. That being said...if the --incremental flag is set, but $totalissues is 0 for a particular biblio, the else clause will still trigger, and wipe out the totalissues value for that biblio! So the if/else isn't correct to begin with - those conditions need to be checked separately. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163766|0 |1 is obsolete| | --- Comment #5 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 164610 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164610&action=edit Bug 36137: Make update_totalissues cron always skip the holds queue To test: 1. Run the update_totalissues cron, the holds queue is updated. 2. APPLY PATCH, restart services 3. Run update_totalissues cron again, this time the holds queue should always be skipped. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 --- Comment #6 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to Emily Lamancusa from comment #4)
(In reply to Lucas Gass from comment #1)
I don't see why an if/else is needed here. We can just pass the value $incremental to UpdateTotalIssues
- my $ret; - if ( $incremental && $totalissues > 0 ) { - $ret = UpdateTotalIssues( $biblionumber, $totalissues ); - } - else { - $ret = UpdateTotalIssues( $biblionumber, 0, $totalissues ); - }
Instead this should work just as good:
+ my $ret = UpdateTotalIssues( $biblionumber, $incremental, $totalissues, 1 );
The if/else is necessary. In the if branch, $totalissues is being passed as the $increase parameter (i.e. the amount to increment the existing value by). In the else branch, $totalissues is being passed as the $value parameter (i.e. the value to overwrite the existing value).
Providing a defined $value parameter causes UpdateTotalIssues to ignore the $increment parameter, so if the --incremental flag is set, the third parameter needs to be undef. Otherwise UpdateTotalIssues will overwrite the existing value instead of incrementing it as desired.
That being said...if the --incremental flag is set, but $totalissues is 0 for a particular biblio, the else clause will still trigger, and wipe out the totalissues value for that biblio! So the if/else isn't correct to begin with - those conditions need to be checked separately.
Thanks Emily. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164610|0 |1 is obsolete| | --- Comment #7 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Created attachment 165842 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165842&action=edit Bug 36137: Make update_totalissues cron always skip the holds queue To test: 1. Run the update_totalissues cron, the holds queue is updated. 2. APPLY PATCH, restart services 3. Run update_totalissues cron again, this time the holds queue should always be skipped. Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165842|0 |1 is obsolete| | --- Comment #8 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 166142 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166142&action=edit Bug 36137: Make update_totalissues cron always skip the holds queue To test: 1. Run the update_totalissues cron, the holds queue is updated. 2. APPLY PATCH, restart services 3. Run update_totalissues cron again, this time the holds queue should always be skipped. Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #9 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Looks good now, thanks Lucas! Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- PS : UpdateTotalIssues() needs a review. Maybe it could be turned into a Koha::RecordProcessor say Koha::Filter::MARC::UpdateTotalIssues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.05.00 |24.05.00,23.11.06 released in| | Status|Pushed to main |Pushed to stable --- Comment #12 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|24.05.00,23.11.06 |24.05.00,23.11.06,23.05.12 released in| | --- Comment #13 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.12 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36137 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org