Summary: | Search for late orders can show received order lines | ||
---|---|---|---|
Product: | Koha | Reporter: | Katrin Fischer <katrin.fischer> |
Component: | Acquisitions | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Katrin Fischer <katrin.fischer> |
Severity: | major | ||
Priority: | P5 - low | CC: | jonathan.druart, kyle, lucas, michaela.sieber, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.05.00,22.11.06
|
|
Circulation function: | |||
Attachments: |
Bug 33653: Never consider received orders as late
Bug 33653: Never consider received orders as late Bug 33653: Never consider received orders as late Bug 33653: Never consider received orders as late Bug 33653: Use filter_by_active instead Bug 33653: Never consider received orders as late Bug 33653: Use filter_by_active instead Bug 33653: Use filter_by_active instead |
Description
Katrin Fischer
2023-05-02 16:07:02 UTC
Created attachment 150582 [details] [review] Bug 33653: Never consider received orders as late We should not list received orders on the late orders page. Test plan: * Create a basket * Create a normal order (not from a subscription, no standing order) * Leave estimated delivery date empty * Close the basket * Create a new invoice and receive the order line * Go to acq > late orders * Verify the order is not in the list * Set filter for 'To' date into the future, i.e. 01/01/2025 => Without this patch the order shows up => With this patch applied the received order does not show up Not sure this is a regression, hence not sure it's a major. Thanks Jonathan! Chose the severity according to embarassment this might cause the library. :) Holding off for QA later. Created attachment 150624 [details] [review] Bug 33653: Never consider received orders as late We should not list received orders on the late orders page. Test plan: * Create a basket * Create a normal order (not from a subscription, no standing order) * Leave estimated delivery date empty * Close the basket * Create a new invoice and receive the order line * Go to acq > late orders * Verify the order is not in the list * Set filter for 'To' date into the future, i.e. 01/01/2025 => Without this patch the order shows up => With this patch applied the received order does not show up Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Thanks Jonathan! Created attachment 150669 [details] [review] Bug 33653: Never consider received orders as late We should not list received orders on the late orders page. Test plan: * Create a basket * Create a normal order (not from a subscription, no standing order) * Leave estimated delivery date empty * Close the basket * Create a new invoice and receive the order line * Go to acq > late orders * Verify the order is not in the list * Set filter for 'To' date into the future, i.e. 01/01/2025 => Without this patch the order shows up => With this patch applied the received order does not show up Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 150686 [details] [review] Bug 33653: Never consider received orders as late We should not list received orders on the late orders page. Test plan: * Create a basket * Create a normal order (not from a subscription, no standing order) * Leave estimated delivery date empty * Close the basket * Create a new invoice and receive the order line * Go to acq > late orders * Verify the order is not in the list * Set filter for 'To' date into the future, i.e. 01/01/2025 => Without this patch the order shows up => With this patch applied the received order does not show up Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Hinemoea Viault <hinemoea.viault@inlibro.com> This is correct but: shouldn't we chain ->filter_by_active instead? Raising author's attention. Created attachment 150821 [details] [review] Bug 33653: Use filter_by_active instead We have filter_by_active that is filtering by status already. (In reply to Tomás Cohen Arazi from comment #8) > This is correct but: shouldn't we chain ->filter_by_active instead? Yes, that makes sense. Katrin, do you agree? I have removed the existing condition on status in filter_by_lates. If I am reading that right: { '-or' => [ { 'basket.is_standing' => 1, 'orderstatus' => [ 'new', 'ordered', 'partial' ] }, { 'orderstatus' => [ 'ordered', 'partial' ] } ] } * It's a standing order with new, ordered or partial * It's ordered or partial That should work ok here. I believe the standing orders could be an issue, but if I understand the code correctly, they are already showing up in the late orders: - ( orderstatus => { '!=' => 'cancelled' } ), + ( orderstatus => { '-not_in' => ['cancelled', 'complete'] } ), If the delivery date is calculated from the order date, they will all be "late" for the 'new' order line and there is no way to tell that it is a standing order in the results list... probably something for a separate bug. After talking to Joubu I realized we only include the new ones for standing orders now - this is another enhancement :) Created attachment 150833 [details] [review] Bug 33653: Never consider received orders as late We should not list received orders on the late orders page. Test plan: * Create a basket * Create a normal order (not from a subscription, no standing order) * Leave estimated delivery date empty * Close the basket * Create a new invoice and receive the order line * Go to acq > late orders * Verify the order is not in the list * Set filter for 'To' date into the future, i.e. 01/01/2025 => Without this patch the order shows up => With this patch applied the received order does not show up Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 150834 [details] [review] Bug 33653: Use filter_by_active instead We have filter_by_active that is filtering by status already. Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 150865 [details] [review] Bug 33653: Use filter_by_active instead This patch makes `filter_by_lates` use the `filter_by_active` method instead of an ad-hoc query filter. It has the advantage that it considers standing orders too. No behavior change, tests should still pass. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed to master for 23.05. Nice work everyone, thanks! Nice work everyone! Pushed to stable for 22.11.x Lots of merge conflicts with 22.05.x, cannot backport unless rebased. |