Bug 20600 - Provide the ability for users to filter ILL requests
Summary: Provide the ability for users to filter ILL requests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Andrew Isherwood
QA Contact: Katrin Fischer
URL:
Keywords:
: 18590 (view as bug list)
Depends on: 20640
Blocks: 21482 20581
  Show dependency treegraph
 
Reported: 2018-04-18 07:58 UTC by Andrew Isherwood
Modified: 2022-11-11 10:32 UTC (History)
7 users (show)

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


Attachments
Bug 20600: API - Format dates according to syspref (2.40 KB, patch)
2018-04-20 13:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (17.77 KB, patch)
2018-04-20 13:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (21.28 KB, patch)
2018-05-09 11:27 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (21.34 KB, patch)
2018-05-11 18:11 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20600: (follow-up) Do not use $_ in loop (1.08 KB, patch)
2018-06-01 11:12 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove date formatting (1.24 KB, patch)
2018-06-01 11:12 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.53 KB, patch)
2018-06-04 11:46 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.49 KB, patch)
2018-06-04 11:50 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix filtering (2.40 KB, patch)
2018-06-06 09:51 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove console.log! (1.10 KB, patch)
2018-06-06 09:53 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (21.39 KB, patch)
2018-06-06 12:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Do not use $_ in loop (1.12 KB, patch)
2018-06-06 12:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove date formatting (1.29 KB, patch)
2018-06-06 12:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.53 KB, patch)
2018-06-06 12:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix filtering (2.45 KB, patch)
2018-06-06 12:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove console.log! (1.14 KB, patch)
2018-06-06 12:09 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Move template JavaScript to the footer (47.01 KB, patch)
2018-06-06 12:19 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (sign-off) Move template JavaScript to the footer (47.09 KB, patch)
2018-06-07 10:30 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (21.40 KB, patch)
2018-08-10 16:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Do not use $_ in loop (1.12 KB, patch)
2018-08-10 16:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove date formatting (1.29 KB, patch)
2018-08-10 16:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.51 KB, patch)
2018-08-10 16:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix filtering (2.45 KB, patch)
2018-08-10 16:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove console.log! (1.14 KB, patch)
2018-08-10 16:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Move template JavaScript to the footer (47.31 KB, patch)
2018-08-10 16:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (21.43 KB, patch)
2018-09-05 15:50 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (21.43 KB, patch)
2018-09-05 15:50 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Do not use $_ in loop (1.13 KB, patch)
2018-09-05 15:51 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove date formatting (1.29 KB, patch)
2018-09-05 15:51 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.53 KB, patch)
2018-09-05 15:51 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix filtering (2.45 KB, patch)
2018-09-05 15:51 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove console.log! (1.15 KB, patch)
2018-09-05 15:51 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Move template JavaScript to the footer (24.83 KB, patch)
2018-09-05 15:51 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (21.43 KB, patch)
2018-09-06 11:25 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Do not use $_ in loop (1.13 KB, patch)
2018-09-06 11:25 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove date formatting (1.29 KB, patch)
2018-09-06 11:25 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.53 KB, patch)
2018-09-06 11:26 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix filtering (2.45 KB, patch)
2018-09-06 11:26 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove console.log! (1.15 KB, patch)
2018-09-06 11:26 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Move template JavaScript to the footer (24.83 KB, patch)
2018-09-06 11:26 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix bad rebase (19.77 KB, patch)
2018-09-06 11:26 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix bad rebase (20.12 KB, patch)
2018-09-20 16:02 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix bad rebase (1.04 KB, patch)
2018-10-12 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Rebase on current master (27.36 KB, patch)
2018-11-06 16:29 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix errant tabs (2.23 KB, patch)
2018-11-06 16:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (20.83 KB, patch)
2018-11-07 14:52 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Do not use $_ in loop (1.13 KB, patch)
2018-11-07 14:52 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove date formatting (1.29 KB, patch)
2018-11-07 14:52 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.53 KB, patch)
2018-11-07 14:52 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix filtering (2.45 KB, patch)
2018-11-07 14:52 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove console.log! (1.15 KB, patch)
2018-11-07 14:52 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Move template JavaScript to the footer (24.83 KB, patch)
2018-11-07 14:52 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Rebase on current master (27.39 KB, patch)
2018-11-07 14:52 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Restore patron name fix (1.67 KB, patch)
2018-11-08 11:04 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix API spec (1.53 KB, patch)
2018-11-09 14:39 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix unit tests (2.96 KB, patch)
2018-11-09 14:39 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (20.85 KB, patch)
2018-11-19 11:30 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Do not use $_ in loop (1.13 KB, patch)
2018-11-19 11:30 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove date formatting (1.29 KB, patch)
2018-11-19 11:30 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.53 KB, patch)
2018-11-19 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix filtering (2.45 KB, patch)
2018-11-19 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove console.log! (1.15 KB, patch)
2018-11-19 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Move template JavaScript to the footer (24.83 KB, patch)
2018-11-19 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Rebase on current master (26.92 KB, patch)
2018-11-19 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Restore patron name fix (1.67 KB, patch)
2018-11-19 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix API spec (1.53 KB, patch)
2018-11-19 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix unit tests (3.07 KB, patch)
2018-11-19 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix tests (2.14 KB, patch)
2019-01-14 10:42 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix critic error (700 bytes, patch)
2019-01-14 13:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (20.91 KB, patch)
2019-02-04 19:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Do not use $_ in loop (1.18 KB, patch)
2019-02-04 19:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove date formatting (1.35 KB, patch)
2019-02-04 19:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.58 KB, patch)
2019-02-04 19:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix filtering (2.51 KB, patch)
2019-02-04 19:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove console.log! (1.20 KB, patch)
2019-02-04 19:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Move template JavaScript to the footer (24.89 KB, patch)
2019-02-04 19:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Rebase on current master (26.98 KB, patch)
2019-02-04 19:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Restore patron name fix (1.73 KB, patch)
2019-02-04 19:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix API spec (1.58 KB, patch)
2019-02-04 19:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix unit tests (3.13 KB, patch)
2019-02-04 19:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix tests (2.20 KB, patch)
2019-02-04 19:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix critic error (757 bytes, patch)
2019-02-04 19:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: (QA follow-up) Fix terminology and capitalization (2.46 KB, patch)
2019-02-04 19:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20600: Add filtering of ILL requests in list (20.91 KB, patch)
2019-02-13 09:17 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Do not use $_ in loop (1.18 KB, patch)
2019-02-13 09:17 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove date formatting (1.35 KB, patch)
2019-02-13 09:17 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Add date range filtering (6.58 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix filtering (2.51 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Remove console.log! (1.20 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Move template JavaScript to the footer (24.89 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Rebase on current master (26.98 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Restore patron name fix (1.73 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix API spec (1.58 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix unit tests (3.13 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix tests (2.20 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (follow-up) Fix critic error (757 bytes, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20600: (QA follow-up) Fix terminology and capitalization (2.46 KB, patch)
2019-02-13 09:18 UTC, Andrew Isherwood
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2018-04-18 07:58:37 UTC
Currently when a staff member is viewing a list of ILL requests, it is only possible to sort the results by column. This bug proposes providing the ability to filter the ILL request list via values displayed in the table columns.
Comment 1 Andrew Isherwood 2018-04-18 08:22:33 UTC
The work done in this bug will need to be revisited once bug 20581 makes it into master as the filters will need to be adapted to allow for the status alias that requests can have.
Comment 2 Andrew Isherwood 2018-04-20 13:32:21 UTC Comment hidden (obsolete)
Comment 3 Andrew Isherwood 2018-04-20 13:33:02 UTC Comment hidden (obsolete)
Comment 4 Magnus Enger 2018-05-03 12:54:18 UTC Comment hidden (obsolete)
Comment 5 Andrew Isherwood 2018-05-09 08:22:06 UTC Comment hidden (obsolete)
Comment 6 Andrew Isherwood 2018-05-09 11:27:05 UTC Comment hidden (obsolete)
Comment 7 Magnus Enger 2018-05-11 18:11:13 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2018-05-30 19:18:01 UTC
1.
Do not use loop variable $_

+    foreach(@req_list) {
+        foreach my $field(@format_dates) {
+            if (defined $_->{$field}) {
+                $_->{$field . "_formatted"} = format_sqldatetime(

2. Why do not you use the datepicker plugin as we use it elsewhere? :)

3. For dates in datatables we usually do not use it that way, search for "title-string" in the *.tt files

I am not sure you need the 2 *_formatted variables.

4. Is it really useful to provide only a specific dates and not a date range?
Comment 9 Andrew Isherwood 2018-06-01 11:12:47 UTC Comment hidden (obsolete)
Comment 10 Andrew Isherwood 2018-06-01 11:12:59 UTC Comment hidden (obsolete)
Comment 11 Andrew Isherwood 2018-06-01 11:17:36 UTC
Hi Jonathan

Thanks for your feedback.

1. OK, addressed in https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75734
2. OK, addressed in https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75735
3. Thanks for the pointer towards title-string. I have used the *_formatted fields to allow me to display the date fields in the datatable according to the date format set in the sysprefs. Since the data is coming in to the datatable from an API call, I'm not sure how else I can do it. $KohaDates obviously won't work, I couldn't get title-string to either, though, having done some research, it appears it is used to control sorting, rather than display formatting.
4. Good point, I'll create a follow-up patch and submit it
Comment 12 Jonathan Druart 2018-06-01 13:08:57 UTC
There is a date range filter example in serials/claims.tt
Comment 13 Andrew Isherwood 2018-06-04 11:46:48 UTC Comment hidden (obsolete)
Comment 14 Andrew Isherwood 2018-06-04 11:48:43 UTC
Hi Jonathan

I've now added a follow-up patch to add the date range filtering. I ended up not following the example you cited, it appears to take a very manual approach of hiding all table rows and then showing them depending on whether they pass the filter test. Doing some research, Datatables has an API that allows custom filters to be defined, it felt cleaner to do this the proper Datatables way.

Anyway, thanks for your help on this! :)
Comment 15 Andrew Isherwood 2018-06-04 11:50:24 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2018-06-04 19:52:38 UTC
I had not time to test, but wondering if this will work with our various dateformat settings? Adding Owen to take a look :)
Comment 17 Owen Leonard 2018-06-05 14:33:30 UTC
(In reply to Katrin Fischer from comment #16)
> I had not time to test, but wondering if this will work with our various
> dateformat settings?

Filtering by date didn't work for me when the dateformat preference was set to dmydot or metric.
Comment 18 Andrew Isherwood 2018-06-06 09:51:03 UTC Comment hidden (obsolete)
Comment 19 Andrew Isherwood 2018-06-06 09:51:43 UTC
Oooh, good spot Owen, thanks!

I've just attached a follow-up patch that should address this.
Comment 20 Andrew Isherwood 2018-06-06 09:53:27 UTC Comment hidden (obsolete)
Comment 21 Owen Leonard 2018-06-06 12:08:43 UTC Comment hidden (obsolete)
Comment 22 Owen Leonard 2018-06-06 12:08:47 UTC Comment hidden (obsolete)
Comment 23 Owen Leonard 2018-06-06 12:08:51 UTC Comment hidden (obsolete)
Comment 24 Owen Leonard 2018-06-06 12:08:54 UTC Comment hidden (obsolete)
Comment 25 Owen Leonard 2018-06-06 12:08:59 UTC Comment hidden (obsolete)
Comment 26 Owen Leonard 2018-06-06 12:09:03 UTC Comment hidden (obsolete)
Comment 27 Owen Leonard 2018-06-06 12:19:43 UTC Comment hidden (obsolete)
Comment 28 Andrew Isherwood 2018-06-07 10:30:14 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2018-06-14 20:39:26 UTC
Well, yes it works, but I do not think it is the way to do. I still think we should use the standard title-string sort method. I am not going to consider it as blocker anyway.
Comment 30 Owen Leonard 2018-08-10 16:00:20 UTC Comment hidden (obsolete)
Comment 31 Owen Leonard 2018-08-10 16:00:25 UTC Comment hidden (obsolete)
Comment 32 Owen Leonard 2018-08-10 16:00:29 UTC Comment hidden (obsolete)
Comment 33 Owen Leonard 2018-08-10 16:00:34 UTC Comment hidden (obsolete)
Comment 34 Owen Leonard 2018-08-10 16:00:38 UTC Comment hidden (obsolete)
Comment 35 Owen Leonard 2018-08-10 16:00:44 UTC Comment hidden (obsolete)
Comment 36 Owen Leonard 2018-08-10 16:00:48 UTC Comment hidden (obsolete)
Comment 37 Owen Leonard 2018-08-10 16:01:56 UTC
I've rebased these patches and moved the changes in staff-global.css to staff-global.scss. Staff client CSS will have to be recompiled for testing.
Comment 38 Andrew Isherwood 2018-09-05 15:50:23 UTC Comment hidden (obsolete)
Comment 39 Andrew Isherwood 2018-09-05 15:50:59 UTC Comment hidden (obsolete)
Comment 40 Andrew Isherwood 2018-09-05 15:51:05 UTC Comment hidden (obsolete)
Comment 41 Andrew Isherwood 2018-09-05 15:51:11 UTC Comment hidden (obsolete)
Comment 42 Andrew Isherwood 2018-09-05 15:51:16 UTC Comment hidden (obsolete)
Comment 43 Andrew Isherwood 2018-09-05 15:51:21 UTC Comment hidden (obsolete)
Comment 44 Andrew Isherwood 2018-09-05 15:51:27 UTC Comment hidden (obsolete)
Comment 45 Andrew Isherwood 2018-09-05 15:51:32 UTC Comment hidden (obsolete)
Comment 46 Andrew Isherwood 2018-09-06 11:25:46 UTC
Created attachment 78463 [details] [review]
Bug 20600: Add filtering of ILL requests in list

This patch adds the ability for the ILL request list in the staff
interface to be filtered by a number of criteria:

- Status
- Date placed
- Date modified
- Pickup branch
- Borrower card number

To test:
1) Apply patch
2) Ensure you have a reasonable range of ILL requests created
3) Navigate to the "View ILL requests" page
4) Choose one or more filtering criteria
5) Click "Search"
6) Observe the results are filtered to match the selected criteria
7) Click "Clear"
8) Observe your results are returned to their initial state
9) Repeat steps 4 -> 6 until you are happy.

Dates supplied by the /api/v1/illrequests API route were not conforming
to the preference specified by the dateformat syspref. This patch
addresses that.

It has been addressed as part of this bug since we are adding filtering
of requests by some date fields and, therefore, needed dates in a
predictable format.

To test:
1) Apply the patch
2) Ensure you have at least one ILL request created
3) Make a request to the /api/v1/illrequests endpoint
4) Observe that dates supplied for "placed_formatted" & "updated_formatted" conform to your
dateformat syspref.
5) Change your date format syspref, repeat steps 3 & 4

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. Nice enh!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 47 Andrew Isherwood 2018-09-06 11:25:51 UTC
Created attachment 78464 [details] [review]
Bug 20600: (follow-up) Do not use $_ in loop

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 48 Andrew Isherwood 2018-09-06 11:25:56 UTC
Created attachment 78465 [details] [review]
Bug 20600: (follow-up) Remove date formatting

