Bug 20581 - Allow manual selection of custom ILL request statuses
Summary: Allow manual selection of custom ILL request statuses
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 1 vote (vote)
Assignee: Andrew Isherwood
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 20600
Blocks: 20639
  Show dependency treegraph
 
Reported: 2018-04-13 15:27 UTC by Andrew Isherwood
Modified: 2020-01-06 20:14 UTC (History)
9 users (show)

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


Attachments
Bug 20581: Modifications to database schema (3.87 KB, patch)
2018-04-17 13:54 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.74 KB, patch)
2018-04-17 13:55 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (2.63 KB, patch)
2018-04-17 13:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (7.52 KB, patch)
2018-04-17 13:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add support for status_alias to OPAC view (2.11 KB, patch)
2018-04-17 13:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.66 KB, patch)
2018-04-17 16:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (3.87 KB, patch)
2018-05-15 14:20 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: DO NOT PUSH: Updated Schema Files. (2.95 KB, patch)
2018-05-15 14:23 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.17 KB, patch)
2018-06-05 11:21 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (3.87 KB, patch)
2018-08-15 14:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.74 KB, patch)
2018-08-15 14:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (2.63 KB, patch)
2018-08-15 14:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (7.52 KB, patch)
2018-08-15 14:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add support for status_alias to OPAC view (2.11 KB, patch)
2018-08-15 14:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.66 KB, patch)
2018-08-15 14:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: DO NOT PUSH: Updated Schema Files. (2.95 KB, patch)
2018-08-15 14:56 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.17 KB, patch)
2018-08-15 14:57 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (3.87 KB, patch)
2018-09-03 13:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.74 KB, patch)
2018-09-03 13:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (2.63 KB, patch)
2018-09-03 13:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (7.66 KB, patch)
2018-09-03 13:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add support for status_alias to OPAC view (2.17 KB, patch)
2018-09-03 13:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.66 KB, patch)
2018-09-03 13:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: DO NOT PUSH: Updated Schema Files. (2.95 KB, patch)
2018-09-03 13:19 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.19 KB, patch)
2018-09-03 13:19 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.11 KB, patch)
2018-09-03 13:19 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.12 KB, patch)
2018-09-05 09:55 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (3.92 KB, patch)
2018-09-07 13:18 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.79 KB, patch)
2018-09-07 13:18 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (2.68 KB, patch)
2018-09-07 13:18 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (7.71 KB, patch)
2018-09-07 13:18 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20581: Add support for status_alias to OPAC view (2.22 KB, patch)
2018-09-07 13:18 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.71 KB, patch)
2018-09-07 13:18 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20581: DO NOT PUSH: Updated Schema Files. (3.00 KB, patch)
2018-09-07 13:18 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.24 KB, patch)
2018-09-07 13:18 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.17 KB, patch)
2018-09-07 13:18 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (3.92 KB, patch)
2018-10-09 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.79 KB, patch)
2018-10-09 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (2.68 KB, patch)
2018-10-09 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (7.72 KB, patch)
2018-10-09 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add support for status_alias to OPAC view (2.22 KB, patch)
2018-10-09 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.71 KB, patch)
2018-10-09 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.25 KB, patch)
2018-10-09 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.17 KB, patch)
2018-10-09 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify DB updgrade (3.43 KB, patch)
2018-10-12 13:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix statusalias return (1.11 KB, patch)
2018-10-12 13:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Allow for undefined lib (1.13 KB, patch)
2018-10-12 14:09 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify DB updgrade (3.88 KB, patch)
2018-10-12 14:12 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix statusalias return (1.12 KB, patch)
2018-10-12 14:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix fallback status_alias (1.24 KB, patch)
2018-10-15 13:34 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify POD (806 bytes, patch)
2018-10-15 13:34 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix AV population (2.14 KB, patch)
2018-10-15 13:49 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (3.92 KB, patch)
2018-11-08 12:01 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify DB updgrade (3.88 KB, patch)
2018-11-08 12:01 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.80 KB, patch)
2018-11-08 12:01 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (2.58 KB, patch)
2018-11-08 12:01 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (6.00 KB, patch)
2018-11-08 12:01 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.71 KB, patch)
2018-11-08 12:01 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.28 KB, patch)
2018-11-08 12:01 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.11 KB, patch)
2018-11-08 12:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Allow for undefined lib (1.17 KB, patch)
2018-11-08 12:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix statusalias return (1.12 KB, patch)
2018-11-08 12:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix fallback status_alias (1.28 KB, patch)
2018-11-08 12:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify POD (806 bytes, patch)
2018-11-08 12:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix AV population (2.17 KB, patch)
2018-11-08 12:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: DO NOT PUSH: Updated Schema Files. (84.05 KB, patch)
2018-11-08 12:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (3.92 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify DB updgrade (3.88 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.80 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (1.11 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (5.74 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.71 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.31 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.11 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Allow for undefined lib (1.17 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix statusalias return (1.12 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix fallback status_alias (1.28 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify POD (806 bytes, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix AV population (2.17 KB, patch)
2018-11-19 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: DO NOT PUSH: Updated Schema Files. (84.05 KB, patch)
2018-11-19 12:15 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (3.92 KB, patch)
2018-11-19 12:15 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify DB updgrade (3.88 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.80 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (1.11 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (26.07 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.71 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.31 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.11 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Allow for undefined lib (1.17 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix statusalias return (1.12 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix fallback status_alias (1.28 KB, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify POD (806 bytes, patch)
2018-11-19 12:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (5.74 KB, patch)
2018-11-19 12:23 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (3.92 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.80 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (1.11 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (5.74 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.11 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.71 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: DO NOT PUSH: Updated Schema Files. (84.05 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify DB updgrade (3.88 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.31 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Allow for undefined lib (1.17 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix statusalias return (1.12 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix fallback status_alias (1.28 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix AV population (2.17 KB, patch)
2018-11-19 12:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify POD (806 bytes, patch)
2018-11-19 12:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix typo in kohastructure (951 bytes, patch)
2018-11-29 12:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix QA errors (1.51 KB, patch)
2019-01-18 15:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix dependency requirement (1.75 KB, patch)
2019-01-25 11:57 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Do not use PK for AV FK (7.53 KB, patch)
2019-02-05 14:25 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (4.00 KB, patch)
2019-02-05 14:35 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.80 KB, patch)
2019-02-05 14:35 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (1.11 KB, patch)
2019-02-05 14:35 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (5.74 KB, patch)
2019-02-05 14:35 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.11 KB, patch)
2019-02-05 14:35 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.71 KB, patch)
2019-02-05 14:35 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.31 KB, patch)
2019-02-05 14:35 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Allow for undefined lib (1.17 KB, patch)
2019-02-05 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix statusalias return (1.12 KB, patch)
2019-02-05 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix fallback status_alias (1.28 KB, patch)
2019-02-05 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix AV population (2.17 KB, patch)
2019-02-05 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify POD (806 bytes, patch)
2019-02-05 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix typo in kohastructure (951 bytes, patch)
2019-02-05 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix QA errors (1.51 KB, patch)
2019-02-05 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix dependency requirement (1.75 KB, patch)
2019-02-05 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Do not use PK for AV FK (7.53 KB, patch)
2019-02-05 14:36 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix typo in DB upgrade (1.31 KB, patch)
2019-02-05 15:19 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Overload status_alias (4.17 KB, patch)
2019-02-06 12:57 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix status_alias comparison (1.44 KB, patch)
2019-02-06 14:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix unit test (894 bytes, patch)
2019-02-06 14:19 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: (follow-up) Passed value to status_alias (1.36 KB, patch)
2019-02-07 10:46 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (4.06 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.85 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (1.17 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (5.79 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.16 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.76 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.37 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Allow for undefined lib (1.22 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix statusalias return (1.17 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix fallback status_alias (1.33 KB, patch)
2019-02-09 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix AV population (2.23 KB, patch)
2019-02-09 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify POD (864 bytes, patch)
2019-02-09 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix typo in kohastructure (1009 bytes, patch)
2019-02-09 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix QA errors (1.57 KB, patch)
2019-02-09 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix dependency requirement (1.81 KB, patch)
2019-02-09 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Do not use PK for AV FK (7.58 KB, patch)
2019-02-09 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix typo in DB upgrade (1.37 KB, patch)
2019-02-09 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Overload status_alias (4.23 KB, patch)
2019-02-09 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix status_alias comparison (1.50 KB, patch)
2019-02-09 21:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix unit test (952 bytes, patch)
2019-02-09 21:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Passed value to status_alias (1.42 KB, patch)
2019-02-09 21:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix status_alias -1 bug (945 bytes, patch)
2019-02-11 11:11 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20581: Modifications to database schema (4.12 KB, patch)
2019-02-14 06:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: Provide status_alias on IllRequest (1.91 KB, patch)
2019-02-14 06:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: API provide status_alias embed (1.23 KB, patch)
2019-02-14 06:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: Add display of status_alias to staff vw (5.85 KB, patch)
2019-02-14 06:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: Fix display of status alias in OPAC (2.22 KB, patch)
2019-02-14 06:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: Unit tests for status_alias (2.82 KB, patch)
2019-02-14 06:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Display status name (2.42 KB, patch)
2019-02-14 06:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Allow for undefined lib (1.28 KB, patch)
2019-02-14 06:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix statusalias return (1.23 KB, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix fallback status_alias (1.39 KB, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix AV population (2.28 KB, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Modify POD (922 bytes, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix typo in kohastructure (1.04 KB, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix QA errors (1.63 KB, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix dependency requirement (1.86 KB, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Do not use PK for AV FK (7.64 KB, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix typo in DB upgrade (1.43 KB, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Overload status_alias (4.29 KB, patch)
2019-02-14 06:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix status_alias comparison (1.55 KB, patch)
2019-02-14 06:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix unit test (1010 bytes, patch)
2019-02-14 06:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Passed value to status_alias (1.48 KB, patch)
2019-02-14 06:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20581: (follow-up) Fix status_alias -1 bug (1003 bytes, patch)
2019-02-14 06:45 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 Andrew Isherwood 2018-04-13 15:27:12 UTC
Currently the displayed status of any given ILL request is strictly prescribed by where it is in the workflow, as defined in the ILL module.

While this gives an accurate indication of the current status of a request, it does not allow for any local variation in the displayed status, which can be limiting for the end user.

This bug proposes adding the ability for the admin, via a syspref, to define a custom set of request statuses, which can be selected for display by the manager of an ILL request. These statuses work alongside the existing "workflow" status that an ILL request has and will, if selected, override *for display* the status of that request.

This proposal will allow the ILL module to continue to strictly prescribe and manage the workflow of requests, but will also allow the end user some flexibility in the display of a given request's status
Comment 1 Andrew Isherwood 2018-04-17 13:50:20 UTC
Work on this bug is being submitted as distinct patches rather than one large patch. Each patch is related to a specific area of work, e.g. database schema changes, API changes, ILLrequest object changes, staff template changes & OPAC changes.

Although these patches are distinct, they are all required for the end result to work. As such, I am adding QA test scenarios here rather than attaching them to the individual patches.

Test custom status creation:
1) Apply this bug's patches
2) Apply database updates
3) Ensure you have at least one ILL backend available:
   https://wiki.koha-community.org/wiki/ILL_backends
4) Ensure you have the "ILLModule" preference enabled
5) Log in as a user with the ability to add authorised values.
6) Depending on whether you opted to accept the "Default ILL request status authorised value category name" data population during install, you may or may not have an authorised values category ILLSTATUS, if not, create it.
7) Add some authorised values to the ILLSTATUS category, these can be custom request statuses of your choice

Test custom status usage:
8) From the staff interface, create an ILL request from a backend of your choice
9) On the resulting "Request details" page, observe that the request's status is "New request"
10) Click the "Edit request" button
11) In the "Request details" pane, observe that the request's status is now available in a drop down box, it should contain the current status PLUS your custom statuses.
12) Select a custom status and Submit the form
13) In the resulting "View ILL requests" page, observe that your request is now displaying your custom status.
14) Click the "Manage request" button for your new request
15) In the "Request details" page, observe that the request's status is displaying as your custom status

Test custom status is overridden when request progresses through workflow:
16) Click "Confirm request", then the "Confirm request" confirmation
17) Observe that in the resulting "Request details" page, the request's status is now no longer displaying your custom status but has switched to "Requested"

Test custom status is removed when authorised value is removed:
18) Observe that the request has the status "Requested", we are going to override that with a custom status, then remove that custom status authorised value
19) Click "Edit request"
20) Select a custom status and Submit the form
21) In the resulting "View ILL requests" page, observe that your request is now displaying your custom status.
22) Go to the authorised values management page
23) Delete the custom status you previously selected in step 19
24) Return to the list of ILL requests
25) Observe that the request that previously had the custom status now has it's former status of "Requested"

Test API response for View ILL requests view:
26) Modify an existing request and set a custom status as discussed above
27) Navigate to the "View ILL requests" page
28) Examine the response to the API call made to the /api/v1/illrequests?embed=metadata,patron,capabilities,library,status_alias route
28) Observe that for requests with a custom status, the status_alias property contains an object for the request's authorised value containing the custom status
29) Observe that for requests without a custom status, status_alias is null

Test OPAC view:
30) Modify an existing request and set a custom status as discussed above
31) In the OPAC, navigate to "your interlibrary loan requests"
32) In the "Interlibrary loan requests" view, observe that the request's status is displaying your custom value
33) Click "View" on the request with a custom value
34) In the "View Interlibrary loan request" view, observe that the request's status is displaying your custom value
Comment 2 Andrew Isherwood 2018-04-17 13:54:04 UTC
Created attachment 74323 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.
Comment 3 Andrew Isherwood 2018-04-17 13:55:30 UTC
Created attachment 74324 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set
Comment 4 Andrew Isherwood 2018-04-17 13:56:04 UTC
Created attachment 74325 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view
Comment 5 Andrew Isherwood 2018-04-17 13:56:31 UTC
Created attachment 74326 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.
Comment 6 Andrew Isherwood 2018-04-17 13:56:54 UTC
Created attachment 74327 [details] [review]
Bug 20581: Add support for status_alias to OPAC view

