Bug 7678 - Statistics wizard: patrons
Summary: Statistics wizard: patrons
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-08 14:25 UTC by delaye
Modified: 2013-12-05 20:04 UTC (History)
8 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 7678: Adds extended patron attributes in patron reports (19.53 KB, patch)
2012-08-08 14:32 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7678: Adds extended patron attributes in patron reports (19.69 KB, patch)
2012-08-14 07:53 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 7678: follow-up to pass QA (2.04 KB, patch)
2012-08-20 07:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7678: follow-up to pass QA (2.09 KB, patch)
2012-10-09 11:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 7678: follow-up to pass QA (2.70 KB, patch)
2012-10-10 08:22 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7678: Adds extended patron attributes in patron reports (19.73 KB, patch)
2012-10-19 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7678: follow-up to pass QA (2.80 KB, patch)
2012-10-19 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7678: Adds extended patron attributes in patron reports (19.76 KB, patch)
2012-12-02 23:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7678: follow-up to pass QA (3.13 KB, patch)
2012-12-02 23:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7678: Adds extended patron attributes in patron reports (975 bytes, patch)
2012-12-10 11:26 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7678: Adds extended patron attributes in patron reports (1.02 KB, patch)
2013-01-18 19:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7678: Adds extended patron attributes in patron reports (1.16 KB, patch)
2013-01-20 20:42 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 delaye 2012-03-08 14:25:41 UTC
The patron stat wizard ( http://koha-community:8080/cgi-bin/koha/reports/borrowers_stats.pl) should also support filter by extended attributes (with the exclusion of those being defined as password) For those who are related to authorised_values, the wizard will display the list on the filter column.

Add filter and information : patron branch
Add filter and information  dateenrolled  (after / before)
Comment 1 Julian Maurice 2012-08-08 14:32:31 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2012-08-14 07:53:36 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2012-08-16 14:07:20 UTC
QA comments:

1/ In the routine "calculate", the variables $line and $digit have to be escaped.

2/ Maybe this line:
  for (my $i = 0; $i <= scalar @$filters; $i++) {
could be replaced with
  for my $i ( 0 .. scalar @$filters ) {
For more readability (not mandatory) :)

Marked as Failed QA for 1
Comment 4 Julian Maurice 2012-08-20 07:04:15 UTC Comment hidden (obsolete)
Comment 5 delaye 2012-08-31 10:02:31 UTC
i test the patch in a sandbox but does not work , i have a white screen when i run the report.....and when i select a  patron attributes in the report

and i don't see the filter  dateenrolled  (after / before)
Comment 6 MathildeF 2012-09-06 13:43:44 UTC
also tested in a sandbox

no white screen for me
but:
- Filters on authorised_value does not work: I have selected a single value, and the results table displays all the values
- I dont see "dateenrolled"
Comment 7 claire.hernandez@biblibre.com 2012-09-07 13:13:57 UTC
I change the patch status for review.
Comment 8 Julian Maurice 2012-10-09 11:33:07 UTC Comment hidden (obsolete)
Comment 9 Julian Maurice 2012-10-10 08:22:49 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2012-10-19 13:17:18 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2012-10-19 13:17:38 UTC Comment hidden (obsolete)
Comment 12 Jared Camins-Esakov 2012-11-30 18:39:46 UTC
Is this patch for 3.8 only? If not, please update the version listed.
Comment 13 Julian Maurice 2012-11-30 22:35:22 UTC
This is an enhancement, so this is for master.
Version updated accordingly.
Comment 14 Katrin Fischer 2012-12-02 22:40:23 UTC
Script fails perlcritic before and after applying the test:
Subroutine prototypes used at line 149, column 1.  See page 194 of PBP.  (Severity: 5)
Subroutine prototypes used at line 154, column 1.  See page 194 of PBP.  (Severity: 5)
Comment 15 Katrin Fischer 2012-12-02 23:04:42 UTC
Created attachment 13829 [details] [review]
[SIGNED-OFF] Bug 7678: Adds extended patron attributes in patron reports

This patch allows to choose row and column amongst extended patron
attributes and to filter on them in reports/borrower_stats.pl

Signed-off-by: Marc Veron <veron@veron.ch>

Great, I was looking for such a feature.
Patch works as expected.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 16 Katrin Fischer 2012-12-02 23:04:52 UTC
Created attachment 13830 [details] [review]
[SIGNED-OFF] Bug 7678: follow-up to pass QA

Check input parameters to avoid SQL injection
Rewrite a for loop for readability

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests pass, perlcritic is unchanged.

Test plan
- Verify ExtendedPatronAttributes hides/shows new patron attribute
  search criteria
- Verify patron attribute show up correctly
  - pull down list for attributes linked to authorized values
  - input field for other attributes
- Verify search works correctly
Comment 17 Jared Camins-Esakov 2012-12-03 11:38:48 UTC
I love the idea, unfortunately I found a problem: if you do a report on an extended attribute that permits both NULL *and* zero values the zero values are not shown:
branchcode / SHOW_BCODE		1	NULL	TOTAL
CPL	 	 			1	1
FRL	 	 			9	9 
MPL	 	 			18	18
PVL	 	 			1	1
SPL			 	1	20	22
TOTAL			0	1	49	51

There is one patron at SPL with SHOW_BCODE set to 0, a fact which is not reflected either in the SPL line or the total.
Comment 18 Julian Maurice 2012-12-10 11:26:31 UTC Comment hidden (obsolete)
Comment 19 Julian Maurice 2012-12-10 11:27:38 UTC
Thanks for the tests Jared, patch above should fix the problem.
Comment 20 Kyle M Hall 2013-01-18 19:56:41 UTC Comment hidden (obsolete)
Comment 21 Katrin Fischer 2013-01-20 20:42:57 UTC
Created attachment 14728 [details] [review]
[SIGNED-OFF] Bug 7678: Adds extended patron attributes in patron reports

Fix bug that prevented zero values to be shown in the results.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixes the problem with differentiating 0 and NULL values.
Passes all tests.
Comment 22 Jared Camins-Esakov 2013-01-22 23:34:36 UTC
This patch has been pushed to master.
Comment 23 Julian Maurice 2013-02-01 11:19:17 UTC
Ok in master.