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

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


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

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