Bug 27894 - Add visual feedback on overridden pickup locations
Summary: Add visual feedback on overridden pickup locations
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 27863
Blocks: 27864 27931 27932
  Show dependency treegraph
 
Reported: 2021-03-08 11:17 UTC by Tomás Cohen Arazi
Modified: 2021-04-07 14:38 UTC (History)
3 users (show)

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


Attachments
Bug 27894: Unit tests (3.61 KB, patch)
2021-03-11 13:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27894: Adapt /holds/:hold_id/pickup_locations (2.80 KB, patch)
2021-03-11 13:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27894: Add visual feedback on overridden pickup locations (4.78 KB, patch)
2021-03-11 13:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27894: Unit tests (3.68 KB, patch)
2021-03-22 11:21 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27894: Adapt /holds/:hold_id/pickup_locations (2.86 KB, patch)
2021-03-22 11:21 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27894: Add visual feedback on overridden pickup locations (4.85 KB, patch)
2021-03-22 11:21 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27894: Unit tests (3.73 KB, patch)
2021-04-03 23:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27894: Adapt /holds/:hold_id/pickup_locations (2.91 KB, patch)
2021-04-03 23:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27894: Add visual feedback on overridden pickup locations (4.89 KB, patch)
2021-04-03 23:23 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 Tomás Cohen Arazi 2021-03-08 11:17:04 UTC
Bug 27863 highlighted that, when we are choosing pickup locations and AllowHoldsPolicyOverride is set, some visual feedback on which are 'valid pickup locations' and which are overridden would be handy for the end user.
Comment 1 Tomás Cohen Arazi 2021-03-11 13:55:18 UTC
Created attachment 118121 [details] [review]
Bug 27894: Unit tests

This patch adds tests for the changed behavior.
Comment 2 Tomás Cohen Arazi 2021-03-11 13:55:21 UTC
Created attachment 118122 [details] [review]
Bug 27894: Adapt /holds/:hold_id/pickup_locations

This patch makes the controller for the route, return all valid pickup
locations (i.e. pickup_location => 1) when AllowHoldPolicyOverride is
set to 'Allow', but also adds a calculated attribute: 'needs_override'
so the consumer knows the specific pickup location needs an override
(and thus be able to provide visual feedback on a single run).

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> FAILURE: Tests fail, the change is not implemented
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Comment 3 Tomás Cohen Arazi 2021-03-11 13:55:25 UTC
Created attachment 118123 [details] [review]
Bug 27894: Add visual feedback on overridden pickup locations

This patch takes advantage of the previous changes, and makes the
select2 dropdown display a warning sign (with a tooltip) on pickup
locations that, if chosen, it would mean a circ rule would be
overridden.

To test:
1. Have some pickup locations that are not valid for existing holds and
   AllowHoldPolicyOverride set to "Don't allow"
2. Visit a biblio with some holds
3. Choose a hold that cannot use some pickup location and use the
   dropdown
