Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request
Summary: Bring back ability to mark item as seen via SIP2 item information request
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Kyle M Hall (khall)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-04-21 11:09 UTC by Kyle M Hall (khall)
Modified: 2023-12-28 20:47 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06
Circulation function:


Attachments
Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request (5.10 KB, patch)
2023-04-21 11:10 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request (5.14 KB, patch)
2023-04-23 06:09 UTC, David Nind
Details | Diff | Splinter Review
Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request (5.24 KB, patch)
2023-05-12 09:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33580: fix tests (1.86 KB, patch)
2023-05-15 08:50 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2023-04-21 11:09:22 UTC
Prior to Koha 22.05, the SIP2 item information message had a side affect of updating the datelastseen field for items. This bug has been fixed, but was being utilized by inventory tools that used SIP2. We should bring back this affect and formalize it as an optional SIP2 config account setting.
Comment 1 Kyle M Hall (khall) 2023-04-21 11:10:17 UTC
Created attachment 150032 [details] [review]
Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request

Prior to Koha 22.05, the SIP2 item information message had a side affect of updating the datelastseen field for items. This bug has been fixed, but was being utilized by inventory tools that used SIP2. We should bring back this affect and formalize it as an optional SIP2 config account setting.

Test Plan:
1) Apply this patch set
2) prove t/db_dependent/SIP/Message.t
Comment 2 David Nind 2023-04-23 06:09:59 UTC
Created attachment 150113 [details] [review]
Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request

Prior to Koha 22.05, the SIP2 item information message had a side affect of updating the datelastseen field for items. This bug has been fixed, but was being utilized by inventory tools that used SIP2. We should bring back this affect and formalize it as an optional SIP2 config account setting.

Test Plan:
1) Apply this patch set
2) prove t/db_dependent/SIP/Message.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Marcel de Rooy 2023-05-12 08:58:59 UTC
Processing additional checks

        * Commit title does not start with 'Bug XXXXX: ' - b097a2bf40
Comment 4 Marcel de Rooy 2023-05-12 09:07:28 UTC
No blocker, but it is a bit obscure how the values of this option work. The documentation is quite poor..
The name could be improved too?

+        my $seen = $account->{seen_on_item_information};
+        ModDateLastSeen( $item->itemnumber, $seen eq 'keep_lost' ) if $seen;

+             seen_on_item_information="mark_found"> <!-- could be "keep_lost", empty to disable -->

Nevertheless passing QA to fix a regression.
Comment 5 Marcel de Rooy 2023-05-12 09:07:53 UTC
Created attachment 151123 [details] [review]
Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request

Prior to Koha 22.05, the SIP2 item information message had a side affect of updating the datelastseen field for items. This bug has been fixed, but was being utilized by inventory tools that used SIP2. We should bring back this affect and formalize it as an optional SIP2 config account setting.

Test Plan:
1) Apply this patch set
2) prove t/db_dependent/SIP/Message.t

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Tomás Cohen Arazi (tcohen) 2023-05-12 14:21:23 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 7 Martin Renvoize (ashimema) 2023-05-15 08:40:03 UTC
Comment on attachment 151123 [details] [review]
Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request

Review of attachment 151123 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/SIP/Message.t
@@ +457,5 @@
> +    $item_object->update({ itemlost => 1, datelastseen => '1900-01-01' });
> +    $server->{account}->{seen_on_item_information} = 'keep_lost';
> +    $msg->handle_item_information( $server );
> +    $item_object = Koha::Items->find( $item_object->id );
> +    isnt( $item_object->datelastseen, "1900-01-01", "datelastseen updated" );

The tests are failing due to date vs datetime format mismatch.

However.. I think I'd also like ot have seen a different date used to prove the change.. at first glance this looks like it's checking the field that's already set.?
Comment 8 Martin Renvoize (ashimema) 2023-05-15 08:50:12 UTC
Doh.. I missed that it's an `isnt`!


So, the test checks out.. but the format is still a problem
Comment 9 Jonathan Druart 2023-05-15 08:50:35 UTC
Created attachment 151188 [details] [review]
Bug 33580: fix tests
Comment 10 Jonathan Druart 2023-05-15 08:51:29 UTC
Looks like some of you forgot that there is a --run-tests flag that you can pass to the qa script...
Comment 11 Jonathan Druart 2023-05-15 08:52:24 UTC
(In reply to Jonathan Druart from comment #9)
> Created attachment 151188 [details] [review] [review]
> Bug 33580: fix tests

My try at fixing the tests, no idea if it's correct but it seems to be what it should have been.
Comment 12 Marcel de Rooy 2023-05-15 08:52:49 UTC
Looks like someone changed datelastseen :)

commit f729d46478fbfbb258262888941a6aa42da04d70
Author: Lucas Gass <lucas@bywatersolutions.com>
Date:   Tue Feb 7 23:14:43 2023 +0000

    Bug 31212: Change datelastseen from date to datetime field
Comment 13 Jonathan Druart 2023-05-15 08:54:40 UTC
Ho right, datelastseen has been changed to datetime on 31212, pushed at the same time
Comment 14 Martin Renvoize (ashimema) 2023-05-16 06:45:28 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release