Bug 12403 - Batch record deletion
Summary: Batch record deletion
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature (vote)
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Keywords:
: 12050 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-06-11 09:36 UTC by Jonathan Druart
Modified: 2016-12-05 21:23 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12403: Add a batch record deletion (23.26 KB, patch)
2014-06-11 09:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12403: Add the new permission tools_records_batchdelitem (9.65 KB, patch)
2014-06-11 09:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12403: Don't check disabled checkboxes (1.23 KB, patch)
2014-06-11 09:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12403: Add a batch record deletion (23.09 KB, patch)
2014-07-08 07:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12403: Add the new permission tools_records_batchdelitem (9.65 KB, patch)
2014-07-08 07:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12403: Don't check disabled checkboxes (1.23 KB, patch)
2014-07-08 07:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12403: Add a batch record deletion (23.27 KB, patch)
2014-08-19 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12403: Add the new permission tools_records_batchdelitem (9.65 KB, patch)
2014-08-19 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12403: Add a batch record deletion (23.34 KB, patch)
2014-10-11 15:33 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 12403: Add the new permission tools_records_batchdelitem (9.71 KB, patch)
2014-10-11 15:33 UTC, Brendan Gallagher
Details | Diff | Splinter Review
[PASSED QA] Bug 12403: Add a batch record deletion (23.40 KB, patch)
2014-10-26 21:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12403: Add the new permission tools_records_batchdelitem (9.77 KB, patch)
2014-10-26 21:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12403: Follow-up - fixing a few typos (4.24 KB, patch)
2014-10-26 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12403 [Follow-up] Batch record deletion (23.65 KB, patch)
2014-10-28 15:15 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12403: Follow-up - fixing a few typos (4.32 KB, patch)
2014-10-28 19:03 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12403 [Follow-up] Batch record deletion (23.96 KB, patch)
2014-10-28 19:03 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 12403: Add a batch record deletion (23.46 KB, patch)
2014-10-28 20:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12403: Add the new permission tools_records_batchdelitem (9.83 KB, patch)
2014-10-28 20:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12403: Follow-up - fixing a few typos (4.37 KB, patch)
2014-10-28 20:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12403 [Follow-up] Batch record deletion (24.01 KB, patch)
2014-10-28 20:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12403: Add a batch record deletion (23.40 KB, patch)
2014-10-28 20:06 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12403: Add the new permission tools_records_batchdelitem (9.78 KB, patch)
2014-10-28 20:06 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12403: Follow-up - fixing a few typos (4.37 KB, patch)
2014-10-28 20:06 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12403 [Follow-up] Batch record deletion (24.01 KB, patch)
2014-10-28 20:06 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2014-06-11 09:36:13 UTC
This will add a new tool for deleting records: biblios and authorities.
Comment 1 Jonathan Druart 2014-06-11 09:38:15 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-06-11 09:38:20 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2014-06-11 09:38:25 UTC Comment hidden (obsolete)
Comment 4 Olli-Antti Kivilahti 2014-07-07 08:47:24 UTC
Hi there Jonathan!
Great to be signing off your patches again!

I am building batch removal of component parts on top of this feature.
Comment 5 Olli-Antti Kivilahti 2014-07-07 09:12:48 UTC
Could you make a C4/Koha-module out of ($op eq 'delete') feature in batch_delete_records.pl?

So it would be easier to reuse that.
Comment 6 Jonathan Druart 2014-07-07 11:39:28 UTC
(In reply to Olli-Antti Kivilahti from comment #5)
> Could you make a C4/Koha-module out of ($op eq 'delete') feature in
> batch_delete_records.pl?
> 
> So it would be easier to reuse that.

Hi Olli,
There is no important logic in the pl file.
Most of the code is error encapsulation.
Where do you want to reuse this part of code?
Comment 7 Olli-Antti Kivilahti 2014-07-07 12:47:20 UTC
Error encapsulation is important logic. :)

Just automatically make a Z39.50 search to find all component parts for a record, then ask does the user want to remove all of them, then remove all of them + parent record.

