| Summary: | Invalid query when receiving an order | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Andreas Jonsson <andreas.jonsson> |
| Component: | Acquisitions | Assignee: | Andreas Jonsson <andreas.jonsson> |
| Status: | RESOLVED FIXED | QA Contact: | David Cook <dcook> |
| Severity: | blocker | ||
| Priority: | P3 | CC: | aleisha, andrew, dcook, jonathan.druart, librarian, martin.renvoize, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
24.11.00,24.05.03,23.11.08,23.05.14
|
|
| Circulation function: | |||
| Bug Depends on: | 37018 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 37533: fix query in orderreceive.tt
Bug 37533: fix query in orderreceive.tt Bug 37533: fix query in orderreceive.tt Bug 37533: fix query in orderreceive.tt |
||
Created attachment 169881 [details] [review] Bug 37533: fix query in orderreceive.tt The new validation in the REST API will no longer allow the operator "in". Consequently, it has to be replaced with the allowed "-in". Test plan: * Open an invoice and click "Go to receipt page" and on any basket click "receive" and make sure the dialog box appears. Created attachment 169882 [details] [review] Bug 37533: fix query in orderreceive.tt The new validation in the REST API will no longer allow the operator "in". Consequently, it has to be replaced with the allowed "-in". Test plan: * Open an invoice and click "Go to receipt page" and on any basket click "receive" and make sure the dialog box appears. Created attachment 169923 [details] [review] Bug 37533: fix query in orderreceive.tt The new validation in the REST API will no longer allow the operator "in". Consequently, it has to be replaced with the allowed "-in". Test plan: * Open an invoice and click "Go to receipt page" and on any basket click "receive" and make sure the dialog box appears. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> I'm going to need more information on this one... Without the patch, I've gone to http://localhost:8081/cgi-bin/koha/acqui/parcel.pl?invoiceid=1 and I've received the item and had no problems. What's the bug that this is fixing? Here's how I reproduced the bug on 23.11.x: 1. Have a vendor and a basket 2. Add an order to the basket 3. Close the basket 4. Receive shipments 5. Add an invoice number and continue 6. On the receiving items page, click Receive in the row of your order 7. Instead of the usual pop-up window to receive my item, you get an error that says Something went wrong when loading the table. 400: Bad request. This patch solved this error and I was able to receive my item (In reply to Aleisha Amohia from comment #5) > Here's how I reproduced the bug on 23.11.x: > > 1. Have a vendor and a basket > 2. Add an order to the basket > 3. Close the basket > 4. Receive shipments > 5. Add an invoice number and continue > 6. On the receiving items page, click Receive in the row of your order > 7. Instead of the usual pop-up window to receive my item, you get an error > that says Something went wrong when loading the table. 400: Bad request. > > This patch solved this error and I was able to receive my item Hmm... that's so interesting. That's the test plan I did on "main" and it worked fine. 172.20.0.1 - - [01/Aug/2024:04:14:13 +0000] "GET /api/v1/app.pl/api/v1/acquisitions/orders?only_active=1&q=%7B%22order_id%22:%7B%22in%22:%5B2%5D%7D%7D&_page=1&_per_page=20&_match=contains&_order_by=%2Bme.order_id HTTP/1.1" 200 - "http:// localhost:8081/cgi-bin/koha/acqui/orderreceive.pl?multiple_orders=2&invoiceid=1" "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36" If bug 37018 were working, I would think that this should cause an error but doesn't. Curious. I suspected I needed to re-run "perl build-resources.PL" to update the API definitions, but even after doing that it is still working fine... Yeah... something is rotten in Denmark. Bug 37018 is in a state of "Passed QA", some of the patches are in "23.11.07", but some are missing from "main". It's a right mess at the moment. I'll bring it up on bug 37018 and Mattermost... -- For this one, it's a simple change, so I'll check that it still works with it (it should), and then stamp it. Created attachment 169924 [details] [review] Bug 37533: fix query in orderreceive.tt The new validation in the REST API will no longer allow the operator "in". Consequently, it has to be replaced with the allowed "-in". Test plan: * Open an invoice and click "Go to receipt page" and on any basket click "receive" and make sure the dialog box appears. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au> (In reply to David Cook from comment #7) > Yeah... something is rotten in Denmark. > > Bug 37018 is in a state of "Passed QA", some of the patches are in > "23.11.07", but some are missing from "main". It's a right mess at the > moment. > > I'll bring it up on bug 37018 and Mattermost... > > -- > > For this one, it's a simple change, so I'll check that it still works with > it (it should), and then stamp it. This was in the security release from Tuesday. The security patches will be the next ones I push, but currently stuck on with some conflicts/failing tests - see security channel if you want to help things along. I can pick this up directly after they have been pushed. Assume the patches are there for testing maybe. Pushed for 24.11! Well done everyone, thank you! *** Bug 37555 has been marked as a duplicate of this bug. *** Pushed to main for 24.11. Nice work everyone, thanks! Why don't we allow "in" then? (In reply to Jonathan Druart from comment #13) > Why don't we allow "in" then? That's a good question. I imagine that "in" must've worked, although SQL::Abstract::Classic (and SQL::Abstract) seems to dictate "-in" in their docs. But the docs don't really explain it. Looking at the SQL::Abstract source code, it looks like it's expecting modifiers to start with "-". It's a bit hard to tell at a glance, but it kind of seems that "-in" and "in" might yield different structures (or at least take different code paths to get to the same place). Not sure if it's worth investigating or not, but probably good to require the "-" prefix. (In reply to Jonathan Druart from comment #13) > Why don't we allow "in" then? I'm not sure how we've built the allow-list on bug 37018. But yes: another option would've been to just add it to the list. I feel like it is not a bad thing to be able to identify more precisely this operands. |
There is an invalid query in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt var base_query = { "order_id": {"in": [[% multiple_orders | html %]]}}; It should be: var base_query = { "order_id": {"-in": [[% multiple_orders | html %]]}}; The validation introduced in bug 37018 will not allow the operator "in".