Bug 18203 - Add per borrower category restrictions on placing ILL requests in OPAC
Summary: Add per borrower category restrictions on placing ILL requests in OPAC
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement with 1 vote (vote)
Assignee: Pedro Amorim
QA Contact: Katrin Fischer
URL:
Keywords:
: 23976 (view as bug list)
Depends on: 7317
Blocks: 35263
  Show dependency treegraph
 
Reported: 2017-03-03 07:43 UTC by Alex Sassmannshausen
Modified: 2023-11-30 16:08 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
When the ILL module is activated, the patron category administration page will include a setting "Can patron place ILL requests in OPAC". For existing installations the flag will be set for all patron categories on update. **Sponsored by** *PTFS Europe Ltd*
Version(s) released in:
23.11.00


Attachments
Add per borrower category create ILL privilege. (14.66 KB, patch)
2017-03-03 07:50 UTC, Alex Sassmannshausen
Details | Diff | Splinter Review
DO NOT PUSH: Schema changes. (6.65 KB, patch)
2017-03-03 07:50 UTC, Alex Sassmannshausen
Details | Diff | Splinter Review
Bug 18203: Add per borrower category restrictions on ILL placement (14.34 KB, patch)
2023-01-16 13:00 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: Add per borrower category restrictions on ILL placement (14.39 KB, patch)
2023-01-17 07:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 18203: Add per borrower category restrictions on ILL placement (14.41 KB, patch)
2023-01-17 10:33 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: (QA follow-up) * Add boolean flag in Category.pm (850 bytes, patch)
2023-03-25 18:20 UTC, Pedro Amorim
Details | Diff | Splinter Review
WIP: Fix categories table presentation (1.15 KB, patch)
2023-03-26 11:36 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: Add per borrower category restrictions on placing ILL requests (14.36 KB, patch)
2023-03-31 04:13 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: DB specific and atomicupdate (2.62 KB, patch)
2023-07-12 09:20 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: dbic specific [DONT PUSH] (1.95 KB, patch)
2023-07-12 09:20 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: Add per borrower category restrictions on placing ILL requests (10.91 KB, patch)
2023-07-12 09:20 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: DB specific and atomicupdate (2.67 KB, patch)
2023-07-12 15:31 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 18203: dbic specific [DONT PUSH] (1.99 KB, patch)
2023-07-12 15:31 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 18203: Add per borrower category restrictions on placing ILL requests (10.96 KB, patch)
2023-07-12 15:31 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 18203: DB specific and atomicupdate (2.67 KB, patch)
2023-09-07 15:02 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: dbic specific [DONT PUSH] (1.99 KB, patch)
2023-09-07 15:02 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: Add per borrower category restrictions on placing ILL requests (10.91 KB, patch)
2023-09-07 15:02 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 18203: DB specific and atomicupdate (2.72 KB, patch)
2023-10-31 14:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18203: dbic specific [DONT PUSH] (2.04 KB, patch)
2023-10-31 14:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18203: Add per borrower category restrictions on placing ILL requests (11.03 KB, patch)
2023-10-31 14:34 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 Alex Sassmannshausen 2017-03-03 07:43:01 UTC
The ILLModule currently allows any borrower category to request new ILLs.  This may not be what is required in many places as ILLs can be very expensive.

This bug should implement a method by which entire borrower categories can be prevented from creating new ILL requests.

Alex
Comment 1 Alex Sassmannshausen 2017-03-03 07:50:00 UTC
Created attachment 60805 [details] [review]
Add per borrower category create ILL privilege.

* admin/categories.pl: Add validation & update for `canplaceill`.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/categories.tt: Add
  `canplaceill` handling.
* ill/ill-requests.pl: Pass `canplaceill` to template.
* koha-tmpl/intranet-tmpl/prog/en/includes/ill-toolbar.inc: Check for
  `canplaceill`.
* opac/opac-illrequests.pl: Pass `canplaceill` to template.
* koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-illrequests.tt: Check
  for `canplaceill`.
* installer/data/mysql/kohastructure.sql: Create `canplaceill`
  column.
