Bug 31948

Summary: Add timestamp to tmp_holdsqueue
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: Hold requestsAssignee: Katrin Fischer <katrin.fischer>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: barton, gmcharlt, lucas, m.de.rooy, michaela.sieber, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This adds a timestamp column to the tmp_holdsqueue table that the holds queue in circulation is built from. With RealTimeHoldsQueue this will enable reporting on the date and time an entry was added to the holds queue.
Version(s) released in:
22.11.00
Bug Depends on: 29346    
Bug Blocks: 2954, 33051    
Attachments: Bug 31948: Add timestamp to tmp_holdsqueue table
Bug 31948: Add timestamp to tmp_holdsqueue table
Bug 31948: Add timestamp to tmp_holdsqueue table

Description Katrin Fischer 2022-10-24 14:49:48 UTC
With the option to have real time updates to tmp_holdsqueue (and maybe don't run the build_holdqueue?) as a consequence, it would be interesting to see when an entry was added or updated in the queue. 
We should have a timestamp!
Comment 1 Katrin Fischer 2022-10-28 14:06:09 UTC
Created attachment 142767 [details] [review]
Bug 31948: Add timestamp to tmp_holdsqueue table

With the option to have real time updates to tmp_holdsqueue
a timestamp will be really helpful to help find the most recently
added entries.

To test:
* Place some holds on a record with available items
* Run build_holds-queue.pl
* Verify the hold shows up under circulation > holds queue
* Apply patch
* Run build_holds_queue.pl again
* Verify this works without any issues
* Turn on RealTimeHoldsQueue
* Update pick-up location, add another hold
* Make sure these changes are reflected in the holds queue as well
* Look at the database and verify timestamps are set
Comment 2 Tomás Cohen Arazi 2022-10-28 14:32:10 UTC
*** Bug 18440 has been marked as a duplicate of this bug. ***
Comment 3 Kyle M Hall 2022-10-28 14:38:10 UTC
Created attachment 142773 [details] [review]
Bug 31948: Add timestamp to tmp_holdsqueue table

With the option to have real time updates to tmp_holdsqueue
a timestamp will be really helpful to help find the most recently
added entries.

To test:
* Place some holds on a record with available items
* Run build_holds-queue.pl
* Verify the hold shows up under circulation > holds queue
* Apply patch
* Run build_holds_queue.pl again
* Verify this works without any issues
* Turn on RealTimeHoldsQueue
* Update pick-up location, add another hold
* Make sure these changes are reflected in the holds queue as well
* Look at the database and verify timestamps are set

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Owen Leonard 2022-10-28 14:47:19 UTC
Created attachment 142774 [details] [review]
Bug 31948: Add timestamp to tmp_holdsqueue table

With the option to have real time updates to tmp_holdsqueue
a timestamp will be really helpful to help find the most recently
added entries.

To test:
* Place some holds on a record with available items
* Run build_holds-queue.pl
* Verify the hold shows up under circulation > holds queue
* Apply patch
* Run build_holds_queue.pl again
* Verify this works without any issues
* Turn on RealTimeHoldsQueue
* Update pick-up location, add another hold
* Make sure these changes are reflected in the holds queue as well
* Look at the database and verify timestamps are set

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Katrin Fischer 2022-10-28 14:59:25 UTC
Kyle, hope it's ok with you - we are counting you for QA and Owen as SO. If you
Comment 6 Tomás Cohen Arazi 2022-10-28 15:46:17 UTC
(In reply to Katrin Fischer from comment #5)
> Kyle, hope it's ok with you - we are counting you for QA and Owen as SO. If
> you

Yeah, he reviewed it per my request so it is PQA.
Comment 7 Tomás Cohen Arazi 2022-10-28 19:47:51 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 8 Marcel de Rooy 2022-12-05 10:37:56 UTC
+        $dbh->do(q{ALTER TABLE `tmp_holdsqueue` ADD `timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP AFTER item_level_request});

Hmm. No check if column exists? Not idempotent..

Resulting in:
Upgrade to 22.06.00.075  [09:27:16]: Bug 31948 - Add timestamp to tmp_holdsqueue table
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate column name 'timestamp' at /usr/share/koha/C4/Installer.pm line 739
Comment 9 Katrin Fischer 2022-12-05 10:41:12 UTC
(In reply to Marcel de Rooy from comment #8)
> +        $dbh->do(q{ALTER TABLE `tmp_holdsqueue` ADD `timestamp` timestamp
> NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP AFTER
> item_level_request});
> 
> Hmm. No check if column exists? Not idempotent..
> 
> Resulting in:
> Upgrade to 22.06.00.075  [09:27:16]: Bug 31948 - Add timestamp to
> tmp_holdsqueue table
> ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate column
> name 'timestamp' at /usr/share/koha/C4/Installer.pm line 739

Hi Marcel, you are right, we should add the check. Sorry for missing it. I probably won't be able to provide the patch today or tomorrow tho :(
Comment 10 Lucas Gass 2022-12-05 23:47:29 UTC
Enhancement will not be backported to 22.05.x
Comment 11 Tomás Cohen Arazi 2023-02-23 11:01:40 UTC
(In reply to Katrin Fischer from comment #9)
> (In reply to Marcel de Rooy from comment #8)
> > +        $dbh->do(q{ALTER TABLE `tmp_holdsqueue` ADD `timestamp` timestamp
> > NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP AFTER
> > item_level_request});
> > 
> > Hmm. No check if column exists? Not idempotent..
> > 
> > Resulting in:
> > Upgrade to 22.06.00.075  [09:27:16]: Bug 31948 - Add timestamp to
> > tmp_holdsqueue table
> > ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate column
> > name 'timestamp' at /usr/share/koha/C4/Installer.pm line 739
> 
> Hi Marcel, you are right, we should add the check. Sorry for missing it. I
> probably won't be able to provide the patch today or tomorrow tho :(

Moved the resolution of this to bug 33051 and removed the additional_work_needed keyword.