Bug 37601 - Add status field to bookings table
Summary: Add status field to bookings table
Status: Pushed to main
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
URL:
Keywords: release-notes-needed
Depends on: 29002 37592
Blocks: 35946 37652
  Show dependency treegraph
 
Reported: 2024-08-08 15:45 UTC by Paul Derscheid
Modified: 2024-09-16 15:46 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
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
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
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
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

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 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 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 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 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.