- This patch adds the abililty for the OPAC to display a set status_alias
value in preference to a request's status value. During ILL request
listing or individual ILL request view, the status_alias value, if set,
will be displayed in favour of the status
Comment 7 Andrew Isherwood 2018-04-17 16:06:08 UTC
Created attachment 74362 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t
Comment 8 Mark Tompsett 2018-04-18 01:31:34 UTC
Let's say I install multiple languages.
If the Authorized Value is not keyed partially on language, then how well will this work in a multilingual setting where one user might use one language, and another one might use some other language.
Comment 9 Andrew Isherwood 2018-04-18 09:02:12 UTC
Hi Mark. That's a very good point. My knowledge of how to add multi language support to authorised values is, unfortunately, non existent. Are you aware of any "good practice" examples for this kind of thing?
Comment 10 Katrin Fischer 2018-04-18 11:10:03 UTC
(In reply to Andrew Isherwood from comment #9)
> Hi Mark. That's a very good point. My knowledge of how to add multi language
> support to authorised values is, unfortunately, non existent. Are you aware
> of any "good practice" examples for this kind of thing?

Hi Andrew,
at the moment it's not possible to have multi-language authorised values. It's kind of an ongoing discussion if we should add more of them. But I think the long term solution woudl be to copy what we do for itemtype descriptions (there is a bug for that, but not spotting it right now). 

So the question is, if maybe there is a better way to achieve what you want to do. 
Could it be an option to have default values that are translatable via templates?
A mix of custom values and and default values?

I am not per se against adding authorised values - it's a problem we need to fix globally at some point. Even better of course if someone would be willing to have a look.
Comment 11 Andrew Isherwood 2018-04-18 13:02:18 UTC
Hi Katrin

Thanks for the suggestions.

I'm wary about cooking up a home baked way of doing it as, at the moment, I'm using a well established method of providing an authorised list of values, which is exactly what the custom statuses need. I think I'd rather keep to a correct "Koha" way of providing this functionality than try to work around the lack of translations with a bespoke solution.

It's tricky when in a transitional phase with anything, but I think having devs coming up with bespoke solutions will likely cause more pain down the line when a standard way of doing it is established and it's necessary to migrate to it.

So I think, for now, it feels more "correct" to use authorised values and accept that they cannot be translated.

Cheers
Andrew
Comment 12 Andrew Isherwood 2018-05-15 14:20:12 UTC
Created attachment 75333 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.
Comment 13 Andrew Isherwood 2018-05-15 14:22:03 UTC
Added an updated patch "Bug 20581: Modifications to database schema" which fixes the filename of the atomicupdate file from .sql to .perl
Comment 14 Andrew Isherwood 2018-05-15 14:23:06 UTC
Created attachment 75334 [details] [review]
Bug 20581: DO NOT PUSH: Updated Schema Files.
Comment 15 Andrew Isherwood 2018-06-05 11:21:07 UTC
Created attachment 75810 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name
Comment 16 Andrew Isherwood 2018-08-15 14:56:24 UTC
Created attachment 77819 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.
Comment 17 Andrew Isherwood 2018-08-15 14:56:33 UTC
Created attachment 77820 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set
Comment 18 Andrew Isherwood 2018-08-15 14:56:39 UTC
Created attachment 77821 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view
Comment 19 Andrew Isherwood 2018-08-15 14:56:44 UTC
Created attachment 77822 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.
Comment 20 Andrew Isherwood 2018-08-15 14:56:49 UTC
Created attachment 77823 [details] [review]
Bug 20581: Add support for status_alias to OPAC view

- This patch adds the abililty for the OPAC to display a set status_alias
value in preference to a request's status value. During ILL request
listing or individual ILL request view, the status_alias value, if set,
will be displayed in favour of the status
Comment 21 Andrew Isherwood 2018-08-15 14:56:54 UTC
Created attachment 77824 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t
Comment 22 Andrew Isherwood 2018-08-15 14:56:59 UTC
Created attachment 77825 [details] [review]
Bug 20581: DO NOT PUSH: Updated Schema Files.
Comment 23 Andrew Isherwood 2018-08-15 14:57:05 UTC
Created attachment 77826 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name
Comment 24 Andrew Isherwood 2018-08-15 14:57:34 UTC
Rebased against current master
Comment 25 Andrew Isherwood 2018-09-03 13:18:27 UTC
Created attachment 78376 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.
Comment 26 Andrew Isherwood 2018-09-03 13:18:33 UTC
Created attachment 78377 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set
Comment 27 Andrew Isherwood 2018-09-03 13:18:39 UTC
Created attachment 78378 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view
Comment 28 Andrew Isherwood 2018-09-03 13:18:44 UTC
Created attachment 78379 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.
Comment 29 Andrew Isherwood 2018-09-03 13:18:49 UTC
Created attachment 78380 [details] [review]
Bug 20581: Add support for status_alias to OPAC view

- This patch adds the abililty for the OPAC to display a set status_alias
value in preference to a request's status value. During ILL request
listing or individual ILL request view, the status_alias value, if set,
will be displayed in favour of the status
Comment 30 Andrew Isherwood 2018-09-03 13:18:54 UTC
Created attachment 78381 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t
Comment 31 Andrew Isherwood 2018-09-03 13:19:00 UTC
Created attachment 78382 [details] [review]
Bug 20581: DO NOT PUSH: Updated Schema Files.
Comment 32 Andrew Isherwood 2018-09-03 13:19:07 UTC
Created attachment 78383 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name
Comment 33 Andrew Isherwood 2018-09-03 13:19:12 UTC
Created attachment 78384 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.
Comment 34 Andrew Isherwood 2018-09-03 13:20:46 UTC
Rebased on current master and provided an additional fix.

Test custom status creation:
1) Apply this bug's patches
2) Apply database updates
3) Ensure you have at least one ILL backend available:
   https://wiki.koha-community.org/wiki/ILL_backends