It is not required

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 49 Andrew Isherwood 2018-09-06 11:26:02 UTC
Created attachment 78466 [details] [review]
Bug 20600: (follow-up) Add date range filtering

It makes more sense to be able to filter placed and updated dates by
range. This patch adds that.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 50 Andrew Isherwood 2018-09-06 11:26:07 UTC
Created attachment 78467 [details] [review]
Bug 20600: (follow-up) Fix filtering

There was a bug whereby filtering by date would fail if the selected
date syspref was set to dmydot or metric. Rather than use the display
date value, We now use the non-display date value that is always in
a predictable ISO8601 format to construct the Date objects used
for comparison.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 51 Andrew Isherwood 2018-09-06 11:26:12 UTC
Created attachment 78468 [details] [review]
Bug 20600: (follow-up) Remove console.log!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 52 Andrew Isherwood 2018-09-06 11:26:19 UTC
Created attachment 78469 [details] [review]
Bug 20600: (follow-up) Move template JavaScript to the footer

This patch moves the ILL requests template's JavaScript to the footer.

Also changed:
 - Removed unused jQuery checkboxes plugin
 - Some <input> tags have been corrected to be self-closing.
 - Title tag has been corrected
 - Table row limit of 10 has been removed in favor of accepting default
   used elsewhere.

