[Bug 36018] New: Improve consistency in Acquisition/Order(s) regarding active/current orders
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Bug ID: 36018 Summary: Improve consistency in Acquisition/Order(s) regarding active/current orders 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: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org See also bug 24430 and bug 35994. Current should be: not cancelled. Active should be: not cancelled and not completed. At least this should be the meaning in both modules and their methods. Since active and current is still quite close, perhaps we should even think about an alternative for current. We mean not cancelled, and they could be complete. -- 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=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=36018 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, | |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24430 -- 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=36018 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Hmm. On the other hand we quite often use filter_by_current and there is just one place where we use filter_by_active. So we could go the other way around.. -- 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=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- git grep -P 'filter.*_out(?!side)' Clubs.pm:=head3 filter_out_empty Clubs.pm: my $filtered_rs = $clubs_rs->filter_out_empty; Clubs.pm:sub filter_out_empty { Holds.pm:=head3 filter_out_has_cancellation_requests Holds.pm: my $holds_without_cancellation_requests = $holds->filter_out_has_cancellation_requests; Holds.pm:sub filter_out_has_cancellation_requests { Items.pm: $result = $result->filter_out_lost; Items.pm:=head3 filter_out_lost Items.pm: my $filered_items = $items->filter_out_lost; Items.pm:sub filter_out_lost { -- 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=36018 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- If filter_out_ is a pattern with consensus, then we need filter_by_current for current orders (not cancelled or completed) and filter_out_cancelled to get the current ones plus the completed ones. -- 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=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 162318 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162318&action=edit Bug 36018: Rename orders->filter_by_current Current is a bit misleading here. It means the ones that are not cancelled. We already use the filter_out_etc naming schema. So in harmony with that, we switch it to filter_out_cancelled. Test plan: Run t/db_dependent/Koha/Acquisition/Order.t Run git grep -l '::Orders' | xargs grep filter_by_current * All occurrences should be related to Recalls, not Orders. Go to acqui/basket.pl. Delete a whole basket who has some active and some cancelled lines. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35994 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35994 [Bug 35994] Need an easy way to see if biblio record or item is still in acquisition -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Hmm. We still have an inconsistency in Koha::Biblio. It also has active_orders but it defines them as not cancelled.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=36018 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 162320 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162320&action=edit Bug 36018: biblio->active_orders should be ->uncancelled_orders Active orders are more than just not cancelled. See filter_by_active in Koha::Acquisition::Orders. They are still in the acq process; we still need to receive items on those orders. When we cancel and want to delete a biblio, we should check for not cancelled orders (broader than active orders as in Orders.pm). Test plan: Git grep active_orders. Run t/db_dependent/Koha/Biblio.t Run t/db_dependent/Koha/Acquisition/Order.t Add two order lines in a basket referring to same biblio. Try to cancel with delete biblio. Should not be possible. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #5)
Hmm. We still have an inconsistency in Koha::Biblio. It also has active_orders but it defines them as not cancelled..
Main take-away for second patch: Biblio->active_orders is misleading. We are not just looking at order still in acq process, but at all not-cancelled orders. We will only delete a biblio if there is not another uncancelled order in the system for that biblio. So we adhere to the active definition of Koha::Acquisition::Orders->filter_by_active. Active is more than not cancelled. Look at the status and if you received all items. The third patch will make that more explicit even. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 162321 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162321&action=edit Bug 36018: $orders->filter_by_active should check quantityreceived An active order that has no items to receive any more is actually not very active anymore :) Test plan: Run t/db_dependent/Koha/Acquisition/Orders.t Enable OPACAcquisitionDetails. Add order on basket for new biblio. Check that opac-detail does not yet count the item as on order (since status is still new). Close basket. Check opac-detail again. Should count it now. Reopen basket. Cancel line. Check opac-detail again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs Signoff |Patch doesn't apply --- Comment #9 from David Nind <david@davidnind.com> --- The patch no longer applies (sha1/fake ancestor error): git bz apply 36018 Bug 36018 - Improve consistency in Acquisition/Order(s) regarding active/current orders 162318 - Bug 36018: Rename orders->filter_by_current 162320 - Bug 36018: biblio->active_orders should be ->uncancelled_orders 162321 - Bug 36018: $orders->filter_by_active should check quantityreceived Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36018: Rename orders->filter_by_current Applying: Bug 36018: biblio->active_orders should be ->uncancelled_orders error: sha1 information is lacking or useless (acqui/basket.pl). error: could not build fake ancestor Patch failed at 0001 Bug 36018: biblio->active_orders should be ->uncancelled_orders -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162318|0 |1 is obsolete| | --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 163769 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163769&action=edit Bug 36018: Rename orders->filter_by_current Current is a bit misleading here. It means the ones that are not cancelled. We already use the filter_out_etc naming schema. So in harmony with that, we switch it to filter_out_cancelled. Test plan: Run t/db_dependent/Koha/Acquisition/Order.t Run git grep -l '::Orders' | xargs grep filter_by_current * All occurrences should be related to Recalls, not Orders. Go to acqui/basket.pl. Delete a whole basket who has some active and some cancelled lines. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162320|0 |1 is obsolete| | --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 163770 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163770&action=edit Bug 36018: biblio->active_orders should be ->uncancelled_orders Active orders are more than just not cancelled. See filter_by_active in Koha::Acquisition::Orders. They are still in the acq process; we still need to receive items on those orders. When we cancel and want to delete a biblio, we should check for not cancelled orders (broader than active orders as in Orders.pm). Test plan: Git grep active_orders. Run t/db_dependent/Koha/Biblio.t Run t/db_dependent/Koha/Acquisition/Order.t Add two order lines in a basket referring to same biblio. Try to cancel with delete biblio. Should not be possible. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162321|0 |1 is obsolete| | --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 163771 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163771&action=edit Bug 36018: $orders->filter_by_active should check quantityreceived An active order that has no items to receive any more is actually not very active anymore :) Test plan: Run t/db_dependent/Koha/Acquisition/Orders.t Enable OPACAcquisitionDetails. Add order on basket for new biblio. Check that opac-detail does not yet count the item as on order (since status is still new). Close basket. Check opac-detail again. Should count it now. Reopen basket. Cancel line. Check opac-detail again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 163772 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163772&action=edit Bug 36018: (follow-up) Do not rely on assumed correct quantities Now filter_by_active checks quantities, we should ascertain that our test data is correct, not leaving it to random values from TestBuilder. Test plan: Run t/db_dependent/Koha/Acquisition/Order.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to David Nind from comment #9)
The patch no longer applies (sha1/fake ancestor error):
Hi David, Thx for testing. I managed them to apply them on current master with auto merge. And still found an issue in a unit test. So added a follow-up as well. Could you retry please? Thanks -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Janusz Kaczmarek <januszop@gmail.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=36018 Janusz Kaczmarek <januszop@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163769|0 |1 is obsolete| | --- Comment #15 from Janusz Kaczmarek <januszop@gmail.com> --- Created attachment 164953 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164953&action=edit Bug 36018: Rename orders->filter_by_current Current is a bit misleading here. It means the ones that are not cancelled. We already use the filter_out_etc naming schema. So in harmony with that, we switch it to filter_out_cancelled. Test plan: Run t/db_dependent/Koha/Acquisition/Order.t Run git grep -l '::Orders' | xargs grep filter_by_current * All occurrences should be related to Recalls, not Orders. Go to acqui/basket.pl. Delete a whole basket who has some active and some cancelled lines. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Janusz Kaczmarek <januszop@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Janusz Kaczmarek <januszop@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163770|0 |1 is obsolete| | --- Comment #16 from Janusz Kaczmarek <januszop@gmail.com> --- Created attachment 164954 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164954&action=edit Bug 36018: biblio->active_orders should be ->uncancelled_orders Active orders are more than just not cancelled. See filter_by_active in Koha::Acquisition::Orders. They are still in the acq process; we still need to receive items on those orders. When we cancel and want to delete a biblio, we should check for not cancelled orders (broader than active orders as in Orders.pm). Test plan: Git grep active_orders. Run t/db_dependent/Koha/Biblio.t Run t/db_dependent/Koha/Acquisition/Order.t Add two order lines in a basket referring to same biblio. Try to cancel with delete biblio. Should not be possible. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Janusz Kaczmarek <januszop@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Janusz Kaczmarek <januszop@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163771|0 |1 is obsolete| | --- Comment #17 from Janusz Kaczmarek <januszop@gmail.com> --- Created attachment 164955 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164955&action=edit Bug 36018: $orders->filter_by_active should check quantityreceived An active order that has no items to receive any more is actually not very active anymore :) Test plan: Run t/db_dependent/Koha/Acquisition/Orders.t Enable OPACAcquisitionDetails. Add order on basket for new biblio. Check that opac-detail does not yet count the item as on order (since status is still new). Close basket. Check opac-detail again. Should count it now. Reopen basket. Cancel line. Check opac-detail again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Janusz Kaczmarek <januszop@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Janusz Kaczmarek <januszop@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163772|0 |1 is obsolete| | --- Comment #18 from Janusz Kaczmarek <januszop@gmail.com> --- Created attachment 164956 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164956&action=edit Bug 36018: (follow-up) Do not rely on assumed correct quantities Now filter_by_active checks quantities, we should ascertain that our test data is correct, not leaving it to random values from TestBuilder. Test plan: Run t/db_dependent/Koha/Acquisition/Order.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Janusz Kaczmarek <januszop@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com Status|Signed Off |Failed QA --- Comment #19 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- I tried to test this, but when I try to add an order to my basket, I have an error message To recreate: 1. Go to Acquisitions 2. Click on the Search button next to the Vendor search 3. Click Add to basket next to 'My basket' 4. In the From an existing record search box, type shakespeare and submit 5. Click 'Add order' under the first result 6. In the item field y - Koha item type, choose Books 7. Click Add item 8. In Fund, choose Secondary fund 9. Click 'Save' --> Error message The method Koha::Biblio->uncancelled_orders is not covered by tests! Followed by a lot of text. I was trying to test this part of the first patch
Go to acqui/basket.pl. Delete a whole basket who has some active and some cancelled lines.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Caroline Cyr La Rose from comment #19)
I tried to test this, but when I try to add an order to my basket, I have an error message
To recreate: 1. Go to Acquisitions 2. Click on the Search button next to the Vendor search 3. Click Add to basket next to 'My basket' 4. In the From an existing record search box, type shakespeare and submit 5. Click 'Add order' under the first result 6. In the item field y - Koha item type, choose Books 7. Click Add item 8. In Fund, choose Secondary fund 9. Click 'Save' --> Error message
The method Koha::Biblio->uncancelled_orders is not covered by tests!
Followed by a lot of text.
I was trying to test this part of the first patch
Go to acqui/basket.pl. Delete a whole basket who has some active and some cancelled lines.
Thx Caroline. Can replicate this. Will have a look now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #20)
(In reply to Caroline Cyr La Rose from comment #19) Thx Caroline. Can replicate this. Will have a look now.
Actually, I replicated it when I did not refresh plack. And after that, I couldnt anymore. Could you retry? Restart all, incl refreshing plack and try again? Back to SO -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Janusz Kaczmarek from comment #18)
Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Thanks Janusz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #23 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- (In reply to Marcel de Rooy from comment #21)
(In reply to Marcel de Rooy from comment #20)
(In reply to Caroline Cyr La Rose from comment #19) Thx Caroline. Can replicate this. Will have a look now.
Actually, I replicated it when I did not refresh plack. And after that, I couldnt anymore. Could you retry? Restart all, incl refreshing plack and try again?
Back to SO
You're right, after restart_all I don't have the error anymore. Sorry about that! I never know when to use reset_all or restart_all (unless it's specified in the test plan) and sometimes (most of the time!) avoid them because they take a while :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov Status|Signed Off |Failed QA --- Comment #24 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- I'm getting an error in the Receive Orders workflow with this patch applied: 1. Have at least one closed basket with an order line (When I reproduced it, I created an order from a new biblio record in KTD's sample basket, then closed the basket) 2. Go to Receive Shipments and add an invoice 3. Proceed to the next step of selecting order(s) to receive When the Pending Orders table tries to load on parcel.pl, a pop-up appears saying there was a 500 error with loading the table. It also produces this error in the API error log: [ERROR] GET /api/v1/acquisitions/orders: unhandled exception (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Column 'quantity' in where clause is ambiguous at /kohadevbox/koha/Koha/Objects.pm line 399>> Tested on a totally fresh KTD instance with just this patch applied, because chasing false alarms because we forgot to restart something happens to the best of us :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 165153 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165153&action=edit Bug 36018: (follow-up) Disambiguate quantity One of the embeds refers to suggestions. This table also has a quantity column. Test plan: Verify that Pending Orders table does load now instead of triggering an internal server error on ambiguous column. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Emily Lamancusa from comment #24)
I'm getting an error in the Receive Orders workflow with this patch applied:
1. Have at least one closed basket with an order line (When I reproduced it, I created an order from a new biblio record in KTD's sample basket, then closed the basket) 2. Go to Receive Shipments and add an invoice 3. Proceed to the next step of selecting order(s) to receive When the Pending Orders table tries to load on parcel.pl, a pop-up appears saying there was a 500 error with loading the table.
It also produces this error in the API error log: [ERROR] GET /api/v1/acquisitions/orders: unhandled exception (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Column 'quantity' in where clause is ambiguous at /kohadevbox/koha/Koha/Objects.pm line 399>>
Tested on a totally fresh KTD instance with just this patch applied, because chasing false alarms because we forgot to restart something happens to the best of us :)
Good catch! Only visible when the API is pulling additional tables.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=36018 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164953|0 |1 is obsolete| | --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 166503 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166503&action=edit Bug 36018: Rename orders->filter_by_current Current is a bit misleading here. It means the ones that are not cancelled. We already use the filter_out_etc naming schema. So in harmony with that, we switch it to filter_out_cancelled. Test plan: Run t/db_dependent/Koha/Acquisition/Order.t Run git grep -l '::Orders' | xargs grep filter_by_current * All occurrences should be related to Recalls, not Orders. Go to acqui/basket.pl. Delete a whole basket who has some active and some cancelled lines. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Janusz Kaczmarek <januszop@gmail.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=36018 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164954|0 |1 is obsolete| | --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 166504 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166504&action=edit Bug 36018: biblio->active_orders should be ->uncancelled_orders Active orders are more than just not cancelled. See filter_by_active in Koha::Acquisition::Orders. They are still in the acq process; we still need to receive items on those orders. When we cancel and want to delete a biblio, we should check for not cancelled orders (broader than active orders as in Orders.pm). Test plan: Git grep active_orders. Run t/db_dependent/Koha/Biblio.t Run t/db_dependent/Koha/Acquisition/Order.t Add two order lines in a basket referring to same biblio. Try to cancel with delete biblio. Should not be possible. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Janusz Kaczmarek <januszop@gmail.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=36018 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164955|0 |1 is obsolete| | --- Comment #29 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 166505 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166505&action=edit Bug 36018: $orders->filter_by_active should check quantityreceived An active order that has no items to receive any more is actually not very active anymore :) Test plan: Run t/db_dependent/Koha/Acquisition/Orders.t Enable OPACAcquisitionDetails. Add order on basket for new biblio. Check that opac-detail does not yet count the item as on order (since status is still new). Close basket. Check opac-detail again. Should count it now. Reopen basket. Cancel line. Check opac-detail again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Janusz Kaczmarek <januszop@gmail.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=36018 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164956|0 |1 is obsolete| | --- Comment #30 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 166506 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166506&action=edit Bug 36018: (follow-up) Do not rely on assumed correct quantities Now filter_by_active checks quantities, we should ascertain that our test data is correct, not leaving it to random values from TestBuilder. Test plan: Run t/db_dependent/Koha/Acquisition/Order.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Janusz Kaczmarek <januszop@gmail.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=36018 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165153|0 |1 is obsolete| | --- Comment #31 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 166507 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166507&action=edit Bug 36018: (follow-up) Disambiguate quantity One of the embeds refers to suggestions. This table also has a quantity column. Test plan: Verify that Pending Orders table does load now instead of triggering an internal server error on ambiguous column. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=36018 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com CC| |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=36018 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All working well and a clear clarification and improvement. QA scripts passing, tests passing and coverage increased. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #34 from Jonathan Druart <jonathan.druart@gmail.com> --- t/db_dependent/api/v1/acquisitions_orders.t .. # Failed test 'Only one order is active' # at t/db_dependent/api/v1/acquisitions_orders.t line 137. # got: '0' # expected: '1' # Looks like you failed 1 test of 24. t/db_dependent/api/v1/acquisitions_orders.t .. 1/5 # Failed test 'list() tests' # at t/db_dependent/api/v1/acquisitions_orders.t line 225. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #35 from Jonathan Druart <jonathan.druart@gmail.com> --- Might be a random failure, it's failing on Koha_Master but passing on Koha_Master_D11 it seems. Easy to recreate locally. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #36 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 166677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166677&action=edit Bug 36018: Prevent random failure from api/v1/acquisitions_orders.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 --- Comment #37 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #36)
Created attachment 166677 [details] [review] Bug 36018: Prevent random failure from api/v1/acquisitions_orders.t
Thx -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Picked the follow-up for aquisition_orders.t for main. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to main |Needs documenting --- Comment #39 from Fridolin Somers <fridolin.somers@biblibre.com> --- Not backported to 23.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|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=36018 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #40 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- I don't think this changes anything in the manual. Please reopen with more details if I'm wrong! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36018 Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |thibaud.guillot@biblibre.co | |m --- Comment #41 from Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> --- *** Bug 32928 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org