* installer/data/mysql/atomicupdate/ill_cat_permission.sql: New file.
Comment 2 Alex Sassmannshausen 2017-03-03 07:50:04 UTC
Created attachment 60806 [details] [review]
DO NOT PUSH: Schema changes.

* Koha/Schema/Result/Category.pm: Add `canplaceill`.
Comment 3 Alex Sassmannshausen 2017-03-03 07:53:52 UTC
Test Plan:
- Have ILLModule working
- With any non-database user verify you can place new ILL requests in OPAC and Intranet
- Apply patches here
- Run dbupdate
- Verify you can no longer place new ILL requests in OPAC and Intranet
- In borrower categories, set 'Can place ILL' to yes.
- Verify you can place new ILL requests in OPAC and Intranet.
Comment 4 Katrin Fischer 2020-06-25 14:04:03 UTC
I think this would still be very useful to have as I see no other option to limit ILL requests for patrons. By patron category should work for most libraries.
Comment 5 Niamh Walker-Headon 2021-04-21 08:58:21 UTC
Has there been any update on this since the DO NOT PUSH? 
Thanks, 
Niamh
Comment 6 Katrin Fischer 2022-02-26 21:23:47 UTC
*** Bug 23976 has been marked as a duplicate of this bug. ***
Comment 7 Katrin Fischer 2022-02-26 21:26:56 UTC
Alex is no longer working on Koha so I am not sure why this was never set to 'needs sign-off'. 

The DO NOT PUSH for schema changes is normal as this file is only used for testing and the RM will not use it when pushing the patch set. So this doesn't tell us much. hm.

I am updating the status to 'Patch doesn't apply' and adding Andrew and Martin who may know what happened here.
Comment 8 Katrin Fischer 2023-01-13 09:29:49 UTC
I believe this would still be a nice to have feature.
Comment 9 Pedro Amorim 2023-01-16 13:00:50 UTC
Created attachment 145331 [details] [review]
Bug 18203: Add per borrower category restrictions on ILL placement

Test Plan:
- Enable ILLModule sys pref and install any backend
- Verify you can place new ILL requests in OPAC and Intranet
- Apply patch
- Run dbic
- Verify you can no longer place new ILL requests in OPAC and Intranet
- In borrower categories, set 'Can place ILL' to yes.
- Verify you can place new ILL requests in OPAC and Intranet.
Comment 10 David Nind 2023-01-17 07:24:31 UTC
Created attachment 145349 [details] [review]
Bug 18203: Add per borrower category restrictions on ILL placement

Test Plan:
- Enable ILLModule sys pref and install any backend
- Verify you can place new ILL requests in OPAC and Intranet
- Apply patch
- Run dbic
- Verify you can no longer place new ILL requests in OPAC and Intranet
- In borrower categories, set 'Can place ILL' to yes.
- Verify you can place new ILL requests in OPAC and Intranet.

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 Pedro Amorim 2023-01-17 10:33:47 UTC
Created attachment 145354 [details] [review]
Bug 18203: Add per borrower category restrictions on ILL placement

Test Plan:
- Enable ILLModule sys pref and install any backend
- Verify you can place new ILL requests in OPAC and Intranet
- Apply patch
- Run dbic
- Verify you can no longer place new ILL requests in OPAC and Intranet
- In borrower categories, set 'Can place ILL' to yes.
- Verify you can place new ILL requests in OPAC and Intranet.

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 Pedro Amorim 2023-01-17 10:34:37 UTC
Fixed commit author, had originally been submitted as John Doe.
Comment 13 Katrin Fischer 2023-02-27 19:45:20 UTC
1) There is a fail in the QA scripts:

 FAIL	installer/data/mysql/kohastructure.sql
   FAIL	  tinyint_has_boolean_flag
		The new column categories.canplaceill is using TINYINT(1) as type but is not defined as boolean in the schema file (Koha/Schema/Result/Category.pm), see the SQL12 coding guideline


2) Behavior in staff interface

I think, we should not use the staff person's patron category for the check on the 'new request' button in the staff interface.

If a staff user has permissions for the interlibrary loans module, they should be able to place ILL requests for the users of the library. They are usually not placing ILL requests for themselves, but for others. Similar to hold requests.

The check on the category should be implemented in the patron search/the user they try to place the request for. But I am not sure if this would have to be done in the backends then?