To test, apply the patch and follow the original test plan for this bug.
Everything should work as expected.

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 53 Andrew Isherwood 2018-09-06 11:26:24 UTC Comment hidden (obsolete)
Comment 54 Owen Leonard 2018-09-20 16:02:25 UTC Comment hidden (obsolete)
Comment 55 Martin Renvoize 2018-10-11 15:38:13 UTC
*** Bug 18590 has been marked as a duplicate of this bug. ***
Comment 56 Andrew Isherwood 2018-10-12 11:06:20 UTC
Created attachment 80468 [details] [review]
Bug 20600: (follow-up) Fix bad rebase
Comment 57 Josef Moravec 2018-10-12 15:27:45 UTC
Comment on attachment 78463 [details] [review]
Bug 20600: Add filtering of ILL requests in list

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

::: Koha/REST/V1/Illrequests.pm
@@ +65,5 @@
>      }
> +
> +    # Create new "formatted" columns for each date column
> +    # that needs formatting
> +    foreach(@req_list) {

I do not like this. The date should not be formatted on API level. It should be left to client/UI.
Comment 58 Andrew Isherwood 2018-10-15 14:41:56 UTC
(In reply to Josef Moravec from comment #57)
> Comment on attachment 78463 [details] [review] [review]
> Bug 20600: Add filtering of ILL requests in list
> 
> Review of attachment 78463 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/REST/V1/Illrequests.pm
> @@ +65,5 @@
> >      }
> > +
> > +    # Create new "formatted" columns for each date column
> > +    # that needs formatting
> > +    foreach(@req_list) {
> 
> I do not like this. The date should not be formatted on API level. It should
> be left to client/UI.

Thanks for this. I don't like it either. But, having spent a long time trying to figure out how, I came to the conclusion that there's no way of sanely using the date formatting syspref setting from Javascript. Hence me having to pre-format at the server end.

There is a precedent for this, though I didn't know it when I implemented it in Illrequests.pm svc/checkouts does exactly the same thing in order to pre-format dates for the API response used in checkouts.js: https://github.com/PTFS-Europe/koha/blob/master/svc/checkouts#L207-L218
Comment 59 Andrew Isherwood 2018-11-06 16:29:35 UTC
Created attachment 81992 [details] [review]
Bug 20600: (follow-up) Rebase on current master

Significant things have changed in master due to ILL enhancement patches
being pushed. This patch rebases this bug on top of them
Comment 60 Andrew Isherwood 2018-11-06 16:32:22 UTC
Created attachment 81993 [details] [review]
Bug 20600: (follow-up) Fix errant tabs

How on earth these tabs crept in I have no idea... <sigh>
Comment 61 Andrew Isherwood 2018-11-07 14:52:28 UTC
Created attachment 82032 [details] [review]
Bug 20600: Add filtering of ILL requests in list

This patch adds the ability for the ILL request list in the staff
interface to be filtered by a number of criteria:

- Status
- Date placed
- Date modified
- Pickup branch
- Borrower card number

To test:
1) Apply patch
2) Ensure you have a reasonable range of ILL requests created
3) Navigate to the "View ILL requests" page
4) Choose one or more filtering criteria
5) Click "Search"
6) Observe the results are filtered to match the selected criteria
7) Click "Clear"
8) Observe your results are returned to their initial state
9) Repeat steps 4 -> 6 until you are happy.

