Bug 22531 - Allow for multiple requests to partners and display partners in audit log
Summary: Allow for multiple requests to partners and display partners in audit log
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Andrew Isherwood
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-18 14:20 UTC by Andrew Isherwood
Modified: 2021-11-01 12:12 UTC (History)
6 users (show)

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


Attachments
Bug 22531: Add logging & display of req partners (14.53 KB, patch)
2019-03-18 16:34 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 22531: Add unit tests (7.06 KB, patch)
2019-03-19 14:17 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 22531: Add logging & display of req partners (14.94 KB, patch)
2021-03-26 10:47 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 22531: Add unit tests (7.64 KB, patch)
2021-03-26 10:47 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 22531: Add logging & display of req partners (14.99 KB, patch)
2021-07-07 08:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22531: Add unit tests (7.68 KB, patch)
2021-07-07 08:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22531: Add logging & display of req partners (14.99 KB, patch)
2021-09-17 11:30 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 22531: Add unit tests (7.54 KB, patch)
2021-09-17 11:30 UTC, Andrew Isherwood
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2019-03-18 14:20:08 UTC
Currently, when an ILL request is sent to partners it is not possible to send to further partners, should the first partner not be able to fulfil the request.

A better approach would be to allow the "Place request with partners" option to be selected even when this action has already taken place. This will allow the user to try multiple partners subsequently.

As such, we should log which partners a request has been sent to and this information should be displayed in the "Display request log" functionality added in Bug 20750
Comment 1 Andrew Isherwood 2019-03-18 16:34:42 UTC
Created attachment 86723 [details] [review]
Bug 22531: Add logging & display of req partners

This patch adds:

- The ability to send request to partners multiple times.
- Logging of requested partners
- Display of requested partners in "ILL Request Log" display
Comment 2 Andrew Isherwood 2019-03-19 14:17:05 UTC
Created attachment 86744 [details] [review]
Bug 22531: Add unit tests

Add tests for new and modified methods
Comment 3 Andrew Isherwood 2019-03-19 14:25:27 UTC
Test plan:

- Ensure you have at least two "partners" set up. These are patrons in the category defined in <interlibrary_loans> -> <partner_code> in koha-conf.xml, the patrons must also have a primary email address defined.
- Create an ILL request using the FreeForm backend
- From the "Manage ILL request" screen, select "Place request with partners"
- In "Select partner libraries" choose one partner library, then click "Send email"
- From the "Manage ILL request" screen, select "ILL request log"
=> TEST: Observe that the status was changed *to* "Requested from partners (first_partner_email_address_here)
- From the "Manage ILL request" screen, select "Place request with partners"
- In "Select partner libraries" choose a different partner library, then click "Send email"
- From the "Manage ILL request" screen, select "ILL request log"
=> TEST: Observe that the status was changed *from* "Requested from partners (first_partner_email_address_here)
=> TEST: Observe that the status was changed *TO* "Requested from partners (second_partner_email_address_here)
Comment 4 Andrew Isherwood 2019-03-19 14:28:18 UTC
Test plan: (ignore the plan in comment #3)

- Ensure you have at least two "partners" set up. These are patrons in the category defined in <interlibrary_loans> -> <partner_code> in koha-conf.xml, the patrons must also have a primary email address defined.
- Create an ILL request using the FreeForm backend
- From the "Manage ILL request" screen, select "Place request with partners"
- In "Select partner libraries" choose one partner library, then click "Send email"
- Go to the "Manage ILL request" screen
=> TEST: Observe that the request "Status" is "Requested from partners : first_partner_email_address_here"
=> TEST: Observe that the first_partner_email_address links to the appropriate patron screen
- From the "Manage ILL request" screen, select "ILL request log"
=> TEST: Observe that the status was changed *to* "Requested from partners (first_partner_email_address_here)
- From the "Manage ILL request" screen, select "Place request with partners"
- In "Select partner libraries" choose a different partner library, then click "Send email"
- From the "Manage ILL request" screen, select "ILL request log"
=> TEST: Observe that the status was changed *from* "Requested from partners (first_partner_email_address_here)
=> TEST: Observe that the status was changed *TO* "Requested from partners (second_partner_email_address_here)
=> TEST: Observe that the email addresses displayed link to the appropriate patron page
Comment 5 Martin Renvoize 2021-03-19 14:37:20 UTC
Hmm, I had a go at rebasing this one.. and it didn't appear too bad.. but my rebased set had all the unit tests failing.

Going to ask Andrew for a rebase.
Comment 6 Andrew Isherwood 2021-03-26 10:47:52 UTC
Created attachment 118843 [details] [review]
Bug 22531: Add logging & display of req partners

This patch adds:

- The ability to send request to partners multiple times.
- Logging of requested partners
- Display of requested partners in "ILL Request Log" display
Comment 7 Andrew Isherwood 2021-03-26 10:47:55 UTC
Created attachment 118844 [details] [review]
Bug 22531: Add unit tests

Add tests for new and modified methods
Comment 8 Andrew Isherwood 2021-03-26 10:49:20 UTC
Revised test plan:

- Ensure you have at least two "partners" set up. These are patrons in the category defined in <interlibrary_loans> -> <partner_code> in koha-conf.xml, the patrons must also have a primary email address defined.
- Ensure the ILLLOG syspref is turned on
- Create an ILL request using the FreeForm backend
- From the "Manage ILL request" screen, select "Place request with partners"
- In "Select partner libraries" choose one partner library, then click "Send email"
- Go to the "Manage ILL request" screen
=> TEST: Observe that the request "Status" is "Requested from partners : first_partner_email_address_here"
=> TEST: Observe that the first_partner_email_address links to the appropriate patron screen
- From the "Manage ILL request" screen, select "ILL request log"
=> TEST: Observe that the status was changed *to* "Requested from partners (first_partner_email_address_here)
- From the "Manage ILL request" screen, select "Place request with partners"
- In "Select partner libraries" choose a different partner library, then click "Send email"
- From the "Manage ILL request" screen, select "ILL request log"
=> TEST: Observe that the status was changed *from* "Requested from partners (first_partner_email_address_here)
=> TEST: Observe that the status was changed *TO* "Requested from partners (second_partner_email_address_here)
=> TEST: Observe that the email addresses displayed link to the appropriate patron page
Comment 9 Martin Renvoize 2021-07-07 08:40:21 UTC
Created attachment 122643 [details] [review]
Bug 22531: Add logging & display of req partners

This patch adds:

- The ability to send request to partners multiple times.
- Logging of requested partners
- Display of requested partners in "ILL Request Log" display

Signed-ff-by: Barry Cannon <bc@interleaf.ie>
Comment 10 Martin Renvoize 2021-07-07 08:40:26 UTC
Created attachment 122644 [details] [review]
Bug 22531: Add unit tests

Add tests for new and modified methods

Signed-ff-by: Barry Cannon <bc@interleaf.ie>
Comment 11 Martin Renvoize 2021-07-07 08:41:21 UTC
Signed off by Barry Cannon on behalf of the Institutes of Technology, whom he supports.  Testing was undertaken at multiple sites and is now in live use.
Comment 12 Kyle M Hall 2021-09-10 15:25:18 UTC
Applying: Bug 22531: Add logging & display of req partners
Applying: Bug 22531: Add unit tests
error: sha1 information is lacking or useless (Koha/Illrequest/Logger.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 22531: Add unit tests
Comment 13 Andrew Isherwood 2021-09-17 11:30:22 UTC
Created attachment 124979 [details] [review]
Bug 22531: Add logging & display of req partners

This patch adds:

- The ability to send request to partners multiple times.
- Logging of requested partners
- Display of requested partners in "ILL Request Log" display

Signed-ff-by: Barry Cannon <bc@interleaf.ie>
Comment 14 Andrew Isherwood 2021-09-17 11:30:26 UTC
Created attachment 124980 [details] [review]
Bug 22531: Add unit tests

Add tests for new and modified methods

Signed-off-by: Barry Cannon <bc@interleaf.ie>
Comment 15 Katrin Fischer 2021-11-01 12:12:48 UTC
I am sorry, but I am having a bit of trouble with this one:

prove t/db_dependent/Illrequests.t
t/db_dependent/Illrequests.t .. 3/13 
    #   Failed test 'new node + core_status_graph = bigger status graph'
    #   at t/db_dependent/Illrequests.t line 272.
    # Compared array length of $data->{"GENREQ"}{"next_actions"}
    #    got : array with 3 element(s)
    # expect : array with 2 element(s)
    # {
    #   'CANCREQ' => {
    #     'id' => 'CANCREQ',
    #     'method' => 0,
    #     'name' => 'Cancellation requested',
    #     'next_actions' => [
    #       'KILL',
    #       'REQ'
    #     ],
    #     'prev_actions' => [
    #       'NEW'
    #     ],
    #     'ui_method_icon' => 0,
    #     'ui_method_name' => 0
    #   },
    #   'CHK' => {
    #     'id' => 'CHK',
    #     'method' => 'check_out',
    #     'name' => 'Checked out',
    #     'needs_all' => [
    #       sub { "DUMMY" }
    #     ],
    #     'needs_perms' => [
    #       'user_circulate_circulate_remaining_permissions'
    #     ],
    #     'needs_prefs' => [
    #       'CirculateILL'
    #     ],
    #     'next_actions' => [],
    #     'prev_actions' => [
    #       'REQ',
    #       'GENREQ',
    #       'COMP'
    #     ],
    #     'ui_method_icon' => 'fa-upload',
    #     'ui_method_name' => 'Check out'
    #   },
    #   'COMP' => {
    #     'id' => 'COMP',
    #     'method' => 'mark_completed',
    #     'name' => 'Completed',
    #     'next_actions' => [
    #       'CHK'
    #     ],
    #     'prev_actions' => [
    #       'REQ'
    #     ],
    #     'ui_method_icon' => 'fa-check',
    #     'ui_method_name' => 'Mark completed'
    #   },
    #   'GENREQ' => {
    #     'id' => 'GENREQ',
    #     'method' => 'generic_confirm',
    #     'name' => 'Requested from partners',
    #     'next_actions' => [
    #       'COMP',
    #       'CHK',
    #       'GENREQ'
    #     ],
    #     'prev_actions' => [
    #       'NEW',
    #       'REQREV',
    #       'GENREQ'
    #     ],
    #     'ui_method_icon' => 'fa-send-o',
    #     'ui_method_name' => 'Place request with partners'
    #   },
    #   'KILL' => {
    #     'id' => 'KILL',
    #     'method' => 'delete',
    #     'name' => 0,
    #     'next_actions' => [],
    #     'prev_actions' => [
    #       'QUEUED',
    #       'REQREV',
    #       'NEW',
    #       'CANCREQ'
    #     ],
    #     'ui_method_icon' => 'fa-trash',
    #     'ui_method_name' => 'Delete request'
    #   },
    #   'NEW' => {
    #     'id' => 'NEW',
    #     'method' => 'create',
    #     'name' => 'New request',
    #     'next_actions' => [
    #       'REQ',
    #       'GENREQ',
    #       'KILL'
    #     ],
    #     'prev_actions' => [],
    #     'ui_method_icon' => 'fa-plus',
    #     'ui_method_name' => 'New request'
    #   },
    #   'QUEUED' => {
    #     'id' => 'QUEUED',
    #     'method' => 0,
    #     'name' => 'Queued request',
    #     'next_actions' => [
    #       'REQ',
    #       'KILL'
    #     ],
    #     'prev_actions' => [],
    #     'ui_method_icon' => 0,
    #     'ui_method_name' => 0
    #   },
    #   'REQ' => {
    #     'id' => 'REQ',
    #     'method' => 'confirm',
    #     'name' => 'Requested',
    #     'next_actions' => [
    #       'REQREV',
    #       'COMP',
    #       'CHK'
    #     ],
    #     'prev_actions' => [
    #       'NEW',
    #       'REQREV',
    #       'QUEUED',
    #       'CANCREQ'
    #     ],
    #     'ui_method_icon' => 'fa-check',
    #     'ui_method_name' => 'Confirm request'
    #   },
    #   'REQREV' => {
    #     'id' => 'REQREV',
    #     'method' => 'cancel',
    #     'name' => 'Request reverted',
    #     'next_actions' => [
    #       'REQ',
    #       'GENREQ',
    #       'KILL'
    #     ],
    #     'prev_actions' => [
    #       'REQ'
    #     ],
    #     'ui_method_icon' => 'fa-times',
    #     'ui_method_name' => 'Revert Request'
    #   },
    #   'RET' => {
    #     'id' => 'RET',
    #     'method' => 'check_in',
    #     'name' => 'Returned to library',
    #     'next_actions' => [
    #       'COMP'
    #     ],
    #     'prev_actions' => [
    #       'CHK'
    #     ],
    #     'ui_method_icon' => 'fa-download',
    #     'ui_method_name' => 'Check in'
    #   },
    #   'TEST' => {
    #     'id' => 'TEST',
    #     'next_actions' => [],
    #     'prev_actions' => []
    #   }
    # }
    # Looks like you failed 1 test of 5.
t/db_dependent/Illrequests.t .. 5/13 
#   Failed test 'Status Graph tests'
#   at t/db_dependent/Illrequests.t line 380.
t/db_dependent/Illrequests.t .. 11/13     # Looks like you planned 3 tests but ran 5.

#   Failed test 'Custom statuses'
#   at t/db_dependent/Illrequests.t line 1343.
t/db_dependent/Illrequests.t .. 13/13 # Looks like you failed 2 tests of 13.
t/db_dependent/Illrequests.t .. Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/13 subtests 

Test Summary Report
-------------------
t/db_dependent/Illrequests.t (Wstat: 512 Tests: 13 Failed: 2)
  Failed tests:  5, 12
  Non-zero exit status: 2


There are also some minor QA fails (tab characters and missing TT filters). I would have fixed those, but holding off now as the tests might need a little work anyway.