3) Changed behaviour for existing installations

As all categories are allowed to place ILL requests now, we should set "Can place ILL requests?" to yes for all categories if the ILLModule is activated. This way we won't break things for libraries and they can implement the new feature if they chose to do so.


But overall this looks quite good already!
Comment 14 Pedro Amorim 2023-03-25 18:20:19 UTC
Created attachment 148707 [details] [review]
Bug 18203: (QA follow-up) * Add boolean flag in Category.pm
Comment 15 Pedro Amorim 2023-03-25 19:00:33 UTC
> 2) Behavior in staff interface
> 
> I think, we should not use the staff person's patron category for the check
> on the 'new request' button in the staff interface.
> 
> If a staff user has permissions for the interlibrary loans module, they
> should be able to place ILL requests for the users of the library. They are
> usually not placing ILL requests for themselves, but for others. Similar to
> hold requests.
> 
> The check on the category should be implemented in the patron search/the
> user they try to place the request for. But I am not sure if this would have
> to be done in the backends then?

Yes, I agree with this idea. But it's the backend who is responsible for attaching the borrowernumber to the illrequest. However, this does seem a bit redundant for the staff UI, i.e. if a staff member already has the ill permission, this check should not even be required (since ill permission needs to be assigned on a per user in the first place anyway).

I suggest we restructure this bug's functionality to just apply to OPAC, since I also believe the original problem it's trying to solve is only on the OPAC anyway (prevent certain patron categories from being able to place requests to avoid costs/noise)

> 3) Changed behaviour for existing installations
> 
> As all categories are allowed to place ILL requests now, we should set "Can
> place ILL requests?" to yes for all categories if the ILLModule is
> activated. This way we won't break things for libraries and they can
> implement the new feature if they chose to do so.

Should we:
1) keep canplaceill default 0, and in atomicupdate set to 1 for every existing installation if ILLModule sys pref is enabled, or
2) change canplaceill default to 1, which means upon enabling ILLModule it just works for everyone and then let staff members untick the canplaceill checkbox for any given category.

Please Katrin let me know what you think of the above, or if some of what I said doesn't make sense or is wrong.
Thanks!
Comment 16 Pedro Amorim 2023-03-26 09:55:33 UTC
Current patches may or may not be causing issues with the table rendered in patron categories /cgi-bin/koha/admin/categories.pl
Need to double-check that.
Comment 17 Pedro Amorim 2023-03-26 11:36:06 UTC
Created attachment 148719 [details] [review]
WIP: Fix categories table presentation

But now there is an error in the console if ILL is disabled
Squash this later
Comment 18 Pedro Amorim 2023-03-31 04:13:25 UTC
Created attachment 149010 [details] [review]
Bug 18203: Add per borrower category restrictions on placing ILL requests

Default value for canplaceillopac is 1
Patron in a category that cannot place ill can still visit illlist and illview
Comment 19 Pedro Amorim 2023-07-12 09:20:39 UTC
Created attachment 153345 [details] [review]
Bug 18203: DB specific and atomicupdate
Comment 20 Pedro Amorim 2023-07-12 09:20:42 UTC
Created attachment 153346 [details] [review]
Bug 18203: dbic specific [DONT PUSH]
Comment 21 Pedro Amorim 2023-07-12 09:20:45 UTC
Created attachment 153347 [details] [review]
Bug 18203: Add per borrower category restrictions on placing ILL requests

    Test Plan:
    - Enable ILLModule sys pref and install any backend, or run
    bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
    - Verify you can place new ILL requests in OPAC and Intranet
    - Apply patch
    - In borrower categories, verify there's a new column for 'can place ILL in opac' and is set to 'yes' by default
    - Edit your borrower's patron category and set 'can place ILL in opac' to 'No'
    - Verify you can no longer place new ILL requests in OPAC
    - Verify you also cannot place new ILL requests through URL:
    :8080/cgi-bin/koha/opac-illrequests.pl?method=create&backend=FreeForm
Comment 22 Pedro Amorim 2023-07-12 09:22:36 UTC
Rebased this after bug 33702.
Updated test plan.
Split submission to multiple patches (db, dbic and feature).

