| Summary: | Remove 'debug' information from patron statistics | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Katrin Fischer <katrin.fischer> |
| Component: | Reports | Assignee: | Andreas Roussos <arouss1980> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | arouss1980, martin.renvoize, mtompset, nick, veron |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | /cgi-bin/koha/reports/borrowers_stats.pl | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
screenshot of patron statistics
Bug 9188 - Remove 'debug' information from patron statistics Bug 9188 - Remove 'debug' information from patron statistics Bug 9188: counter patch -- just meaningful filter information Bug 9188: counter patch -- just meaningful filter information Bug 9188: Remove 'debug' information from patron statistics |
||
Apparently this is still relevant as of the current master (8a74bb2). It's easy to fix, so I'll submit a patch soon. Created attachment 55526 [details] [review] Bug 9188 - Remove 'debug' information from patron statistics In the staff client, if you submit a report under Reports -> Statistics wizards -> Patrons you see a lot of debug info with the heading "Filtered on:". This patch fixes that by adding an extra template parameter ('filtered_on_debug') in reports/borrowers_stats.pl and setting it to 0. The relevant template file is also updated to include a conditional directive which checks for the 'filtered_on_debug' variable. Debug info will only be shown if the variable is set to something other than 0. Finally, the associated help file template is updated to reflect the changes made. Test plan: 1) In the staff client, submit a report under Reports -> Statistics wizards -> Patrons. Observe that you see a lot of debug information. 2) Apply the patch. 3) Repeat step 1. Confirm that the patch works, i.e. you no longer see the debug information. Created attachment 55625 [details] [review] Bug 9188 - Remove 'debug' information from patron statistics In the staff client, if you submit a report under Reports -> Statistics wizards -> Patrons you see a lot of debug info with the heading "Filtered on:". This patch fixes that by adding an extra template parameter ('filtered_on_debug') in reports/borrowers_stats.pl and setting it to 0. The relevant template file is also updated to include a conditional directive which checks for the 'filtered_on_debug' variable. Debug info will only be shown if the variable is set to something other than 0. Finally, the associated help file template is updated to reflect the changes made. Test plan: 1) In the staff client, submit a report under Reports -> Statistics wizards -> Patrons. Observe that you see a lot of debug information. 2) Apply the patch. 3) Repeat step 1. Confirm that the patch works, i.e. you no longer see the debug information. Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Checking the other statistical reports I notice that they all show a 'Filtered on' - maybe instead of hiding this totally, we should try to clean up the bits that are not easy to understand? Or should we try and clean up the other reports too? What do you think? (In reply to Katrin Fischer from comment #4) > Checking the other statistical reports I notice that they all show a > 'Filtered on' - maybe instead of hiding this totally, we should try to clean > up the bits that are not easy to understand? Or should we try and clean up > the other reports too? > > What do you think? Well, the "Query =" bits are probably the ones that can confuse a non-tech savvy user the most and IMHO should not be displayed at all. As for the "Filtered on:" info, I think it clutters the display i.e. you have asked for a report, so you should get a report (and just that), not a repeat of what filters you chose when designing the report. Besides, when outputting to a file, you never get to see this "debug" info which (again, IMHO) is yet another reason to not display it at all unless a flag is set. Perhaps we've missed something, though. Usually when more people express their views a better decision can be made, so I was wondering if this bug is worth adding to the agenda of the next dev meeting on 28/09? May I also suggest changing the bug title to "Remove 'debug' information from report results" or something similar to reflect its wider scope? Sorry Andreas, this got lost in my bug mails. I am resetting to 'Needs sign-off' - maybe this will get us another point of view! I agree the top part is messy. However, I believe the things which are not filtered expressly don't need to be listed. For example, I chose a particular category code and branch code, and there would have been no way to know the branch code without the mess at the top. And yes, it was just a single number. I agree the Query parts are rather pointless. I think if you choose a row and column, there is no need to display a value for either the row or column unless it is filtered to that value. This means there should only be two lines at most at the top. As such, I don't think this patch is the correct way to do it. Created attachment 64087 [details] [review] Bug 9188: counter patch -- just meaningful filter information TEST PLAN --------- run patron statistics without filtering -- notice lots of text at top. run patron statistics with filtering -- notice lots of text at top. apply patch run it without and without filtering -- amount of text should be small or none and be related to the filtering selected. run koha qa test tools If you believe the counter patch is good, obsolete the first patch and sign off the second. If you believe there is need for more discussion, discuss away. Do not apply both patches, just one or the other. P.S. This does NOT fix the row/column not fully selected problem, as that is beyond scope. (In reply to M. Tompsett from comment #9) > If you believe the counter patch is good, obsolete the first patch and sign > off the second. If you believe there is need for more discussion, discuss > away. Do not apply both patches, just one or the other. Please accept my apologies for taking so long to reply. Thank you for providing a counter patch, I've had a look at it and it's good, definitely a better approach than mine ;-) Interestingly, it still applies to the current master after all this time without any need for rebasing. I'm going to sign it off and obsolete mine. Also, as per the patch submitting guidelines in the wiki, I'm raising the patch complexity from trivial to small. Created attachment 79659 [details] [review] Bug 9188: counter patch -- just meaningful filter information TEST PLAN --------- run patron statistics without filtering -- notice lots of text at top. run patron statistics with filtering -- notice lots of text at top. apply patch run it without and without filtering -- amount of text should be small or none and be related to the filtering selected. run koha qa test tools Signed-off-by: Andreas Roussos <arouss1980@gmail.com> Works as expected, i.e. only filtering information is displayed. Sorry, I've set the wrong status after signing off Mark's patch. Created attachment 79666 [details] [review] Bug 9188: Remove 'debug' information from patron statistics TEST PLAN --------- run patron statistics without filtering -- notice lots of text at top. run patron statistics with filtering -- notice lots of text at top. apply patch run it without and without filtering -- amount of text should be small or none and be related to the filtering selected. run koha qa test tools Signed-off-by: Andreas Roussos <arouss1980@gmail.com> Works as expected, i.e. only filtering information is displayed. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Awesome work all! Pushed to master for 18.11 Enhancement, will not be backported. |
Created attachment 13828 [details] screenshot of patron statistics Patron statistics show a lot of information that is confusing to a normal user and looks more like debug information than information that is intended for the end user. I think this information should be removed or hidden by default.