4) Ensure you have the "ILLModule" preference enabled
5) Log in as a user with the ability to add authorised values.
6) Depending on whether you opted to accept the "Default ILL request status authorised value category name" data population during install, you may or may not have an authorised values category ILLSTATUS, if not, create it.
7) Add some authorised values to the ILLSTATUS category, these can be custom request statuses of your choice

Test custom status usage:
8) From the staff interface, create an ILL request from a backend of your choice
9) On the resulting "Request details" page, observe that the request's status is "New request"
10) Click the "Edit request" button
11) In the "Request details" pane, observe that the request's status is now available in a drop down box, it should contain the current status PLUS your custom statuses.
12) Select a custom status and Submit the form
13) In the resulting "View ILL requests" page, observe that your request is now displaying your custom status.
14) Click the "Manage request" button for your new request
15) In the "Request details" page, observe that the request's status is displaying as your custom status

Test custom status is overridden when request progresses through workflow:
16) Click "Confirm request", then the "Confirm request" confirmation
17) Observe that in the resulting "Request details" page, the request's status is now no longer displaying your custom status but has switched to "Requested"

Test custom status is removed when authorised value is removed:
18) Observe that the request has the status "Requested", we are going to override that with a custom status, then remove that custom status authorised value
19) Click "Edit request"
20) Select a custom status and Submit the form
21) In the resulting "View ILL requests" page, observe that your request is now displaying your custom status.
22) Go to the authorised values management page
23) Delete the custom status you previously selected in step 19
24) Return to the list of ILL requests
25) Observe that the request that previously had the custom status now has it's former status of "Requested"

Test API response for View ILL requests view:
26) Modify an existing request and set a custom status as discussed above
27) Navigate to the "View ILL requests" page
28) Examine the response to the API call made to the /api/v1/illrequests?embed=metadata,patron,capabilities,library,status_alias route
28) Observe that for requests with a custom status, the status_alias property contains an object for the request's authorised value containing the custom status
29) Observe that for requests without a custom status, status_alias is null

Test OPAC view:
30) Modify an existing request and set a custom status as discussed above
31) In the OPAC, navigate to "your interlibrary loan requests"
32) In the "Interlibrary loan requests" view, observe that the request's status is displaying your custom value
33) Click "View" on the request with a custom value
34) In the "View Interlibrary loan request" view, observe that the request's status is displaying your custom value
Comment 35 Andrew Isherwood 2018-09-03 13:21:11 UTC
Rebased on current master and provided an additional fix.

Test custom status creation:
1) Apply this bug's patches
2) Apply database updates
3) Ensure you have at least one ILL backend available:
   https://wiki.koha-community.org/wiki/ILL_backends
4) Ensure you have the "ILLModule" preference enabled
5) Log in as a user with the ability to add authorised values.
6) Depending on whether you opted to accept the "Default ILL request status authorised value category name" data population during install, you may or may not have an authorised values category ILLSTATUS, if not, create it.
7) Add some authorised values to the ILLSTATUS category, these can be custom request statuses of your choice

Test custom status usage:
8) From the staff interface, create an ILL request from a backend of your choice
9) On the resulting "Request details" page, observe that the request's status is "New request"
10) Click the "Edit request" button
11) In the "Request details" pane, observe that the request's status is now available in a drop down box, it should contain the current status PLUS your custom statuses.
12) Select a custom status and Submit the form
13) In the resulting "View ILL requests" page, observe that your request is now displaying your custom status.
14) Click the "Manage request" button for your new request
15) In the "Request details" page, observe that the request's status is displaying as your custom status

Test custom status is overridden when request progresses through workflow:
16) Click "Confirm request", then the "Confirm request" confirmation
17) Observe that in the resulting "Request details" page, the request's status is now no longer displaying your custom status but has switched to "Requested"

Test custom status is removed when authorised value is removed:
18) Observe that the request has the status "Requested", we are going to override that with a custom status, then remove that custom status authorised value
19) Click "Edit request"
20) Select a custom status and Submit the form
21) In the resulting "View ILL requests" page, observe that your request is now displaying your custom status.
22) Go to the authorised values management page
23) Delete the custom status you previously selected in step 19
24) Return to the list of ILL requests
25) Observe that the request that previously had the custom status now has it's former status of "Requested"

Test API response for View ILL requests view:
26) Modify an existing request and set a custom status as discussed above
27) Navigate to the "View ILL requests" page
28) Examine the response to the API call made to the /api/v1/illrequests?embed=metadata,patron,capabilities,library,status_alias route
28) Observe that for requests with a custom status, the status_alias property contains an object for the request's authorised value containing the custom status
29) Observe that for requests without a custom status, status_alias is null

Test OPAC view:
30) Modify an existing request and set a custom status as discussed above
31) In the OPAC, navigate to "your interlibrary loan requests"
32) In the "Interlibrary loan requests" view, observe that the request's status is displaying your custom value
33) Click "View" on the request with a custom value
34) In the "View Interlibrary loan request" view, observe that the request's status is displaying your custom value
Comment 36 Andrew Isherwood 2018-09-05 09:55:15 UTC
Created attachment 78431 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.
Comment 37 PTFS Europe Sandboxes 2018-09-07 13:18:07 UTC
Created attachment 78515 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 38 PTFS Europe Sandboxes 2018-09-07 13:18:11 UTC
Created attachment 78516 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 39 PTFS Europe Sandboxes 2018-09-07 13:18:15 UTC
Created attachment 78517 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 40 PTFS Europe Sandboxes 2018-09-07 13:18:19 UTC
Created attachment 78518 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 41 PTFS Europe Sandboxes 2018-09-07 13:18:23 UTC
Created attachment 78519 [details] [review]
Bug 20581: Add support for status_alias to OPAC view

- This patch adds the abililty for the OPAC to display a set status_alias
value in preference to a request's status value. During ILL request
listing or individual ILL request view, the status_alias value, if set,
will be displayed in favour of the status

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 42 PTFS Europe Sandboxes 2018-09-07 13:18:27 UTC
Created attachment 78520 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 43 PTFS Europe Sandboxes 2018-09-07 13:18:31 UTC
Created attachment 78521 [details] [review]
Bug 20581: DO NOT PUSH: Updated Schema Files.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 44 PTFS Europe Sandboxes 2018-09-07 13:18:35 UTC
Created attachment 78522 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 45 PTFS Europe Sandboxes 2018-09-07 13:18:39 UTC
Created attachment 78523 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 46 Josef Moravec 2018-10-09 14:15:49 UTC
Could you rebase please?
Comment 47 Andrew Isherwood 2018-10-09 14:36:25 UTC
Created attachment 80279 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 48 Andrew Isherwood 2018-10-09 14:36:30 UTC
Created attachment 80280 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 49 Andrew Isherwood 2018-10-09 14:36:34 UTC
Created attachment 80281 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 50 Andrew Isherwood 2018-10-09 14:36:39 UTC
Created attachment 80282 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 51 Andrew Isherwood 2018-10-09 14:36:44 UTC
Created attachment 80283 [details] [review]
Bug 20581: Add support for status_alias to OPAC view

