[Bug 23166] New: Simplify code related to orders in catalogue/*detail.pl
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Bug ID: 23166 Summary: Simplify code related to orders in catalogue/*detail.pl 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: 23084, 9780 Target Milestone: --- Since bug 9780 we are the ability to alert the librarian when a record which has been ordered will be deleted. Then the code to handle the alert has been copied to other catalogue/*detail.pl script, as the JS check is in the toolbar. This code is really unnecessary complicated and sometime simply not necessary. It could be replaced with Koha::Acquisition::Orders->search and make it more readable. (found it working on bug 23084, so writing it on top of it). Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9780 [Bug 9780] Restrict the right to suppress a record used in some order https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23084 [Bug 23084] Replace grep {^$var$} with grep {$var eq $_} -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 90824 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90824&action=edit Bug 23166: Replace the count of orders with Koha::Acq::Orders -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90824|0 |1 is obsolete| | --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 90825 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90825&action=edit Bug 23084: Replace grep {^$var$} with grep {$_ eq $var} We certainly faced 3 similar bugs due to this syntax: bug 23006, bug 22941 and bug 17526. To prevent other issues related to this syntax this patch suggests to replace them all in one go. Test plan: Confirm that the 2 syntaxes are similar Eyeball the patch and confirm that there is no typo! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90825|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 90826 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90826&action=edit Bug 23166: Remove baskets_orders code This is never used. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 90827 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90827&action=edit Bug 23166: Remove baskets_deletedorders code This is never used. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 90828 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90828&action=edit Bug 23166: Replace the count of orders with Koha::Acq::Orders -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |In Discussion CC| |martin.renvoize@ptfs-europe | |.com --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Martin, any suggestions to make this even more readable? Maybe using filter_by? I will be waiting for a decision before applying the change to other *detail.pl scripts. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90826|0 |1 is obsolete| | Attachment #90827|0 |1 is obsolete| | Attachment #90828|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104365 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104365&action=edit Bug 23166: Remove baskets_orders code This is never used. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104366 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104366&action=edit Bug 23166: Remove baskets_deletedorders code This is never used. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104367 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104367&action=edit Bug 23166: Replace the count of orders with Koha::Acq::Orders -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104368 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104368&action=edit Bug 23166: Remove filtering from controllers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104369 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104369&action=edit Bug 23166: Add filter_by_current and _cancelled Koha::Acq::Orders methods -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104370 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104370&action=edit Bug 23166: Call the methods from the .inc We do not longer need the order variables to be passed from the controllers, we can call the methods on the biblio object instead. There is something wrong with our ->search method and TT behaviours, it is hard to retrieve object list in a scalar context. If [% objects.method.count %] is called, objects.method will get the first object of the list and count will explode (Koha::Object->count does not exist) We need to force the call in a scalar context to retrieve an iterator and prevent to fetch all the objects (we could have called all then size, but it's not efficient) If adopted I will move the plugin on a separate bug report to ease backport -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Test plan: Create several orders (on at least 2 bibliographic records) Cancel some Try to delete the bibliographic records => You should the JS alert about existing orders The delete button from the different views where the toolbar is displayed should be tested: - normal detail - ISBD detail - MARC detail - image viewer - labeled MARC - More detail No behaviour changes expected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- This is ready for testing (at least receive feedback on the different changes made by this patch set) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26139 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26139 [Bug 26139] 'Place Hold' button isn't hidden in all biblio views if there's no items available for loan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com, | |tomascohen@gmail.com Keywords| |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|26139 | See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=26139 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26139 [Bug 26139] 'Place hold' button isn't hidden in all detail views if there are no items available for loan -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26139 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26139 [Bug 26139] 'Place hold' button isn't hidden in all detail views if there are no items available for loan -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104365|0 |1 is obsolete| | --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 108991 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108991&action=edit Bug 23166: Remove baskets_orders code This is never used. 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104366|0 |1 is obsolete| | --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 108992 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108992&action=edit Bug 23166: Remove baskets_deletedorders code This is never used. 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104367|0 |1 is obsolete| | --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 108993 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108993&action=edit Bug 23166: Replace the count of orders with Koha::Acq::Orders 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104368|0 |1 is obsolete| | --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 108994 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108994&action=edit Bug 23166: Remove filtering from controllers 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104369|0 |1 is obsolete| | --- Comment #19 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 108995 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108995&action=edit Bug 23166: Add filter_by_current and _cancelled Koha::Acq::Orders methods 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104370|0 |1 is obsolete| | --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 108996 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108996&action=edit Bug 23166: Call the methods from the .inc We do not longer need the order variables to be passed from the controllers, we can call the methods on the biblio object instead. There is something wrong with our ->search method and TT behaviours, it is hard to retrieve object list in a scalar context. If [% objects.method.count %] is called, objects.method will get the first object of the list and count will explode (Koha::Object->count does not exist) We need to force the call in a scalar context to retrieve an iterator and prevent to fetch all the objects (we could have called all then size, but it's not efficient) If adopted I will move the plugin on a separate bug report to ease backport 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #21 from Nick Clemens <nick@bywatersolutions.com> --- Qa tool has complaints: FAIL Koha/Template/Plugin/Context.pm FAIL pod coverage POD is missing for 'Scalar' FAIL koha-tmpl/intranet-tmpl/prog/en/includes/catalog-strings.inc FAIL valid_template Can't call method "filter_by_current" without a package or object reference at /kohadevbox/koha/Koha/Template/Plugin/Context.pm line 43. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #108991|0 |1 is obsolete| | Attachment #108992|0 |1 is obsolete| | Attachment #108993|0 |1 is obsolete| | Attachment #108994|0 |1 is obsolete| | Attachment #108995|0 |1 is obsolete| | Attachment #108996|0 |1 is obsolete| | --- Comment #22 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 108997 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108997&action=edit Bug 23166: Remove baskets_orders code This is never used. Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #23 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 108998 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108998&action=edit Bug 23166: Remove baskets_deletedorders code This is never used. Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #24 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 108999 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108999&action=edit Bug 23166: Replace the count of orders with Koha::Acq::Orders Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #25 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109000 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109000&action=edit Bug 23166: Remove filtering from controllers Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #26 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109001 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109001&action=edit Bug 23166: Add filter_by_current and _cancelled Koha::Acq::Orders methods Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #27 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109002 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109002&action=edit Bug 23166: Call the methods from the .inc We do not longer need the order variables to be passed from the controllers, we can call the methods on the biblio object instead. There is something wrong with our ->search method and TT behaviours, it is hard to retrieve object list in a scalar context. If [% objects.method.count %] is called, objects.method will get the first object of the list and count will explode (Koha::Object->count does not exist) We need to force the call in a scalar context to retrieve an iterator and prevent to fetch all the objects (we could have called all then size, but it's not efficient) If adopted I will move the plugin on a separate bug report to ease backport Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff CC| |joonas.kylmala@helsinki.fi --- Comment #28 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- I totally missed running the qa tool, thanks for noticing Nick! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #29 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109019 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109019&action=edit Bug 23166: (QA follow-up) Add POD text -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #108997|0 |1 is obsolete| | Attachment #108998|0 |1 is obsolete| | Attachment #108999|0 |1 is obsolete| | Attachment #109000|0 |1 is obsolete| | Attachment #109001|0 |1 is obsolete| | Attachment #109002|0 |1 is obsolete| | Attachment #109019|0 |1 is obsolete| | --- Comment #30 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109020 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109020&action=edit Bug 23166: Remove baskets_orders code This is never used. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #31 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109021 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109021&action=edit Bug 23166: Remove baskets_deletedorders code This is never used. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #32 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109022 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109022&action=edit Bug 23166: Replace the count of orders with Koha::Acq::Orders -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #33 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109023 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109023&action=edit Bug 23166: Remove filtering from controllers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #34 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109024 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109024&action=edit Bug 23166: Add filter_by_current and _cancelled Koha::Acq::Orders methods -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #35 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109025 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109025&action=edit Bug 23166: Call the methods from the .inc We do not longer need the order variables to be passed from the controllers, we can call the methods on the biblio object instead. There is something wrong with our ->search method and TT behaviours, it is hard to retrieve object list in a scalar context. If [% objects.method.count %] is called, objects.method will get the first object of the list and count will explode (Koha::Object->count does not exist) We need to force the call in a scalar context to retrieve an iterator and prevent to fetch all the objects (we could have called all then size, but it's not efficient) If adopted I will move the plugin on a separate bug report to ease backport -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #36 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109026 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109026&action=edit Bug 23166: (QA follow-up) Add POD text -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #37 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- I attached a follow-up for the POD and removed the sign-offs given the other issue Nick pointed out still remains:
FAIL koha-tmpl/intranet-tmpl/prog/en/includes/catalog-strings.inc FAIL valid_template Can't call method "filter_by_current" without a package or object reference at /kohadevbox/koha/Koha/Template/Plugin/Context.pm line 43.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #38 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- There seems to be nothing wrong with the code, it is just that the QA tool doesn't add the "orders" object because this is not being executed from the real .pl script, so therefore the $set variable is undef. I think we could say this is false positive then, right? Or if we want to prevent the QA tool failing maybe such a check can be added: sub Scalar { my ( $self, $set, $method ) = @_; + return undef unless $set; $set = $set->$method; return $set; } What do you think? I think it is safe to ignore this here and we could go ahead with the current patches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 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=23166 --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Joonas Kylmälä from comment #37)
I attached a follow-up for the POD and removed the sign-offs given the other issue Nick pointed out still remains:
FAIL koha-tmpl/intranet-tmpl/prog/en/includes/catalog-strings.inc FAIL valid_template Can't call method "filter_by_current" without a package or object reference at /kohadevbox/koha/Koha/Template/Plugin/Context.pm line 43.
Hi Joonas, we usually don't remove the sign-off lines - that's one reason we prefer follow-ups - because it allows to see the changes, but also because that means you don't change what has been signed off by someone. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109020|0 |1 is obsolete| | --- Comment #40 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109037 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109037&action=edit Bug 23166: Remove baskets_orders code This is never used. 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109021|0 |1 is obsolete| | --- Comment #41 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109038&action=edit Bug 23166: Remove baskets_deletedorders code This is never used. 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109022|0 |1 is obsolete| | --- Comment #42 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109039&action=edit Bug 23166: Replace the count of orders with Koha::Acq::Orders 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109023|0 |1 is obsolete| | --- Comment #43 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109040&action=edit Bug 23166: Remove filtering from controllers 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109024|0 |1 is obsolete| | --- Comment #44 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109041&action=edit Bug 23166: Add filter_by_current and _cancelled Koha::Acq::Orders methods 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109025|0 |1 is obsolete| | --- Comment #45 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109042 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109042&action=edit Bug 23166: Call the methods from the .inc We do not longer need the order variables to be passed from the controllers, we can call the methods on the biblio object instead. There is something wrong with our ->search method and TT behaviours, it is hard to retrieve object list in a scalar context. If [% objects.method.count %] is called, objects.method will get the first object of the list and count will explode (Koha::Object->count does not exist) We need to force the call in a scalar context to retrieve an iterator and prevent to fetch all the objects (we could have called all then size, but it's not efficient) If adopted I will move the plugin on a separate bug report to ease backport 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=23166 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109026|0 |1 is obsolete| | --- Comment #46 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 109043 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109043&action=edit Bug 23166: (QA follow-up) Add POD text 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=23166 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109037|0 |1 is obsolete| | Attachment #109038|0 |1 is obsolete| | Attachment #109039|0 |1 is obsolete| | Attachment #109040|0 |1 is obsolete| | Attachment #109041|0 |1 is obsolete| | Attachment #109042|0 |1 is obsolete| | Attachment #109043|0 |1 is obsolete| | --- Comment #47 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109044 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109044&action=edit Bug 23166: Remove baskets_orders code This is never used. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #48 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109045 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109045&action=edit Bug 23166: Remove baskets_deletedorders code This is never used. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #49 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109046 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109046&action=edit Bug 23166: Replace the count of orders with Koha::Acq::Orders Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #50 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109047 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109047&action=edit Bug 23166: Remove filtering from controllers Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #51 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109048 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109048&action=edit Bug 23166: Add filter_by_current and _cancelled Koha::Acq::Orders methods Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #52 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109049 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109049&action=edit Bug 23166: Call the methods from the .inc We do not longer need the order variables to be passed from the controllers, we can call the methods on the biblio object instead. There is something wrong with our ->search method and TT behaviours, it is hard to retrieve object list in a scalar context. If [% objects.method.count %] is called, objects.method will get the first object of the list and count will explode (Koha::Object->count does not exist) We need to force the call in a scalar context to retrieve an iterator and prevent to fetch all the objects (we could have called all then size, but it's not efficient) If adopted I will move the plugin on a separate bug report to ease backport Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #53 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109050 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109050&action=edit Bug 23166: (QA follow-up) Add POD text Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #54 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Katrin Fischer from comment #39)
we usually don't remove the sign-off lines - that's one reason we prefer follow-ups - because it allows to see the changes, but also because that means you don't change what has been signed off by someone.
sorry for the mess! I was working on signing-off the patches the same time as Nick and hadn't noticed he had submitted his signed-off-by lines just some minutes earlier so when I ran git-bz attach with my patches it overrode his signed-off-by lines with mine. And then Nick had noticed the QA tool failure after his sign-off and put the bug to FQA state so I thought it would be best to wait for him to sign-off again, which also prompted for me to remove my own SO lines. Anyhow, Nick signed-off this now and I added my QA sign-off so ready for RM to merge :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #55 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Sounds good and I can stop to stare at this because I am getting nowhere tonight :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #56 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 109058 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109058&action=edit Bug 23166: Early return if no set It prevents valid_template check to raise Can't call method "filter_by_current" without a package or object reference at /kohadevbox/koha/Koha/Template/Plugin/Context.pm line 49. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #57 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Joonas Kylmälä from comment #38)
There seems to be nothing wrong with the code, it is just that the QA tool doesn't add the "orders" object because this is not being executed from the real .pl script, so therefore the $set variable is undef. I think we could say this is false positive then, right? Or if we want to prevent the QA tool failing maybe such a check can be added:
sub Scalar { my ( $self, $set, $method ) = @_; + return undef unless $set; $set = $set->$method; return $set; }
What do you think? I think it is safe to ignore this here and we could go ahead with the current patches.
I've decided to fix it here, it should not hurt. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109058|0 |1 is obsolete| | --- Comment #58 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 109059 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=109059&action=edit Bug 23166: Early return if no set It prevents valid_template check to raise Can't call method "filter_by_current" without a package or object reference at /kohadevbox/koha/Koha/Template/Plugin/Context.pm line 49. Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 --- Comment #59 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Jonathan Druart from comment #57)
I've decided to fix it here, it should not hurt.
Ok, I added my SO for it in case you didn't push yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 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=23166 --- Comment #60 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=23166 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|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=23166 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26365 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26365 [Bug 26365] Koha/Acquisition/Order.t is failing with MySQL 8 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #61 from Lucas Gass <lucas@bywatersolutions.com> --- enhancement will not be backported to 20.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Bug 23166 depends on bug 23084, which changed state. Bug 23084 Summary: Replace grep {^$var$} with grep {$var eq $_} https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23084 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=23166 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29283 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29283 [Bug 29283] Cannot delete basket with cancelled order for deleted biblio -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23166 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30915 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30915 [Bug 30915] "Scalar" TT plugin no longer needed -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org