Bug 20548 - Remove copyright clearance workflow from staff created ILL requests
Summary: Remove copyright clearance workflow from staff created ILL requests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Andrew Isherwood
QA Contact:
URL:
Keywords:
Depends on: 20284 20941
Blocks: 20996
  Show dependency treegraph
 
Reported: 2018-04-09 13:50 UTC by Andrew Isherwood
Modified: 2022-11-11 10:32 UTC (History)
8 users (show)

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


Attachments
Bug [20548] - [Remove copyright clearance workfl..] (4.93 KB, patch)
2018-04-09 14:12 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20548: Remove copyright clearance for staff (4.98 KB, patch)
2018-04-13 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Remove copyright clearance for staff (5.01 KB, patch)
2018-04-19 13:08 UTC, Barry Cannon
Details | Diff | Splinter Review
Bug 20548: Remove copyright clearance for staff (5.12 KB, patch)
2018-09-18 13:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20548: (follow-up) Fix unit tests (1.85 KB, patch)
2018-09-26 13:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20548: Remove copyright clearance for staff (5.15 KB, patch)
2018-10-08 23:41 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 20548: (follow-up) Fix unit tests (1.90 KB, patch)
2018-10-08 23:42 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2018-04-09 13:50:59 UTC
The workflow for a staff member to accept the copyright clearance declaration defined in the ILLModuleCopyrightClearance preference is erroneous. It is only necessary for OPAC users to accept the declaration. (Currently this functionality is broken, see bug 20284).

This bug is for the removal of the workflow step requiring staff members to accept the declaration.
Comment 1 Andrew Isherwood 2018-04-09 14:12:11 UTC
Created attachment 73871 [details] [review]
Bug [20548] - [Remove copyright clearance workfl..]

This patch removes the erroneously added workflow that requires staff
users to agree to the copyright clearance declaration defined in the
ILLModuleCopyrightClearance preference. Only OPAC users should be
required to accept the declaration.

TEST PLAN
---------

- Ensure you have at least one ILL backend available:
  https://wiki.koha-community.org/wiki/ILL_backends
- Ensure you have the "ILLModule" preference enabled
- Add some text to the "ILLModuleCopyrightClearance" preference
- As an OPAC user make an ILL request:
  - Navigate to a search results page in the catalog
  - Click the "Make an Interlibrary Loan request" link at the bottom
  - Choose "Create a new request", then select a backend
  - TEST: Check the text you added earlier is displayed with buttons for
  agreeing or disagreeing
- As a staff user, select the "ILL requests" button on the front page of
the intranet site
  - Choose "Create a new request", then select a backend
  - TEST: Check that you are NOT prompted to agree to the text you added
  earlier
Comment 2 Andrew Isherwood 2018-04-13 11:06:14 UTC
Created attachment 74155 [details] [review]
Bug 20548: Remove copyright clearance for staff

This patch removes the erroneously added workflow that requires staff
users to agree to the copyright clearance declaration defined in the
ILLModuleCopyrightClearance preference. Only OPAC users should be
required to accept the declaration.

To test:
1) Ensure you have at least one ILL backend available:
  https://wiki.koha-community.org/wiki/ILL_backends
2) Ensure you have the "ILLModule" preference enabled
3) Add some text to the "ILLModuleCopyrightClearance" preference
4) As an OPAC user make an ILL request:
  a) Navigate to a search results page in the catalog
  b) Click the "Make an Interlibrary Loan request" link at the bottom
  c) Choose "Create a new request", then select a backend
  d) Observe the text you added earlier is displayed with buttons for
  agreeing or disagreeing
5) As a staff user, select the "ILL requests" button on the front page of
the intranet site
  a) Choose "Create a new request", then select a backend
  b) Observe that you are NOT prompted to agree to the text you added
  earlier

Assigned-to: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 3 Barry Cannon 2018-04-19 13:06:48 UTC
This patch removes the erroneously added workflow that requires staff
users to agree to the copyright clearance declaration defined in the
ILLModuleCopyrightClearance preference. Only OPAC users should be
required to accept the declaration.

To test:
1) Ensure you have at least one ILL backend available:
  https://wiki.koha-community.org/wiki/ILL_backends
