To recreate: 1. Have at least two libraries (Library A & Library B) and at least two patrons. One patron for Library A & one for Library B. 2. Create a manual fee for both patrons and write them off. 3. Go to the cash register statistics report and change the transaction type to "Write off" and your transaction library to Library A. 4. Notice that when you submit the report the transaction library defaults back to "All" and shows both patrons instead of just the patron from Library A.
Created attachment 155915 [details] [review] Bug 34829: Cash register report reverts to 'All libraries' after every search To test: 1. Have at least two libraries (Library A & Library B) and at least two patrons. One patron for Library A & one for Library B. 2. Create a manual fee for both patrons and write them off. 3. Go to the cash register statistics report and change the transaction type to "Write off" and your transaction library to Library A. 4. Notice that when you submit the report the transaction library defaults back to "All" and shows both patrons instead of just the patron from Library A. 5. Apply patch 6. Verify that the library chosen now remains selected Note: the export format is faulty, but this is unrelated to this patch.
Created attachment 155950 [details] [review] Bug 34829: Cash register report reverts to 'All libraries' after every search To test: 1. Have at least two libraries (Library A & Library B) and at least two patrons. One patron for Library A & one for Library B. 2. Create a manual fee for both patrons and write them off. 3. Go to the cash register statistics report and change the transaction type to "Write off" and your transaction library to Library A. 4. Notice that when you submit the report the transaction library defaults back to "All" and shows both patrons instead of just the patron from Library A. 5. Apply patch 6. Verify that the library chosen now remains selected Note: the export format is faulty, but this is unrelated to this patch. Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Works as described! Thanks for the patch, Katrin!
my $branch = $input->param("branch"); my $manager_branchcode = $input->param("branch") || C4::Context->userenv->{'branch'}; This does not look good. And this looks bad: - <option value="ALL">All</option> Script still has it: if ($manager_branchcode ne 'ALL') {
(In reply to Marcel de Rooy from comment #4) > my $branch = $input->param("branch"); > my $manager_branchcode = $input->param("branch") || > C4::Context->userenv->{'branch'}; > This does not look good. I am sorry, but this is not constructive. Especially since I didn't change this line. It only appears in the patch because of the perltidy. > > > And this looks bad: > - <option value="ALL">All</option> > Script still has it: > if ($manager_branchcode ne 'ALL') { Still, no explanation.
I am not an expert programmer, I am happy to change things around, but I need a little bit more insight in why you think something is wrong.
(In reply to Katrin Fischer from comment #6) > I am not an expert programmer, I am happy to change things around, but I > need a little bit more insight in why you think something is wrong. I will try. Should not be very hard?
(In reply to Marcel de Rooy from comment #4) > my $branch = $input->param("branch"); > my $manager_branchcode = $input->param("branch") || > C4::Context->userenv->{'branch'}; > This does not look good. You added $branch here, but we already have $manager_branchcode using the same CGI parameter. We should not do that. It will create confusion. If you need to know the branch code passed, use the existing variable. Change that etc. Do not clone it. > And this looks bad: > - <option value="ALL">All</option> > Script still has it: > if ($manager_branchcode ne 'ALL') { In the template you remove the ALL value of a combo box. You replace it by: + <option value="">All libraries</option> But the script still contains reference to the old value ALL. You should also solve those references. HTH