[Bug 24430] New: Remove C4::Biblio::CountBiblioInOrders
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Bug ID: 24430 Summary: Remove C4::Biblio::CountBiblioInOrders Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Using the DBIC relationship with aqorders and count, we can get rid of it. -- 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=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- 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=24430 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 97414 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97414&action=edit Bug 24430: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 97415 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97415&action=edit Bug 24430: Add ->orders and ->orders_count to Koha::Biblio This patch introduces an accessor to the related orders, and a method that returns the orders count. The target usage for the count is the API. In the rest of the codebase, we would just call: $biblio->orders->count To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 97416 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97416&action=edit Bug 24430: Remove CountBiblioInOrders and its traces This patch replaces the only uses of CountBiblioInOrders and makes that code use $biblio->orders->count instead. Test nothing breaks in basket.pl and parcel.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17628, 20212 Status|NEW |Needs Signoff CC| |kyle@bywatersolutions.com, | |liz@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17628 [Bug 17628] Move bibliographic related code to Koha::Biblio https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20212 [Bug 20212] Slowness in Receiving in Acquisitions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97416|0 |1 is obsolete| | --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 97443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97443&action=edit Bug 24430: Remove CountBiblioInOrders and its traces This patch replaces the only uses of CountBiblioInOrders and makes that code use $biblio->orders->count instead. Test nothing breaks in basket.pl and parcel.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97443|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97414|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97415|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 97446 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97446&action=edit Bug 24430: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 97447 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97447&action=edit Bug 24430: Add ->orders and ->active_orders_count to Koha::Biblio This patch introduces an accessor to the related orders, and a method that returns the active orders count. The target usage for the count is the API. In the rest of the codebase, we would just call: $biblio->orders->count To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 97448 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97448&action=edit Bug 24430: Remove CountBiblioInOrders and its traces This patch replaces the only uses of CountBiblioInOrders and makes that code use $biblio->orders->count instead. Test nothing breaks in basket.pl and parcel.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |24418 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24418 [Bug 24418] Add Koha::Biblio->suggestions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24435 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24435 [Bug 24435] Add Koha::Biblio->items_count -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Nick Clemens <nick@bywatersolutions.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=24430 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97446|0 |1 is obsolete| | Attachment #97447|0 |1 is obsolete| | Attachment #97448|0 |1 is obsolete| | --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 97563 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97563&action=edit Bug 24430: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 97564 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97564&action=edit Bug 24430: Add ->orders and ->active_orders_count to Koha::Biblio This patch introduces an accessor to the related orders, and a method that returns the active orders count. The target usage for the count is the API. In the rest of the codebase, we would just call: $biblio->orders->count To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 97565 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97565&action=edit Bug 24430: Remove CountBiblioInOrders and its traces This patch replaces the only uses of CountBiblioInOrders and makes that code use $biblio->orders->count instead. Test nothing breaks in basket.pl and parcel.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nicolas.legrand@bulac.fr -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97563|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 97652 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97652&action=edit Bug 24430: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=24430 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97564|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 97653 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97653&action=edit Bug 24430: Add ->orders and ->active_orders_count to Koha::Biblio This patch introduces an accessor to the related orders, and a method that returns the active orders count. The target usage for the count is the API. In the rest of the codebase, we would just call: $biblio->orders->count To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=24430 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97565|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 97654 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97654&action=edit Bug 24430: Remove CountBiblioInOrders and its traces This patch replaces the only uses of CountBiblioInOrders and makes that code use $biblio->orders->count instead. Test nothing breaks in basket.pl and parcel.pl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=24430 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Trivial improvements that simplify code.. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24467 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24467 [Bug 24467] *_count methods should be avoided -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|24467 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24467 [Bug 24467] Add a syntax for specifying counts on x-koha-embed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24467 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24467 [Bug 24467] *_count methods should be avoided -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Bug 24430 depends on bug 24418, which changed state. Bug 24418 Summary: Add Koha::Biblio->suggestions https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24418 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=24430 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |joy@bywatersolutions.com Status|Pushed to master |RESOLVED --- Comment #16 from Joy Nelson <joy@bywatersolutions.com> --- 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=24430 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24672 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24672 [Bug 24672] Error on receiving orders when there is an order with a deleted record -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- A bit late, but wondering why active_orders include completed lines ? Looks a bit strange to me.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #0)
Using the DBIC relationship with aqorders and count, we can get rid of it.
You could have used aqorders but added orders? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35994 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Adding the orders relation alias is forward thinking for the API here. I agree with the approach Tomas took -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Martin Renvoize from comment #19)
Adding the orders relation alias is forward thinking for the API here. I agree with the approach Tomas took
Yeah no problem. But couldnt you add an ->orders method and use the aqorders relation inside. Preventing custom relations in DBIx schema? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Not with how the API embeds get translated. The real fix is to fix our database field names and database table names to generally be consistency.. but that would upset everyone with reports. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #17)
A bit late, but wondering why active_orders include completed lines ? Looks a bit strange to me..
Looking at Koha::Acquisition::Orders it feels like I might have mixed 'current' vs. 'active'. So a terminology problem. And 'filter_by_active' seems to have a wrong POD (should be 'filtering OUT orders that are not active'). We could have a problem with this embed in 'biblio.active_orders+count' in /acquisitions/orders. Please file a bug! (In reply to Marcel de Rooy from comment #20)
Yeah no problem. But couldnt you add an ->orders method and use the aqorders relation inside. Preventing custom relations in DBIx schema?
We could even do it the other way around: not having any automatic relationship generated, and only define what we are gonna use! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #22)
Looking at Koha::Acquisition::Orders it feels like I might have mixed 'current' vs. 'active'. So a terminology problem. And 'filter_by_active' seems to have a wrong POD (should be 'filtering OUT orders that are not active'). We could have a problem with this embed in 'biblio.active_orders+count' in /acquisitions/orders.
Please file a bug!
Thanks for your feedback. I came here from bug 35994 but I may open yet another bug to address the 'intermediate concerns' as you mentioned. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36018 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #22)
Looking at Koha::Acquisition::Orders it feels like I might have mixed 'current' vs. 'active'. So a terminology problem. And 'filter_by_active' seems to have a wrong POD (should be 'filtering OUT orders that are not active'). We could have a problem with this embed in 'biblio.active_orders+count' in /acquisitions/orders.
Please file a bug!
This is bug 36018. Renaming the _current method there. I checked on the use of active_orders_count and it is only used in parcel.tt. This all looks okay. Do not allow order cancellation with deletion of biblio if there are multiple active orders. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24430 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #24)
I checked on the use of active_orders_count and it is only used in parcel.tt. This all looks okay. Do not allow order cancellation with deletion of biblio if there are multiple active orders.
Hmm. Scratch that. Continued on 36018. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org