2) Ensure you have the "ILLModule" preference enabled
3) Add some text to the "ILLModuleCopyrightClearance" preference
4) As an OPAC user make an ILL request:
  a) Navigate to a search results page in the catalog
  b) Click the "Make an Interlibrary Loan request" link at the bottom
  c) Choose "Create a new request", then select a backend
  d) Observe the text you added earlier is displayed with buttons for
  agreeing or disagreeing
5) As a staff user, select the "ILL requests" button on the front page of
the intranet site
  a) Choose "Create a new request", then select a backend
  b) Observe that you are NOT prompted to agree to the text you added
  earlier

Assigned-to: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Barry Cannon <bc@interleaf.ie>
Comment 4 Barry Cannon 2018-04-19 13:08:49 UTC
Created attachment 74549 [details] [review]
Remove copyright clearance for staff

This patch removes the erroneously added workflow that requires staff
users to agree to the copyright clearance declaration defined in the
ILLModuleCopyrightClearance preference. Only OPAC users should be
required to accept the declaration.

To test:
1) Ensure you have at least one ILL backend available:
  https://wiki.koha-community.org/wiki/ILL_backends
2) Ensure you have the "ILLModule" preference enabled
3) Add some text to the "ILLModuleCopyrightClearance" preference
4) As an OPAC user make an ILL request:
  a) Navigate to a search results page in the catalog
  b) Click the "Make an Interlibrary Loan request" link at the bottom
  c) Choose "Create a new request", then select a backend
  d) Observe the text you added earlier is displayed with buttons for
  agreeing or disagreeing
5) As a staff user, select the "ILL requests" button on the front page of
the intranet site
  a) Choose "Create a new request", then select a backend
  b) Observe that you are NOT prompted to agree to the text you added
  earlier

Assigned-to: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Barry Cannon <bc@interleaf.ie>
Comment 5 Marcel de Rooy 2018-04-20 08:11:46 UTC
Dependency in FQA
Comment 6 Claire Gravely 2018-09-16 18:42:48 UTC
dependency is pushed, changing back to signed off.
Comment 7 Jonathan Druart 2018-09-18 13:23:22 UTC
Created attachment 79051 [details] [review]
Bug 20548: Remove copyright clearance for staff

This patch removes the erroneously added workflow that requires staff
users to agree to the copyright clearance declaration defined in the
ILLModuleCopyrightClearance preference. Only OPAC users should be
required to accept the declaration.

To test:
1) Ensure you have at least one ILL backend available:
  https://wiki.koha-community.org/wiki/ILL_backends
2) Ensure you have the "ILLModule" preference enabled
3) Add some text to the "ILLModuleCopyrightClearance" preference
4) As an OPAC user make an ILL request:
  a) Navigate to a search results page in the catalog
  b) Click the "Make an Interlibrary Loan request" link at the bottom
  c) Choose "Create a new request", then select a backend
  d) Observe the text you added earlier is displayed with buttons for
  agreeing or disagreeing
5) As a staff user, select the "ILL requests" button on the front page of
the intranet site
  a) Choose "Create a new request", then select a backend
  b) Observe that you are NOT prompted to agree to the text you added
  earlier

Assigned-to: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Barry Cannon <bc@interleaf.ie>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2018-09-18 13:24:44 UTC
Andrew, did you notice the empty button (with and without the patch):

https://snag.gy/9PQ5yL.jpg

on /cgi-bin/koha/ill/ill-requests.pl?method=illview&illrequest_id=1
Comment 9 Andrew Isherwood 2018-09-19 08:26:31 UTC
Hi Jonathan, this is actually a bug in master. I spotted it the other day but completely forgot to do anything about it, thanks for the reminder. It seems that at some point the "| html" filter was erroneously added to some variable declarations that weren't strings, these specifically:

https://github.com/PTFS-Europe/koha/blob/master/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt#L483-L485

Since these variables are either hashrefs or arrayrefs, the filter breaks them, hence the problem you're seeing.

I'll create a new bug and get them fixed.
Comment 10 Andrew Isherwood 2018-09-19 08:38:24 UTC
A patch for this is now at https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21377
Comment 11 Nick Clemens 2018-09-19 13:17:05 UTC
please add unit tests to cover this change. After applying:

vagrant@kohadevbox:kohaclone(master)$ prove /home/vagrant/kohaclone/t/db_dependent/Illrequests.t
/home/vagrant/kohaclone/t/db_dependent/Illrequests.t .. 4/11 Use of uninitialized value $backend_dir in join or string at /home/vagrant/kohaclone/Koha/Illrequest.pm line 665.

    #   Failed test 'Backend create: copyright clearance.'
    #   at /home/vagrant/kohaclone/t/db_dependent/Illrequests.t line 412.
    #     Structures begin differing at:
    #          $got->{value} = Does not exist
    #     $expected->{value} = HASH(0x971d1e0)

    #   Failed test 'Backend create: arbitrary stage.'
    #   at /home/vagrant/kohaclone/t/db_dependent/Illrequests.t line 426.
    #     Structures begin differing at:
    #          $got->{stage} = 'commit'
    #     $expected->{stage} = 'bar'
Use of uninitialized value $stage in string eq at /home/vagrant/kohaclone/Koha/Illrequest.pm line 630.
Use of uninitialized value in concatenation (.) or string at /home/vagrant/kohaclone/Koha/Illrequest.pm line 666.
Use of uninitialized value in concatenation (.) or string at /home/vagrant/kohaclone/Koha/Illrequest.pm line 668.

    #   Failed test 'Backend create: arbitrary stage, permitted.'
    #   at /home/vagrant/kohaclone/t/db_dependent/Illrequests.t line 444.
    #     Structures begin differing at:
    #          $got->{permitted} = Does not exist
    #     $expected->{permitted} = '1'
    # Looks like you failed 3 tests of 18.

#   Failed test 'Backend core methods'
#   at /home/vagrant/kohaclone/t/db_dependent/Illrequests.t line 534.
# Looks like you failed 1 test of 11.
/home/vagrant/kohaclone/t/db_dependent/Illrequests.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/11 subtests
Comment 12 Andrew Isherwood 2018-09-26 13:06:18 UTC
Created attachment 79414 [details] [review]
Bug 20548: (follow-up) Fix unit tests

Removed the "Backend create: copyright clearance" unit test since we no
longer have a "copyrightclearance" stage, hence we don't have a block in
the template handling it, hence the test failure!
Comment 13 Andrew Isherwood 2018-09-26 13:07:35 UTC
Thanks Nick. It actually required the removal of a test that is no longer applicable due to the original patch removing the code that it tested!
Comment 14 Chris Cormack 2018-10-08 23:41:23 UTC
Created attachment 80243 [details] [review]
Bug 20548: Remove copyright clearance for staff

This patch removes the erroneously added workflow that requires staff
users to agree to the copyright clearance declaration defined in the
ILLModuleCopyrightClearance preference. Only OPAC users should be
required to accept the declaration.

To test:
1) Ensure you have at least one ILL backend available:
  https://wiki.koha-community.org/wiki/ILL_backends
2) Ensure you have the "ILLModule" preference enabled
3) Add some text to the "ILLModuleCopyrightClearance" preference
4) As an OPAC user make an ILL request:
  a) Navigate to a search results page in the catalog
  b) Click the "Make an Interlibrary Loan request" link at the bottom
  c) Choose "Create a new request", then select a backend
  d) Observe the text you added earlier is displayed with buttons for
  agreeing or disagreeing
5) As a staff user, select the "ILL requests" button on the front page of
the intranet site
  a) Choose "Create a new request", then select a backend
  b) Observe that you are NOT prompted to agree to the text you added
  earlier

Assigned-to: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Barry Cannon <bc@interleaf.ie>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 15 Chris Cormack 2018-10-08 23:42:23 UTC
Created attachment 80244 [details] [review]
Bug 20548: (follow-up) Fix unit tests

Removed the "Backend create: copyright clearance" unit test since we no
longer have a "copyrightclearance" stage, hence we don't have a block in
the template handling it, hence the test failure!

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 16 Nick Clemens 2018-10-09 16:25:17 UTC
Awesome work all!

Pushed to master for 18.11
Comment 17 Martin Renvoize 2018-10-15 12:40:13 UTC
Pushed to 18.05.x for 18.05.05
Comment 18 Fridolin Somers 2018-11-09 07:07:19 UTC
Depends on Bug 20284 not in 17.11.x