---- Reported by nengard@gmail.com 2009-04-18 19:40:40 ---- It would be nice if there were check boxes next to reports so you can select this month's reports and delete them when you run next month's ... that way you don't end up with 100s of reports. --- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:06 UTC --- This bug was previously known as _bug_ 3134 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3134 Actual time not defined. Setting to 0.0 CC member bchurch@ptfs.com does not have an account here
Created attachment 14604 [details] [review] Bug 3134 - Ability to selelct multiple reports to delete at once This patch adds the option to select multiple saved reports for deletion. Text output of the report id (of little information value to the user) is replaced with a checkbox, and sorting has been removed from that table column. To test you must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion.
Owen, What was wrong with this patch? Nicole
Created attachment 19376 [details] [review] Bug 3134 - Ability to selelct multiple reports to delete at once This patch adds the option to select multiple saved reports for deletion. To test you must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion.
Comment on attachment 19376 [details] [review] Bug 3134 - Ability to selelct multiple reports to delete at once Grrr... Another prematurely-sent version of this patch. This one breaks the report group tabs. Back to the drawing board.
I actually think the number serves a purpose - rather than the name, which is not unique, the number gives librarians a way to identify a specific report - I often find myself asking "which number report are you running? There are three labeled "Circ stats - TEST"" or somesuch. Showing the number saves a lot of back and forth on that. Librarians often refer to the reports by number as well, just naturally. So please don't remove them. :) Liz
I'd like to keep the report numbers too - apart from what Liz said it's an easy way to find the newest reports.
Created attachment 19806 [details] [review] Bug 3134 - Ability to selelct multiple reports to delete at once This patch adds the option to select multiple saved reports for deletion. To test you must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion.
Created attachment 19843 [details] [review] Bug 3134 - Ability to selelct multiple reports to delete at once This patch adds the option to select multiple saved reports for deletion. To test you must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Functional tests pass, template tests pass.
Hi Owen, could you rebase your patch please? I tried but might have gone wrong somewhere. Would it be possible that you provide a unit test for the changed sub delete_report?
(In reply to Katrin Fischer from comment #9) > Hi Owen, > could you rebase your patch please? I tried but might have gone wrong > somewhere. Would it be possible that you provide a unit test for the changed > sub delete_report? I can provide the unit test for delete_report if necessary.
(In reply to kenza from comment #10) > (In reply to Katrin Fischer from comment #9) > > Hi Owen, > > could you rebase your patch please? I tried but might have gone wrong > > somewhere. Would it be possible that you provide a unit test for the changed > > sub delete_report? > > I can provide the unit test for delete_report if necessary. Bug 10761 - UT: saved_report and delete_report in C4::Reports::Report_Guided.pm need unit tests.
(In reply to kenza from comment #10) > I can provide the unit test for delete_report if necessary. I would appreciate that very much kenza.
Created attachment 20461 [details] [review] [SIGNED-OFF] Bug 3134 - Ability to selelct multiple reports to delete at once This patch adds the option to select multiple saved reports for deletion. To test you must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Functional tests pass, template tests pass.
QA comment: The first column contains the report id *and* a checkbox => checkboxes are not aligned. I suspect a wrong rebase. I think we want only the checkbox. Marked as Failed QA.
(In reply to Jonathan Druart from comment #14) > QA comment: > > The first column contains the report id *and* a checkbox => checkboxes are > not aligned. I suspect a wrong rebase. I think we want only the checkbox. > > Marked as Failed QA. Or maybe the checkbox should be before the id. Like that, checkboxes will be aligned.
Created attachment 20465 [details] [review] Bug 3134 [Follow-up] Ability to select multiple reports to delete at once This follow-up moves the checkboxes into a separate column from the report ID in order to prevent visual inconsistencies. To test, repeat the test plan from the original patch: You must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion.
Created attachment 20516 [details] [review] [SIGNED-OFF] Bug 3134 - Ability to selelct multiple reports to delete at once This patch adds the option to select multiple saved reports for deletion. To test you must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Functional tests pass, template tests pass. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 20517 [details] [review] [SIGNED OFF] Bug 3134 [Follow-up] Ability to select multiple reports to delete at once This follow-up moves the checkboxes into a separate column from the report ID in order to prevent visual inconsistencies. To test, repeat the test plan from the original patch: You must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> This works really nicely, passes all tests and the QA script. There is one little thing to note: If you delete a single report using the option from the menu, the line you want to delete is highlighted in red. It would be nice to do that also for deleting multiple reports.
QA comment: These patches add the ability to delete multiple reports. The second one fixes the ergonomic issue (described in comment 14). I will provide 3 followups: 1/ Fix the following issue: (In reply to Katrin Fischer from comment #18) > There is one little thing to note: If you delete a single report > using the option from the menu, the line you want to delete > is highlighted in red. It would be nice to do that also for > deleting multiple reports. 2/ Fix the sort on id and report name 3/ Reindent the delete_report routine (shouldn't it be delete_reports?) and remove the $sth->finish statement. Note for RM: feel free to push only the 2 first patches and to ask for a new report for the 3 followups. Marked as Passed QA.
Created attachment 20524 [details] [review] Bug 3134 - Ability to selelct multiple reports to delete at once This patch adds the option to select multiple saved reports for deletion. To test you must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Functional tests pass, template tests pass. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 20525 [details] [review] Bug 3134 [Follow-up] Ability to select multiple reports to delete at once This follow-up moves the checkboxes into a separate column from the report ID in order to prevent visual inconsistencies. To test, repeat the test plan from the original patch: You must have two or more saved reports to delete. Deletion should work properly when: - Selecting one report for deletion by checking the box. - Selecting more than one report for deletion by checking boxes. - Clicking the old "Delete" link Clicking the delete button should prompt you to confirm. Clicking cancel should cancel. Clicking the delete button when no boxes are checked should trigger an alert asking you to select reports for deletion. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> This works really nicely, passes all tests and the QA script. There is one little thing to note: If you delete a single report using the option from the menu, the line you want to delete is highlighted in red. It would be nice to do that also for deleting multiple reports. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 20526 [details] [review] Bug 3134: Add the warn class on tr elements This patch adds the same behavior when deleting 1 or more reports. When checkboxes are checked and the "delete selected" button is pressed, checked lines are hightlighted in red. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 20527 [details] [review] Bug 3134: ID and Report name columns are not well sorted. The sort for ID is alpha instead of numerical, same for the report names. To test: create id=1, name=Report 1 create id=2, name=Report 2 create id=10, name=Report 10 create id=11, name=Report 11 Sorting by id: without this patch 1, 10, 11, 2 with this patch: 1, 2, 10, 11 Sorting by name: without this patch: Report 1, Report 10, Report 11, Report 2 with this patch: Report 1, Report 2, Report 10, Report 11 Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 20528 [details] [review] Bug 3134: Reindent delete_report The first patch add a bad indentation for this routine. This patch fixes that. The $sth->finish statement is useless and should be removed. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Pushed to master. Thanks, Owen and Jonathan!