This field should be updated to be a datetime field and the interface adapted to display the full date and time to help librarians track items
Created attachment 138053 [details] [review] Bug 31212: Change datelastseen from date to datetime field This patch changes the datelastseen field to a datetime field for better time accuracy. It also fixes the interface in order to display datelastseen as a datetime field. To test: 1. Apply patch 2. Search the catalogue and select a biblio with at least one item. Confirm in the holdings table that every entry in the "Last Seen" column is in date-time format. 3. Navigate to "Items" in for that same biblio and check the item "History" section to have "Last Seen" in the correct format 4. Click "Edit" then "Edit Items" and confirm that the table has date-time format for the "Date Last Seen" Column entries 5. Edit one of the items to have lost status, then navigate to the Reports section, find "Items Lost", and search for the item with the barcode. Confirm the proper date-time format in the result. 6. Navigate to Tools, then Inventory, run inventory and confirm that the "Date Last Seen" column entries are in proper format
Failing this one because although it does change the field from date to datetime, the time is not accurately recorded. In all of the tests I ran, it records datelastseen as always being at midnight - e.g. 07/25/2022 00:00 Examples of tests: - Check an item in - Manually edit the datelastseen in items editor - Initiate a manual transfer of the item to another library
Hi Michal, I concur with Sally; your patch, as it stands, does not work 100%. However, fear not! The Community is here to help! ;-) Since you're changing a column definition in the DB, you may want to have a look at these guidelines: https://wiki.koha-community.org/wiki/Database_updates The gist of it is that when you alter the database schema, you need to accommodate for both a) new installations (which you did by modifying the installer's DB structure in installer/data/mysql/kohastructure.sql), *and* b) existing installations by means of creating an atomicupdate file. Also, since the DATETIME column will now be able to store the exact time the item was last seen down to the second, you'll have to ensure the timestamp is stored with the correct accuracy in the DB. Finally, don't forget the unit tests whenever you want to submit a patch, as they can help you find oversights in your submitted code. As usual, the details are in the Wiki: https://wiki.koha-community.org/wiki/Unit_Tests#Good_practices_for_running_tests I hope this helps!
Created attachment 139510 [details] [review] Bug 31212: Add Database update
Patch no longer applies: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 31212: Change datelastseen from date to datetime field Using index info to reconstruct a base tree... M C4/Items.pm M Koha/Schema/Result/Deleteditem.pm M Koha/Schema/Result/Item.pm M installer/data/mysql/kohastructure.sql M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt M koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt M koha-tmpl/intranet-tmpl/prog/en/modules/reports/itemslost.tt M koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reports/itemslost.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt Auto-merging installer/data/mysql/kohastructure.sql Auto-merging Koha/Schema/Result/Item.pm Auto-merging Koha/Schema/Result/Deleteditem.pm Auto-merging C4/Items.pm CONFLICT (content): Merge conflict in C4/Items.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 31212: Change datelastseen from date to datetime field
Created attachment 142262 [details] [review] Bug 31212: Change datelastseen from date to datetime field This patch changes the datelastseen field to a datetime field for better time accuracy. It also fixes the interface in order to display datelastseen as a datetime field. To test: 1. Apply patch 2. Search the catalogue and select a biblio with at least one item. Confirm in the holdings table that every entry in the "Last Seen" column is in date-time format. 3. Navigate to "Items" in for that same biblio and check the item "History" section to have "Last Seen" in the correct format 4. Click "Edit" then "Edit Items" and confirm that the table has date-time format for the "Date Last Seen" Column entries 5. Edit one of the items to have lost status, then navigate to the Reports section, find "Items Lost", and search for the item with the barcode. Confirm the proper date-time format in the result. 6. Navigate to Tools, then Inventory, run inventory and confirm that the "Date Last Seen" column entries are in proper format
Created attachment 142263 [details] [review] Bug 31212: Add Database update
Created attachment 142264 [details] [review] Bug 31212: Fix date_from_string and explicitly called output_pref subroutine
Rebased for current master
Created attachment 142282 [details] [review] Bug 31212: Change datelastseen from date to datetime field This patch changes the datelastseen field to a datetime field for better time accuracy. It also fixes the interface in order to display datelastseen as a datetime field. To test: 1. Apply patch 2. Search the catalogue and select a biblio with at least one item. Confirm in the holdings table that every entry in the "Last Seen" column is in date-time format. 3. Navigate to "Items" in for that same biblio and check the item "History" section to have "Last Seen" in the correct format 4. Click "Edit" then "Edit Items" and confirm that the table has date-time format for the "Date Last Seen" Column entries 5. Edit one of the items to have lost status, then navigate to the Reports section, find "Items Lost", and search for the item with the barcode. Confirm the proper date-time format in the result. 6. Navigate to Tools, then Inventory, run inventory and confirm that the "Date Last Seen" column entries are in proper format Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com>
Created attachment 142283 [details] [review] Bug 31212: Add Database update Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com>
Created attachment 142284 [details] [review] Bug 31212: Fix date_from_string and explicitly called output_pref subroutine Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com>
Created attachment 142286 [details] [review] Bug 31212: Change datelastseen from date to datetime field This patch changes the datelastseen field to a datetime field for better time accuracy. It also fixes the interface in order to display datelastseen as a datetime field. To test: 1. Apply patch 2. Search the catalogue and select a biblio with at least one item. Confirm in the holdings table that every entry in the "Last Seen" column is in date-time format. 3. Navigate to "Items" in for that same biblio and check the item "History" section to have "Last Seen" in the correct format 4. Click "Edit" then "Edit Items" and confirm that the table has date-time format for the "Date Last Seen" Column entries 5. Edit one of the items to have lost status, then navigate to the Reports section, find "Items Lost", and search for the item with the barcode. Confirm the proper date-time format in the result. 6. Navigate to Tools, then Inventory, run inventory and confirm that the "Date Last Seen" column entries are in proper format Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 142287 [details] [review] Bug 31212: Add Database update Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 142288 [details] [review] Bug 31212: Fix date_from_string and explicitly called output_pref subroutine Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com> Signed-off-by: David Nind <david@davidnind.com>
I've signed off, but then I read through Sally's comments (comment #2). For editing the date last seen in the item editor, I: - Changed the BKS framework for 952$r so that it displays in the editor with the dateaccessioned.pl plugin - Edited an item: saves date and time with 00:00 for the time - Without the daetaccessioned.pl plugin you have to enter the date and time including seconds, otherwise you get an error (edited item so no date, added date without time): "Invalid value passed, items.datelastseen=09/05/2022 expected type is datetime at /...." Can confirm that check in and manual transfer are working as expected (with the date time displayed correctly). Also, when you edit an item, the date last seen changes to the current date but with the time set as 00:00.
(In reply to David Nind from comment #16) > I've signed off, but then I read through Sally's comments (comment #2). > > For editing the date last seen in the item editor, I: > - Changed the BKS framework for 952$r so that it displays in the editor with > the dateaccessioned.pl plugin > - Edited an item: saves date and time with 00:00 for the time > - Without the daetaccessioned.pl plugin you have to enter the date and time > including seconds, otherwise you get an error (edited item so no date, added > date without time): "Invalid value passed, items.datelastseen=09/05/2022 > expected type is datetime at /...." > > Can confirm that check in and manual transfer are working as expected (with > the date time displayed correctly). > > Also, when you edit an item, the date last seen changes to the current date > but with the time set as 00:00. Thanks for your testing David, I'll set this one to FQA and try to fix those problems.
> > Also, when you edit an item, the date last seen changes to the current date > > but with the time set as 00:00. Editing an item should not update the datelastseen. For me it doesn't update without this patchset and it should not with it either.
Created attachment 143595 [details] [review] Bug 31212: (follow-up) Fix datelastseen when returning item
> > - Changed the BKS framework for 952$r so that it displays in the editor with > > the dateaccessioned.pl plugin > > - Edited an item: saves date and time with 00:00 for the time > > - Without the daetaccessioned.pl plugin you have to enter the date and time > > including seconds, otherwise you get an error (edited item so no date, added > > date without time): "Invalid value passed, items.datelastseen=09/05/2022 > > expected type is datetime at /...." I think this is a separate bug about how we handle dates when not using the dateaccessioned plugin. I have filed it as such: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32168
Updated test plan: 1. Apply patch, updatedatabase, restart_all 2. Check out an item, look at the datelastseen and make sure it is correctly updated with the correct timestamp. 3. Check in an item, look at the datelastseen and make sure it is correctly updated with the correct timestamp. 4. Search the catalogue and select a biblio with at least one item. Confirm in the holdings table that every entry in the "Last Seen" column is in date-time format. 5. Navigate to "Items" in for that same biblio and check the item "History" section to have "Last Seen" in the correct format 6. Click "Edit" then "Edit Items" and confirm that the table has date-time format for the "Date Last Seen" Column entries 7. Edit one of the items to have lost status, then navigate to the Reports section, find "Items Lost", and search for the item with the barcode. Confirm the proper date-time format in the result. 8. Navigate to Tools, then Inventory, run inventory and confirm that the "Date Last Seen" column entries are in proper format 9. Update MARC frameworks to expose the datelastseen column in the editor, also turn on the dateaccessioned plugin. Make sure you can properly edit the datelastseen using the datepicker
Hi Lucas. I've re-tested, but I'm still having an issue with the date last seen updating to the current date when editing an item. If I edit an item, the date last seen field is now pre-populated with the current date (952$r setup to display in staff interface and date_accesioned plugin added). This isn't the case before the patch is applied - it is populated with the date originally recorded before editing. Everything else seems to be working as expected in the test plan. David
Created attachment 146369 [details] [review] Bug 31212: Change datelastseen from date to datetime field
Created attachment 146370 [details] [review] Bug 31212: Add Database update Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 146371 [details] [review] Bug 31212: Fix date_from_string and explicitly called output_pref subroutine Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 146372 [details] [review] Bug 31212: (follow-up) Fix datelastseen when returning item
This patchset is again ready for testing.
Created attachment 147664 [details] [review] Bug 31212: Change datelastseen from date to datetime field Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Created attachment 147665 [details] [review] Bug 31212: Add Database update Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Created attachment 147666 [details] [review] Bug 31212: Fix date_from_string and explicitly called output_pref subroutine Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Created attachment 147667 [details] [review] Bug 31212: (follow-up) Fix datelastseen when returning item Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Can you explain the changes in C4/Items.pm?
(In reply to Jonathan Druart from comment #32) > Can you explain the changes in C4/Items.pm? I mean the ones from the first patch. This one is definitely not needed: + my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 0 }); This one should not be needed either: + $datelastseen = output_pref({ str => $datelastseen, dateformat => 'iso', dateonly => 0 });
(In reply to Jonathan Druart from comment #33) > (In reply to Jonathan Druart from comment #32) > > Can you explain the changes in C4/Items.pm? > > I mean the ones from the first patch. > > This one is definitely not needed: > + my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', > dateonly => 0 }); > > This one should not be needed either: > + $datelastseen = output_pref({ str => $datelastseen, dateformat => > 'iso', dateonly => 0 }); I took over this bug from someone else, so I cannot explain those changes. However I can removed those unnecessary lines.
Created attachment 148939 [details] [review] Bug 31212: (follow-up) Remove unnessesary changes from Items.pm
Created attachment 150378 [details] [review] Bug 31212: Change datelastseen from date to datetime field Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 150379 [details] [review] Bug 31212: Add Database update Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 150380 [details] [review] Bug 31212: Fix date_from_string and explicitly called output_pref subroutine Signed-off-by: Ann Flournoy <aflournoy@cityofkeller.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 150381 [details] [review] Bug 31212: (follow-up) Fix datelastseen when returning item Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 150382 [details] [review] Bug 31212: (follow-up) Remove unnessesary changes from Items.pm Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 150383 [details] [review] Bug 31212: (QA follow-up) Fix unit test, remove unused imports Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Please avoid including schema file updates along with other changes.
Pushed to master for 23.05. Nice work everyone, thanks!
Created attachment 151189 [details] [review] Bug 31212: Fix api/v1/items.
Created attachment 151190 [details] [review] Bug 31212: Fix api/v1/items.t
(In reply to Jonathan Druart from comment #45) > Created attachment 151190 [details] [review] [review] > Bug 31212: Fix api/v1/items.t it should fix api/v1/biblios.t as well
Enhancement with additional_work_needed flag active - not backporting to 22.11.x
Created attachment 151194 [details] [review] Bug 31212: Adjust item spec for last_seen_date This bug changed the column to a date-time and the spec gets adjusted as well. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This development introduced a small regression in which the item gets stored twice during the tests in t/db_dependent/Koha/SearchEngine/Indexer.t because it used to store a date a couple times, with no real changes to the object (i.e. dt_from_string->ymd) and now it always differs (i.e. the attribute is marked as dirty). Options: 1. Adjust the tests to this situation and add FIXME and file a new bug to deal with the fact 2. Fix it here Ideas?