Dates supplied by the /api/v1/illrequests API route were not conforming
to the preference specified by the dateformat syspref. This patch
addresses that.

It has been addressed as part of this bug since we are adding filtering
of requests by some date fields and, therefore, needed dates in a
predictable format.

To test:
1) Apply the patch
2) Ensure you have at least one ILL request created
3) Make a request to the /api/v1/illrequests endpoint
4) Observe that dates supplied for "placed_formatted" & "updated_formatted" conform to your
dateformat syspref.
5) Change your date format syspref, repeat steps 3 & 4

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. Nice enh!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 62 Andrew Isherwood 2018-11-07 14:52:32 UTC
Created attachment 82033 [details] [review]
Bug 20600: (follow-up) Do not use $_ in loop

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 63 Andrew Isherwood 2018-11-07 14:52:37 UTC
Created attachment 82034 [details] [review]
Bug 20600: (follow-up) Remove date formatting

It is not required

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 64 Andrew Isherwood 2018-11-07 14:52:41 UTC
Created attachment 82035 [details] [review]
Bug 20600: (follow-up) Add date range filtering

It makes more sense to be able to filter placed and updated dates by
range. This patch adds that.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 65 Andrew Isherwood 2018-11-07 14:52:45 UTC
Created attachment 82036 [details] [review]
Bug 20600: (follow-up) Fix filtering

