Bug 37601 - Add status field to bookings table
Summary: Add status field to bookings table
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Paul Derscheid
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 37592
Blocks: 35946 37652
  Show dependency tree / graph
 
Reported: 2024-08-08 15:45 UTC by Paul Derscheid
Modified: 2024-11-18 21:59 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds a status column to the bookings table to track the state of a booking, including 'new', 'cancelled', and 'completed' statuses. Future statuses will be handled dynamically for improved API response handling and search functionality. **Sponsored by** *Büchereizentrale Schleswig-Holstein*
Version(s) released in:
24.11.00
Circulation function: bookings

Attachments
Bug 37601: Add status field to bookings table (2.57 KB, patch)
2024-08-13 10:08 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Bug 37601: (follow-up) Change updated_at to updated_on in accordance with Bug 37592 (1.26 KB, patch)
2024-08-27 11:45 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Bug 37601: (follow-up) Change updated_at to updated_on in accordance with Bug 37592 (1.30 KB, patch)
2024-09-10 08:30 UTC, Biblibre Sandboxes 		Details | Diff | Splinter Review
Bug 37601: Add status field to bookings table (2.68 KB, patch)
2024-09-12 11:06 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 37601: (follow-up) Change updated_at to updated_on in accordance with Bug 37592 (1.50 KB, patch)
2024-09-12 11:06 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 37601: (QA follow-up) Add status to api definition (886 bytes, patch)
2024-09-12 11:06 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 37601: (QA follow-up) Change updated_on to modification_date in accordance with Bug 37592 (1.29 KB, patch)
2024-09-16 10:31 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Bug 37601: (QA follow-up) Rename status 'created' as 'new' to be more consistent with other modules (1.37 KB, patch)
2024-09-16 10:31 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Bug 37601: (QA follow-up) Add status 'completed' (1.40 KB, patch)
2024-09-16 10:31 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Bug 37601: Add status field to bookings table (2.90 KB, patch)
2024-09-18 08:08 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Bug 37601: Add status field to bookings table (2.82 KB, patch)
2024-09-18 08:32 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Bug 37601: (QA follow-up) Adjust kohastructure.sql (1.24 KB, patch)
2024-09-18 08:36 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Derscheid 2024-08-08 15:45:56 UTC 
The goal is to add a status column to the bookings table to track a bookings state

Two existing ideas are:
- confirmed (the default state for a freshly created booking, open for suggestions on a better name for this one)
- cancelled


For a follow-up bug:

- active (booking is currently in the boundary of start and end date)
- expired (bookings end date is in the past)
- pending (bookings start date is in the future)

should be calculated at runtime and zipped into the api response so that the datatable ordering and searching still works as expected. 


Additional opinions would be appreciated!
Comment 1 Paul Derscheid 2024-08-13 10:08:46 UTC 
Created attachment 170260 [details] [review]
Bug 37601: Add status field to bookings table
Comment 2 Martin Renvoize (ashimema) 2024-08-15 14:45:32 UTC 
This is a great start, I can see it being really useful in the future of bookings.

For example, I can see there being a process for opac bookings that would allow for booking request followed by confirmation by a librarian (certainly not everyone's workflow need, but some might want that.. this gives us the option to use status's to denote that flow)

I like 'cancelled' to allow for historic booking storage.. right now we just delete.. we can certainly introduce that state at a later if we opt for a workflow that cares about cancelled booking data staying around.

I can see there also being 'collected'/'onloan' for a booking having been picked up.. right now we just leave the booking hanging around once it's collected but there's also a bug that converts the booking into a checkout by deleting the row.. we might want to change that to a status.

Anyway.. I really like the direction of travel here.
Comment 3 Paul Derscheid 2024-08-27 11:45:17 UTC 
Created attachment 170767 [details] [review]
Bug 37601: (follow-up) Change updated_at to updated_on in accordance with Bug 37592
Comment 4 Biblibre Sandboxes 2024-09-10 08:30:53 UTC 
Created attachment 171241 [details] [review]
Bug 37601: (follow-up) Change updated_at to updated_on in accordance with Bug 37592

Signed-off-by: LEBSimonsen <simonsen@bz-sh.de>
Comment 5 Martin Renvoize (ashimema) 2024-09-12 11:06:17 UTC 
Created attachment 171380 [details] [review]
Bug 37601: Add status field to bookings table

Signed-off-by: LEBSimonsen <simonsen@bz-sh.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2024-09-12 11:06:20 UTC 
Created attachment 171381 [details] [review]
Bug 37601: (follow-up) Change updated_at to updated_on in accordance with Bug 37592

Signed-off-by: LEBSimonsen <simonsen@bz-sh.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2024-09-12 11:06:22 UTC 
Created attachment 171382 [details] [review]
Bug 37601: (QA follow-up) Add status to api definition

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Katrin Fischer 2024-09-16 08:48:17 UTC 
1) I wonder about only having 2 status:
* created
* cancelled

Cancelled makes perfect sense to me. 
Created seems a bit unusual compared to other status. Maybe it would be a little nicer as 'new' to match a bit closer how we handle article requests? (suggestions use "ASEKD", which also seems not ideal)

