Bug 32928 - Filters missing in SQL builder on Orders::filter_by_active
Summary: Filters missing in SQL builder on Orders::filter_by_active
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal with 8 votes (vote)
Assignee: Thibaud Guillot
QA Contact: Testopia
URL:
Keywords: regression
Depends on: 20212
Blocks:
  Show dependency treegraph
 
Reported: 2023-02-09 15:21 UTC by Thibaud Guillot
Modified: 2024-03-04 13:29 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 32928: Add filters on Orders::filter_by_active (1.79 KB, patch)
2023-02-09 15:38 UTC, Thibaud Guillot
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Thibaud Guillot 2023-02-09 15:21:01 UTC
Since version 21.xx.xx there has been a change in the way to retrieve the data of the orders to receive. 

The table is built from a call to the API and then the data can be filtered. Bug_20212 has integrated this change. This is a good thing but two filters are missing on the query construction in the Koha::Acquisition::Orders::filter_by_active function



Here is the query in version 20.11 : 

<pre>

SELECT aqbasket.basketno,
               borrowers.surname,
               borrowers.firstname,
               biblio.*,
               biblioitems.isbn,
               biblioitems.biblioitemnumber,
               biblioitems.publishercode,
               biblioitems.publicationyear,
               aqbasket.authorisedby,
               aqbasket.booksellerid,
               aqbasket.closedate,
               aqbasket.creationdate,
               aqbasket.basketname,
               aqbasketgroups.id as basketgroupid,
               aqbasketgroups.name as basketgroupname,
               aqorders.*
        FROM aqorders
            LEFT JOIN aqbasket ON aqorders.basketno = aqbasket.basketno
            LEFT JOIN aqbasketgroups ON aqbasket.basketgroupid = aqbasketgroups.id
            LEFT JOIN borrowers ON aqbasket.authorisedby=borrowers.borrowernumber
            LEFT JOIN biblio ON aqorders.biblionumber=biblio.biblionumber
            LEFT JOIN biblioitems ON biblioitems.biblionumber=biblio.biblionumber

        WHERE (datecancellationprinted is NULL)

            AND (
                ( aqbasket.is_standing AND aqorders.orderstatus IN ( "new", "ordered", "partial" ) )
                OR (
                    ( quantity > quantityreceived OR quantityreceived is NULL )
         AND aqorders.orderstatus IN ( "ordered", "partial" )
                )
            )
        AND aqbasket.booksellerid = ? ORDER BY aqbasket.basketno';
</pre>



The condition "datecancellationprinted is NULL" is missing as well as the quantity control to display the results only if there are still quantities to receive.



And so the result on the same database between the different versions can be very different!
Comment 1 Thibaud Guillot 2023-02-09 15:38:26 UTC
Created attachment 146437 [details] [review]
Bug 32928: Add filters on Orders::filter_by_active

To have the same behavior than before 21.xx.xx filters must be added to
the filter_by_active function.

If not you can have on your list, some orders that can be cancelled, or
orders with quantity totally received.

Requirements:
Have instances with different Koha version (20.11 VS Master for example)

Test plan:

1) Apply this patch on Master

Just have the same database with pending orders, and see differences between both instances on parcel.pl receptions.

You will see that total of orders will be not the same
Comment 2 Jonathan Druart 2023-02-14 09:33:28 UTC
Please provide tests.
Comment 3 Fridolin Somers 2024-02-12 14:51:44 UTC
Oups there is a typo.
Backslash there :
+                    'quantityreceived' => { '<' => \'me.quantity' }
Comment 4 Fridolin Somers 2024-03-04 13:29:24 UTC
(In reply to Fridolin Somers from comment #3)
> Oups there is a typo.
> Backslash there :
> +                    'quantityreceived' => { '<' => \'me.quantity' }

Forget that, it is a string ref, completely normal :D