Bug 33817 - Composition of an item bundle can be changed if checked out
Summary: Composition of an item bundle can be changed if checked out
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 28854
Blocks: 33568 31082
  Show dependency treegraph
 
Reported: 2023-05-24 08:54 UTC by Jonathan Druart
Modified: 2023-09-13 20:12 UTC (History)
5 users (show)

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


Attachments
Bug 33817: Prevent item bundle that is checked out to be modified (1.89 KB, patch)
2023-05-24 11:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33817: Enforce server-side (6.21 KB, patch)
2023-05-25 09:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33817: Prevent item bundle that is checked out to be modified (1.95 KB, patch)
2023-05-25 09:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33817: Enforce server-side (6.27 KB, patch)
2023-05-25 09:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33817: Prevent item bundle that is checked out to be modified (1.95 KB, patch)
2023-06-27 11:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33817: Enforce server-side (6.27 KB, patch)
2023-06-27 11:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33817: (follow-up) Fix spec and messages (3.13 KB, patch)
2023-06-27 11:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33817: Prevent item bundle that is checked out to be modified (2.41 KB, patch)
2023-07-09 15:30 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33817: Enforce server-side (6.32 KB, patch)
2023-07-09 15:30 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33817: (follow-up) Fix spec and messages (3.19 KB, patch)
2023-07-09 15:30 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33817: (QA follow-up) Fix tidyness (1.81 KB, patch)
2023-07-10 17:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2023-05-24 08:54:31 UTC

    
Comment 1 Jonathan Druart 2023-05-24 11:15:51 UTC
From bug 28854 comment 515

(In reply to Martin Renvoize from comment #269)
> (In reply to Katrin Fischer from comment #224)
> > d) Should we be able to change contents of a bundle while it is checked out?
> > At the moment it's possible. Maybe a 'safety question'? (question/suggestion)
> 
> Fixed in follow-up.

This is not working, there is some code in the template to disable the buttons but it does not work. I have a patch to fix it, but I am wondering if it is correct to disable the edition of a bundle item that is checked out.

There is no code server-side to prevent that (and so it's currently possible).

Should we fix or remove this restriction?
Comment 2 Jonathan Druart 2023-05-24 11:16:36 UTC
Created attachment 151623 [details] [review]
Bug 33817: Prevent item bundle that is checked out to be modified
Comment 3 Martin Renvoize 2023-05-24 11:23:49 UTC
From IRC we came to the conclusion:

* Bundles should not be editable whilst checked out (Removing and Adding should both be blocked at client and server side)
* Items should not be available for adding to a bundle when they are individually checked out (Again, this should be blocked at both the client and the server).

The real world idea of this was that you physically had a 'box' you were packing your items into to create the bundle.. you scan your items into the box and out of the box.. thus they should physically be there.

We then track their presence at the checkin/checkout actions.
Comment 4 Jonathan Druart 2023-05-25 09:39:41 UTC
Created attachment 151660 [details] [review]
Bug 33817: Enforce server-side
Comment 5 Martin Renvoize 2023-05-25 09:43:13 UTC
Thanks for your work here Jonathan.., testing not, but the code looks solid :)
Comment 6 Martin Renvoize 2023-05-25 09:54:53 UTC
Created attachment 151662 [details] [review]
Bug 33817: Prevent item bundle that is checked out to be modified

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2023-05-25 09:54:55 UTC
Created attachment 151663 [details] [review]
Bug 33817: Enforce server-side

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2023-05-25 09:55:25 UTC
All working as originally intended and covered by tests too.. Signing off.
Comment 9 Victor Grousset/tuxayo 2023-06-17 01:13:12 UTC
Found this when testing for server side enforcement:

0. apply patches
1. have a bundle
2. have it's page opened and it's manage bundle menu too
3. in another tab, check the bundle out 😈
4. back to the 1st tab, try to add another item to the bundle
5. error: "Failure: Item 'BARCODE' belongs to another bundle"
   this is false!
   note: API response: 409 and error bundle_checked_out, nothing wrong here it seems
6. try to remove an item
7. nothing happens, which is okay
8. API response: 501 not implemented, error: No response rule for "409"

Is the wrong message UI at step 5 a big deal?
Same question for API response at step 8.
Comment 10 Martin Renvoize 2023-06-27 11:04:57 UTC
Created attachment 152723 [details] [review]
Bug 33817: Prevent item bundle that is checked out to be modified

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2023-06-27 11:05:00 UTC
Created attachment 152724 [details] [review]
Bug 33817: Enforce server-side

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2023-06-27 11:05:04 UTC
Created attachment 152725 [details] [review]
Bug 33817: (follow-up) Fix spec and messages

This patch should resolve the issues highlighted at signoff time by
Victor.
Comment 13 Katrin Fischer 2023-07-09 15:22:12 UTC
...problem descriptions and test plans in commit messages please?
Comment 14 Katrin Fischer 2023-07-09 15:30:48 UTC
Created attachment 153228 [details] [review]
Bug 33817: Prevent item bundle that is checked out to be modified

While an item bundle is checked out, we don't want any items to be
added or removed from the bundle.

To test:
* Create a record with LDR Pos. 7 = c
* Add an item for your bundle
* Add one or more existing items from other records to the bundle
* Check out the bundle item
* Verify it appears correctly on the patron account
* Verify you can still add/remove items from the bundle
* Apply patch set
* Verify that the add/remove links are now disabled (greyed out)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Katrin Fischer 2023-07-09 15:30:51 UTC
Created attachment 153229 [details] [review]
Bug 33817: Enforce server-side

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2023-07-09 15:30:54 UTC
Created attachment 153230 [details] [review]
Bug 33817: (follow-up) Fix spec and messages

This patch should resolve the issues highlighted at signoff time by
Victor.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Tomás Cohen Arazi 2023-07-10 17:40:34 UTC
Created attachment 153293 [details] [review]
Bug 33817: (QA follow-up) Fix tidyness

This patch fixes some reported tidyness issues with the patches.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Tomás Cohen Arazi 2023-07-10 18:43:05 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 19 Martin Renvoize 2023-07-18 09:33:29 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 20 Matt Blenkinsop 2023-07-18 15:21:10 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x