Bug 17202

Summary: Deleting a rotating collection with items should either be prohibited or items should be removed
Product: Koha Reporter: Nick Clemens <nick>
Component: ToolsAssignee: Josef Moravec <josef.moravec>
Status: CLOSED FIXED QA Contact: Kyle M Hall <kyle>
Severity: normal    
Priority: P5 - low CC: andrewfh, fridolin.somers, janet.mcgowan, jonathan.druart, josef.moravec, kyle, nick
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18606
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00
Bug Depends on:    
Bug Blocks: 18606    
Attachments: Bug 17202: Don't report an item in a collection if the collection doesn't exist
Bug 17202: Don't report an item in a collection if the collection doesn't exist
Bug 17202: Don't report an item in a collection if the collection doesn't exist
Bug 17202: (QA follow-up) Add foreign key constraint
Bug 17202: Unit tests
Bug 17202: Don't report an item in a collection if the collection doesn't exist
Bug 17202: (QA follow-up) Add foreign key constraint
Bug 17202: (follow-up) Conditionally add FK

Description Nick Clemens 2016-08-26 13:11:11 UTC
To recreate:
Create a rotating collection
Add an item
Delete the rotating collection
View the db (SELECT * FROM collections_tracking;)
The item is still in the collection
Comment 1 Fridolin Somers 2017-05-17 09:58:41 UTC
Indeed, there are no foreign keys in collections_tracking table.
Comment 2 Josef Moravec 2017-05-17 10:22:08 UTC
(In reply to Fridolin SOMERS from comment #1)
> Indeed, there are no foreign keys in collections_tracking table.

I will add it as a part of bug 18606
Comment 3 Janet McGowan 2019-11-21 15:19:03 UTC
I've found a problematic side-effect of this I think:
- someone deletes a rotating collection without removing the items on that collection first
- The itemnumbers remain in the collections_tracking table
- Those items on that deleted collection no longer are subject to usual transfers - if you return an item it no longer gives the popup message to transfer the item to its home library until the rows are removed in the database.
Comment 4 Andrew Fuerste-Henry 2020-02-05 22:05:08 UTC
Looks like items that have been left behind in collections_tracking after their collection has been deleted are also blocked from being added to a new collection.
Comment 5 Andrew Fuerste-Henry 2020-02-05 22:05:23 UTC
Confirmed this is still true in master.
Comment 6 Nick Clemens 2021-04-07 14:31:56 UTC
Created attachment 119306 [details] [review]
Bug 17202: Don't report an item in a collection if the collection doesn't exist

This simply adds a JOIN to the collections table - if the collection is gone
then the item is not considered to be in a rotating collection

Also remove variable to store the return that is not used later

To test:
Create a rotating collection
Add an item
Delete the rotating collection
View the db (SELECT * FROM collections_tracking;)
The item is still in the collection
Enable AutomaticItemReturn
Checkin the item at a branch not its home
The item does not get transferred home
Apply patch
Restart all the things
Check in the item, it is transferred home
Comment 7 Owen Leonard 2021-04-08 15:00:22 UTC
Created attachment 119330 [details] [review]
Bug 17202: Don't report an item in a collection if the collection doesn't exist

This simply adds a JOIN to the collections table - if the collection is gone
then the item is not considered to be in a rotating collection

Also remove variable to store the return that is not used later

To test:
Create a rotating collection
Add an item
Delete the rotating collection
View the db (SELECT * FROM collections_tracking;)
The item is still in the collection
Enable AutomaticItemReturn
Checkin the item at a branch not its home
The item does not get transferred home
Apply patch
Restart all the things
Check in the item, it is transferred home

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Kyle M Hall 2021-04-23 13:36:36 UTC
Created attachment 120099 [details] [review]
Bug 17202: Don't report an item in a collection if the collection doesn't exist

This simply adds a JOIN to the collections table - if the collection is gone
then the item is not considered to be in a rotating collection

Also remove variable to store the return that is not used later

To test:
Create a rotating collection
Add an item
Delete the rotating collection
View the db (SELECT * FROM collections_tracking;)
The item is still in the collection
Enable AutomaticItemReturn
Checkin the item at a branch not its home
The item does not get transferred home
Apply patch
Restart all the things
Check in the item, it is transferred home

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall 2021-04-23 13:36:49 UTC
Created attachment 120100 [details] [review]
Bug 17202: (QA follow-up) Add foreign key constraint

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Jonathan Druart 2021-04-27 10:20:34 UTC
Missing test coverage.
Comment 11 Nick Clemens 2021-04-27 12:53:53 UTC
Created attachment 120216 [details] [review]
Bug 17202: Unit tests
Comment 12 Nick Clemens 2021-04-27 12:53:57 UTC
Created attachment 120217 [details] [review]
Bug 17202: Don't report an item in a collection if the collection doesn't exist

This simply adds a JOIN to the collections table - if the collection is gone
then the item is not considered to be in a rotating collection

Also remove variable to store the return that is not used later

To test:
Create a rotating collection
Add an item
Delete the rotating collection
View the db (SELECT * FROM collections_tracking;)
The item is still in the collection
Enable AutomaticItemReturn
Checkin the item at a branch not its home
The item does not get transferred home
Apply patch
Restart all the things
Check in the item, it is transferred home

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Nick Clemens 2021-04-27 12:54:05 UTC
Created attachment 120218 [details] [review]
Bug 17202: (QA follow-up) Add foreign key constraint

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 14 Nick Clemens 2021-04-27 12:54:10 UTC
Created attachment 120219 [details] [review]
Bug 17202: (follow-up) Conditionally add FK
Comment 15 Jonathan Druart 2021-04-28 09:16:41 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 16 Fridolin Somers 2021-04-30 09:22:01 UTC
Does not apply in 20.11.x, please provide rebase if needed.