Bug 37536 - Cataloging add item js needs to update conditional that checks op
Summary: Cataloging add item js needs to update conditional that checks op
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Brendan Lawlor
QA Contact: Emily Lamancusa
URL:
Keywords:
: 37589 (view as bug list)
Depends on:
Blocks: 36192
  Show dependency treegraph
 
Reported: 2024-07-31 13:41 UTC by Brendan Lawlor
Modified: 2024-08-16 15:37 UTC (History)
5 users (show)

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


Attachments
Bug 37536: Update conditional to check for 'cud-saveitem' (1.30 KB, patch)
2024-07-31 13:51 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37536: Update conditional to check for 'cud-saveitem' (1.38 KB, patch)
2024-08-03 23:12 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 37536: Update conditional to check for 'cud-saveitem' (1.45 KB, patch)
2024-08-09 20:25 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 37536: Update conditional to check for 'cud-saveitem' (1.85 KB, patch)
2024-08-09 20:30 UTC, Emily Lamancusa
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Brendan Lawlor 2024-07-31 13:41:22 UTC
In cataloging_additem.js there is a check for op != 'saveitem' that needs to be updated to op != 'cud-saveitem'

In additem.pl and additem.tt all instances of 'saveitem' have been updated to 'cud-save-item'

The javascript's functionality is explained in bug 22669. It's related to editing items when receiving an order.
Comment 1 Brendan Lawlor 2024-07-31 13:51:43 UTC
Created attachment 169890 [details] [review]
Bug 37536: Update conditional to check for 'cud-saveitem'

In additem.pl and additem.tt all instances of 'saveitem' have been updated to 'cud-save-item'
The javascript needs to be updated in the same way.

Test plan:
1. git grep 'saveitem'
2. Notice all instances in additem.pl and additem.tt have been updated to 'cud-saveitem'
3. Notice cataloging_additem.js checks if op != 'saveitem'
4. Check the files and verify the change makes sense
5. Try the test plan from Bug 22669
Comment 2 Victor Grousset/tuxayo 2024-08-03 04:33:01 UTC
From bug 22669 test plan:

> - Make sure acqcreateitem is set to "when placing an order"

where is that acqcreateitem thing?

---

> Test plan:
> 1. git grep 'saveitem'
> 2. Notice all instances in additem.pl and additem.tt have been updated to
> 'cud-saveitem'
> 3. Notice cataloging_additem.js checks if op != 'saveitem'
> 4. Check the files and verify the change makes sense

These 1st steps are actually additional and welcome QA notes to help the code review. The test plan is just about blindly following instructions to confirm that the patch does what is says it does.
So the actual test plan would be the one from Bug 22669 remixed to witness a CSRF error before the patch and then witnessing it being gone after the patches.
Comment 3 Brendan Lawlor 2024-08-03 21:19:48 UTC
Thanks Victor.

acqcreateitem is a system preference and I believe the default is 'when placing an order'

Yes the actual test is on Bug 22669. When I tested I didn't see the CSRF error, but the pop up opens and closes abruptly as described in that bug.
Comment 4 Victor Grousset/tuxayo 2024-08-03 23:12:31 UTC
Created attachment 170044 [details] [review]
Bug 37536: Update conditional to check for 'cud-saveitem'

In additem.pl and additem.tt all instances of 'saveitem' have been updated to 'cud-save-item'
The javascript needs to be updated in the same way.

Test plan:
1. Same as test plan from Bug 22669. Same symptom.

QA notes:
1. git grep 'saveitem'
2. Notice all instances in additem.pl and additem.tt have been updated to 'cud-saveitem'
3. Notice cataloging_additem.js checks if op != 'saveitem'
4. Check the files and verify the change makes sense

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2024-08-03 23:13:41 UTC
> acqcreateitem is a system preference and I believe the default is 'when placing an order'

Thanks, somehow I didn't find it and when searching in the code I forgot to search case insensitively ^^"

---

> Yes the actual test is on Bug 22669. When I tested I didn't see the CSRF error, but the pop up opens and closes abruptly as described in that bug.


Ok it's the same symptom.


---

Patch works! :)
Comment 6 Emily Lamancusa 2024-08-09 20:25:23 UTC
Created attachment 170220 [details] [review]
Bug 37536: Update conditional to check for 'cud-saveitem'

In additem.pl and additem.tt all instances of 'saveitem' have been updated to 'cud-save-item'
The javascript needs to be updated in the same way.

Test plan:
1. Same as test plan from Bug 22669. Same symptom.

QA notes:
1. git grep 'saveitem'
2. Notice all instances in additem.pl and additem.tt have been updated to 'cud-saveitem'
3. Notice cataloging_additem.js checks if op != 'saveitem'
4. Check the files and verify the change makes sense

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 7 Emily Lamancusa 2024-08-09 20:26:06 UTC
Thanks for the fix! QA tool passes, and it works and makes sense. Passing QA
Comment 8 Emily Lamancusa 2024-08-09 20:30:00 UTC
Created attachment 170221 [details] [review]
Bug 37536: Update conditional to check for 'cud-saveitem'

In additem.pl and additem.tt all instances of 'saveitem' have been updated to 'cud-save-item'
The javascript needs to be updated in the same way.

Test plan:
- Make sure acqcreateitem is set to "when placing an order"
- Create a basket with some orders
- Close the basket
- Go to your vendor and receive an order
- On the receive page, try to edit your item
=> Without the patch, the pop up page will open and then close, not allowing the item to be edited.
=> With this patch applied you will see the item edit form. Save and
confirm that the parent window is updated with the new value (actually
it's refreshed)

QA notes:
1. git grep 'saveitem'
2. Notice all instances in additem.pl and additem.tt have been updated to 'cud-saveitem'
3. Notice cataloging_additem.js checks if op != 'saveitem'
4. Check the files and verify the change makes sense

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 9 Brendan Lawlor 2024-08-14 19:58:20 UTC
*** Bug 37589 has been marked as a duplicate of this bug. ***
Comment 10 Katrin Fischer 2024-08-16 15:37:23 UTC
Pushed for 24.11!

Well done everyone, thank you!