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)
Created attachment 11445 [details] [review] 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
Created attachment 11597 [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.
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
Created attachment 11687 [details] [review] Bug 7678: follow-up to pass QA Check input parameters to avoid SQL injection Rewrite a for loop for readability
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)
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"
I change the patch status for review.
Created attachment 12742 [details] [review] Bug 7678: follow-up to pass QA Check input parameters to avoid SQL injection Rewrite a for loop for readability -- Previous patch didn't take into account patron attributes. That's why we had blank screen.
Created attachment 12753 [details] [review] Bug 7678: follow-up to pass QA Check input parameters to avoid SQL injection Rewrite a for loop for readability -- Improved checks on patron attribute types
Created attachment 12941 [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>
Created attachment 12942 [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>
Is this patch for 3.8 only? If not, please update the version listed.
This is an enhancement, so this is for master. Version updated accordingly.
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)
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>
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
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.
Created attachment 13975 [details] [review] Bug 7678: Adds extended patron attributes in patron reports Fix bug that prevented zero values to be shown in the results.
Thanks for the tests Jared, patch above should fix the problem.
Created attachment 14710 [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>
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.
This patch has been pushed to master.
Ok in master.