Would someone be able to take a look at this? It's a quite small patch! (easy sign-off).
Comment 23 Sam Lau 2023-07-12 15:31:09 UTC
Created attachment 153370 [details] [review]
Bug 18203: DB specific and atomicupdate

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 24 Sam Lau 2023-07-12 15:31:12 UTC
Created attachment 153371 [details] [review]
Bug 18203: dbic specific [DONT PUSH]

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 25 Sam Lau 2023-07-12 15:31:15 UTC
Created attachment 153372 [details] [review]
Bug 18203: Add per borrower category restrictions on placing ILL requests

    Test Plan:
    - Enable ILLModule sys pref and install any backend, or run
    bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
    - Verify you can place new ILL requests in OPAC and Intranet
    - Apply patch
    - In borrower categories, verify there's a new column for 'can place ILL in opac' and is set to 'yes' by default
    - Edit your borrower's patron category and set 'can place ILL in opac' to 'No'
    - Verify you can no longer place new ILL requests in OPAC
    - Verify you also cannot place new ILL requests through URL:
    :8080/cgi-bin/koha/opac-illrequests.pl?method=create&backend=FreeForm

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 26 Pedro Amorim 2023-09-01 09:30:47 UTC Comment hidden (obsolete)
Comment 27 Pedro Amorim 2023-09-07 14:47:16 UTC
Needs a rebase on top of bug 34382
Comment 28 Pedro Amorim 2023-09-07 15:02:34 UTC
Created attachment 155316 [details] [review]
Bug 18203: DB specific and atomicupdate

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 29 Pedro Amorim 2023-09-07 15:02:37 UTC
Created attachment 155317 [details] [review]
Bug 18203: dbic specific [DONT PUSH]

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 30 Pedro Amorim 2023-09-07 15:02:41 UTC
Created attachment 155318 [details] [review]
Bug 18203: Add per borrower category restrictions on placing ILL requests

    Test Plan:
    - Enable ILLModule sys pref and install any backend, or run
    bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
    - Verify you can place new ILL requests in OPAC and Intranet
    - Apply patch
    - In borrower categories, verify there's a new column for 'can place ILL in opac' and is set to 'yes' by default
    - Edit your borrower's patron category and set 'can place ILL in opac' to 'No'
    - Verify you can no longer place new ILL requests in OPAC
    - Verify you also cannot place new ILL requests through URL:
    :8080/cgi-bin/koha/opac-illrequests.pl?method=create&backend=FreeForm

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 31 Katrin Fischer 2023-10-31 14:34:49 UTC
Created attachment 158126 [details] [review]
Bug 18203: DB specific and atomicupdate

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 32 Katrin Fischer 2023-10-31 14:34:52 UTC
Created attachment 158127 [details] [review]
Bug 18203: dbic specific [DONT PUSH]

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 33 Katrin Fischer 2023-10-31 14:34:55 UTC
Created attachment 158128 [details] [review]
Bug 18203: Add per borrower category restrictions on placing ILL requests

    Test Plan:
    - Enable ILLModule sys pref and install any backend, or run
    bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
    - Verify you can place new ILL requests in OPAC and Intranet
    - Apply patch
    - In borrower categories, verify there's a new column for 'can place ILL in opac' and is set to 'yes' by default
    - Edit your borrower's patron category and set 'can place ILL in opac' to 'No'
    - Verify you can no longer place new ILL requests in OPAC
    - Verify you also cannot place new ILL requests through URL:
    :8080/cgi-bin/koha/opac-illrequests.pl?method=create&backend=FreeForm

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 34 Tomás Cohen Arazi 2023-11-01 21:03:40 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 35 Tomás Cohen Arazi 2023-11-01 21:05:49 UTC
I made a last minute change on the column name. I hope you agree.
Comment 36 Pedro Amorim 2023-11-02 09:20:23 UTC
That's awesome Tomas thank you.
Comment 37 Katrin Fischer 2023-11-05 12:26:59 UTC
(In reply to Pedro Amorim from comment #36)
> That's awesome Tomas thank you.

We have a problem here, see bug 35263 - cannot edit patron categories
Comment 38 Fridolin Somers 2023-11-10 01:53:40 UTC
Enhancement not pushed to 23.05.x