There was a bug whereby filtering by date would fail if the selected
date syspref was set to dmydot or metric. Rather than use the display
date value, We now use the non-display date value that is always in
a predictable ISO8601 format to construct the Date objects used
for comparison.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 66 Andrew Isherwood 2018-11-07 14:52:49 UTC
Created attachment 82037 [details] [review]
Bug 20600: (follow-up) Remove console.log!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 67 Andrew Isherwood 2018-11-07 14:52:53 UTC
Created attachment 82038 [details] [review]
Bug 20600: (follow-up) Move template JavaScript to the footer

This patch moves the ILL requests template's JavaScript to the footer.

Also changed:
 - Removed unused jQuery checkboxes plugin
 - Some <input> tags have been corrected to be self-closing.
 - Title tag has been corrected
 - Table row limit of 10 has been removed in favor of accepting default
   used elsewhere.

To test, apply the patch and follow the original test plan for this bug.
Everything should work as expected.

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 68 Andrew Isherwood 2018-11-07 14:52:58 UTC
Created attachment 82039 [details] [review]
Bug 20600: (follow-up) Rebase on current master

Significant things have changed in master due to ILL enhancement patches
being pushed. This patch rebases this bug on top of them
Comment 69 Andrew Isherwood 2018-11-08 11:04:54 UTC
Created attachment 82062 [details] [review]
Bug 20600: (follow-up) Restore patron name fix

The fix for patron name display when no first name is specified was lost
in the rebase. This patch restores it.
Comment 70 Andrew Isherwood 2018-11-09 14:39:09 UTC
Created attachment 82170 [details] [review]
Bug 20600: (follow-up) Fix API spec

We now return placed_formatted and updated_formatted, the API spec
should reflect this
Comment 71 Andrew Isherwood 2018-11-09 14:39:13 UTC
Created attachment 82171 [details] [review]
Bug 20600: (follow-up) Fix unit tests

The unit tests now need to be aware of the calculated placed_formatted
and updated_formatted properties in the API response object
Comment 72 Andrew Isherwood 2018-11-19 11:30:46 UTC
Created attachment 82432 [details] [review]
Bug 20600: Add filtering of ILL requests in list

This patch adds the ability for the ILL request list in the staff
interface to be filtered by a number of criteria:

- Status
- Date placed
- Date modified
- Pickup branch
- Borrower card number

To test:
1) Apply patch
2) Ensure you have a reasonable range of ILL requests created
3) Navigate to the "View ILL requests" page
4) Choose one or more filtering criteria
5) Click "Search"
6) Observe the results are filtered to match the selected criteria
7) Click "Clear"
8) Observe your results are returned to their initial state
9) Repeat steps 4 -> 6 until you are happy.

Dates supplied by the /api/v1/illrequests API route were not conforming
to the preference specified by the dateformat syspref. This patch
addresses that.

It has been addressed as part of this bug since we are adding filtering
of requests by some date fields and, therefore, needed dates in a
predictable format.

To test:
1) Apply the patch
2) Ensure you have at least one ILL request created
3) Make a request to the /api/v1/illrequests endpoint
4) Observe that dates supplied for "placed_formatted" & "updated_formatted" conform to your
dateformat syspref.
5) Change your date format syspref, repeat steps 3 & 4

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. Nice enh!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 73 Andrew Isherwood 2018-11-19 11:30:51 UTC
Created attachment 82433 [details] [review]
Bug 20600: (follow-up) Do not use $_ in loop

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 74 Andrew Isherwood 2018-11-19 11:30:55 UTC
Created attachment 82434 [details] [review]
Bug 20600: (follow-up) Remove date formatting

