Bug 25260 - Merge 'reserves' and 'old_reserves' into a new 'holds' table
Summary: Merge 'reserves' and 'old_reserves' into a new 'holds' table
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL: https://gitlab.com/thekesolutions/Koh...
Keywords: rel_20_11_target
: 23133 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-04-23 01:37 UTC by Tomás Cohen Arazi
Modified: 2020-08-26 13:49 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Large patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 25260: Add a 'holds' table (20.01 KB, patch)
2020-04-29 21:11 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Adapt Koha::Hold(s) (12.45 KB, patch)
2020-04-29 21:11 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Atomic update (11.77 KB, patch)
2020-04-29 21:11 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Schema updates [DO NOT PUSH] (31.39 KB, patch)
2020-04-29 21:11 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Adapt all the things (194.13 KB, patch)
2020-04-29 21:11 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Add a 'holds' table (20.02 KB, patch)
2020-04-29 21:21 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Adapt Koha::Hold(s) (12.46 KB, patch)
2020-04-29 21:21 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Atomic update (11.83 KB, patch)
2020-04-29 21:21 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Schema updates [DO NOT PUSH] (31.45 KB, patch)
2020-04-29 21:21 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Adapt all the things (194.21 KB, patch)
2020-04-29 21:21 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 25260: Add a 'holds' table (19.99 KB, patch)
2020-04-30 20:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25260: Adapt Koha::Hold(s) (12.51 KB, patch)
2020-04-30 20:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25260: Atomic update (11.82 KB, patch)
2020-04-30 20:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25260: Schema updates [DO NOT PUSH] (31.35 KB, patch)
2020-04-30 20:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25260: Adapt all the things (197.08 KB, patch)
2020-04-30 20:52 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 Tomás Cohen Arazi 2020-04-23 01:37:17 UTC
Reengineering the current 'reserves' and 'old_reserves' tables, we propose to introduce a new table, following our current agreements on the design and addressing or use cases.
An RFC will be submitted and linked here.
Comment 1 Tomás Cohen Arazi 2020-04-23 01:40:34 UTC Comment hidden (obsolete)
Comment 2 Agustín Moyano 2020-04-29 21:11:19 UTC Comment hidden (obsolete)
Comment 3 Agustín Moyano 2020-04-29 21:11:22 UTC Comment hidden (obsolete)
Comment 4 Agustín Moyano 2020-04-29 21:11:26 UTC Comment hidden (obsolete)
Comment 5 Agustín Moyano 2020-04-29 21:11:30 UTC Comment hidden (obsolete)
Comment 6 Agustín Moyano 2020-04-29 21:11:35 UTC Comment hidden (obsolete)
Comment 7 Agustín Moyano 2020-04-29 21:21:34 UTC Comment hidden (obsolete)
Comment 8 Agustín Moyano 2020-04-29 21:21:38 UTC Comment hidden (obsolete)
Comment 9 Agustín Moyano 2020-04-29 21:21:41 UTC Comment hidden (obsolete)
Comment 10 Agustín Moyano 2020-04-29 21:21:45 UTC Comment hidden (obsolete)
Comment 11 Agustín Moyano 2020-04-29 21:21:51 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2020-04-30 20:52:16 UTC
Created attachment 104053 [details] [review]
Bug 25260: Add a 'holds' table

Sponsored-by: ByWater Solutions
Sponsored-by: Theke Solutions
Comment 13 Tomás Cohen Arazi 2020-04-30 20:52:24 UTC
Created attachment 104054 [details] [review]
Bug 25260: Adapt Koha::Hold(s)

Sponsored-by: ByWater Solutions
Sponsored-by: Theke Solutions
Comment 14 Tomás Cohen Arazi 2020-04-30 20:52:31 UTC
Created attachment 104055 [details] [review]
Bug 25260: Atomic update

Sponsored-by: ByWater Solutions
Sponsored-by: Theke Solutions
Comment 15 Tomás Cohen Arazi 2020-04-30 20:52:38 UTC
Created attachment 104056 [details] [review]
Bug 25260: Schema updates [DO NOT PUSH]

Sponsored-by: ByWater Solutions
Sponsored-by: Theke Solutions
Comment 16 Tomás Cohen Arazi 2020-04-30 20:52:47 UTC
Created attachment 104057 [details] [review]
Bug 25260: Adapt all the things

This patch adapts many pm and test files to work with unified holds
table.

To test:

1. Remove all previous holds in reserves and old_reserves tables.
2. Create 6 holds. Cancel 2 of them, fulfill one, set one in waiting and
   another in transit.
CHECK => "select reserve_id, found from reserves" query in mysql should return 3 rows, where in found
column should find 'W', 'T' and another with null values.
      => "select reserve_id, found from old_reserves" query in mysql should return 3 rows, where in found column you should find 2 with 'C', and one with 'F' values.
3. in mysql change reserve_id to the value of a hold in reserve table.
4. apply patches
5. updatedatabase
SUCCESS => you should get the following warning "There were 1 old reserves that could not be moved, please check '_old_reserves'"
        => "select id, status, completed from holds" query in mysql should return 5 rows.
          => The hold that had null value in found column, now should have 'placed' status and 0 in completed.
          => The one that had 'W' in found column, now should have 'waiting' status and 0 in completed.
          => The one that had 'T' in found column, now should have 'in_transit' status and 0 in completed.
          => The one that had 'F' in found column, now should have 'fulfilled' status and 1 in completed.
          => The unmodified cancelled hold, now should have 'cancelled' status and 1 in completed.
        => "select * from _old_reserves" should give you one row with the modified hold.
6. in staff interface create, cancel, modify priorities, etc and everything should work exactly as before.

 # Run every test that has reserve and hold word wihtin
 # check that you are in main koha directory
 # check you have correctly setted KOHA_INTRANET_URL and KOHA_OPAC_URL environment variables
 # check plack is up and running.. tests will use selenium
 # prepare yourself a mug of coffe/te or take a nap.. this will take a while
7. prove $(grep -ilr -e reserve -e hold t|grep "\.t$")

 # finally
8. Sign off

Sponsored-by: ByWater Solutions
Sponsored-by: Theke Solutions
Comment 17 Tomás Cohen Arazi 2020-05-11 11:49:06 UTC
*** Bug 23133 has been marked as a duplicate of this bug. ***
Comment 18 Tomás Cohen Arazi 2020-07-23 15:39:05 UTC
I've added a remote branch on the URL field on this bug. That's where I'll keep a rebased patchset up-to-date. Please test and let me know if I should add your signature to the patchset.

https://gitlab.com/thekesolutions/Koha/-/tree/bug_25260_holds_table
Comment 19 Tomás Cohen Arazi 2020-07-23 15:41:06 UTC
In order to test this, because of the DB structure changes, you need to either:
- Have a master DB, checkout this branch and upgrade
- Perform the webinstaller steps

This is because the sample data in misc4dev is not adapted to this changes, so the handy reset_all command won't work (most probably).
Comment 20 Jonathan Druart 2020-08-26 13:45:01 UTC
Thinking loudly

What happens if you create an item, place a hold on it, remove the hold, delete the item.
In current master the old_reserves entry still exist (for stats for instance)
Comment 21 Tomás Cohen Arazi 2020-08-26 13:49:15 UTC
(In reply to Jonathan Druart from comment #20)
> Thinking loudly
> 
> What happens if you create an item, place a hold on it, remove the hold,
> delete the item.
> In current master the old_reserves entry still exist (for stats for instance)

That behaviour should be preserved and it would be a bug if it didn't.
Both biblio_id and item_id are defined as ON DELETE SET NULL. So I guess this should be ok.