=> SUCCESS: Only valid pickup locations are retrieved
4. Set Allow HoldPolicyOverride to "Allow"
5. Refresh the page (I usually click on the Holds tab)
6 .Repeat 3
=> SUCCESS: All pickup locations in the system are displayed
=> FAIL: No visual feedback on overridden pickup locations :-(
7. Apply this patch and refresh the page
8. Repeat 3
=> SUCCESS: All pickup locations in the systema are displayed
=> SUCCESS: Visual feedback on overridden pickup locations is shown!
9. Set AllowHoldPolicyOverride to "Don't allow"
10. Repeat 3
=> SUCCESS: Only valid pickup locations are retrieved
=> SUCCESS: No overrridden warn/symbol, as expected
11. Sign off :-D
Comment 4 Kyle M Hall 2021-03-22 11:21:12 UTC
Created attachment 118583 [details] [review]
Bug 27894: Unit tests

This patch adds tests for the changed behavior.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2021-03-22 11:21:20 UTC
Created attachment 118584 [details] [review]
Bug 27894: Adapt /holds/:hold_id/pickup_locations

This patch makes the controller for the route, return all valid pickup
locations (i.e. pickup_location => 1) when AllowHoldPolicyOverride is
set to 'Allow', but also adds a calculated attribute: 'needs_override'
so the consumer knows the specific pickup location needs an override
(and thus be able to provide visual feedback on a single run).

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> FAILURE: Tests fail, the change is not implemented
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2021-03-22 11:21:23 UTC
Created attachment 118585 [details] [review]
Bug 27894: Add visual feedback on overridden pickup locations

This patch takes advantage of the previous changes, and makes the
select2 dropdown display a warning sign (with a tooltip) on pickup
locations that, if chosen, it would mean a circ rule would be
overridden.

To test:
1. Have some pickup locations that are not valid for existing holds and
   AllowHoldPolicyOverride set to "Don't allow"
2. Visit a biblio with some holds
3. Choose a hold that cannot use some pickup location and use the
   dropdown
=> SUCCESS: Only valid pickup locations are retrieved
4. Set Allow HoldPolicyOverride to "Allow"
5. Refresh the page (I usually click on the Holds tab)
6 .Repeat 3
=> SUCCESS: All pickup locations in the system are displayed
=> FAIL: No visual feedback on overridden pickup locations :-(
7. Apply this patch and refresh the page
8. Repeat 3
=> SUCCESS: All pickup locations in the systema are displayed
=> SUCCESS: Visual feedback on overridden pickup locations is shown!
9. Set AllowHoldPolicyOverride to "Don't allow"
10. Repeat 3
=> SUCCESS: Only valid pickup locations are retrieved
=> SUCCESS: No overrridden warn/symbol, as expected
11. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Katrin Fischer 2021-04-03 23:23:40 UTC
Created attachment 119167 [details] [review]
Bug 27894: Unit tests

This patch adds tests for the changed behavior.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2021-04-03 23:23:44 UTC
Created attachment 119168 [details] [review]
Bug 27894: Adapt /holds/:hold_id/pickup_locations

This patch makes the controller for the route, return all valid pickup
locations (i.e. pickup_location => 1) when AllowHoldPolicyOverride is
set to 'Allow', but also adds a calculated attribute: 'needs_override'
so the consumer knows the specific pickup location needs an override
(and thus be able to provide visual feedback on a single run).

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> FAILURE: Tests fail, the change is not implemented
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2021-04-03 23:23:47 UTC
Created attachment 119169 [details] [review]
Bug 27894: Add visual feedback on overridden pickup locations

This patch takes advantage of the previous changes, and makes the
select2 dropdown display a warning sign (with a tooltip) on pickup
locations that, if chosen, it would mean a circ rule would be
overridden.

To test:
1. Have some pickup locations that are not valid for existing holds and
   AllowHoldPolicyOverride set to "Don't allow"
2. Visit a biblio with some holds
3. Choose a hold that cannot use some pickup location and use the
   dropdown
=> SUCCESS: Only valid pickup locations are retrieved
4. Set Allow HoldPolicyOverride to "Allow"
5. Refresh the page (I usually click on the Holds tab)
6 .Repeat 3
=> SUCCESS: All pickup locations in the system are displayed
=> FAIL: No visual feedback on overridden pickup locations :-(
7. Apply this patch and refresh the page
8. Repeat 3
=> SUCCESS: All pickup locations in the systema are displayed
=> SUCCESS: Visual feedback on overridden pickup locations is shown!
9. Set AllowHoldPolicyOverride to "Don't allow"
10. Repeat 3
=> SUCCESS: Only valid pickup locations are retrieved
=> SUCCESS: No overrridden warn/symbol, as expected
11. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Jonathan Druart 2021-04-07 14:38:30 UTC
Pushed to master for 21.05, thanks to everybody involved!