- This patch adds the abililty for the OPAC to display a set status_alias
value in preference to a request's status value. During ILL request
listing or individual ILL request view, the status_alias value, if set,
will be displayed in favour of the status

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 52 Andrew Isherwood 2018-10-09 14:36:48 UTC
Created attachment 80284 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 53 Andrew Isherwood 2018-10-09 14:36:52 UTC
Created attachment 80285 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 54 Andrew Isherwood 2018-10-09 14:36:57 UTC
Created attachment 80286 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 55 Andrew Isherwood 2018-10-09 14:37:24 UTC
Done, thanks Josef!
Comment 56 Josef Moravec 2018-10-12 10:41:44 UTC
Comment on attachment 80279 [details] [review]
Bug 20581: Modifications to database schema

Review of attachment 80279 [details] [review]:
-----------------------------------------------------------------

I would not add the file ill_auth_cat.sql, but as you add the category on database update, it makes more sense to add it to mandatory/auth_val_cat.sql

::: installer/data/mysql/atomicupdate/bug_20581-add_new_illrequests_status_alias_column.perl
@@ +1,4 @@
> +$DBversion = 'XXX';  # will be replaced by the RM
> +if( CheckVersion( $DBversion ) ) {
> +
> +    $dbh->do( "ALTER TABLE illrequests ADD COLUMN status_alias integer DEFAULT NULL" );

Please add a test on column existance and add AFTER statement to be sure the column order is same on all installations

@@ +1,5 @@
> +$DBversion = 'XXX';  # will be replaced by the RM
> +if( CheckVersion( $DBversion ) ) {
> +
> +    $dbh->do( "ALTER TABLE illrequests ADD COLUMN status_alias integer DEFAULT NULL" );
> +    $dbh->do( "ALTER TABLE illrequests ADD CONSTRAINT illrequests_safk FOREIGN KEY (status_alias) REFERENCES authorised_values(id) ON DELETE SET NULL" );

Please add ON UPDATE statement to exactly say what will happen

::: installer/data/mysql/kohastructure.sql
@@ +4157,5 @@
>        ON UPDATE CASCADE ON DELETE CASCADE
> +    CONSTRAINT `illrequests_safk`
> +      FOREIGN KEY (`status_alias`)
> +      REFERENCES `authorised_values` (`id`)
> +      ON DELETE SET NULL

Add ON UPDATE statement
Comment 57 Josef Moravec 2018-10-12 11:00:31 UTC
Comment on attachment 80280 [details] [review]
Bug 20581: Provide status_alias on IllRequest

Review of attachment 80280 [details] [review]:
-----------------------------------------------------------------

::: Koha/Illrequest.pm
@@ +118,5 @@
> +    return $self->status_alias ?
> +        Koha::AuthorisedValue->_new_from_dbic(
> +            scalar $self->_result->status_alias
> +        ) :
> +        undef;

You shouldn't return explicit undef. And please add to pod information about return type.
Comment 58 Josef Moravec 2018-10-12 11:15:43 UTC
Comment on attachment 80282 [details] [review]
Bug 20581: Add display of status_alias to staff vw

Review of attachment 80282 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
@@ +448,5 @@
> +                                            <option value="[% alias.id %]">
> +                                            [% END %]
> +                                                [% alias.authorised_value | html %]
> +                                            </option>
> +                                        [% END %]

You could use av-build-dropbox.inc for making select from AV, then you don't even need the status_aliases param
Comment 59 Josef Moravec 2018-10-12 11:22:21 UTC
Comment on attachment 80285 [details] [review]
Bug 20581: (follow-up) Display status name

Review of attachment 80285 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
@@ +129,4 @@
>          // Render function for request status
>          var createStatus = function(data, type, row, meta) {
>              if (row.status_alias) {
> +                return row.status_alias.lib;

What if lib is not defined?
Comment 60 Josef Moravec 2018-10-12 12:16:17 UTC
Overall it is looking OK, just few relatively small issues I found (see above comments)

Great work.
Comment 61 Andrew Isherwood 2018-10-12 13:32:55 UTC
Created attachment 80474 [details] [review]
Bug 20581: (follow-up) Modify DB updgrade

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c56
Comment 62 Andrew Isherwood 2018-10-12 13:33:00 UTC
Created attachment 80475 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c57
Comment 63 Andrew Isherwood 2018-10-12 13:56:22 UTC
(In reply to Josef Moravec from comment #58)
> Comment on attachment 80282 [details] [review] [review]
> Bug 20581: Add display of status_alias to staff vw
> 
> Review of attachment 80282 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
> @@ +448,5 @@
> > +                                            <option value="[% alias.id %]">
> > +                                            [% END %]
> > +                                                [% alias.authorised_value | html %]
> > +                                            </option>
> > +                                        [% END %]
> 
> You could use av-build-dropbox.inc for making select from AV, then you don't
> even need the status_aliases param

Thanks for the tip about av-build-dropbox.inc Josef. Unfortunately, in this case I don't think I can make use of it since we don't need to just display a list of AV values. We need to display a list of AV values plus a possible additional value. It looks like av-build-dropbox.inc generates the entire <select> element, rather than just the AV <option> elements, so I don't think I can make use of it on this occassion. Well worth knowing about though, thanks!
Comment 64 Andrew Isherwood 2018-10-12 14:09:35 UTC
Created attachment 80501 [details] [review]
Bug 20581: (follow-up) Allow for undefined lib

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c59
Comment 65 Josef Moravec 2018-10-12 14:09:57 UTC
Comment on attachment 80475 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

Review of attachment 80475 [details] [review]:
-----------------------------------------------------------------

::: Koha/Illrequest.pm
@@ +125,5 @@
>              scalar $self->_result->status_alias
> +        );
> +    } else {
> +        return;
> +    }

This could be written even simpler:

return unless $self->status_alias
return Koha::AuthorisedValue->_new_from_dbic(
    scalar $self->_result->status_alias
);

But no blocker
Comment 66 Andrew Isherwood 2018-10-12 14:12:37 UTC
Created attachment 80503 [details] [review]
Bug 20581: (follow-up) Modify DB updgrade

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c56
Comment 67 Andrew Isherwood 2018-10-12 14:16:21 UTC
Created attachment 80505 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c57
Comment 68 Josef Moravec 2018-10-12 14:41:43 UTC
Comment on attachment 80501 [details] [review]
Bug 20581: (follow-up) Allow for undefined lib

Review of attachment 80501 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
@@ +129,4 @@
>          // Render function for request status
>          var createStatus = function(data, type, row, meta) {
>              if (row.status_alias) {
> +                return row.status_alias.lib ? row.status_alias.lib : 'N/A';

I would use the code instead of 'N/A'
Comment 69 Josef Moravec 2018-10-12 14:43:00 UTC
Comment on attachment 80505 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

Review of attachment 80505 [details] [review]:
-----------------------------------------------------------------

::: Koha/Illrequest.pm
@@ +113,5 @@
>  
> +    my $statusalias = $request->statusalias;
> +
> +Return a request's status alias, if one is defined, otherwise
> +return implicit undef

Please add the information that status alias is of type Koha::AuthorisedValue
Comment 70 Josef Moravec 2018-10-12 15:14:28 UTC
One more think on authorised values: generaly it is better to take them in from AuthorisedValues template plugin
Comment 71 Andrew Isherwood 2018-10-15 13:34:35 UTC
Created attachment 80598 [details] [review]
Bug 20581: (follow-up) Fix fallback status_alias

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c68
Comment 72 Andrew Isherwood 2018-10-15 13:34:40 UTC
Created attachment 80599 [details] [review]
Bug 20581: (follow-up) Modify POD

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c69
Comment 73 Andrew Isherwood 2018-10-15 13:49:53 UTC
Created attachment 80600 [details] [review]
Bug 20581: (follow-up) Fix AV population

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c72
Comment 74 Andrew Isherwood 2018-11-08 12:01:29 UTC
Created attachment 82063 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 75 Andrew Isherwood 2018-11-08 12:01:34 UTC
Created attachment 82064 [details] [review]
Bug 20581: (follow-up) Modify DB updgrade

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c56
Comment 76 Andrew Isherwood 2018-11-08 12:01:39 UTC
Created attachment 82065 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 77 Andrew Isherwood 2018-11-08 12:01:44 UTC
Created attachment 82066 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 78 Andrew Isherwood 2018-11-08 12:01:48 UTC
Created attachment 82067 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 79 Andrew Isherwood 2018-11-08 12:01:53 UTC
Created attachment 82068 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 80 Andrew Isherwood 2018-11-08 12:01:57 UTC
Created attachment 82069 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 81 Andrew Isherwood 2018-11-08 12:02:03 UTC
Created attachment 82070 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 82 Andrew Isherwood 2018-11-08 12:02:08 UTC
Created attachment 82071 [details] [review]
Bug 20581: (follow-up) Allow for undefined lib

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c59
Comment 83 Andrew Isherwood 2018-11-08 12:02:14 UTC
Created attachment 82072 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c57
Comment 84 Andrew Isherwood 2018-11-08 12:02:19 UTC
Created attachment 82073 [details] [review]
Bug 20581: (follow-up) Fix fallback status_alias

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c68
Comment 85 Andrew Isherwood 2018-11-08 12:02:23 UTC
Created attachment 82074 [details] [review]
Bug 20581: (follow-up) Modify POD

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c69
Comment 86 Andrew Isherwood 2018-11-08 12:02:29 UTC
Created attachment 82075 [details] [review]
Bug 20581: (follow-up) Fix AV population

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c72
Comment 87 Andrew Isherwood 2018-11-08 12:02:34 UTC
Created attachment 82076 [details] [review]
Bug 20581: DO NOT PUSH: Updated Schema Files.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 88 Andrew Isherwood 2018-11-08 12:13:03 UTC
Rebased on top of master then the current dependency tree
Comment 89 Andrew Isherwood 2018-11-19 12:06:02 UTC
Created attachment 82443 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 90 Andrew Isherwood 2018-11-19 12:06:07 UTC
Created attachment 82444 [details] [review]
Bug 20581: (follow-up) Modify DB updgrade

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c56
Comment 91 Andrew Isherwood 2018-11-19 12:06:11 UTC
Created attachment 82445 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 92 Andrew Isherwood 2018-11-19 12:06:15 UTC
Created attachment 82446 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 93 Andrew Isherwood 2018-11-19 12:06:20 UTC
Created attachment 82447 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 94 Andrew Isherwood 2018-11-19 12:06:25 UTC
Created attachment 82448 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 95 Andrew Isherwood 2018-11-19 12:06:30 UTC
Created attachment 82449 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 96 Andrew Isherwood 2018-11-19 12:06:35 UTC
Created attachment 82450 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 97 Andrew Isherwood 2018-11-19 12:06:39 UTC
Created attachment 82451 [details] [review]
Bug 20581: (follow-up) Allow for undefined lib

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c59
Comment 98 Andrew Isherwood 2018-11-19 12:06:44 UTC
Created attachment 82452 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c57
Comment 99 Andrew Isherwood 2018-11-19 12:06:48 UTC
Created attachment 82453 [details] [review]
Bug 20581: (follow-up) Fix fallback status_alias

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c68
Comment 100 Andrew Isherwood 2018-11-19 12:06:53 UTC
Created attachment 82454 [details] [review]
Bug 20581: (follow-up) Modify POD

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c69
Comment 101 Andrew Isherwood 2018-11-19 12:06:57 UTC
Created attachment 82455 [details] [review]
Bug 20581: (follow-up) Fix AV population

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c72
Comment 102 Andrew Isherwood 2018-11-19 12:15:52 UTC
Created attachment 82456 [details] [review]
Bug 20581: DO NOT PUSH: Updated Schema Files.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 103 Andrew Isherwood 2018-11-19 12:15:57 UTC
Created attachment 82457 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 104 Andrew Isherwood 2018-11-19 12:16:01 UTC
Created attachment 82458 [details] [review]
Bug 20581: (follow-up) Modify DB updgrade

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c56
Comment 105 Andrew Isherwood 2018-11-19 12:16:06 UTC
Created attachment 82459 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 106 Andrew Isherwood 2018-11-19 12:16:11 UTC
Created attachment 82460 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 107 Andrew Isherwood 2018-11-19 12:16:15 UTC
Created attachment 82461 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 108 Andrew Isherwood 2018-11-19 12:16:20 UTC
Created attachment 82462 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 109 Andrew Isherwood 2018-11-19 12:16:25 UTC
Created attachment 82463 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 110 Andrew Isherwood 2018-11-19 12:16:29 UTC
Created attachment 82464 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 111 Andrew Isherwood 2018-11-19 12:16:34 UTC
Created attachment 82465 [details] [review]
Bug 20581: (follow-up) Allow for undefined lib

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c59
Comment 112 Andrew Isherwood 2018-11-19 12:16:39 UTC
Created attachment 82466 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c57
Comment 113 Andrew Isherwood 2018-11-19 12:16:44 UTC
Created attachment 82467 [details] [review]
Bug 20581: (follow-up) Fix fallback status_alias

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c68
Comment 114 Andrew Isherwood 2018-11-19 12:16:48 UTC
Created attachment 82468 [details] [review]
Bug 20581: (follow-up) Modify POD

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c69
Comment 115 Andrew Isherwood 2018-11-19 12:23:12 UTC
Created attachment 82472 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 116 Andrew Isherwood 2018-11-19 12:32:01 UTC
Created attachment 82483 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 117 Andrew Isherwood 2018-11-19 12:32:06 UTC
Created attachment 82484 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 118 Andrew Isherwood 2018-11-19 12:32:11 UTC
Created attachment 82485 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 119 Andrew Isherwood 2018-11-19 12:32:16 UTC
Created attachment 82486 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 120 Andrew Isherwood 2018-11-19 12:32:20 UTC
Created attachment 82487 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 121 Andrew Isherwood 2018-11-19 12:32:25 UTC
Created attachment 82488 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 122 Andrew Isherwood 2018-11-19 12:32:30 UTC
Created attachment 82489 [details] [review]
Bug 20581: DO NOT PUSH: Updated Schema Files.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 123 Andrew Isherwood 2018-11-19 12:32:35 UTC
Created attachment 82490 [details] [review]
Bug 20581: (follow-up) Modify DB updgrade

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c56
Comment 124 Andrew Isherwood 2018-11-19 12:32:40 UTC
Created attachment 82491 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 125 Andrew Isherwood 2018-11-19 12:32:45 UTC
Created attachment 82492 [details] [review]
Bug 20581: (follow-up) Allow for undefined lib

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c59
Comment 126 Andrew Isherwood 2018-11-19 12:32:50 UTC
Created attachment 82493 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c57
Comment 127 Andrew Isherwood 2018-11-19 12:32:54 UTC
Created attachment 82494 [details] [review]
Bug 20581: (follow-up) Fix fallback status_alias

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c68
Comment 128 Andrew Isherwood 2018-11-19 12:32:59 UTC
Created attachment 82495 [details] [review]
Bug 20581: (follow-up) Fix AV population

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c72
Comment 129 Andrew Isherwood 2018-11-19 12:33:04 UTC
Created attachment 82496 [details] [review]
Bug 20581: (follow-up) Modify POD

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c69
Comment 130 Andrew Isherwood 2018-11-29 12:06:41 UTC
Created attachment 82733 [details] [review]
Bug 20581: (follow-up) Fix typo in kohastructure

Fix the missing comma
Comment 131 Andrew Isherwood 2019-01-18 15:32:23 UTC
Created attachment 84202 [details] [review]
Bug 20581: (follow-up) Fix QA errors

This patch fixes the "missing_filter" errors on ill-requests.tt
Comment 132 Andrew Isherwood 2019-01-25 11:57:30 UTC
Created attachment 84404 [details] [review]
Bug 20581: (follow-up) Fix dependency requirement

Since this bug is now dependent on Bug 20600 (Provide the ability for
users to filter ILL requests) we need to make the status filtering aware
of the custom statuses. This patch does that.
Comment 133 Katrin Fischer 2019-02-04 19:37:52 UTC
I have an issue here with the data structures: In Koha we ususally store the code (authorised_value) for referencing, this patch uses the authorised_values.id which is the numerical PK of the table. I can see where this is coming from, but as it would be the only place in Koha working like that. It will make it impossible to use our TT plugins for dealing with the descriptions etc. 

From the QA script:

Maybe we could improve that with a bit of squashing:

 FAIL	installer/data/mysql/en/optional/ill_auth_cat.sql
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset

 FAIL	installer/data/mysql/en/optional/ill_auth_cat.txt
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset
Comment 134 Andrew Isherwood 2019-02-05 14:25:02 UTC
Created attachment 84768 [details] [review]
Bug 20581: (follow-up) Do not use PK for AV FK

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c133

We switch from using authorised_values.id when creating the foreign key
illrequests.status_alias, we now use authorised_values.authorised_value

I have added a migration from using id -> authorised_value, so when
existing users of this bug get this version, their DB migrates nicely
instead of just breaking.
Comment 135 Andrew Isherwood 2019-02-05 14:35:25 UTC
Created attachment 84770 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.

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

Following commit was squashed into this one:

Bug 20581: (follow-up) Modify DB updgrade

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c56
Comment 136 Andrew Isherwood 2019-02-05 14:35:30 UTC
Created attachment 84771 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 137 Andrew Isherwood 2019-02-05 14:35:35 UTC
Created attachment 84772 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 138 Andrew Isherwood 2019-02-05 14:35:40 UTC
Created attachment 84773 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 139 Andrew Isherwood 2019-02-05 14:35:45 UTC
Created attachment 84774 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 140 Andrew Isherwood 2019-02-05 14:35:50 UTC
Created attachment 84775 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 141 Andrew Isherwood 2019-02-05 14:35:55 UTC
Created attachment 84776 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name

Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Comment 142 Andrew Isherwood 2019-02-05 14:36:00 UTC
Created attachment 84777 [details] [review]
Bug 20581: (follow-up) Allow for undefined lib

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c59
Comment 143 Andrew Isherwood 2019-02-05 14:36:05 UTC
Created attachment 84778 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c57
Comment 144 Andrew Isherwood 2019-02-05 14:36:10 UTC
Created attachment 84779 [details] [review]
Bug 20581: (follow-up) Fix fallback status_alias

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c68
Comment 145 Andrew Isherwood 2019-02-05 14:36:15 UTC
Created attachment 84780 [details] [review]
Bug 20581: (follow-up) Fix AV population

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c72
Comment 146 Andrew Isherwood 2019-02-05 14:36:20 UTC
Created attachment 84781 [details] [review]
Bug 20581: (follow-up) Modify POD

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c69
Comment 147 Andrew Isherwood 2019-02-05 14:36:25 UTC
Created attachment 84782 [details] [review]
Bug 20581: (follow-up) Fix typo in kohastructure

Fix the missing comma
Comment 148 Andrew Isherwood 2019-02-05 14:36:30 UTC
Created attachment 84783 [details] [review]
Bug 20581: (follow-up) Fix QA errors

This patch fixes the "missing_filter" errors on ill-requests.tt
Comment 149 Andrew Isherwood 2019-02-05 14:36:35 UTC
Created attachment 84784 [details] [review]
Bug 20581: (follow-up) Fix dependency requirement

Since this bug is now dependent on Bug 20600 (Provide the ability for
users to filter ILL requests) we need to make the status filtering aware
of the custom statuses. This patch does that.
Comment 150 Andrew Isherwood 2019-02-05 14:36:40 UTC
Created attachment 84785 [details] [review]
Bug 20581: (follow-up) Do not use PK for AV FK

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c133

We switch from using authorised_values.id when creating the foreign key
illrequests.status_alias, we now use authorised_values.authorised_value

I have added a migration from using id -> authorised_value, so when
existing users of this bug get this version, their DB migrates nicely
instead of just breaking.
Comment 151 Andrew Isherwood 2019-02-05 14:37:33 UTC
Hi Katrin

Thanks for that. I've now switched from using authorised_values.id to authorised_values.authorised_value.

I've also squashed the commits that were causing that QA error. All should be well now.

Cheers
Andrew
Comment 152 Katrin Fischer 2019-02-05 14:53:35 UTC
Moving back to signed off.
Comment 153 Andrew Isherwood 2019-02-05 15:19:49 UTC
Created attachment 84791 [details] [review]
Bug 20581: (follow-up) Fix typo in DB upgrade

IN -> ON
Comment 154 Katrin Fischer 2019-02-06 06:25:19 UTC
Comment on attachment 82489 [details] [review]
Bug 20581: DO NOT PUSH: Updated Schema Files.

This is too big, including more than the expected changed files. I am obsoleting it using 'dbic' on kohadevbox.
Comment 155 Katrin Fischer 2019-02-06 06:36:24 UTC
Hi Andrew, found a little issue:

On a database with the sample data, create an ILLSTATUS:  PROC with something like: Internal processing

Add the new status to an existing or new ILL request. 

Check the status filter on the side of the ill requests table: It pulls the wrong description as there is also a PROC value for location: Processing Center

I think you need to add the authorised value category into your look-up somewhere.
Comment 156 Andrew Isherwood 2019-02-06 11:05:35 UTC
Hi Katrin, rant incoming, though it's not aimed at you!

To be honest, this is actually a problem that is caused by using a foreign key that does not contain unique values.

I was originally using authorised_values.id (which *is* unique) as the foreign key from illrequests.status_alias, but after you said this is the non-standard way of doing things, I changed it to use authorised_values.authorised_value, which has no unique constraint on it, as you have demonstrated.

Thinking around the problem, I suspect this issue will affect any part of Koha that uses authorised values. The DBIx relationship makes the reasonable assumption that the column used as a foreign key contains unique values, so it just returns the first value it finds, which in this case is incorrect.

As far as addressing it in this case. Whatever I do is going to be hackery, since it's patching over a fundamental DB design problem. Since we're using the Koha objects/DBIx::Class accessors to get the request's status, I'm going to have to provide an overloaded method that only gets the authorised values in the correct category.

I do think it's worth considering in the context of the DB design error. This is a fairly nasty problem, I suspect it's just not very commonly seen since creating two authorised values with the same "Authorised value" value is probably quite rare.

I'll create a bug to start a discussion on it.
Comment 157 Katrin Fischer 2019-02-06 12:12:48 UTC
No problem ;)

Some of the authorised values are numeric (lost, damaged, restricted, not_for_loan), so it's very safe to assume that there is a lot of double ups actually there and in other places. I think other parts of the code MUST deal with it in some way. It's the reason why I didn't set FQA last time but "In discussion" actually. 

Getting some discussion on how to deal with this among the devs familiar with Koha::Object is probably best.
Comment 158 Katrin Fischer 2019-02-06 12:15:56 UTC
A first idea I had was to add prefixes... CATEGORY_AUTHVALUE, but the numerical ones are a problem there. The datatypes won't make it possible without more changes. Having a new table for each of those is also not the right way.

Would a combined primary key not solve the issue? (category, authorised_value) - those ARE unique and enforced.
Comment 159 Andrew Isherwood 2019-02-06 12:26:07 UTC
Hmmm, prefixes feels like a hack.

Using a composite primary key as you suggest might be a step in the right direction. However, as I understand it, to then use that as a foreign key in other tables would require each table to have a corresponding column for each of the authorised_values columns that comprise the key.

As an aside, I'm not convinced the category and authorised_value columns do currently have any unique constraint on them. Looking at the schema, it looks like category, authorised_value and lib are defined as keys, but I can only think that was does to create indexes on them as there appears to be no unique constraints on them.

Definitely worth a discussion I think...
Comment 160 Katrin Fischer 2019-02-06 12:30:35 UTC
(In reply to Andrew Isherwood from comment #159)
> Hmmm, prefixes feels like a hack.
> 
> Using a composite primary key as you suggest might be a step in the right
> direction. However, as I understand it, to then use that as a foreign key in
> other tables would require each table to have a corresponding column for
> each of the authorised_values columns that comprise the key.
> 
> As an aside, I'm not convinced the category and authorised_value columns do
> currently have any unique constraint on them. Looking at the schema, it
> looks like category, authorised_value and lib are defined as keys, but I can
> only think that was does to create indexes on them as there appears to be no
> unique constraints on them.
> 
> Definitely worth a discussion I think...

It might only be enforced on GUI side. Jonathan worked on some of this as part of the move to Koha introducing the authorised_value_category table as a first step to improve the data structures. Looking at some of this code might be helpful? Note: In order to translate the code to a description you can use TT plugins. No needs to pass the descriptions usually - so that might avoid some of the issue?
Comment 161 Andrew Isherwood 2019-02-06 12:57:29 UTC
Created attachment 84814 [details] [review]
Bug 20581: (follow-up) Overload status_alias

We need Koha::Illrequest->status_alias to return the correct authorised value.
Since the foreign key uses a non-unique column, we need to take into
account the authorised value category too.

This patch overloads status_alias with some additional logic that
considers the request's branch and the authorised value category to
attempt to return the correct value.

The same logic is applied to the custom statusalias method, which
returns the entire AuthorisedValue object
Comment 162 Andrew Isherwood 2019-02-06 14:18:41 UTC
Created attachment 84818 [details] [review]
Bug 20581: (follow-up) Fix status_alias comparison

status_alias will arrive in the Koha object as a string, so when we
special case it, we should also pass a string
Comment 163 Andrew Isherwood 2019-02-06 14:19:46 UTC
Created attachment 84819 [details] [review]
Bug 20581: (follow-up) Fix unit test

Since we're now storing the authorised_value column as the foreign key,
rather than the id, we need to account for this in the test
Comment 164 Katrin Fischer 2019-02-06 15:43:35 UTC
Andrew, can this be switched back to Signed off = ready for more QA?
Comment 165 Katrin Fischer 2019-02-07 06:32:02 UTC
Sorry Andrew, I wish I had better news:
- Correct descriptions now show

But: 
When I edit one of the ILL requests I set to "Internal processing" and change it back to "New request" it doesn't take on saving. I stays at the custom status and I can't change it. There is no error in the plack logs, just:

[Mon Feb  4 19:33:12 2019] [debug] GET "/api/v1/illrequests"
[Mon Feb  4 19:33:12 2019] [debug] Routing to controller "Koha::REST::V1::Auth" and action "under"
[Mon Feb  4 19:33:12 2019] [debug] Routing to controller "Koha::REST::V1::Illrequests" and action "list"
[Mon Feb  4 19:33:12 2019] [debug] 200 OK (0.0728s, 13.736/s)

This happens for both my FreeForm and BLDSS requests.

If I confirm the request, setting "Requested" seems to work ok, But in the datbase I see -1 for alias_status. I'd say this should be NULL or "", as -1 seems unusual and a bit dangerous (considering the 'on order' authorised value is -1 for example)

Not sure if this is related to the patch, but noting just in case: I can't confirm the request either, but get an error instead. This is a BLDSS request - might just be related to that as my FreeForm one behaves nicely.
Comment 166 Katrin Fischer 2019-02-07 06:39:16 UTC
I am wondering about the intended mechanics here. Looking at the test plan again maybe setting it "back" was not an intended behaviour? 
I am wondering if you could think of the status_alias as a sub_status to the status and in that case if it would not be better to display them both separately. If the ILL request is "new" + "custom", "confirmed" + "custom" etc. And offer an empty option in the pull down for unsetting?

I might not fully grasp the use case yet :(
Comment 167 Andrew Isherwood 2019-02-07 09:18:10 UTC
Hi Katrin

Thanks very much for the info. A bit of context might be in order.

An ILL request has, at any given time, a single "status". The workflow the request passes through is dictated by those statuses. The ILL module has the idea of a "status graph", which essentially says "if you're at this status, you can have come from this/these status(es) and you can go to this/these status(es)." This is how we can determine what actions the user can take on any given request.

This bug adds the concept of status aliases, which are user defined as authorised values. These have no bearing on any of the above, the request's status is still used in the same way. The status alias is purely used for display. The display logic says "if a request has a status alias, display that, otherwise display the request's status". This enables the user to select one of their user defined status aliases, which will be displayed in favour of the request's "actual" status. This allows the user to assign a more locally meaningful status to a request, rather than rely on those provided by ILL.

I mentioned above that the status alias has no bearing on the actual status. The request's status is still there, and still determines, say, which action buttons are displayed for a request, but it is not displayed if an alias is also assigned to that request. However, if a request that has a status alias is moved forward through the workflow by clicking one of the action buttons (e.g. Confirm request), we assume that the status alias is no longer applicable and nullify it in the request, causing the request's actual status to now display, until such a time as the user selects a new status alias.

So, to address what you're seeing above with -1 being assigned. In the "Edit request" screen, the status dropdown contains all of the status aliases PLUS an option for the request's actual status, which effectively allows the user to nullify the request's status alias and revert the request to the actual status. You said you selected "New request", which will have been the request's actual status. But it sounds like, rather than nullifying the status alias, it set it to -1. Which is odd.

I'll investigate and get back to you. Thanks again for your ongoing efforts on getting this one through!
Comment 168 Andrew Isherwood 2019-02-07 10:46:06 UTC
Created attachment 84844 [details] [review]
Bug 20581: (follow-up) Passed value to status_alias

Now we have overloaded status_alias, we can no longer pass it undef (as
it uses that to determine if it's being used as a getter or setter). So
we now pass it -1 to indicate it should actually be setting undef (a
hack, I know, but what are you going to do...)

This patch makes sure ill-request.pl conforms to that. It also fixes the
testing of the passed parameter, which was wrong.
Comment 169 Andrew Isherwood 2019-02-07 10:48:48 UTC
(In reply to Katrin Fischer from comment #165)
> When I edit one of the ILL requests I set to "Internal processing" and
> change it back to "New request" it doesn't take on saving. I stays at the
> custom status and I can't change it.

This is now fixed. The hackery I had to introduce into Illrequest.pm requires a special casing for when we want to nullify status_alias, I had forgotten to introduce that here.

> If I confirm the request, setting "Requested" seems to work ok, But in the
> datbase I see -1 for alias_status.

See above

> Not sure if this is related to the patch, but noting just in case: I can't
> confirm the request either, but get an error instead. This is a BLDSS
> request - might just be related to that as my FreeForm one behaves nicely.

This is unrelated, it does actually appear that the BLDSS API is suddenly returning an error for the confirm call :( I'll investigate...
Comment 170 Katrin Fischer 2019-02-09 21:52:04 UTC
Created attachment 84911 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.

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

Following commit was squashed into this one:

Bug 20581: (follow-up) Modify DB updgrade

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c56

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 171 Katrin Fischer 2019-02-09 21:52:11 UTC
Created attachment 84912 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 172 Katrin Fischer 2019-02-09 21:52:17 UTC
Created attachment 84913 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 173 Katrin Fischer 2019-02-09 21:52:23 UTC
Created attachment 84914 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 174 Katrin Fischer 2019-02-09 21:52:29 UTC
Created attachment 84915 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 175 Katrin Fischer 2019-02-09 21:52:35 UTC
Created attachment 84916 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 176 Katrin Fischer 2019-02-09 21:52:41 UTC
Created attachment 84917 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 177 Katrin Fischer 2019-02-09 21:52:47 UTC
Created attachment 84918 [details] [review]
Bug 20581: (follow-up) Allow for undefined lib

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c59

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 178 Katrin Fischer 2019-02-09 21:52:52 UTC
Created attachment 84919 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c57

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 179 Katrin Fischer 2019-02-09 21:52:59 UTC
Created attachment 84920 [details] [review]
Bug 20581: (follow-up) Fix fallback status_alias

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c68

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 180 Katrin Fischer 2019-02-09 21:53:05 UTC
Created attachment 84921 [details] [review]
Bug 20581: (follow-up) Fix AV population

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c72

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 181 Katrin Fischer 2019-02-09 21:53:11 UTC
Created attachment 84922 [details] [review]
Bug 20581: (follow-up) Modify POD

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c69

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 182 Katrin Fischer 2019-02-09 21:53:17 UTC
Created attachment 84923 [details] [review]
Bug 20581: (follow-up) Fix typo in kohastructure

Fix the missing comma

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 183 Katrin Fischer 2019-02-09 21:53:23 UTC
Created attachment 84924 [details] [review]
Bug 20581: (follow-up) Fix QA errors

This patch fixes the "missing_filter" errors on ill-requests.tt

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 184 Katrin Fischer 2019-02-09 21:53:29 UTC
Created attachment 84925 [details] [review]
Bug 20581: (follow-up) Fix dependency requirement

Since this bug is now dependent on Bug 20600 (Provide the ability for
users to filter ILL requests) we need to make the status filtering aware
of the custom statuses. This patch does that.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 185 Katrin Fischer 2019-02-09 21:53:35 UTC
Created attachment 84926 [details] [review]
Bug 20581: (follow-up) Do not use PK for AV FK

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c133

We switch from using authorised_values.id when creating the foreign key
illrequests.status_alias, we now use authorised_values.authorised_value

I have added a migration from using id -> authorised_value, so when
existing users of this bug get this version, their DB migrates nicely
instead of just breaking.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 186 Katrin Fischer 2019-02-09 21:53:40 UTC
Created attachment 84927 [details] [review]
Bug 20581: (follow-up) Fix typo in DB upgrade

IN -> ON

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 187 Katrin Fischer 2019-02-09 21:53:47 UTC
Created attachment 84928 [details] [review]
Bug 20581: (follow-up) Overload status_alias

We need Koha::Illrequest->status_alias to return the correct authorised value.
Since the foreign key uses a non-unique column, we need to take into
account the authorised value category too.

This patch overloads status_alias with some additional logic that
considers the request's branch and the authorised value category to
attempt to return the correct value.

The same logic is applied to the custom statusalias method, which
returns the entire AuthorisedValue object

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 188 Katrin Fischer 2019-02-09 21:53:53 UTC
Created attachment 84929 [details] [review]
Bug 20581: (follow-up) Fix status_alias comparison

status_alias will arrive in the Koha object as a string, so when we
special case it, we should also pass a string

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 189 Katrin Fischer 2019-02-09 21:54:00 UTC
Created attachment 84930 [details] [review]
Bug 20581: (follow-up) Fix unit test

Since we're now storing the authorised_value column as the foreign key,
rather than the id, we need to account for this in the test

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 190 Katrin Fischer 2019-02-09 21:54:06 UTC
Created attachment 84931 [details] [review]
Bug 20581: (follow-up) Passed value to status_alias

Now we have overloaded status_alias, we can no longer pass it undef (as
it uses that to determine if it's being used as a getter or setter). So
we now pass it -1 to indicate it should actually be setting undef (a
hack, I know, but what are you going to do...)

This patch makes sure ill-request.pl conforms to that. It also fixes the
testing of the passed parameter, which was wrong.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 191 Katrin Fischer 2019-02-09 21:57:11 UTC
I still have one issue with this: 

I don't like -1 in illrequests.status_alias meaning 'no status set'. I'd be much happier with NULL. But I think we should be able to work this out separately from this bug and maybe at the end or on another branch of the 'ILL tree'. Maybe others can give some input then too on how to handle the situation with the Authorised values best.

The functionality works now well enough for me to pass it and move on.
Comment 192 Andrew Isherwood 2019-02-10 10:08:16 UTC
(In reply to Katrin Fischer from comment #191)

> I don't like -1 in illrequests.status_alias meaning 'no status set'. I'd be
> much happier with NULL.

That's a bug, it should not be recording -1 for no status_alias, it should indeed be NULL. If it's saving -1 that's potentially very problematic. Is it definitely still doing that? The fix I did the other day should have resolved that. I'll set this to Failed QA and investigate tomorrow.
Comment 193 Katrin Fischer 2019-02-10 10:13:09 UTC
(In reply to Andrew Isherwood from comment #192)
> (In reply to Katrin Fischer from comment #191)
> 
> > I don't like -1 in illrequests.status_alias meaning 'no status set'. I'd be
> > much happier with NULL.
> 
> That's a bug, it should not be recording -1 for no status_alias, it should
> indeed be NULL. If it's saving -1 that's potentially very problematic. Is it
> definitely still doing that? The fix I did the other day should have
> resolved that. I'll set this to Failed QA and investigate tomorrow.

Thx for the reply! Yes, it still does, my illrequets are both set to -1 now. Unsetting and resetting status seem to work ok, so I assumed it was intentional at this point.
Comment 194 Andrew Isherwood 2019-02-11 11:11:59 UTC
Created attachment 84964 [details] [review]
Bug 20581: (follow-up) Fix status_alias -1 bug

Stupid bug, we need to pass NULL not -1 when unsetting status_alias
Comment 195 Katrin Fischer 2019-02-14 06:43:17 UTC
Created attachment 85075 [details] [review]
Bug 20581: Modifications to database schema

- This patch provides an illrequests.status_alias column that is a foreign key to
authorised_values.id. This provides the ability for an ILL request to
have an optional status alias that is defined as an authorised valued.
- A new ILLSTATUS authorised value category is also made available during install.

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

Following commit was squashed into this one:

Bug 20581: (follow-up) Modify DB updgrade

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c56

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 196 Katrin Fischer 2019-02-14 06:43:22 UTC
Created attachment 85076 [details] [review]
Bug 20581: Provide status_alias on IllRequest

This patch provides status_alias support for the Koha::Illrequest
object.

- New 'statusalias' getter method, returns the request's status_alias
authorised value object if set, otherwise undef
- Overloads existing 'status' method, getter/setter accessors intact,
but setting the request's status now implicitly removes any status_alias
that has been set

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 197 Katrin Fischer 2019-02-14 06:43:28 UTC
Created attachment 85077 [details] [review]
Bug 20581: API provide status_alias embed

- This patch provides the illrequests API route with an additional embed,
called 'status_alias'. Requesting this embed returns an object for the
request's status_alias authorised value, or null if one is not set.
- This patch also adds the new status_alias as a default embed on the API
call made by the illlist view

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 198 Katrin Fischer 2019-02-14 06:43:34 UTC
Created attachment 85078 [details] [review]
Bug 20581: Add display of status_alias to staff vw

- This patch adds display of the new status_alias value, when set, in
favour of the request's status value.
- It also allows the user to set a status_alias for an existing request.
When set, this value will be displayed in preference to the status.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 199 Katrin Fischer 2019-02-14 06:43:40 UTC
Created attachment 85079 [details] [review]
Bug 20581: Fix display of status alias in OPAC

Fix for status alias display in OPAC, we should be displaying the status
name, not the status code.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 200 Katrin Fischer 2019-02-14 06:43:45 UTC
Created attachment 85080 [details] [review]
Bug 20581: Unit tests for status_alias

This patch adds unit tests for the specific status_alias functionality
added in this bug

- Creation of the ILLSTATUS authorised value
- Illrequest->statusalias accessor
- Illrequest->status overloading to reset status_alias

To test:
1) Apply this patch
2) prove t/db_dependent/Illrequests.t

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 201 Katrin Fischer 2019-02-14 06:43:51 UTC
Created attachment 85081 [details] [review]
Bug 20581: (follow-up) Display status name

Rather than display the status code for custom statuses, we should be
displaying the name

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 202 Katrin Fischer 2019-02-14 06:43:56 UTC
Created attachment 85082 [details] [review]
Bug 20581: (follow-up) Allow for undefined lib

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c59

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 203 Katrin Fischer 2019-02-14 06:44:02 UTC
Created attachment 85083 [details] [review]
Bug 20581: (follow-up) Fix statusalias return

As per
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c57

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 204 Katrin Fischer 2019-02-14 06:44:08 UTC
Created attachment 85084 [details] [review]
Bug 20581: (follow-up) Fix fallback status_alias

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c68

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 205 Katrin Fischer 2019-02-14 06:44:14 UTC
Created attachment 85085 [details] [review]
Bug 20581: (follow-up) Fix AV population

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c72

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 206 Katrin Fischer 2019-02-14 06:44:20 UTC
Created attachment 85086 [details] [review]
Bug 20581: (follow-up) Modify POD

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c69

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 207 Katrin Fischer 2019-02-14 06:44:26 UTC
Created attachment 85087 [details] [review]
Bug 20581: (follow-up) Fix typo in kohastructure

Fix the missing comma

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 208 Katrin Fischer 2019-02-14 06:44:31 UTC
Created attachment 85088 [details] [review]
Bug 20581: (follow-up) Fix QA errors

This patch fixes the "missing_filter" errors on ill-requests.tt

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 209 Katrin Fischer 2019-02-14 06:44:37 UTC
Created attachment 85089 [details] [review]
Bug 20581: (follow-up) Fix dependency requirement

Since this bug is now dependent on Bug 20600 (Provide the ability for
users to filter ILL requests) we need to make the status filtering aware
of the custom statuses. This patch does that.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 210 Katrin Fischer 2019-02-14 06:44:43 UTC
Created attachment 85090 [details] [review]
Bug 20581: (follow-up) Do not use PK for AV FK

As per:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c133

We switch from using authorised_values.id when creating the foreign key
illrequests.status_alias, we now use authorised_values.authorised_value

I have added a migration from using id -> authorised_value, so when
existing users of this bug get this version, their DB migrates nicely
instead of just breaking.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 211 Katrin Fischer 2019-02-14 06:44:49 UTC
Created attachment 85091 [details] [review]
Bug 20581: (follow-up) Fix typo in DB upgrade

IN -> ON

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 212 Katrin Fischer 2019-02-14 06:44:54 UTC
Created attachment 85092 [details] [review]
Bug 20581: (follow-up) Overload status_alias

We need Koha::Illrequest->status_alias to return the correct authorised value.
Since the foreign key uses a non-unique column, we need to take into
account the authorised value category too.

This patch overloads status_alias with some additional logic that
considers the request's branch and the authorised value category to
attempt to return the correct value.

The same logic is applied to the custom statusalias method, which
returns the entire AuthorisedValue object

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 213 Katrin Fischer 2019-02-14 06:45:00 UTC
Created attachment 85093 [details] [review]
Bug 20581: (follow-up) Fix status_alias comparison

status_alias will arrive in the Koha object as a string, so when we
special case it, we should also pass a string

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 214 Katrin Fischer 2019-02-14 06:45:08 UTC
Created attachment 85094 [details] [review]
Bug 20581: (follow-up) Fix unit test

Since we're now storing the authorised_value column as the foreign key,
rather than the id, we need to account for this in the test

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 215 Katrin Fischer 2019-02-14 06:45:14 UTC
Created attachment 85095 [details] [review]
Bug 20581: (follow-up) Passed value to status_alias

Now we have overloaded status_alias, we can no longer pass it undef (as
it uses that to determine if it's being used as a getter or setter). So
we now pass it -1 to indicate it should actually be setting undef (a
hack, I know, but what are you going to do...)

This patch makes sure ill-request.pl conforms to that. It also fixes the
testing of the passed parameter, which was wrong.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 216 Katrin Fischer 2019-02-14 06:45:20 UTC
Created attachment 85096 [details] [review]
Bug 20581: (follow-up) Fix status_alias -1 bug

Stupid bug, we need to pass NULL not -1 when unsetting status_alias

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 217 Nick Clemens 2019-02-22 15:05:19 UTC
Awesome work all!

Pushed to master for 19.05
Comment 218 Martin Renvoize 2019-02-26 10:03:12 UTC
Enhancement, will not be backported to 18.11.x series