Bug 21979 - Add option to SIP2 config to send arbitrary item field in CR instead of collection code
Summary: Add option to SIP2 config to send arbitrary item field in CR instead of colle...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 27589
  Show dependency treegraph
 
Reported: 2018-12-07 19:37 UTC by Kyle M Hall
Modified: 2022-06-06 20:24 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code (4.52 KB, patch)
2018-12-07 19:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code (4.53 KB, patch)
2018-12-12 12:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code (5.80 KB, patch)
2019-10-01 11:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Works as expected. Thanks. (5.84 KB, patch)
2019-10-03 08:57 UTC, Jesse Maseto
Details | Diff | Splinter Review
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code (5.80 KB, patch)
2020-01-29 12:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code (5.82 KB, patch)
2020-03-20 18:14 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code (5.35 KB, patch)
2020-08-18 13:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code (5.40 KB, patch)
2020-09-25 10:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21979: Add unit tests (2.83 KB, patch)
2020-09-25 13:36 UTC, Kyle M Hall
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 2018-12-07 19:37:08 UTC
Some libraries need to send a different field as the collection code, depending on how the library catalogs items. We should allow any arbitrary item field to be used as the value for the CR field.
Comment 1 Kyle M Hall 2018-12-07 19:43:54 UTC
Created attachment 82998 [details] [review]
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code

Some libraries need to send a different field as the collection code,
depending on how the library catalogs items. We should allow any arbitrary
item field to be used as the value for the CR field.

Test Plan:
1) Apply this patch
2) Set the new option cr_item_field to 'shelving_location'
3) Restart the SIP server
3) Perform a checkin via SIP
4) Note the CR field contains the shelving location code in the response
5) Perform an item information request
6) Note the CR field contains the shelving location code in the response
Comment 2 Kyle M Hall 2018-12-12 12:19:24 UTC
Created attachment 83102 [details] [review]
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code

Some libraries need to send a different field as the collection code,
depending on how the library catalogs items. We should allow any arbitrary
item field to be used as the value for the CR field.

Test Plan:
1) Apply this patch
2) Set the new option cr_item_field to 'shelving_location'
3) Restart the SIP server
3) Perform a checkin via SIP
4) Note the CR field contains the shelving location code in the response
5) Perform an item information request
6) Note the CR field contains the shelving location code in the response
Comment 3 Jesse Maseto 2019-10-01 08:59:40 UTC
Hi Kyle, There are a few merge conflicts with this patch.
Comment 4 Kyle M Hall 2019-10-01 11:23:52 UTC
Created attachment 93358 [details] [review]
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code

Some libraries need to send a different field as the collection code,
depending on how the library catalogs items. We should allow any arbitrary
item field to be used as the value for the CR field.

Test Plan:
1) Apply this patch
2) Set the new option cr_item_field to 'shelving_location'
3) Restart the SIP server
3) Perform a checkin via SIP
4) Note the CR field contains the shelving location code in the response
5) Perform an item information request
6) Note the CR field contains the shelving location code in the response
Comment 5 Jesse Maseto 2019-10-03 08:57:15 UTC
Created attachment 93535 [details] [review]
Works as expected. Thanks.

-JesseM


Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code

Some libraries need to send a different field as the collection code,
depending on how the library catalogs items. We should allow any arbitrary
item field to be used as the value for the CR field.

Test Plan:
1) Apply this patch
2) Set the new option cr_item_field to 'shelving_location'
3) Restart the SIP server
3) Perform a checkin via SIP
4) Note the CR field contains the shelving location code in the response
5) Perform an item information request
6) Note the CR field contains the shelving location code in the response

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Comment 6 Bouzid Fergani 2019-11-08 20:26:23 UTC
Hi Kyle,
We have conflict in etc/SIPconfig.xml when applying.

Bug 21979 - Add option to SIP2 config to send arbitrary item field in CR instead of collection code

93535 - Works as expected. Thanks.

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code
Using index info to reconstruct a base tree...
M	etc/SIPconfig.xml
Falling back to patching base and 3-way merge...
Auto-merging etc/SIPconfig.xml
CONFLICT (content): Merge conflict in etc/SIPconfig.xml
error: Failed to merge in the changes.
Patch failed at 0001 Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Works-as-expected-Thanks-ELLAUG.patch
Comment 7 Jonathan Druart 2020-01-23 10:37:49 UTC
Kyle, I did not test but I see:

+            $resp .= maybe_add( FID_COLLECTION_CODE,      $item->collection_code,     $server );

then:

