Bug 33580

Summary: Bring back ability to mark item as seen via SIP2 item information request
Product: Koha Reporter: Kyle M Hall <kyle>
Component: SIP2Assignee: Kyle M Hall <kyle>
Status: RESOLVED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, m.de.rooy, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
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
Attachments: Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request
Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request
Bug 33580 - Bring back ability to mark item as seen via SIP2 item information request
Bug 33580: fix tests

Description Kyle M Hall 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 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 2023-05-12 14:21:23 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 7 Martin Renvoize 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 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 2023-05-16 06:45:28 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release