2) You say below that at the moment we just delete the filled bookings. Is there a plan to keep them later on? I wonder about statistics about successfully filled booking requests especially, which could be interesting to libraries. (question, not blocker)
Comment 9 Paul Derscheid 2024-09-16 08:59:34 UTC 
I gladly use 'new' if that's more in line with the conventions.

The plan for me is just to add stati on demand, meaning another bug needs an additional status, we push it onto the enum.

Concerning your second question:
We can of course also add a 'deleted' status to the enum, then we don't need another column.
Comment 10 Paul Derscheid 2024-09-16 09:05:52 UTC 
On the other hand, maybe it shouldn't be a status.

If all now to be combined tables have a column for that, it's better to follow the convention then.
Comment 11 Katrin Fischer 2024-09-16 09:07:19 UTC 
I think the status would probably be "complete" or similar. And the clean-up would be done separately, if we went down the route of keeping them around at first.
Comment 12 Paul Derscheid 2024-09-16 09:08:51 UTC 
Then we can add 'completed' and introduce the clean-up in a follow-up bug.
Comment 13 Katrin Fischer 2024-09-16 09:49:28 UTC 
Talked to Martin and he is OK with switching to "new" - after reviewing the code base it's a bit of the usual mess...
Comment 14 Paul Derscheid 2024-09-16 10:31:52 UTC 
Created attachment 171526 [details] [review]
Bug 37601: (QA follow-up) Change updated_on to modification_date in accordance with Bug 37592
Comment 15 Paul Derscheid 2024-09-16 10:31:55 UTC 
Created attachment 171527 [details] [review]
Bug 37601: (QA follow-up) Rename status 'created' as 'new' to be more consistent with other modules
Comment 16 Paul Derscheid 2024-09-16 10:31:57 UTC 
Created attachment 171528 [details] [review]
Bug 37601: (QA follow-up) Add status 'completed'

This will probably be useful when transforming a booking into a checkout.
Comment 17 Katrin Fischer 2024-09-16 11:49:07 UTC 
Pushed for 24.11!

Well done everyone, thank you!
Comment 18 Jonathan Druart 2024-09-16 12:07:26 UTC 
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'AFTER `end_date`' at line 2 at /kohadevbox/koha/C4/Installer.pm line 742
Comment 19 Katrin Fischer 2024-09-16 12:55:36 UTC 
This one is on me, fix tested and pushed. Sorry for breaking your patch, Paul!
Comment 20 Paul Derscheid 2024-09-16 12:56:02 UTC 
No worries.
Comment 21 Jonathan Druart 2024-09-17 09:29:54 UTC 
Missing DBIC schema changes patch in main.
Comment 22 Katrin Fischer 2024-09-17 10:11:42 UTC 
(In reply to Jonathan Druart from comment #21)
> Missing DBIC schema changes patch in main.

Argh. I'll fix in a few min.
Comment 23 Katrin Fischer 2024-09-17 10:45:44 UTC 
(In reply to Katrin Fischer from comment #22)
> (In reply to Jonathan Druart from comment #21)
> > Missing DBIC schema changes patch in main.
> 
> Argh. I'll fix in a few min.

DBIC schema update pushed.
Comment 24 Jonathan Druart 2024-09-18 08:01:05 UTC 
Something is wrong:

dbrev:
+            ADD COLUMN `status` ENUM('new', 'cancelled', 'completed') NOT NULL DEFAULT 'new' COMMENT 'current status of the booking' $after;

kohastructure:
1222   `status` enum('created', 'cancelled') NOT NULL DEFAULT 'created' COMMENT 'current status of the booking',

Currently I have this in DB:
| status            | enum('new','cancelled','completed') | NO   |     | new                 |                               |

And the tests are failing:
#   Failed test 'list() tests'
#   at t/db_dependent/api/v1/bookings.t line 145.
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Data truncated for column 'status' at row 1 at /kohadevbox/koha/t/lib/TestBuilder.pm line 385
# Looks like your test exited with 2 just after 1.
Comment 25 Paul Derscheid 2024-09-18 08:08:00 UTC 
Created attachment 171653 [details] [review]
Bug 37601: Add status field to bookings table

Signed-off-by: LEBSimonsen <simonsen@bz-sh.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Paul Derscheid 2024-09-18 08:32:47 UTC 
Created attachment 171654 [details] [review]
Bug 37601: Add status field to bookings table

Signed-off-by: LEBSimonsen <simonsen@bz-sh.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 27 Paul Derscheid 2024-09-18 08:36:36 UTC 
Created attachment 171655 [details] [review]
Bug 37601: (QA follow-up) Adjust kohastructure.sql
Comment 28 Jonathan Druart 2024-09-18 09:03:25 UTC 
(In reply to Paul Derscheid from comment #27)
> Created attachment 171655 [details] [review] [review]
> Bug 37601: (QA follow-up) Adjust kohastructure.sql

Pushed to main.

Confirmed that it fixes the failure.