|
Description
Paul Derscheid
2024-08-07 13:49:31 UTC
Created attachment 170144 [details] [review] Bug 37592: Add created_at, updated_at fields to bookings table It would be helpful for some institutions to track when bookings were created and updated, for example when communicating with patrons. Created attachment 170161 [details] [review] Bug 37592: Add created_at, updated_at fields to bookings table Signed-off-by: Owen Leonard <oleonard@myacpl.org> I like this.. but lets make sure we have a 100% consensus on the API guidelines and have it cloned to Database guidelines before we push it: https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#REST1.3.4.1_date.2Fdatetime.2Ftimestamp_fields Currently the guidelines says: * Where a field contains a 'date' it should be consistently named thing_date as opposed to date_thing and it should always return a full datetime. Good call! Created attachment 170763 [details] [review] Bug 37592: (follow-up) Change created_at, updated_at to created_on, updated_on As per a discussion in the community chat, this change is more in line with the existing schema. Created attachment 171254 [details] [review] Bug 37592: (follow-up) Change created_at, updated_at to created_on, updated_on As per a discussion in the community chat, this change is more in line with the existing schema. Signed-off-by: LEBSimonsen <simonsen@bz-sh.de> Created attachment 171376 [details] [review] Bug 37592: Add created_at, updated_at fields to bookings table Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 171377 [details] [review] Bug 37592: (follow-up) Change created_at, updated_at to created_on, updated_on As per a discussion in the community chat, this change is more in line with the existing schema. Signed-off-by: LEBSimonsen <simonsen@bz-sh.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 171378 [details] [review] Bug 37592: (QA follow-up) Add API mapping and definition Sticking to API guidelines, this adds the creation_date and modification_date fields to the api definitions and the required to_api_mappings for those fields to be properly populated. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> I don't want to block this, but forgive me this question:
Independent of the general discussion on naming things, should we not try to avoid API mappings for new columns? I think if we use creation_date in the API we might also name the column that. What do you think?
sub to_api_mapping {
- return {};
+ return {
+ created_on => "creation_date",
+ updated_on => "modification_date"
+ };
}
Also a question for the release notes :)
Signed-off-by: LEBSimonsen <simonsen@bz-sh.de>
This will show as LEBSimonsen in the release notes - would you like to update to the name or another term?
I'll gladly refactor it, if it's pushed immediately afterwards as these are literally just name changes. (In reply to Paul Derscheid from comment #12) > I'll gladly refactor it, if it's pushed immediately afterwards as these are > literally just name changes. Once the patch has reached my queue and it comes back again, it automatically goes on top of the enhancements pile. Check the dashboard :) Created attachment 171524 [details] [review] Bug 37592: (QA follow-up) Change created_on, updated_on to creation_date, modification_date It makes sense not to introduce mapping code if there's no reason for it. Accordingly the the columns are now of type DATETIME instead. Pushed for 24.11! Well done everyone, thank you! Created attachment 171645 [details] [review] Bug 37592: (QA follow-up) Fetch database fields for api return Creation and Modification times are maintained by the database, but on add/update we were not fetching the updated fields from the database for the api response. This patch corrects that and also updates the api schema to reflect that these are readOnly fields. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Follow-up pushed as RM Assistant; This gets the tests passing again and fixes a flaw in our logic. Thanks for the assist Martin! |