Bug 23273 - Downloading from overdues.pl doesn't use set filters
Summary: Downloading from overdues.pl doesn't use set filters
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 21852
  Show dependency treegraph
 
Reported: 2019-07-05 20:31 UTC by Andrew Fuerste-Henry
Modified: 2019-09-24 18:24 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.04


Attachments
Bug 23273: Fix CSV export for overdues (6.75 KB, patch)
2019-08-19 22:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23281: Cleanup Koha::Patron::Attribute::Types->search (1.73 KB, patch)
2019-08-19 23:47 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 23273: Fix CSV export for overdues (6.80 KB, patch)
2019-08-19 23:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 23273: Fix CSV export for overdues (6.86 KB, patch)
2019-08-20 05:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23273: Remove debug statement (701 bytes, patch)
2019-09-01 15:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23273: Remove debug statement (758 bytes, patch)
2019-09-01 21:04 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 Fuerste-Henry 2019-07-05 20:31:35 UTC
To Test:
- go to overdues.pl
- construct a search limiting by date due and library
- note number of overdues in results
- click "Download file of displayed overdues"
- note that downloaded file contains all overdues, not just those in your search
Comment 2 Katrin Fischer 2019-08-18 21:16:47 UTC
Confirmed on master.

This looks like it could be a problem with the construction of the URL/use of filters.
Comment 3 Jonathan Druart 2019-08-19 22:13:10 UTC
Created attachment 92365 [details] [review]
Bug 23273: Fix CSV export for overdues

We should construct the URI parameters string manually to avoid
filtering problems.
We cannot send the full query_string to the template and expect that the
string will be escaped correctly.

Test plan:
- go to overdues.pl
- construct a search limiting by date due and library
- note number of overdues in results
- click "Download file of displayed overdues"
=> note that downloaded file contains just those in your search
Comment 4 Owen Leonard 2019-08-19 23:47:46 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2019-08-19 23:51:04 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2019-08-19 23:52:40 UTC
Created attachment 92368 [details] [review]
Bug 23273: Fix CSV export for overdues

We should construct the URI parameters string manually to avoid
filtering problems.
We cannot send the full query_string to the template and expect that the
string will be escaped correctly.

Test plan:
- go to overdues.pl
- construct a search limiting by date due and library
- note number of overdues in results
- click "Download file of displayed overdues"
=> note that downloaded file contains just those in your search

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 7 Katrin Fischer 2019-08-20 05:55:31 UTC
Created attachment 92372 [details] [review]
Bug 23273: Fix CSV export for overdues

We should construct the URI parameters string manually to avoid
filtering problems.
We cannot send the full query_string to the template and expect that the
string will be escaped correctly.

Test plan:
- go to overdues.pl
- construct a search limiting by date due and library
- note number of overdues in results
- click "Download file of displayed overdues"
=> note that downloaded file contains just those in your search

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2019-08-20 05:56:44 UTC
We are missing a filter here, should be an easy fix, but was not sure which one to use here:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt
   FAIL	  filters
		missing_filter at line 39 (            [% url_params = BLOCK %][% url_params %]&amp;[% var | uri %]=[% filters.$var | uri %][% END %])

Leaving PQA for now.
Comment 9 Martin Renvoize 2019-08-20 14:02:30 UTC
Nice work!

Pushed to master for 19.11.00
Comment 10 Jonathan Druart 2019-09-01 14:46:28 UTC
(In reply to Katrin Fischer from comment #8)
> We are missing a filter here, should be an easy fix, but was not sure which
> one to use here:
> 
>  FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt
>    FAIL	  filters
> 		missing_filter at line 39 (            [% url_params = BLOCK %][%
> url_params %]&amp;[% var | uri %]=[% filters.$var | uri %][% END %])
> 
> Leaving PQA for now.

Martin, you added:
-            [% url_params = BLOCK %][% url_params %]&amp;[% var | uri %]=[% filters.$var | uri %][% END %]
+            [% url_params = BLOCK %][% url_params | uri %]&amp;[% var | uri %]=[% filters.$var | uri %][% END %]


It feel wrong, it should be $raw, otherwise it will be double encoded.
Comment 11 Jonathan Druart 2019-09-01 15:35:07 UTC
Also, there was a debug statement:

+use Data::Printer colored => 1; warn p $filters;
Comment 12 Jonathan Druart 2019-09-01 15:35:53 UTC
Created attachment 92517 [details] [review]
Bug 23273: Remove debug statement
Comment 13 Katrin Fischer 2019-09-01 21:04:22 UTC
Created attachment 92527 [details] [review]
Bug 23273: Remove debug statement

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2019-09-01 21:04:43 UTC
Moving into PQA queue for better visibility.
Comment 15 Fridolin SOMERS 2019-09-05 15:13:54 UTC
Pushed to 19.05.x for 19.05.04
Comment 16 Fridolin SOMERS 2019-09-06 08:05:58 UTC
Sorry I went to fast on backporting this.
A patch is missing in master and i found an issue.
I revert for now from 19.05.x
Comment 17 Martin Renvoize 2019-09-06 09:19:25 UTC
Nice work!

Pushed to master for 19.11.00
Comment 18 Martin Renvoize 2019-09-06 09:20:42 UTC
Followup pushed and filters corrected.
Comment 19 Fridolin SOMERS 2019-09-06 09:40:14 UTC
Pushed to 19.05.x for 19.05.04
Comment 20 Lucas Gass 2019-09-24 18:24:24 UTC
backported to 18.11.x for 18.11.10