Bug 34868 - Add ability for SIP2 to distinguish missing item from other lost types
Summary: Add ability for SIP2 to distinguish missing item from other lost types
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-09-21 15:43 UTC by Kyle M Hall
Modified: 2023-11-30 16:39 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
**Sponsored by** *ByWater Solutions*
Version(s) released in:
23.11.00


Attachments
Bug 34868: Add ability for SIP2 to distinguish missing item from other lost types (3.77 KB, patch)
2023-09-21 15:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34868: Add ability for SIP2 to distinguish missing item from other lost types (3.81 KB, patch)
2023-09-22 00:22 UTC, David Nind
Details | Diff | Splinter Review
Bug Bug 34868: (QA follow-up) Rename new option, add comments (2.40 KB, patch)
2023-10-31 17:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34868: Add ability for SIP2 to distinguish missing item from other lost types (3.82 KB, patch)
2023-11-07 19:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34868: (QA follow-up) Rename new option, add comments (2.40 KB, patch)
2023-11-07 19:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34868: (QA follow-up) Fix XML (1.52 KB, patch)
2023-11-07 19:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34868: (QA follow-up) Tidy sip_circulation_status (2.46 KB, patch)
2023-11-07 19:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34868: Add ability for SIP2 to distinguish missing item from other lost types (3.85 KB, patch)
2023-11-09 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34868: (QA follow-up) Rename new option, add comments (2.42 KB, patch)
2023-11-09 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34868: (QA follow-up) Fix XML (1.70 KB, patch)
2023-11-09 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34868: (QA follow-up) Tidy sip_circulation_status (2.51 KB, patch)
2023-11-09 21:18 UTC, Katrin Fischer
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 2023-09-21 15:43:35 UTC
The SIP circulation status specifies that a 12 means an item is lost, and 13 means an item is missing. In Koha, missing items are simply a type of lost item so we never send a 13. This is an important distinction for some SIP based inventory tools. It would be good to be able to specify when lost status means "missing" at the SIP login level.
Comment 1 Kyle M Hall 2023-09-21 15:48:12 UTC
Created attachment 156003 [details] [review]
Bug 34868: Add ability for SIP2 to distinguish missing item from other lost types

The SIP circulation status specifies that a 12 means an item is lost, and 13 means an item is missing. In Koha, missing items are simply a type of lost item so we never send a 13. This is an important distinction for some SIP based inventory tools. It would be good to be able to specify when lost status means "missing" at the SIP login level.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/SIP/Transaction.t
Comment 2 David Nind 2023-09-22 00:22:50 UTC
Created attachment 156027 [details] [review]
Bug 34868: Add ability for SIP2 to distinguish missing item from other lost types

The SIP circulation status specifies that a 12 means an item is lost, and 13 means an item is missing. In Koha, missing items are simply a type of lost item so we never send a 13. This is an important distinction for some SIP based inventory tools. It would be good to be able to specify when lost status means "missing" at the SIP login level.

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

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Katrin Fischer 2023-10-31 16:05:52 UTC
I feel like the naming is a bit confusing:

+             missing_lost_status="4"

Maybe: lost_status_for_missing?

And it would be good to have some additional documentation in the form of a note/comment. 

I'd also like to see a test where we see what happens when the SIP option is not configured.
Comment 4 Kyle M Hall 2023-10-31 17:33:58 UTC
Created attachment 158151 [details] [review]
Bug Bug 34868: (QA follow-up) Rename new option, add comments
Comment 5 Katrin Fischer 2023-11-01 10:59:28 UTC
I am sorry, but the follow-up appears untested:

 FAIL	etc/SIPconfig.xml
   FAIL	  xml_valid
		83: parser error : error parsing attribute name
		83: parser error : attributes construct error
		83: parser error : Couldn't find end of Start Tag login line 61
		100: parser error : Opening and ending tag mismatch: accounts line 0 and login
		101: parser error : Opening and ending tag mismatch: acsconfig line 0 and accounts
		110: parser error : Extra content at the end of the document

 OK	t/db_dependent/SIP/Transaction.t