But I got it covered. Component parts don't have items so there are no different states to check for.
Comment 8 Bernardo Gonzalez Kriegel 2014-07-07 15:56:46 UTC
Hi Jonathan,

 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_delete_records.tt
   FAIL   forbidden patterns
                forbidden pattern: To include datatables files, please use the include file (see bug 10868) (line 6)

Captured by your own code :)
[http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10868#c4]
Comment 9 Jonathan Druart 2014-07-08 07:11:18 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2014-07-08 07:11:22 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2014-07-08 07:11:27 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2014-07-08 07:17:40 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #8)
> Captured by your own code :)
> [http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10868#c4]

Ooops :)
Actually I rebased this patch from an old version and I forgot to launch the qa script.

Note that the qa script still raises an error:

 FAIL   tools/batch_delete_records.pl
   FAIL   valid
                Smartmatch is experimental 
                Useless use of '\'; doesn't escape metacharacter '(' 

But it is not relevant, it's just because the script uses C4::Authorities (which raises this warning).
Comment 13 Bernardo Gonzalez Kriegel 2014-08-09 13:46:27 UTC
Hi Jonathan, testing again.
Now koha-qa pass, but:

1) Can't delete biblio records, got something like

Can't use string ("51") as a HASH ref while "strict refs" in use at /home/bgkriegel/kohaclone/C4/Reserves.pm line 292

2) Perhaps ergonomic issue, when you put some records on cart, and go to actions 'delete' what you get is the full tools > batch record deletion page, not a big big deal.

3) Deleting authorities works, but the message is confusing
It said

0 / 1 records have successfully been deleted. Some errors occurred

for one successful deletion, should be 1 / 1 i think
Comment 14 Jonathan Druart 2014-08-19 14:24:01 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2014-08-19 14:24:06 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2014-08-19 14:25:22 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #13)
> Hi Jonathan, testing again.

Thanks Bernardo!

> 1) Can't delete biblio records, got something like
> 
> Can't use string ("51") as a HASH ref while "strict refs" in use at
> /home/bgkriegel/kohaclone/C4/Reserves.pm line 292

Arg wrong call to GetReservesFromBiblionumber, should be fixed now.

> 2) Perhaps ergonomic issue, when you put some records on cart, and go to
> actions 'delete' what you get is the full tools > batch record deletion
> page, not a big big deal.

It's what is expected. What are you expected?

> 3) Deleting authorities works, but the message is confusing
> It said
> 
> 0 / 1 records have successfully been deleted. Some errors occurred
> 
> for one successful deletion, should be 1 / 1 i think

Fixed.
Comment 17 Brendan Gallagher 2014-10-11 15:33:12 UTC Comment hidden (obsolete)
Comment 18 Brendan Gallagher 2014-10-11 15:33:21 UTC Comment hidden (obsolete)
Comment 19 Katrin Fischer 2014-10-26 20:46:25 UTC
Working on this now.
Comment 20 Katrin Fischer 2014-10-26 21:44:04 UTC
I like this. :)

A possible enhancement I'd like to see would be adding an order and subscription count to the table as well. This would match the checks we do in acquisitions to make sure we don't delete something, that would have unwanted consequences. But not a blocker.
Comment 21 Katrin Fischer 2014-10-26 21:45:08 UTC Comment hidden (obsolete)
Comment 22 Katrin Fischer 2014-10-26 21:45:20 UTC Comment hidden (obsolete)
Comment 23 Katrin Fischer 2014-10-26 21:46:01 UTC Comment hidden (obsolete)
Comment 24 Owen Leonard 2014-10-28 15:15:24 UTC Comment hidden (obsolete)
Comment 25 Bernardo Gonzalez Kriegel 2014-10-28 19:03:30 UTC Comment hidden (obsolete)
Comment 26 Bernardo Gonzalez Kriegel 2014-10-28 19:03:39 UTC Comment hidden (obsolete)
Comment 27 Katrin Fischer 2014-10-28 20:01:52 UTC Comment hidden (obsolete)
Comment 28 Katrin Fischer 2014-10-28 20:01:55 UTC Comment hidden (obsolete)
Comment 29 Katrin Fischer 2014-10-28 20:01:59 UTC Comment hidden (obsolete)
Comment 30 Katrin Fischer 2014-10-28 20:02:02 UTC Comment hidden (obsolete)
Comment 31 Katrin Fischer 2014-10-28 20:06:41 UTC
Created attachment 32863 [details] [review]
[PASSED QA] Bug 12403: Add a batch record deletion

