Bug 20639 - Allow setting a default/single backend for OPAC driven requests
Summary: Allow setting a default/single backend for OPAC driven requests
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Andrew Isherwood
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 20581
Blocks: 20750
  Show dependency treegraph
 
Reported: 2018-04-23 15:47 UTC by Martin Renvoize
Modified: 2019-03-12 08:43 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20639: Add ILLOpacbackends syspref (4.84 KB, patch)
2018-04-24 13:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20639: Add ILLOpacbackends syspref (4.84 KB, patch)
2018-04-24 13:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20639: Add ILLOpacbackends syspref (5.07 KB, patch)
2018-05-08 15:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20639: Add ILLOpacbackends syspref (5.07 KB, patch)
2018-08-24 09:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20639: (follow-up) Fix variable not passed (2.49 KB, patch)
2018-09-17 14:11 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20639: (follow-up) Fix population of backends (1.34 KB, patch)
2018-09-27 10:49 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20639: Add ILLOpacbackends syspref (5.12 KB, patch)
2018-09-27 11:19 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20639: (follow-up) Fix population of backends (1.39 KB, patch)
2018-09-27 11:19 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20639: Add ILLOpacbackends syspref (5.18 KB, patch)
2019-02-09 22:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20639: (follow-up) Fix population of backends (1.44 KB, patch)
2019-02-09 22:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20639: (QA follow-up) Rephrase system preference (1.29 KB, patch)
2019-02-09 22:12 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 Martin Renvoize 2018-04-23 15:47:38 UTC
If you have more than one ILL backend installed, and OPAC ILL Requests enabled the OPAC users are currently faced with a dropdown list of backends to start a request on. This is often meaningless to the end user and as such a default should be configurable at the least and preferably the ability to set a fast path to only allow a prefered backend to be used from the OPAC.
Comment 1 Martin Renvoize 2018-04-24 13:17:23 UTC
Created attachment 74806 [details] [review]
Bug 20639: Add ILLOpacbackends syspref

This adds the ILLOpacbackends syspref, allowing users to refine the ill
backends available to opac users for initiating ill requests
Comment 2 Martin Renvoize 2018-04-24 13:57:04 UTC
Created attachment 74811 [details] [review]
Bug 20639: Add ILLOpacbackends syspref

This adds the ILLOpacbackends syspref, allowing users to refine the ill
backends available to opac users for initiating ill requests
Comment 3 Andrew Isherwood 2018-05-08 15:17:46 UTC
Comment on attachment 74811 [details] [review]
Bug 20639: Add ILLOpacbackends syspref

Obsoleted due to updated patch
Comment 4 Andrew Isherwood 2018-05-08 15:18:33 UTC
Created attachment 75165 [details] [review]
Bug 20639: Add ILLOpacbackends syspref

This adds the ILLOpacbackends syspref, allowing users to refine the ill
backends available to opac users for initiating ill requests

Remove default assignment for backends

We don't need a default assignment for the ILLOpacbackends assignment,
if the pref isn't set, it returns undef anyway. Also, having this
default assignment actually breaks the fetching of the preference
Comment 5 Andrew Isherwood 2018-08-24 09:16:56 UTC
Created attachment 78119 [details] [review]
Bug 20639: Add ILLOpacbackends syspref

This adds the ILLOpacbackends syspref, allowing users to refine the ill
backends available to opac users for initiating ill requests

Remove default assignment for backends

We don't need a default assignment for the ILLOpacbackends assignment,
if the pref isn't set, it returns undef anyway. Also, having this
default assignment actually breaks the fetching of the preference
Comment 6 Andrew Isherwood 2018-09-17 14:11:55 UTC
Created attachment 79033 [details] [review]
Bug 20639: (follow-up) Fix variable not passed

available_backends expects a variable declaring the contents
of the ILLopacbackends syspref. This wasn't being passed in all calls to
it, so I've removed the requirement for it being passed, we now check
the contents of the syspref in available_backends itselfBug
Comment 7 Andrew Isherwood 2018-09-17 14:23:01 UTC
To test:

1. Ensure you have at least two ILL backends installed
2. From the OPAC user's home page, choose "your interlibrary loan requests"
3. Click the "Create a new request" button
4. TEST: Observe that all backends are available
5. As a user with sufficient privileges, go to the System Preferences
6. Search the privileges for "interlibrary"
7. TEST: Observe that a "ILLOpacbackends" preference is available
8. Enter the name of *one* of the installed backends in the input box for "ILLOpacbackends", for example BLDSS
9. From the OPAC user's home page, choose "your interlibrary loan requests"
10. TEST: Observe that the "Create a new request" button is no longer a select box
11. TEST: Observe that clicking the "Create a new request" button takes you straight to the form for creating a request with the single available backend
12. As a user with sufficient privileges, go to the System Preferences
13. Search the privileges for "interlibrary"
14. Enter the name of *two* of the installed backends in the input box for "ILLOpacbackends", separated by a | character for example: BLDSS|Koha
15. From the OPAC user's home page, choose "your interlibrary loan requests"
16. TEST: Observe that the "Create a new request" button is now a select box
17. TEST: Observe that clicking the "Create a new request" button only offers the backends specified in step 14
Comment 8 Andrew Isherwood 2018-09-27 10:49:06 UTC
Created attachment 79474 [details] [review]
Bug 20639: (follow-up) Fix population of backends

