Summary: | Holds to Pull : Show pull list on load. Otherwise, it looks like the pull list is empty. | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Hold requests | Assignee: | David Cook <dcook> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | gmcharlt, jonathan.druart, paul.poulain |
Version: | 3.10 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 18079 | ||
Attachments: |
Bug 8454 - Holds to Pull : Show pull list on load. Otherwise, it looks like the pull list is empty.
Bug 8454 - Holds to Pull : Show pull list on load. Otherwise, it looks like the pull list is empty. Bug 8454 - Show pull list on load. Otherwise, it looks like the pull list is empty - Amended Bug 8454 - Holds to Pull : Show pull list on load. Otherwise, it looks like the pull list is empty. |
Description
David Cook
2012-07-17 01:26:04 UTC
Created attachment 10880 [details] [review] Bug 8454 - Holds to Pull : Show pull list on load. Otherwise, it looks like the pull list is empty. Currently, when you click "Holds to pull" from the Circulation menu, the only thing that you see is a white screen and a side navigation bar that says "Refine Results". Since there appear to be no results to refine, the holds to pull list appears completely empty. Staff have to know to click "Submit" on the "Refine Results" form to get anything. I propose that we have the "run_report" flag set to ON for the initial load. The report uses the default 2 days mentioned in the "Refine Results" form, so there is continuity across the board. Patch tested with a sandbox, by Delaye Stephane <stephane.delaye@biblibre.com> Created attachment 11499 [details] [review] Bug 8454 - Holds to Pull : Show pull list on load. Otherwise, it looks like the pull list is empty. Currently, when you click "Holds to pull" from the Circulation menu, the only thing that you see is a white screen and a side navigation bar that says "Refine Results". Since there appear to be no results to refine, the holds to pull list appears completely empty. Staff have to know to click "Submit" on the "Refine Results" form to get anything. I propose that we have the "run_report" flag set to ON for the initial load. The report uses the default 2 days mentioned in the "Refine Results" form, so there is continuity across the board. Signed-off-by: Delaye Stephane <stephane.delaye@biblibre.com> QA Comments: Your patch is the same as if ( 1 ){} I think a better way to do this is : my $run_report = ( not defined $input->param('run_report') ) ? 1 : $input->param('run_report'); If someone wants to load this page without running the report it is possible. I figured the "if ( 1 ){}" approach would be the easiest, as it meant the fewest changes to the code, but I'm certainly up for alternative strategies :). I'm not sure that I understand your conditional here though. Doesn't that say that $run_report will be 1, if $input->param('run_report') isn't set/defined? Otherwise, if it is defined...then it will just be the param that's passed, which amounts to 1 anyway? In either case, it seems inevitable that the page will load the report upon page load, which I'm proposing is a good idea. However, in that event, how would it be possible for someone to load this page without running the report, as you suggest? (In reply to comment #4) > QA Comments: > > Your patch is the same as > if ( 1 ){} > > I think a better way to do this is : > my $run_report = ( not defined $input->param('run_report') ) ? 1 : > $input->param('run_report'); > > If someone wants to load this page without running the report it is possible. (In reply to comment #5) > I figured the "if ( 1 ){}" approach would be the easiest, as it meant the > fewest changes to the code, but I'm certainly up for alternative strategies > :). > > I'm not sure that I understand your conditional here though. Doesn't that > say that $run_report will be 1, if $input->param('run_report') isn't > set/defined? Otherwise, if it is defined...then it will just be the param > that's passed, which amounts to 1 anyway? > > In either case, it seems inevitable that the page will load the report upon > page load, which I'm proposing is a good idea. > > However, in that event, how would it be possible for someone to load this > page without running the report, as you suggest? Hi David, Your patch does not allow to open the page without launching the report. For me it is not logical to have a cgi param (run_report) and to not check it. And "the fewest changes to the code" is not a good reason for me if it is not consistent :) If, on the master code, you replace my $run_report=$input->param('run_report'); with my $run_report = ( not defined $input->param('run_report') ) ? 1 : $input->param('run_report'); the page can be displayed without the report if you call it with run_report=0. In all others cases, the report will be displayed (even if the param is not passed). I think it is a good behaviour. Created attachment 12181 [details] [review] Bug 8454 - Show pull list on load. Otherwise, it looks like the pull list is empty - Amended Ahh, I see your logic now. Here's an amended patch that uses your idea. Created attachment 12182 [details] [review] Bug 8454 - Holds to Pull : Show pull list on load. Otherwise, it looks like the pull list is empty. Currently, when you click "Holds to pull" from the Circulation menu, the only thing that you see is a white screen and a side navigation bar that says "Refine Results". Since there appear to be no results to refine, the holds to pull list appears completely empty. Staff have to know to click "Submit" on the "Refine Results" form to get anything. I propose that we have the "run_report" flag set to ON for the initial load. The report uses the default 2 days mentioned in the "Refine Results" form, so there is continuity across the board. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> QA comment: * very tiny patch, no side effect risk, just a default value added * koha-qa.pl OK Patch pushed to master |