+            if ( my $CR = $server->{account}->{cr_item_field} ) {
+                $resp .= maybe_add( FID_COLLECTION_CODE, $item->{$CR} );
+            } else {
+                $resp .= maybe_add( FID_COLLECTION_CODE, $item->collection_code );
+            }

Is that correct? (twice maybe_add with FID_COLLECTION_CODE)
Comment 8 Kyle M Hall 2020-01-29 12:47:23 UTC
Created attachment 98066 [details] [review]
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code

Some libraries need to send a different field as the collection code,
depending on how the library catalogs items. We should allow any arbitrary
item field to be used as the value for the CR field.

Test Plan:
1) Apply this patch
2) Set the new option cr_item_field to 'shelving_location'
3) Restart the SIP server
3) Perform a checkin via SIP
4) Note the CR field contains the shelving location code in the response
5) Perform an item information request
6) Note the CR field contains the shelving location code in the response

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Comment 9 Kyle M Hall 2020-01-29 12:48:28 UTC
(In reply to Jonathan Druart from comment #7)
> Kyle, I did not test but I see:
> 
> +            $resp .= maybe_add( FID_COLLECTION_CODE,     
> $item->collection_code,     $server );
> 
> then:
> 
> +            if ( my $CR = $server->{account}->{cr_item_field} ) {
> +                $resp .= maybe_add( FID_COLLECTION_CODE, $item->{$CR} );
> +            } else {
> +                $resp .= maybe_add( FID_COLLECTION_CODE,
> $item->collection_code );
> +            }
> 
> Is that correct? (twice maybe_add with FID_COLLECTION_CODE)

Fixed! Looks like the original patch got a bad rebase perhaps.
Comment 10 Bouzid Fergani 2020-03-20 18:14:39 UTC
Created attachment 101308 [details] [review]
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code

Some libraries need to send a different field as the collection code,
depending on how the library catalogs items. We should allow any arbitrary
item field to be used as the value for the CR field.

Test Plan:
1) Apply this patch
2) Set the new option cr_item_field to 'shelving_location'
3) Restart the SIP server
3) Perform a checkin via SIP
4) Note the CR field contains the shelving location code in the response
5) Perform an item information request
6) Note the CR field contains the shelving location code in the response

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 11 Katrin Fischer 2020-08-16 09:38:38 UTC
Patch no longer applies, please rebase!

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code
Using index info to reconstruct a base tree...
M	C4/SIP/ILS/Item.pm
M	C4/SIP/Sip/MsgType.pm
M	etc/SIPconfig.xml
Falling back to patching base and 3-way merge...
Auto-merging etc/SIPconfig.xml
Auto-merging C4/SIP/Sip/MsgType.pm
Auto-merging C4/SIP/ILS/Item.pm
CONFLICT (content): Merge conflict in C4/SIP/ILS/Item.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-21979-Add-option-to-SIP2-config-to-send-arbitr-uD5jqS.patch


I have some issues setting up SIP on my kohadevbox - looking for a tutor!
Comment 12 Kyle M Hall 2020-08-18 13:12:54 UTC
Created attachment 108502 [details] [review]
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code

Some libraries need to send a different field as the collection code,
depending on how the library catalogs items. We should allow any arbitrary
item field to be used as the value for the CR field.

Test Plan:
1) Apply this patch
2) Set the new option cr_item_field to 'shelving_location'
3) Restart the SIP server
3) Perform a checkin via SIP
4) Note the CR field contains the shelving location code in the response
5) Perform an item information request
6) Note the CR field contains the shelving location code in the response

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 13 Martin Renvoize 2020-09-25 10:45:42 UTC
Created attachment 110746 [details] [review]
Bug 21979: Add option to SIP2 config to send arbitrary item field in CR instead of collection code

Some libraries need to send a different field as the collection code,
depending on how the library catalogs items. We should allow any arbitrary
item field to be used as the value for the CR field.

Test Plan:
1) Apply this patch
2) Set the new option cr_item_field to 'shelving_location'
3) Restart the SIP server
3) Perform a checkin via SIP
4) Note the CR field contains the shelving location code in the response
5) Perform an item information request
6) Note the CR field contains the shelving location code in the response

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2020-09-25 10:46:46 UTC
This works as expected and the code reads well...

Should we be adding Unit Tests for these SIP additions however...?

Passing QA and letting the RM weight in on the above.
Comment 15 Kyle M Hall 2020-09-25 13:36:09 UTC
Created attachment 110757 [details] [review]
Bug 21979: Add unit tests
Comment 16 Jonathan Druart 2020-10-01 09:44:15 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 17 Lucas Gass 2020-10-20 17:56:26 UTC
enhancement will not be backported to 20.05.x