The OPAC view wasn't correctly restricting the display of backends when
the ILLOpacbackends preference was set.

See the test plan on comment 7
Comment 9 PTFS Europe Sandboxes 2018-09-27 11:19:17 UTC
Created attachment 79477 [details] [review]
Bug 20639: Add ILLOpacbackends syspref

This adds the ILLOpacbackends syspref, allowing users to refine the ill
backends available to opac users for initiating ill requests

Remove default assignment for backends

We don't need a default assignment for the ILLOpacbackends assignment,
if the pref isn't set, it returns undef anyway. Also, having this
default assignment actually breaks the fetching of the preference

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 10 PTFS Europe Sandboxes 2018-09-27 11:19:40 UTC
Created attachment 79478 [details] [review]
Bug 20639: (follow-up) Fix population of backends

The OPAC view wasn't correctly restricting the display of backends when
the ILLOpacbackends preference was set.

See the test plan on comment 7

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 11 Katrin Fischer 2019-02-09 22:08:59 UTC
Created attachment 84932 [details] [review]
Bug 20639: Add ILLOpacbackends syspref

This adds the ILLOpacbackends syspref, allowing users to refine the ill
backends available to opac users for initiating ill requests

Remove default assignment for backends

We don't need a default assignment for the ILLOpacbackends assignment,
if the pref isn't set, it returns undef anyway. Also, having this
default assignment actually breaks the fetching of the preference

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2019-02-09 22:09:03 UTC
Created attachment 84933 [details] [review]
Bug 20639: (follow-up) Fix population of backends

The OPAC view wasn't correctly restricting the display of backends when
the ILLOpacbackends preference was set.

See the test plan on comment 7

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2019-02-09 22:12:53 UTC
Created attachment 84934 [details] [review]
Bug 20639: (QA follow-up) Rephrase system preference

Rephrases system preference a tiny bit and explaining case
of leaving the pref empty.
Comment 14 Katrin Fischer 2019-02-09 22:16:05 UTC
This works as described but doesn't cover one use case: A library that doens't want to allow OPAC initiated requests. Something to think about later maybe?
Comment 15 Katrin Fischer 2019-02-09 22:21:30 UTC
On second thought - Andrew, can we have some unit tests for testing the new behaviour of the method please?
Comment 16 Andrew Isherwood 2019-02-10 10:09:20 UTC
(In reply to Katrin Fischer from comment #14)
> This works as described but doesn't cover one use case: A library that
> doens't want to allow OPAC initiated requests. Something to think about
> later maybe?

Yes, indeed. Having ILL enabled only for the staff interface is definitely outside the scope of this bug I think.
Comment 17 Andrew Isherwood 2019-02-11 11:25:38 UTC
(In reply to Katrin Fischer from comment #14)
> This works as described but doesn't cover one use case: A library that
> doens't want to allow OPAC initiated requests. Something to think about
> later maybe?

I have now added Bug 22305 to record this
Comment 18 Andrew Isherwood 2019-02-11 11:46:51 UTC
(In reply to Katrin Fischer from comment #15)
> On second thought - Andrew, can we have some unit tests for testing the new
> behaviour of the method please?

Hi Katrin - The method in question "available_backends" has never had tests due to it needing at least one backend installing in the filesystem in order to run, it looks at the directory specified in the config and enumerates the backends it finds.
Comment 19 Martin Renvoize 2019-02-11 15:07:42 UTC
I think we should let the tests slip for this one for a couple of reasons.

1) It wouldn't be trivial to write the tests, adding and removing files as part of a mocking process which could be fraught with permissions issues.
2) We're still aiming to shift to Koha::Plugins infrastructure for backends in which case this call would likely be replaced by the use of GetPlugins or similar from that area of code.

I will write a bug report for that piece and note/link it to this, but for now I certainly think we can get away without one and let the tree that's hanging of this continue to shrink.
Comment 20 Nick Clemens 2019-02-22 15:05:29 UTC
Awesome work all!

Pushed to master for 19.05
Comment 21 Martin Renvoize 2019-02-26 10:04:15 UTC
Enhancement, will not be backported to 18.11.x series
Comment 22 Josef Moravec 2019-03-07 09:11:32 UTC
This should be in master, but it looks like it is not pushed!
Comment 23 Nick Clemens 2019-03-07 21:20:43 UTC
Oops!

Actually , pushed to master for 19.05
Comment 24 Martin Renvoize 2019-03-12 08:43:39 UTC
Enhancement, will not be backported to 18.11.x series.