It is not required

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 75 Andrew Isherwood 2018-11-19 11:31:00 UTC
Created attachment 82435 [details] [review]
Bug 20600: (follow-up) Add date range filtering

It makes more sense to be able to filter placed and updated dates by
range. This patch adds that.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 76 Andrew Isherwood 2018-11-19 11:31:04 UTC
Created attachment 82436 [details] [review]
Bug 20600: (follow-up) Fix filtering

There was a bug whereby filtering by date would fail if the selected
date syspref was set to dmydot or metric. Rather than use the display
date value, We now use the non-display date value that is always in
a predictable ISO8601 format to construct the Date objects used
for comparison.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 77 Andrew Isherwood 2018-11-19 11:31:09 UTC
Created attachment 82437 [details] [review]
Bug 20600: (follow-up) Remove console.log!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 78 Andrew Isherwood 2018-11-19 11:31:13 UTC
Created attachment 82438 [details] [review]
Bug 20600: (follow-up) Move template JavaScript to the footer

This patch moves the ILL requests template's JavaScript to the footer.

Also changed:
 - Removed unused jQuery checkboxes plugin
 - Some <input> tags have been corrected to be self-closing.
 - Title tag has been corrected
 - Table row limit of 10 has been removed in favor of accepting default
   used elsewhere.

To test, apply the patch and follow the original test plan for this bug.
Everything should work as expected.

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 79 Andrew Isherwood 2018-11-19 11:31:17 UTC
Created attachment 82439 [details] [review]
Bug 20600: (follow-up) Rebase on current master

Significant things have changed in master due to ILL enhancement patches
being pushed. This patch rebases this bug on top of them
Comment 80 Andrew Isherwood 2018-11-19 11:31:22 UTC
Created attachment 82440 [details] [review]
Bug 20600: (follow-up) Restore patron name fix

The fix for patron name display when no first name is specified was lost
in the rebase. This patch restores it.
Comment 81 Andrew Isherwood 2018-11-19 11:31:26 UTC
Created attachment 82441 [details] [review]
Bug 20600: (follow-up) Fix API spec

We now return placed_formatted and updated_formatted, the API spec
should reflect this
Comment 82 Andrew Isherwood 2018-11-19 11:31:31 UTC
Created attachment 82442 [details] [review]
Bug 20600: (follow-up) Fix unit tests

The unit tests now need to be aware of the calculated placed_formatted
and updated_formatted properties in the API response object
Comment 83 Katrin Fischer 2019-01-11 16:34:35 UTC
Hi Andrew,

here some first findings:

1) Unit tests failing:

t/db_dependent/api/v1/illrequests.t .. 
    #   Failed test 'exact match for JSON Pointer ""'
    #   at t/db_dependent/api/v1/illrequests.t line 108.
    #     Structures begin differing at:
    #          $got->[0]{id_prefix} = ''
    #     $expected->[0]{id_prefix} = Does not exist

    #   Failed test 'exact match for JSON Pointer ""'
    #   at t/db_dependent/api/v1/illrequests.t line 143.
    #     Structures begin differing at:
    #          $got->[0]{id_prefix} = ''
    #     $expected->[0]{id_prefix} = Does not exist
    # Looks like you failed 2 tests of 18.
t/db_dependent/api/v1/illrequests.t .. 1/1 
#   Failed test 'list() tests'
#   at t/db_dependent/api/v1/illrequests.t line 155.
# Looks like you failed 1 test of 1.

2) QA tool complaining:


 FAIL	Koha/REST/V1/Illrequests.pm
   FAIL	  critic
		# Variables::ProhibitUnusedVariables: Got 1 violation(s).  

I am having some issues with my yarn... so couldn't test further yet. Can you please have a look at the above?
Comment 84 Andrew Isherwood 2019-01-14 10:42:36 UTC
Created attachment 83882 [details] [review]
Bug 20600: (follow-up) Fix tests

Some silly variable name typos in the tests
Comment 85 Andrew Isherwood 2019-01-14 11:08:24 UTC
(In reply to Katrin Fischer from comment #83)
> 
> 1) Unit tests failing:

Silly mistake in the test script, latest patch fixes it

> 2) QA tool complaining:
> 
> 
>  FAIL	Koha/REST/V1/Illrequests.pm
>    FAIL	  critic
> 		# Variables::ProhibitUnusedVariables: Got 1 violation(s).  
> 

I can't replicate this, I ran:

koha-qa.pl -v 2 -c 8

and didn't get any failures. What command did you run and did the error give any indication of what the unused variable was?
Comment 86 Martin Renvoize 2019-01-14 13:16:26 UTC
Created attachment 83887 [details] [review]
Bug 20600: (follow-up) Fix critic error
Comment 87 Martin Renvoize 2019-01-14 13:18:02 UTC
Sorted the critic error for you.. setting back to SO
Comment 88 Katrin Fischer 2019-02-04 19:08:16 UTC
Created attachment 84736 [details] [review]
Bug 20600: Add filtering of ILL requests in list