Processing additional checks

	* Commit title does not start with 'Bug XXXXX: ' - c91e7b374a

A comment within an XML element is not allowed, maybe move to top or below?

Also please have a look at the tests again:

(In reply to Katrin Fischer from comment #3)
> I'd also like to see a test where we see what happens when the SIP option is
> not configured.
Comment 6 Kyle M Hall 2023-11-07 19:18:57 UTC
(In reply to Katrin Fischer from comment #5)
> I am sorry, but the follow-up appears untested:
> 
>  FAIL	etc/SIPconfig.xml
>    FAIL	  xml_valid
> 		83: parser error : error parsing attribute name
> 		83: parser error : attributes construct error
> 		83: parser error : Couldn't find end of Start Tag login line 61
> 		100: parser error : Opening and ending tag mismatch: accounts line 0 and
> login
> 		101: parser error : Opening and ending tag mismatch: acsconfig line 0 and
> accounts
> 		110: parser error : Extra content at the end of the document
> 
>  OK	t/db_dependent/SIP/Transaction.t

Ack! I thought I'd fixed that! Maybe I didn't commit those changes.
Comment 7 Kyle M Hall 2023-11-07 19:26:14 UTC
Created attachment 158614 [details] [review]
Bug 34868: Add ability for SIP2 to distinguish missing item from other lost types

The SIP circulation status specifies that a 12 means an item is lost, and 13 means an item is missing. In Koha, missing items are simply a type of lost item so we never send a 13. This is an important distinction for some SIP based inventory tools. It would be good to be able to specify when lost status means "missing" at the SIP login level.

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

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 Kyle M Hall 2023-11-07 19:26:21 UTC
Created attachment 158615 [details] [review]
Bug 34868: (QA follow-up) Rename new option, add comments
Comment 9 Kyle M Hall 2023-11-07 19:26:22 UTC
Created attachment 158616 [details] [review]
Bug 34868: (QA follow-up) Fix XML
Comment 10 Kyle M Hall 2023-11-07 19:30:49 UTC
Created attachment 158617 [details] [review]
Bug 34868: (QA follow-up) Tidy sip_circulation_status
Comment 11 Kyle M Hall 2023-11-07 19:38:29 UTC
(In reply to Katrin Fischer from comment #5)
>  FAIL	etc/SIPconfig.xml

Fixed!

> Processing additional checks
> 
> 	* Commit title does not start with 'Bug XXXXX: ' - c91e7b374a

Fixed!

> (In reply to Katrin Fischer from comment #3)
> > I'd also like to see a test where we see what happens when the SIP option is
> > not configured.

There is already a test for when the option is off. It's the two lines in the unit test file above the two lines I added!
Comment 12 Katrin Fischer 2023-11-09 21:18:11 UTC
Created attachment 158768 [details] [review]
Bug 34868: Add ability for SIP2 to distinguish missing item from other lost types

The SIP circulation status specifies that a 12 means an item is lost, and 13 means an item is missing. In Koha, missing items are simply a type of lost item so we never send a 13. This is an important distinction for some SIP based inventory tools. It would be good to be able to specify when lost status means "missing" at the SIP login level.

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

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2023-11-09 21:18:14 UTC
Created attachment 158769 [details] [review]
Bug 34868: (QA follow-up) Rename new option, add comments

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2023-11-09 21:18:18 UTC
Created attachment 158770 [details] [review]
Bug 34868: (QA follow-up) Fix XML

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2023-11-09 21:18:21 UTC
Created attachment 158771 [details] [review]
Bug 34868: (QA follow-up) Tidy sip_circulation_status

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Tomás Cohen Arazi 2023-11-10 14:02:22 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 17 Fridolin Somers 2023-11-11 08:04:47 UTC
Enhancement not pushed to 23.05.x