Bug 20058 - Option to use shelving location instead of homebranch for sorting
Summary: Option to use shelving location instead of homebranch for sorting
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Stefan Berndtsson
QA Contact: Testopia
URL:
Keywords: release-notes-needed
Depends on:
Blocks: 33060
  Show dependency treegraph
 
Reported: 2018-01-22 11:54 UTC by Stefan Berndtsson
Modified: 2023-12-28 20:45 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/665
Text to go in the release notes:
Version(s) released in:
22.11.00


Attachments
Add option to send permanent_location as AQ in SIP response (2.56 KB, patch)
2018-01-22 11:54 UTC, Stefan Berndtsson
Details | Diff | Splinter Review
Bug 20058: Add option to send permanent_location as AQ in SIP response (4.58 KB, patch)
2021-09-27 14:49 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 20058: Add tests (3.04 KB, patch)
2021-09-27 14:50 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 20058: Add tests (2.67 KB, patch)
2021-09-27 14:52 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 20058: Add option to send permanent_location as AQ in SIP response (4.43 KB, patch)
2021-09-29 14:25 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 20058: Add tests (2.67 KB, patch)
2021-09-29 14:25 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 20058: Add option to send permanent_location as AQ in SIP response (4.15 KB, patch)
2022-08-09 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20058: Add tests (2.74 KB, patch)
2022-08-09 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20058: Add option to send permanent_location as AQ in SIP response (4.15 KB, patch)
2022-09-02 07:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20058: Add tests (2.74 KB, patch)
2022-09-02 07:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20058: (QA follow-up) Add exec flag to install file (681 bytes, patch)
2022-09-02 07:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20058: (QA follow-up) Simplify test code (2.20 KB, patch)
2022-09-02 07:01 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Berndtsson 2018-01-22 11:54:15 UTC
By default a checkin response via SIP2 has homebranch in AQ. AQ is used for sorting by our (Gothenburg University) machines.

The provided patch adds a syspref to select permanent_location instead of homebranch as the value put in AQ.
Comment 1 Stefan Berndtsson 2018-01-22 11:54:32 UTC
Created attachment 70791 [details] [review]
Add option to send permanent_location as AQ in SIP response
Comment 2 David Gustafsson 2021-09-27 14:49:56 UTC
Created attachment 125364 [details] [review]
Bug 20058: Add option to send permanent_location as AQ in SIP response
Comment 3 David Gustafsson 2021-09-27 14:50:00 UTC
Created attachment 125365 [details] [review]
Bug 20058: Add tests
Comment 4 David Gustafsson 2021-09-27 14:50:23 UTC
Rebased against master and added tests.
Comment 5 David Gustafsson 2021-09-27 14:52:25 UTC
Created attachment 125366 [details] [review]
Bug 20058: Add tests
Comment 6 Fridolin Somers 2021-09-28 06:50:20 UTC
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ -655,10 +655,19 @@ Circulation:
             - pref: StoreLastBorrower
               default: 0
               choices:
+        -
                   1: Store
                   0: "Don't store"

This change is a mistake no ?
Comment 7 David Gustafsson 2021-09-29 14:17:40 UTC
Yes! Will fix this.
Comment 8 David Gustafsson 2021-09-29 14:25:16 UTC
Created attachment 125454 [details] [review]
Bug 20058: Add option to send permanent_location as AQ in SIP response
Comment 9 David Gustafsson 2021-09-29 14:25:20 UTC
Created attachment 125455 [details] [review]
Bug 20058: Add tests
Comment 10 Katrin Fischer 2022-08-07 21:22:25 UTC
A quick observation here:

1) I notice that the new system preference is added under Holds policy, I think that doesn't fit well and should better bemoved to be under the new SIP2 heading.

2) As this is supposed to be used for sorting: there is now a configuration option specific to sort bins with the SIP2SortBinMapping system preference, but it's setting the CL field. Is this still required with this new development?

Also adding Kyle and Martin as they know SIP2 well, I am not sure if we don't have a way to achieve that yet via the conf file.
Comment 11 Martin Renvoize 2022-08-08 20:59:04 UTC
Hmm, well bug 20517 uses the CL field for sorter, which from memory is what sorters here use.. I'd have to look at the docs again to remind myself what AQ is for, but from the code this looks reasonable.
Comment 12 Stefan Berndtsson 2022-08-09 08:59:54 UTC
The main difference for us, and the reason for making the patch in the first place, is that our sorting is done in each machine based on CT (branchcode) and AQ (permanent_location).

CT is only set when the item is being sent to another library, so we first sort on that, and if it is not set, we sort on AQ for different destinations within the same library.

Using CL is likely a functional way forward for sorting, but it moves the sorting rules to the server, which is not something that's setup yet, so I haven't looked into that here.

When we first wrote this patch (in 2018 or so), Koha did not produce CT, so AQ was all we had. Now that Koha sets CT to homebranch when transferring, the default behaviour of also setting AQ to the same thing seems a bit redundant.

The SIP name of the AQ field is "permanent location" (FID_PERM_LOCN), although what that technically means is of course an interpretation.

To me, AQ and CL has two different meanings. One is information on the returned item (AQ), one is instructions to a machine (CL).
Comment 13 Martin Renvoize 2022-08-09 15:31:18 UTC
Created attachment 138925 [details] [review]
Bug 20058: Add option to send permanent_location as AQ in SIP response

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2022-08-09 15:31:22 UTC
Created attachment 138926 [details] [review]
Bug 20058: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2022-08-09 15:32:19 UTC
Thanks for the explanation.. all works as described to signing off.  Over to Kyle for a final QA run :)

PS, whilst rebasing it to apply I moved the syspref into the SIP2 section for display.
Comment 16 Katrin Fischer 2022-08-09 16:40:06 UTC
(In reply to Martin Renvoize from comment #15)
> Thanks for the explanation.. all works as described to signing off.  Over to
> Kyle for a final QA run :)
> 
> PS, whilst rebasing it to apply I moved the syspref into the SIP2 section
> for display.

Thanks!
Comment 17 Nick Clemens 2022-09-02 07:01:22 UTC
Created attachment 140068 [details] [review]
Bug 20058: Add option to send permanent_location as AQ in SIP response

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Nick Clemens 2022-09-02 07:01:27 UTC
Created attachment 140069 [details] [review]
Bug 20058: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Nick Clemens 2022-09-02 07:01:32 UTC
Created attachment 140070 [details] [review]
Bug 20058: (QA follow-up) Add exec flag to install file
Comment 20 Nick Clemens 2022-09-02 07:01:36 UTC
Created attachment 140071 [details] [review]
Bug 20058: (QA follow-up) Simplify test code
Comment 21 Tomás Cohen Arazi 2022-09-12 20:47:25 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 22 Jonathan Druart 2023-02-23 09:50:54 UTC
+                  yes: permanent_location
+                  no: homebranch

This is wrong, it must be 1 and 0, no "yes" and "no".
Comment 23 Katrin Fischer 2023-02-23 22:18:01 UTC
There are more of these, I've filed a separate bug and provided a patch:
Bug 33060 - Fix yes/no setting to 1/0 in system preference YAML files