This patch adds the ability for the ILL request list in the staff
interface to be filtered by a number of criteria:

- Status
- Date placed
- Date modified
- Pickup branch
- Borrower card number

To test:
1) Apply patch
2) Ensure you have a reasonable range of ILL requests created
3) Navigate to the "View ILL requests" page
4) Choose one or more filtering criteria
5) Click "Search"
6) Observe the results are filtered to match the selected criteria
7) Click "Clear"
8) Observe your results are returned to their initial state
9) Repeat steps 4 -> 6 until you are happy.

Dates supplied by the /api/v1/illrequests API route were not conforming
to the preference specified by the dateformat syspref. This patch
addresses that.

It has been addressed as part of this bug since we are adding filtering
of requests by some date fields and, therefore, needed dates in a
predictable format.

To test:
1) Apply the patch
2) Ensure you have at least one ILL request created
3) Make a request to the /api/v1/illrequests endpoint
4) Observe that dates supplied for "placed_formatted" & "updated_formatted" conform to your
dateformat syspref.
5) Change your date format syspref, repeat steps 3 & 4

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. Nice enh!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 89 Katrin Fischer 2019-02-04 19:08:21 UTC
Created attachment 84737 [details] [review]
Bug 20600: (follow-up) Do not use $_ in loop

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 90 Katrin Fischer 2019-02-04 19:08:26 UTC
Created attachment 84738 [details] [review]
Bug 20600: (follow-up) Remove date formatting

It is not required

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 91 Katrin Fischer 2019-02-04 19:08:32 UTC
Created attachment 84739 [details] [review]
Bug 20600: (follow-up) Add date range filtering

It makes more sense to be able to filter placed and updated dates by
range. This patch adds that.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 92 Katrin Fischer 2019-02-04 19:08:37 UTC
Created attachment 84740 [details] [review]
Bug 20600: (follow-up) Fix filtering

There was a bug whereby filtering by date would fail if the selected
date syspref was set to dmydot or metric. Rather than use the display
date value, We now use the non-display date value that is always in
a predictable ISO8601 format to construct the Date objects used
for comparison.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 93 Katrin Fischer 2019-02-04 19:08:42 UTC
Created attachment 84741 [details] [review]
Bug 20600: (follow-up) Remove console.log!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 94 Katrin Fischer 2019-02-04 19:08:47 UTC
Created attachment 84742 [details] [review]
Bug 20600: (follow-up) Move template JavaScript to the footer

This patch moves the ILL requests template's JavaScript to the footer.

Also changed:
 - Removed unused jQuery checkboxes plugin
 - Some <input> tags have been corrected to be self-closing.
 - Title tag has been corrected
 - Table row limit of 10 has been removed in favor of accepting default
   used elsewhere.

To test, apply the patch and follow the original test plan for this bug.
Everything should work as expected.

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 95 Katrin Fischer 2019-02-04 19:08:53 UTC
Created attachment 84743 [details] [review]
Bug 20600: (follow-up) Rebase on current master

Significant things have changed in master due to ILL enhancement patches
being pushed. This patch rebases this bug on top of them

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 96 Katrin Fischer 2019-02-04 19:08:58 UTC
Created attachment 84744 [details] [review]
Bug 20600: (follow-up) Restore patron name fix

The fix for patron name display when no first name is specified was lost
in the rebase. This patch restores it.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 97 Katrin Fischer 2019-02-04 19:09:04 UTC
Created attachment 84745 [details] [review]
Bug 20600: (follow-up) Fix API spec

We now return placed_formatted and updated_formatted, the API spec
should reflect this

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 98 Katrin Fischer 2019-02-04 19:09:10 UTC
Created attachment 84746 [details] [review]
Bug 20600: (follow-up) Fix unit tests

The unit tests now need to be aware of the calculated placed_formatted
and updated_formatted properties in the API response object

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 99 Katrin Fischer 2019-02-04 19:09:15 UTC
Created attachment 84747 [details] [review]
Bug 20600: (follow-up) Fix tests

Some silly variable name typos in the tests

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 100 Katrin Fischer 2019-02-04 19:09:20 UTC
Created attachment 84748 [details] [review]
Bug 20600: (follow-up) Fix critic error

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 101 Katrin Fischer 2019-02-04 19:09:25 UTC
Created attachment 84749 [details] [review]
Bug 20600: (QA follow-up) Fix terminology and capitalization

- Subject Line > Subject line
- Patron barcode > Cardnumber
Comment 102 Katrin Fischer 2019-02-12 07:19:06 UTC
Hi Andrew, can you please rebase? One issue was from me adding the price filter, but was not sure about the second, so it would be great if you could take a look.
Comment 103 Katrin Fischer 2019-02-13 07:18:23 UTC
Even with the revert this still doesn't apply for me?
Comment 104 Katrin Fischer 2019-02-13 07:21:47 UTC
It's the same issue with the datatables code I had before it looks like, maybe the trouble was not the price patch? Please check.
Comment 105 Andrew Isherwood 2019-02-13 09:17:51 UTC
Created attachment 85037 [details] [review]
Bug 20600: Add filtering of ILL requests in list