This patch offers a new tool for deleting records.
Biblios and authorities will can to be deleted with a simple list of
biblionumber or authid.

This feature adds:
- a new pl/tt files tools/batch_delete_records
- a new permission: tools > records_batchdel

Test plan for biblios:
1/ There are two ways to generate a list of biblionumbers:
- using the basket: do a search, add some biblio to your basket, open
  the basket and click on the "Action" button > "Delete"
- generating a list from a report
2/ On the "Batch record deletion" tool verify:
- biblios with issues cannot be deleted (checkbox disabled and line in
  red).
- information is correct.
- sort functions work on each columns.
- the items, reserves and issues values are correct.
3/ After clicking on the "Delete selected recors" button, verify:
- reserves, items and biblio have successful been deleted.
- if an error occurs, the tool display an error message.

Test plan for authority:
1/ Generate a list of authid using a report:
2/ On the "Batch record deletion" tool verify:
- authorities are display with the summary.
- the count usage (used in X biblios) is correct.
3/ After clicking on the "Delete selected recors" button, verify:
- The authorities have successful been deleted.
- if an error occurs, the tool display an error message.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 32 Katrin Fischer 2014-10-28 20:06:45 UTC
Created attachment 32864 [details] [review]
[PASSED QA] Bug 12403: Add the new permission tools_records_batchdelitem

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 33 Katrin Fischer 2014-10-28 20:06:49 UTC
Created attachment 32865 [details] [review]
[PASSED QA] Bug 12403: Follow-up - fixing a few typos

- Fixes the permission on the tools page
- Fixes some capitalization
- Changes 'Reserves' to 'Holds'

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 34 Katrin Fischer 2014-10-28 20:06:53 UTC
Created attachment 32866 [details] [review]
[PASSED QA] Bug 12403 [Follow-up] Batch record deletion

This follow-up makes several changes to language and markup with one
change in behavior:

- Updated permission description to avoid the use of the term "biblio"
- Updated batch delete template to avoid the use of the term
  "biblio" and to improve clarity.
- Replaced instances of terms "issue" and "reserve" with "checkout" and
  "hold" respectively.
- On bibliographic record batch delete, wrap biblionumber in <label>.
- On bibliographic record batch delete, show subtitle via Keywords to
  MARC mapping.
- On bibliographic record batch delete, respect BiblioDefaultView system
  preference.
- In the staff client cart, move batch delete link from "Action" menu
  to text link below (alongside "add to list" and "place hold"). The
  buttons are actions which affect the whole Cart. The links are actions
  which can be applied to selected items. I think this change makes it
  more consistent with the kind of operation being performed.

To test:

- Submit a batch of bibliographic records and confirm that the
  list of titles shows subtitle data as defined in Keywords to MARC
  mapping. Confirm also that the title links respect your
  BiblioDefaultView system preference.

- Perform a catalog search, select several titles, and add them to the
  Cart. Open the Cart and test the "Batch delete" link with and without
  titles selected. Test as a user who lacks batch biblio delete
  permission and confirm that the link does not appear.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described, no koha-qa errors

I love the rewording, we always translated 'biblio' as 'bibliographic record',
it makes a lot of sense.

Tested using direct input of biblionumber or cart, subtitle display shows
correctly, link respect preference, user without permission cant acces the
tool or links.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 35 Katrin Fischer 2014-11-02 08:35:46 UTC
*** Bug 12050 has been marked as a duplicate of this bug. ***
Comment 36 Tomás Cohen Arazi 2014-11-07 18:32:47 UTC
Patches pushed to master.

Thanks Jonathan!