[Bug 23070] New: Use Koha::Hold in C4::Reserves::RevertWaitingStatus
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Bug ID: 23070 Summary: Use Koha::Hold in C4::Reserves::RevertWaitingStatus Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 9834 Target Milestone: --- We are using raw SQL statements, we should use Koha::Hold instead. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9834 [Bug 9834] Reverting a waiting hold should lead to the former hold type (item or biblio level) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 90400 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90400&action=edit Bug 23070: Use Koha::Hold in C4::Reserves::RevertWaitingStatus We are using raw SQL statements, we should use Koha::Hold instead. This patch does not seem optimal, we would like to increment priority in only 1 statement and without the need to fetch and loop all holds. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com Status|ASSIGNED |In Discussion --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I am missing a DBIx::Class trick to make this better. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #2)
I am missing a DBIx::Class trick to make this better.
If we were not getting rid of being able to call dbic's 'update' routine there might be.. though even then I think you'd need to pass some raw sql via a scalar ref.. something along the lines `Koha::Holds->search({ biblionumber => $hold->biblionumber, priority => { '>' => 0 } })->update({ priority => \'priority + 1' });` However, as we're keen to ensure we always go via our 'store' codepath to catch triggers then that is the equivalent to having to call ->update_all in DBIx::Class::ResultSet world.. which as that triggers a loop internally and a db hit per row rather than a db hit for the set then what you've got here seems the best we can do. We could possibly get clever when we work on a solution for our version of update and check whether there are triggers in the local store routing and either call dbics underlying update directly or manipulate it into a loop which calls our store routines much like dbic does for update_all. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I do not think we should force developers to use store. For instance here it's not needed, we do not want to change the value for priority. The problem appears especially when we add or update from the interface. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Catch me on IRC.. I'm not sure I understood your last comment.. might just be 'early morning brain' of course. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- <Joubu> About 23070, not sure I will manage to rephrase it :) <Joubu> ->store will check the default values (triggering Koha::Object->store), and also do some stuffs (like logging, etc.) <Joubu> But in some cases (like the one we have on 23070), we know that it won't be necessary. The +1 query will not need the default values check or anything else, we just want to increment a value in DB. <Joubu> That a good example to let people use ->update directly, even if it should considered as a not recommended method <Joubu> And QA will have to be careful about that -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23185 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 97976 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97976&action=edit Bug 23070: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 97977 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97977&action=edit Bug 23070: Increment all priorities in 1 query -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #3)
(In reply to Jonathan Druart from comment #2)
I am missing a DBIx::Class trick to make this better.
If we were not getting rid of being able to call dbic's 'update' routine there might be.. though even then I think you'd need to pass some raw sql via a scalar ref.. something along the lines
`Koha::Holds->search({ biblionumber => $hold->biblionumber, priority => { '>' => 0 } })->update({ priority => \'priority + 1' });`
However, as we're keen to ensure we always go via our 'store' codepath to catch triggers then that is the equivalent to having to call ->update_all in DBIx::Class::ResultSet world.. which as that triggers a loop internally and a db hit per row rather than a db hit for the set then what you've got here seems the best we can do.
We could possibly get clever when we work on a solution for our version of update and check whether there are triggers in the local store routing and either call dbics underlying update directly or manipulate it into a loop which calls our store routines much like dbic does for update_all.
I'm leaning towards thinking we shouldn't use a catch-all ->store method, but an ->insert and an ->update methods... There's a reasoning behind how DBIC does things... We've gone too far with our use of ->update_or_insert and we end up having a big IF clause on the more interesting Koha::Object-derived classes, to detect if it is an update or an insert... And I also belive many business stuffs should be on the controllers instead. We are inserting too many things in the DAO (not exactly the pattern, I know) only to be backwards compatible. That said, this patches look correct the way our codebase is going forward. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90400|0 |1 is obsolete| | Attachment #97976|0 |1 is obsolete| | Attachment #97977|0 |1 is obsolete| | --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 99168 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99168&action=edit Bug 23070: Use Koha::Hold in C4::Reserves::RevertWaitingStatus We are using raw SQL statements, we should use Koha::Hold instead. This patch does not seem optimal, we would like to increment priority in only 1 statement and without the need to fetch and loop all holds. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 99169 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99169&action=edit Bug 23070: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 99170 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99170&action=edit Bug 23070: Increment all priorities in 1 query -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |21944 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Patches rebased on top of bug 21944. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21944 [Bug 21944] Fix waiting holds at wrong location bug -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |23185 Status|Needs Signoff |ASSIGNED Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23185 [Bug 23185] Koha::Objects supports passing through 'update' which means we can side step 'set' + 'store' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_05_candidate, | |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #14 from Victor Grousset/tuxayo <victor@tuxayo.net> --- status: ASSIGNED Should it be needs signoff? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Victor Grousset/tuxayo from comment #14)
status: ASSIGNED
Should it be needs signoff?
No, if it is ASSIGNED, it's not ready yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99168|0 |1 is obsolete| | Attachment #99169|0 |1 is obsolete| | Attachment #99170|0 |1 is obsolete| | --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 103907 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103907&action=edit Bug 23070: Use Koha::Hold in C4::Reserves::RevertWaitingStatus We are using raw SQL statements, we should use Koha::Hold instead. This patch does not seem optimal, we would like to increment priority in only 1 statement and without the need to fetch and loop all holds. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 103908 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103908&action=edit Bug 23070: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 103909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103909&action=edit Bug 23070: Increment all priorities in 1 query -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 103910 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103910&action=edit Bug 23070: Pass no_triggers => 1 to Koha::Objects->update To make sure we will update all the objects in one go (and no trigger the ->set->store from Koha::Object->update) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bgkriegel@gmail.com --- Comment #20 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Test pass, is there anything else to test? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Bernardo Gonzalez Kriegel from comment #20)
Test pass, is there anything else to test?
Yes, place some holds on the same record, mark on as waiting then revert the waiting status and confirm that the priorities are recalculated correctly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #22 from Victor Grousset/tuxayo <victor@tuxayo.net> --- I got an error on the tests, is there any thing else to do before running them? #### Error: kohadev-koha@661a25cf6198:/kohadevbox/koha$ time prove t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 8/63 Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 558. Use of uninitialized value in string eq at /kohadevbox/koha/C4/Reserves.pm line 558. t/db_dependent/Reserves.t .. 60/63 # Looks like you planned 2 tests but ran 1. # Failed test 'item level hold' # at t/db_dependent/Reserves.t line 961. # Looks like you planned 2 tests but ran 1. # Looks like you failed 1 test of 1 run. # Failed test 'reserves.item_level_hold' # at t/db_dependent/Reserves.t line 992. DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'no_triggers' in 'field list' [for Statement "UPDATE `reserves` SET `no_triggers` = ?, `priority` = priority + 1 WHERE ( ( `biblionumber` = ? AND `priority` > ? ) )" with ParamValues: 0=1, 1=458, 2=0] at /kohadevbox/koha/Koha/Objects.pm line 222 # Looks like your test exited with 11 just after 61. t/db_dependent/Reserves.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 3/63 subtests Test Summary Report ------------------- t/db_dependent/Reserves.t (Wstat: 2816 Tests: 61 Failed: 1) Failed test: 61 Non-zero exit status: 11 Parse errors: Bad plan. You planned 63 tests but ran 61. Files=1, Tests=61, 6 wallclock secs ( 0.04 usr 0.01 sys + 4.78 cusr 0.72 csys = 5.55 CPU) Result: FAIL real 0m6.765s user 0m4.890s sys 0m0.749s -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 didier <didier.gautheron@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |didier.gautheron@biblibre.c | |om --- Comment #23 from didier <didier.gautheron@biblibre.com> --- (In reply to Victor Grousset/tuxayo from comment #22)
I got an error on the tests, is there any thing else to do before running them? Look like theres's a bug should be
diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 38db7d6e0e..397c33f0bd 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1994,7 +1994,7 @@ sub RevertWaitingStatus { ## Increment the priority of all other non-waiting ## reserves for this bib record my $holds = Koha::Holds->search({ biblionumber => $hold->biblionumber, priority => { '>' => 0 } }) - ->update({ priority => \'priority + 1', no_triggers => 1 }); + ->update({ priority => \'priority + 1'}, {no_triggers => 1 }); ## Fix up the currently waiting reserve $hold->set( -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #24 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi didier, if you spot something wrong, please don't hesitate to set "Failed QA" as this usually alerts the dev. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 --- Comment #25 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Thanks Didier, I wasn't sure if I could set to "Failed QA" because my setup or way of testing could be wrong. However, you definitely found something fishy. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103910|0 |1 is obsolete| | --- Comment #26 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104253 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104253&action=edit Bug 23070: Pass no_triggers => 1 to Koha::Objects->update To make sure we will update all the objects in one go (and no trigger the ->set->store from Koha::Object->update) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #27 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- A follow-up from bug 23185 modified the way we pass no_triggers. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103907|0 |1 is obsolete| | --- Comment #28 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 104261 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104261&action=edit Bug 23070: Use Koha::Hold in C4::Reserves::RevertWaitingStatus We are using raw SQL statements, we should use Koha::Hold instead. This patch does not seem optimal, we would like to increment priority in only 1 statement and without the need to fetch and loop all holds. == Test plan == - apply patch - place some holds on the same record - check that the priorities look good - mark one hold as waiting by doing a check-in - revert the waiting status - confirm that the priorities are recalculated correctly Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103908|0 |1 is obsolete| | --- Comment #29 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 104262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104262&action=edit Bug 23070: Add tests Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103909|0 |1 is obsolete| | --- Comment #30 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 104263 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104263&action=edit Bug 23070: Increment all priorities in 1 query Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104253|0 |1 is obsolete| | --- Comment #31 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 104264 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104264&action=edit Bug 23070: Pass no_triggers => 1 to Koha::Objects->update To make sure we will update all the objects in one go (and no trigger the ->set->store from Koha::Object->update) Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #32 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Jonathan Druart from comment #21)
(In reply to Bernardo Gonzalez Kriegel from comment #20)
Test pass, is there anything else to test?
Yes, place some holds on the same record, mark on as waiting then revert the waiting status and confirm that the priorities are recalculated correctly.
Test plan added to the patch based on the above. Details of what I tried: the holds: patron 1 prio 1 patron 2 prio 2 patron 3 prio 3 patron 4 prio 4 patron 5 prio 5 patron 6 prio 6 check in patron 1 waiting patron 2 prio 1 patron 3 prio 2 patron 4 prio 3 patron 5 prio 4 patron 6 prio 5 revert prio same a before :D automated tests passes :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_05_candidate, |rel_20_11_target |RM_priority | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104261|0 |1 is obsolete| | --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 106556 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106556&action=edit Bug 23070: Use Koha::Hold in C4::Reserves::RevertWaitingStatus We are using raw SQL statements, we should use Koha::Hold instead. This patch does not seem optimal, we would like to increment priority in only 1 statement and without the need to fetch and loop all holds. == Test plan == - apply patch - place some holds on the same record - check that the priorities look good - mark one hold as waiting by doing a check-in - revert the waiting status - confirm that the priorities are recalculated correctly Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104262|0 |1 is obsolete| | --- Comment #34 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 106557 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106557&action=edit Bug 23070: Add tests Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104263|0 |1 is obsolete| | --- Comment #35 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 106558 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106558&action=edit Bug 23070: Increment all priorities in 1 query Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104264|0 |1 is obsolete| | --- Comment #36 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 106559 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106559&action=edit Bug 23070: Pass no_triggers => 1 to Koha::Objects->update To make sure we will update all the objects in one go (and no trigger the ->set->store from Koha::Object->update) Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.11.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=23070 --- Comment #37 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to master |Pushed to stable Version(s)|20.11.00 |20.11.00, 20.05.03 released in| | --- Comment #38 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 20.05.x for 20.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #39 from Aleisha Amohia <aleisha@catalyst.net.nz> --- enhancement, not backported to 19.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_11_target | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Bug 23070 depends on bug 23185, which changed state. Bug 23185 Summary: Koha::Objects supports passing through 'update' which means we can side step 'set' + 'store' https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23185 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23070 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30630 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30630 [Bug 30630] Checking in a waiting hold at another branch when HoldsAutoFill is enabled causes errors -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org