This patch adds the ability for the ILL request list in the staff
interface to be filtered by a number of criteria:

- Status
- Date placed
- Date modified
- Pickup branch
- Borrower card number

To test:
1) Apply patch
2) Ensure you have a reasonable range of ILL requests created
3) Navigate to the "View ILL requests" page
4) Choose one or more filtering criteria
5) Click "Search"
6) Observe the results are filtered to match the selected criteria
7) Click "Clear"
8) Observe your results are returned to their initial state
9) Repeat steps 4 -> 6 until you are happy.

Dates supplied by the /api/v1/illrequests API route were not conforming
to the preference specified by the dateformat syspref. This patch
addresses that.

It has been addressed as part of this bug since we are adding filtering
of requests by some date fields and, therefore, needed dates in a
predictable format.

To test:
1) Apply the patch
2) Ensure you have at least one ILL request created
3) Make a request to the /api/v1/illrequests endpoint
4) Observe that dates supplied for "placed_formatted" & "updated_formatted" conform to your
dateformat syspref.
5) Change your date format syspref, repeat steps 3 & 4

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. Nice enh!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 106 Andrew Isherwood 2019-02-13 09:17:55 UTC
Created attachment 85038 [details] [review]
Bug 20600: (follow-up) Do not use $_ in loop

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 107 Andrew Isherwood 2019-02-13 09:17:59 UTC
Created attachment 85039 [details] [review]
Bug 20600: (follow-up) Remove date formatting

It is not required

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 108 Andrew Isherwood 2019-02-13 09:18:03 UTC
Created attachment 85040 [details] [review]
Bug 20600: (follow-up) Add date range filtering

It makes more sense to be able to filter placed and updated dates by
range. This patch adds that.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 109 Andrew Isherwood 2019-02-13 09:18:07 UTC
Created attachment 85041 [details] [review]
Bug 20600: (follow-up) Fix filtering

There was a bug whereby filtering by date would fail if the selected
date syspref was set to dmydot or metric. Rather than use the display
date value, We now use the non-display date value that is always in
a predictable ISO8601 format to construct the Date objects used
for comparison.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 110 Andrew Isherwood 2019-02-13 09:18:11 UTC
Created attachment 85042 [details] [review]
Bug 20600: (follow-up) Remove console.log!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 111 Andrew Isherwood 2019-02-13 09:18:15 UTC
Created attachment 85043 [details] [review]
Bug 20600: (follow-up) Move template JavaScript to the footer

This patch moves the ILL requests template's JavaScript to the footer.

Also changed:
 - Removed unused jQuery checkboxes plugin
 - Some <input> tags have been corrected to be self-closing.
 - Title tag has been corrected
 - Table row limit of 10 has been removed in favor of accepting default
   used elsewhere.

To test, apply the patch and follow the original test plan for this bug.
Everything should work as expected.

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 112 Andrew Isherwood 2019-02-13 09:18:19 UTC
Created attachment 85044 [details] [review]
Bug 20600: (follow-up) Rebase on current master

Significant things have changed in master due to ILL enhancement patches
being pushed. This patch rebases this bug on top of them

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 113 Andrew Isherwood 2019-02-13 09:18:23 UTC
Created attachment 85045 [details] [review]
Bug 20600: (follow-up) Restore patron name fix

The fix for patron name display when no first name is specified was lost
in the rebase. This patch restores it.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 114 Andrew Isherwood 2019-02-13 09:18:27 UTC
Created attachment 85046 [details] [review]
Bug 20600: (follow-up) Fix API spec

We now return placed_formatted and updated_formatted, the API spec
should reflect this

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 115 Andrew Isherwood 2019-02-13 09:18:31 UTC
Created attachment 85047 [details] [review]
Bug 20600: (follow-up) Fix unit tests

The unit tests now need to be aware of the calculated placed_formatted
and updated_formatted properties in the API response object

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 116 Andrew Isherwood 2019-02-13 09:18:35 UTC
Created attachment 85048 [details] [review]
Bug 20600: (follow-up) Fix tests

Some silly variable name typos in the tests

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 117 Andrew Isherwood 2019-02-13 09:18:39 UTC
Created attachment 85049 [details] [review]
Bug 20600: (follow-up) Fix critic error

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 118 Andrew Isherwood 2019-02-13 09:18:43 UTC
Created attachment 85050 [details] [review]
Bug 20600: (QA follow-up) Fix terminology and capitalization

- Subject Line > Subject line
- Patron barcode > Cardnumber
Comment 119 Andrew Isherwood 2019-02-13 09:20:48 UTC
Hi Katrin - I've now fixed the merge conflict. It should apply cleanly
Comment 120 Nick Clemens 2019-02-19 16:37:42 UTC
Awesome work all!

Pushed to master for 19.05
Comment 121 Martin Renvoize 2019-02-26 09:13:34 UTC
Enhancement, will not be backported to 18.11.x series.