Bug 11395

Summary: Batch modifications for records
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: ToolsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: new feature    
Priority: P5 - low CC: bob, brendan, cedric.vita, francois.pichenot, helen.linda, katrin.fischer, koha, kyle, luce.barbey, m.de.rooy, marjorie.barry-vila, nengard, paola.rossi, patrick.robitaille, tomascohen, veron
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5725
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8015
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14098
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 11413    
Bug Blocks: 12994, 13606, 14795, 15189, 15579, 22245    
Attachments: Bug 11395: Add a batch record modification
Bug 11395: Add links to the new tool page
Bug 11395: Fix return for ModifyRecordWithTemplate
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: New service to preview a record.
form screen
list screen
Bug 11395: The modification template should be changed on the list view
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: The modification template should be changed on the list view
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: Add a batch record modification
Bug 11395: Add links to the new tool page
Bug 11395: Fix return for ModifyRecordWithTemplate
Bug 11395: New service to preview a record.
Bug 11395: The modification template should be changed on the list view
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: Fix field_numbers
Bug 11395: Fix path for the checkboxes jQuery plugin
Bug 11395: prevent processing no record
Bug 11395: Raise an alert if control field is used
screen-1
screen-2
Bug 11395: exit should be done after displaying the output
Bug 11395: Add a batch record modification
Bug 11395: Add links to the new tool page
Bug 11395: Fix return for ModifyRecordWithTemplate
Bug 11395: New service to preview a record.
Bug 11395: The modification template should be changed on the list view
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: Fix field_numbers
Bug 11395: Fix path for the checkboxes jQuery plugin
Bug 11395: prevent processing no record
Bug 11395: Raise an alert if control field is used
Bug 11395: exit should be done after displaying the output
Bug 11395: Add a batch record modification
Bug 11395: Add links to the new tool page
Bug 11395: Fix return for ModifyRecordWithTemplate
Bug 11395: New service to preview a record.
Bug 11395: The modification template should be changed on the list view
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: Fix field_numbers
Bug 11395: Fix path for the checkboxes jQuery plugin
Bug 11395: prevent processing no record
Bug 11395: Raise an alert if control field is used
Bug 11395: exit should be done after displaying the output
Bug 11395: Add a batch record modification
Bug 11395: Add links to the new tool page
Bug 11395: New service to preview a record.
Bug 11395: The modification template should be changed on the list view
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: Fix path for the checkboxes jQuery plugin
Bug 11395: prevent processing no record
Bug 11395: Raise an alert if control field is used
Bug 11395: exit should be done after displaying the output
Bug 11395: A control field is a field with tag < 10
[PASSED QA] Bug 11395: Add a batch record modification
[PASSED QA] Bug 11395: Add a batch record modification
[PASSED QA] Bug 11395: Add links to the new tool page
[PASSED QA] Bug 11395: New service to preview a record.
[PASSED QA] Bug 11395: The modification template should be changed on the list view
[PASSED QA] Bug 11395: DB: Add permission tools_records_batchmod
[PASSED QA] Bug 11395: Fix path for the checkboxes jQuery plugin
[PASSED QA] Bug 11395: prevent processing no record
[PASSED QA] Bug 11395: Raise an alert if control field is used
[PASSED QA] Bug 11395: exit should be done after displaying the output
[PASSED QA] Bug 11395: A control field is a field with tag < 10
Bug 11395: Add a batch record modification
Bug 11395: Add links to the new tool page
Bug 11395: New service to preview a record.
Bug 11395: The modification template should be changed on the list view
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: Fix path for the checkboxes jQuery plugin
Bug 11395: prevent processing no record
Bug 11395: Raise an alert if control field is used
Bug 11395: exit should be done after displaying the output
Bug 11395: A control field is a field with tag < 10
Bug 11395: Add a batch record modification
Bug 11395: Add links to the new tool page
Bug 11395: New service to preview a record.
Bug 11395: The modification template should be changed on the list view
Bug 11395: DB: Add permission tools_records_batchmod
Bug 11395: Fix path for the checkboxes jQuery plugin
Bug 11395: prevent processing no record
Bug 11395: exit should be done after displaying the output
[FAILED_QA] Bug 11395: Raise an alert if control field is used
[FAILED_QA] Bug 11395: A control field is a field with tag < 10

Description Jonathan Druart 2013-12-13 09:45:10 UTC
This report revives bug 5725.
Since bug 8015 is pushed, it is easy to provide a new batch record modification using the existing API.
Here, we just have to create a new script without any logic on the record modifications.
By "records", I include both biblios and authorities.
Comment 1 Jonathan Druart 2013-12-13 10:36:49 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-12-13 10:36:55 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2013-12-13 10:37:03 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2013-12-13 10:37:12 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2013-12-13 10:37:20 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2013-12-13 10:42:31 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2013-12-13 10:43:07 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2013-12-13 10:47:11 UTC
Note that bug 11319 improves and fixes some inconsistencies of the marc modification templates feature.
I tested this feature on top of the improvement but it applies well against master.
Comment 9 Jonathan Druart 2013-12-17 10:43:09 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2014-01-23 08:44:43 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2014-03-11 11:03:08 UTC Comment hidden (obsolete)
Comment 12 francois.pichenot 2014-03-11 12:50:35 UTC
I tried to add a new value in 606$a.
Modifications appear in 'normal' view.
They don't appear in :
• 'marc' view
• 'ISBD' views
• SQL report like "select ExtractValue(marcxml,'//datafield[@tag="606"]/subfield[@code>="a"]') ..."
Comment 13 Jonathan Druart 2014-03-11 14:57:18 UTC
(In reply to francois.pichenot from comment #12)
> I tried to add a new value in 606$a.

Could you please detail the rule you used?
Comment 14 francois.pichenot 2014-03-12 08:29:04 UTC
(In reply to Jonathan Druart from comment #13)
> (In reply to francois.pichenot from comment #12)
> > I tried to add a new value in 606$a.
> 
> Could you please detail the rule you used?

The rule was 'Update field 609$a with value pomme de terre' and not 606$a... Sorry.
So, the SQL report is right, but not marc and ISBD view. Maybe a problem with 609 and xslt ?

With the rule 'Update field 606$a with value carottes', all is right.
Comment 15 Jonathan Druart 2014-04-10 15:22:08 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2014-04-11 12:41:53 UTC
None of these patches appear to be signed off, change status to "Needs Signoff".
Comment 17 Koha Team University Lyon 3 2014-06-18 15:33:54 UTC
<h1>Something went wrong !</h1>Applying: Bug 11395: Add a batch record modification
Applying: Bug 11395: Add links to the new tool page
Applying: Bug 11395: Fix return for ModifyRecordWithTemplate
Applying: Bug 11395: New service to preview a record.
Applying: Bug 11395: DB: Add permission tools_records_batchmod Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/de-DE/mandatory/userpermissions.sql
Auto-merging installer/data/mysql/en/mandatory/userpermissions.sql
Auto-merging installer/data/mysql/es-ES/mandatory/userpermissions.sql
Auto-merging installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql
Auto-merging installer/data/mysql/it-IT/necessari/userpermissions.sql
CONFLICT (content): Merge conflict in installer/data/mysql/it-IT/necessari/userpermissions.sql
Auto-merging installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql
Auto-merging installer/data/mysql/pl-PL/mandatory/userpermissions.sql
Auto-merging installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql
Auto-merging installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 11395: DB: Add permission tools_records_batchmod When you have resolved this problem run git bz apply --continue.
Comment 18 Jonathan Druart 2014-06-19 08:06:08 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2014-06-19 10:44:56 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2014-06-19 10:45:01 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2014-06-19 10:45:06 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2014-06-19 10:45:12 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2014-06-19 10:45:16 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2014-06-19 10:45:21 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2014-06-19 10:47:02 UTC
I added the dependency on bug 11413 since this enhancement is dangerous without fixing the current issues.
Comment 26 Jonathan Druart 2014-06-19 16:21:16 UTC Comment hidden (obsolete)
Comment 27 Jonathan Druart 2014-06-19 16:22:15 UTC
(In reply to Jonathan Druart from comment #26)
> Created attachment 29056 [details] [review] [review]
> Bug 11395: Fix field_numbers

This issue has been raised by the Lyon 3 Team during the SO day.
Comment 28 Kyle M Hall 2014-07-03 15:00:16 UTC
Applying: Bug 11395: Fix return for ModifyRecordWithTemplate
fatal: sha1 information is lacking or useless (C4/MarcModificationTemplates.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Comment 29 Kyle M Hall 2014-07-03 18:00:38 UTC
* Select All / Clear All links do not work
* Links should probably be "Select all" and "Clear all"
* Preview MARC does not open record in a modal window
* When I click the "Modify selected records" button, nothing happens.
Comment 30 Kyle M Hall 2014-07-03 18:01:15 UTC
GET http://staff-kohaqa.koha-dev.com/intranettmpl/prog/en/lib/jquery/plugins/jquery.checkboxes.min.js 404 (Not Found) batch_record_modification.pl:113

Uncaught TypeError: undefined is not a function batch_record_modification.pl:120
Comment 31 Jonathan Druart 2014-07-04 08:43:33 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2014-07-04 08:45:34 UTC
(In reply to Kyle M Hall from comment #30)
> GET
> http://staff-kohaqa.koha-dev.com/intranettmpl/prog/en/lib/jquery/plugins/
> jquery.checkboxes.min.js 404 (Not Found) batch_record_modification.pl:113
> 
> Uncaught TypeError: undefined is not a function
> batch_record_modification.pl:120

Kyle, thank you for testing!
The JS error caused unexpected behaviors. Last patch fixes the path (and the wording issue).
All issues you raised should be fixed.
Comment 33 Paola Rossi 2014-07-07 16:09:57 UTC
Today I've tried to test the patches against master 3.17.00.009

I've applied:
5 patches of 11319,
3 patches of 11914,
and the 8 patches of the current 11395.

I've found three errors.

1) ERROR on MARC modification templates: unavailable "Every" option:

I've edited this action:
Copy field 650$x to 650$y if 650$x matches RegEx m/ ^second 650 / 

Then I've changed all the 650-s into 750-s. I see "All" becoming "Every" (I have not understood why it has happened). I've saved, and I've editing the action again: now "All" is shown.
I've not been able to set "Every" option. 
I think that showing "Every" is an error, or there's a lack in managing that option (see C) below).

So I've passed the patch to "Failed QA" status.
--------------------------------------------------------------------
2) On Batch record modification [and on cart Action Modify too], I've cleared all the listed records.
I've selected "Modify the selected records". Instead of "No record was selected", the program has started, and then has ended showing:
"All records have successfully been modified! ".

--------------------------------------------------------------------
3) ERROR on batch_record_modification.pl [I've checked only "Preview MARC"]:
case of repeteable subfield [x] in repeteable fields [650]:

My biblio record had 3 fields 650, each one had 3 $x:
650  0 ......
       _x1-650 x 1 Digital techniques 1
       _xfirst 650, second $x
       _x1-650 x 3 Digital techniques 3
650  0 ........
       _x2-650 x 1 Digital techniques 1
       _xsecond 650, second $x
       _x2-650 x 3 Digital techniques 3
650  0 ......
       _x3-650 x 1 Digital techniques 1
       _xthird 650, second $x
       _x3-650 x 3 Digital techniques 3

I wanted to delete only the second occurrence of $x, only in the second occurrence of 650 field. So my action was:
(A) Delete field 650$x if 650$x matches RegEx m/ ^second 650 / 

On preview MARC the result is not what I wanted, but:
650  0 ...
       _x1-650 x 1 Digital techniques 1
       _xfirst 650, second $x
       _x1-650 x 3 Digital techniques 3
650  0 ....
650  0 ....
       _x3-650 x 1 Digital techniques 1
       _xthird 650, second $x
       _x3-650 x 3 Digital techniques 3

Then I've tried another action, selecting the "first" option:
B) Delete 1st field 650$x if 650$x matches RegEx m/ ^second 650 / 
but the result remained the same as for "All" (A).

Neither the 2 negative forms gave me what I wanted:
C) Delete field 650$x unless 650$x does not match RegEx m/ ^second 650 /
D) Delete 1st field 650$x unless 650$x does not match RegEx m/ ^second 650 /

Similar behaviours occurred in Copy, Update and Move cases.
Comment 34 Jonathan Druart 2014-07-08 08:16:46 UTC
(In reply to Paola Rossi from comment #33)
> Today I've tried to test the patches against master 3.17.00.009

Paola,
Thank you for testing!

> 1) ERROR on MARC modification templates: unavailable "Every" option:

Yes, maybe it's is not obvious.
Actually if the condition is on the same field, the tool won't process "all" field, but only the ones matching the condition.
I preferred to change to "all" value with something clearer.
I consider what you described as a very minor issue (it's just ergonomic).
So I prefer not to fix it for now but I keep it in my mind.

> 2) On Batch record modification [and on cart Action Modify too], I've
> cleared all the listed records.
> I've selected "Modify the selected records". Instead of "No record was
> selected", the program has started, and then has ended showing:
> "All records have successfully been modified! ".

It's definitely an issue for this bug report, patch coming...

> 3) ERROR on batch_record_modification.pl [I've checked only "Preview MARC"]:
> case of repeteable subfield [x] in repeteable fields [650]:

I will have a look at this but it's not for this bug report. I will open a new bug for that (linked to bug 11413 and bug 11319).
Comment 35 Jonathan Druart 2014-07-08 08:17:34 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2014-09-03 13:12:19 UTC Comment hidden (obsolete)
Comment 37 Marcel de Rooy 2014-10-02 13:19:53 UTC
(In reply to Jonathan Druart from comment #36)
> Created attachment 31344 [details] [review] [review]
> Bug 11395: Raise an alert if control field is used
> 
> If a control field is used with a copy or move action, the interface
> should block the user in some cases:
> - If a control field is used with a regular field
> - The subfield should be empty

Jonathan, I would rather suggest:

A control field can be used with another control field or a subfield of a regular field. (Note: For some external records I would like to move for instance 001 to 029a.)
A regular field can not be used with a control field only with another regular field.
A regular subfield can be used with another regular subfield or a control field.

Note that some moves are of course without any use, but this would be the general rule. What do you think?
Comment 38 Jonathan Druart 2014-10-02 13:24:10 UTC
(In reply to M. de Rooy from comment #37)

> Jonathan, I would rather suggest:
> 
> A control field can be used with another control field or a subfield of a
> regular field. (Note: For some external records I would like to move for
> instance 001 to 029a.)
> A regular field can not be used with a control field only with another
> regular field.
> A regular subfield can be used with another regular subfield or a control
> field.
> 
> Note that some moves are of course without any use, but this would be the
> general rule. What do you think?

I don't remember exactly, but I think I blocked the actions which don't work/don't be implemented.
Comment 39 Paola Rossi 2014-10-10 13:24:06 UTC
I've applied the patches against 3.17.00.025

Firstly I've applied the signed-off patches:
11319 (5) 11413 (3)

At step 0/ Create a new marc modification template with some actions,
I've created a template with some actions. Then I've selected the "Edit" link, at the right of one of the created actions, but I could not edit this action.

Instead of seeing the values of the action I wanted to edit, under the "Actions for this template" table, the default page "Add a new action" with the default values: "Delete ", "All" and none, has been shown.

So I pass the patch to "Failed QA" status.
Comment 40 Jonathan Druart 2014-10-10 13:46:50 UTC
It should work. Please try refreshing completely (ctrl+r / ctrl+f5) your cache.
Did you get a JS error (using Firebug)?
Comment 41 Paola Rossi 2014-10-10 16:23:05 UTC
(In reply to Jonathan Druart from comment #40)
> It should work. Please try refreshing completely (ctrl+r / ctrl+f5) your
> cache.

It has worked! Then I could change the actions. Thanks, Jonathan.

I'm sorry, indeed there was another error. It occurred before adding templates [more generally, it occurred when no template exists]. It is not resolved by (ctrl+r / ctrl+f5).

Firstly on Tools I've selected "Batch record modifications". Koha answered by writing [as expected, it's OK]:
"No MARC modification template is defined. You have to create at least one template for using this tool."

But the whole shown page was not OK: Koha has shown a sort a double page (in screen 1 there is the top of the shown page, in screen 2 there is the middle part of the whole "double" shown page). 

Under the "Quote editor" of "Additional tools" I saw the message (see screen 2):

Status: 200 OK Set-Cookie: CGISESSID=023a87ce0c543bc9822811b2fe51b209; path=/; HttpOnly Date: Fri, 10 Oct 2014 15:24:25 GMT Pragma: no-cache Cache-control: no-cache Content-script-type: text/javascript Content-style-type: text/css Content-Type: text/html; charset=UTF-8  

More below, beyond the message, another "Batch record modifications" page was shown; in front of the second part of the there are three search fields [instead of 1]. Then the whole double page ends with the [second] "Quote editor".

I didn't get any JS error using Firebug on this "double" page.

I pass the patch to "Failed QA" status.
Comment 42 Paola Rossi 2014-10-10 16:24:37 UTC Comment hidden (obsolete)
Comment 43 Paola Rossi 2014-10-10 16:25:29 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2014-10-10 18:21:30 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2014-10-10 18:22:07 UTC
(In reply to Paola Rossi from comment #41)

Good catch! Should be fixed now!
Comment 46 Brendan Gallagher 2014-10-10 21:12:30 UTC
Hi Jonathan - 

To see the tool appear in my tools section - (I was logged in as a superlibrarian) I had to add to C4/Auth.pm

$template->param( CAN_user_tools_records_batchmod => 1 );

Did I miss a patch somewhere?  I quickly tried to look for that in the patches but it didn't jump out to me.  Just let me know if I missed it.

Thanks,
Brendan
Comment 47 Jonathan Druart 2014-10-10 21:20:37 UTC
Brendan, it should work if you launch the updatedatabase.pl script.
It will add the permission into the DB and the variable will be sent to the template automatically.
Comment 48 Brendan Gallagher 2014-10-10 21:57:36 UTC
Ok works for me - and follows test plan - I will sign off on this.  Probably take me a bit of time to walk through all 11 patches, I'll do that either tonight or tomorrow morning at the hackfest.

-Brendan
Comment 49 Brendan Gallagher 2014-10-11 15:13:18 UTC Comment hidden (obsolete)
Comment 50 Brendan Gallagher 2014-10-11 15:13:39 UTC Comment hidden (obsolete)
Comment 51 Brendan Gallagher 2014-10-11 15:13:48 UTC Comment hidden (obsolete)
Comment 52 Brendan Gallagher 2014-10-11 15:13:54 UTC Comment hidden (obsolete)
Comment 53 Brendan Gallagher 2014-10-11 15:14:01 UTC Comment hidden (obsolete)
Comment 54 Brendan Gallagher 2014-10-11 15:14:07 UTC Comment hidden (obsolete)
Comment 55 Brendan Gallagher 2014-10-11 15:14:12 UTC Comment hidden (obsolete)
Comment 56 Brendan Gallagher 2014-10-11 15:14:19 UTC Comment hidden (obsolete)
Comment 57 Brendan Gallagher 2014-10-11 15:14:25 UTC Comment hidden (obsolete)
Comment 58 Brendan Gallagher 2014-10-11 15:14:31 UTC Comment hidden (obsolete)
Comment 59 Brendan Gallagher 2014-10-11 15:14:39 UTC Comment hidden (obsolete)
Comment 60 Marcel de Rooy 2014-10-31 10:57:31 UTC
Please set back to Signed off when bug 11413 comes back from FQA
Comment 61 Jonathan Druart 2014-11-03 14:09:39 UTC Comment hidden (obsolete)
Comment 62 Jonathan Druart 2014-11-03 14:09:44 UTC Comment hidden (obsolete)
Comment 63 Jonathan Druart 2014-11-03 14:09:48 UTC Comment hidden (obsolete)
Comment 64 Jonathan Druart 2014-11-03 14:09:52 UTC Comment hidden (obsolete)
Comment 65 Jonathan Druart 2014-11-03 14:09:56 UTC Comment hidden (obsolete)
Comment 66 Jonathan Druart 2014-11-03 14:10:00 UTC Comment hidden (obsolete)
Comment 67 Jonathan Druart 2014-11-03 14:10:07 UTC Comment hidden (obsolete)
Comment 68 Jonathan Druart 2014-11-03 14:10:12 UTC Comment hidden (obsolete)
Comment 69 Jonathan Druart 2014-11-03 14:10:16 UTC Comment hidden (obsolete)
Comment 70 Jonathan Druart 2014-11-03 14:10:21 UTC Comment hidden (obsolete)
Comment 71 Jonathan Druart 2014-11-03 14:10:25 UTC Comment hidden (obsolete)
Comment 72 Jonathan Druart 2014-11-03 14:10:59 UTC
Patches rebased.
Comment 73 Jonathan Druart 2014-11-12 13:17:11 UTC
Comment on attachment 33151 [details] [review]
Bug 11395: Fix field_numbers

This should be on bug 11413.
Comment 74 Jonathan Druart 2014-11-12 13:17:35 UTC
Comment on attachment 33147 [details] [review]
Bug 11395: Fix return for ModifyRecordWithTemplate

This should be on bug 11413.
Comment 75 Jonathan Druart 2014-12-15 09:31:59 UTC Comment hidden (obsolete)
Comment 76 Jonathan Druart 2014-12-15 09:32:05 UTC Comment hidden (obsolete)
Comment 77 Jonathan Druart 2014-12-15 09:32:11 UTC Comment hidden (obsolete)
Comment 78 Jonathan Druart 2014-12-15 09:32:19 UTC Comment hidden (obsolete)
Comment 79 Jonathan Druart 2014-12-15 09:32:29 UTC Comment hidden (obsolete)
Comment 80 Jonathan Druart 2014-12-15 09:32:34 UTC Comment hidden (obsolete)
Comment 81 Jonathan Druart 2014-12-15 09:32:39 UTC Comment hidden (obsolete)
Comment 82 Jonathan Druart 2014-12-15 09:32:44 UTC Comment hidden (obsolete)
Comment 83 Jonathan Druart 2014-12-15 09:32:50 UTC Comment hidden (obsolete)
Comment 84 Jonathan Druart 2014-12-15 09:33:16 UTC
Patches rebased
Comment 85 Jonathan Druart 2014-12-15 09:33:31 UTC Comment hidden (obsolete)
Comment 86 Kyle M Hall 2014-12-19 15:00:42 UTC Comment hidden (obsolete)
Comment 87 Kyle M Hall 2014-12-19 15:01:05 UTC Comment hidden (obsolete)
Comment 88 Kyle M Hall 2014-12-19 15:01:13 UTC Comment hidden (obsolete)
Comment 89 Kyle M Hall 2014-12-19 15:01:17 UTC Comment hidden (obsolete)
Comment 90 Kyle M Hall 2014-12-19 15:01:23 UTC Comment hidden (obsolete)
Comment 91 Kyle M Hall 2014-12-19 15:01:31 UTC Comment hidden (obsolete)
Comment 92 Kyle M Hall 2014-12-19 15:01:35 UTC Comment hidden (obsolete)
Comment 93 Kyle M Hall 2014-12-19 15:01:40 UTC Comment hidden (obsolete)
Comment 94 Kyle M Hall 2014-12-19 15:01:49 UTC Comment hidden (obsolete)
Comment 95 Kyle M Hall 2014-12-19 15:01:53 UTC Comment hidden (obsolete)
Comment 96 Kyle M Hall 2014-12-19 15:01:57 UTC Comment hidden (obsolete)
Comment 97 Jonathan Druart 2015-01-21 14:34:45 UTC
This is limited to 500, see bug 13606 for more information.
Comment 98 Tomás Cohen Arazi 2015-02-10 13:53:32 UTC Comment hidden (obsolete)
Comment 99 Tomás Cohen Arazi 2015-02-10 13:53:46 UTC Comment hidden (obsolete)
Comment 100 Tomás Cohen Arazi 2015-02-10 13:54:05 UTC Comment hidden (obsolete)
Comment 101 Tomás Cohen Arazi 2015-02-10 13:54:37 UTC Comment hidden (obsolete)
Comment 102 Tomás Cohen Arazi 2015-02-10 13:54:55 UTC Comment hidden (obsolete)
Comment 103 Tomás Cohen Arazi 2015-02-10 13:55:10 UTC Comment hidden (obsolete)
Comment 104 Tomás Cohen Arazi 2015-02-10 13:55:24 UTC Comment hidden (obsolete)
Comment 105 Tomás Cohen Arazi 2015-02-10 13:55:36 UTC Comment hidden (obsolete)
Comment 106 Tomás Cohen Arazi 2015-02-10 13:55:50 UTC Comment hidden (obsolete)
Comment 107 Tomás Cohen Arazi 2015-02-10 13:56:04 UTC Comment hidden (obsolete)
Comment 108 Tomás Cohen Arazi 2015-02-10 13:58:51 UTC
Hi, i like this patch set, I signed it myself too.
But I'm marking as Signed-off because I want more people involved. It is a great feature, that could potentially break people's records. So I'm asking Marcel and/or Katrin to give it a ride before pushing it.

I rebased it to current master also.
Comment 109 Marcel de Rooy 2015-02-11 12:14:32 UTC
(In reply to Tomás Cohen Arazi from comment #108)
> Hi, i like this patch set, I signed it myself too.
> But I'm marking as Signed-off because I want more people involved. It is a
> great feature, that could potentially break people's records. So I'm asking
> Marcel and/or Katrin to give it a ride before pushing it.

I will try to do it within reasonable time (..) I have been working on some of the dependent reports before. If a day just had 28 hours.
Comment 110 Marcel de Rooy 2015-03-02 11:09:55 UTC
QA: Picking this up again now.
Comment 111 Marcel de Rooy 2015-03-02 13:35:08 UTC
Created attachment 36295 [details] [review]
Bug 11395: Add a batch record modification

This patch offers a new tool to modify records in a batch.

This feature adds:
- a new pl/tt files tools/batch_record_modification
- a new permission: tools > records_batchmod

Test plan for biblios:
0/ Create a new marc modification template with some actions.
1/ Generate a list of biblionumbers you want to modify.
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 > "Modify"
- generating a list from a report
2/ On the "Batch record modification" tool verify:
- information is correct.
- the preview link show you the needed record.
3/ After clicking on the "Modify selected recors" button, verify
the records have been modified as you wanted.

Test plan for authority:
0/ Create a new marc modification template with some actions.
1/ Generate a list of authid using a report:
2/ On the "Batch record modification" tool verify:
- authorities are display with the summary.
- the preview link show you the needed record.
3/ After clicking on the "Modify selected recors" button, verify
the records have been modified as you wanted.

Catch of errors:
if an error occurs during the modification process, the tool
displays an error message.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 112 Marcel de Rooy 2015-03-02 13:35:14 UTC
Created attachment 36296 [details] [review]
Bug 11395: Add links to the new tool page

This patch adds links in the tool module to the new batch tool and in
the basket.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 113 Marcel de Rooy 2015-03-02 13:35:20 UTC
Created attachment 36297 [details] [review]
Bug 11395: New service to preview a record.

The main patch adds the ability to preview a record. This is done using
an AJAX way. The preview record is processed on the server side.

Note: I don't think an authentification check is necessary here.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 114 Marcel de Rooy 2015-03-02 13:35:25 UTC
Created attachment 36298 [details] [review]
Bug 11395: The modification template should be changed on the list view

This patch fixes the following issue:
If the user comes from a basket, the list view is displayed but there is
no way to select the modification template.

Now the template can be chosen on the list view. This way, the user is
able to change the modifications to apply and see the previewed records.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 115 Marcel de Rooy 2015-03-02 13:35:31 UTC
Created attachment 36299 [details] [review]
Bug 11395: DB: Add permission tools_records_batchmod

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 116 Marcel de Rooy 2015-03-02 13:35:37 UTC
Created attachment 36300 [details] [review]
Bug 11395: Fix path for the checkboxes jQuery plugin

This patch fixes a bad resolution conflict (bug 12107).

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 117 Marcel de Rooy 2015-03-02 13:35:42 UTC
Created attachment 36301 [details] [review]
Bug 11395: prevent processing no record

If no record is selected, the modification should not be launched.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 118 Marcel de Rooy 2015-03-02 13:35:48 UTC
Created attachment 36302 [details] [review]
Bug 11395: exit should be done after displaying the output

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 119 Marcel de Rooy 2015-03-02 13:35:54 UTC
Created attachment 36303 [details] [review]
[FAILED_QA] Bug 11395: Raise an alert if control field is used

If a control field is used with a copy or move action, the interface
should block the user in some cases:
- If a control field is used with a regular field
- The subfield should be empty

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 120 Marcel de Rooy 2015-03-02 13:36:00 UTC
Created attachment 36304 [details] [review]
[FAILED_QA] Bug 11395: A control field is a field with tag < 10

This patch fixes the existing test about control fields.
A control field is < 10, not <=10!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 121 Marcel de Rooy 2015-03-02 13:39:27 UTC
QA Comment: 

The new code looks quite good to me. I have some minor comments as to design issues (no blockers).
My only concerns about code concentrate on changes to marc_mod_templates. Since this report actually is a new interface on top of those rules, I would not oppose pushing it without the patches changing marc_mod_templates. 
If we want to push it with those changes (last two now), I think we should first make some corrections (see below). It is still somewhat unfinished now.
Aside from that, I would also suggest changes to marc mod templates to prevent unneeded record saving (with ModBiblio).
And aside from that, I would love to see additional code for indicators in Koha/SimpleMarc.pm (containing the actual routines for the record updates). 

My conclusion: I am setting the status back to Passed QA to pass the ball to Tomas. If you also feel that we should wait for an improvement of the last two patches, please set to FQA. Or push without those two?

=== Details:

tools/batch_record_modification.pl: Design may still need some work (no blocker). E.g. label before the selected marc mod template (position&length, color).
Typing an empty line in the biblio number field gives the warning: This biblionumber does not exist in the database. (I would personally ignore the whitespace, not warn here).
In the selected biblio number table you have Preview MARC. Just a style issue again: It made me wonder if we had two links there. But it is only one. Note that your column title is already Preview. In line with Cataloging Search, please remove the Preview string in the table and just keep MARC.

About Preview: It was not clear to me at first sight that this preview would show a modified record. I interpreted this view as a way to check the record just before updating it. But it actually is a way to test the result of the rule. If you have another name that makes this intuitively more clear, you are welcome :)

The results list every biblio number successfully modified. This list could be very long. Since I gave Koha the biblio numbers (in a file or via text box), I would be more interested in the numbers where an error came up. At the same time not all "successfully modified" records have actually been modified. Which ones were really changed? That may however be considered outside the scope of this report and would need some code adjustments in MarcModificationTemplates.pm to make the copy/edit/delete routines return some meaningful value. In that case it would not be necessary in this code to always run ModBiblio (with less [theoretical] risks to hurt data that you did not want to change in the first place)..

*** Bug-11395-Raise-an-alert-if-control-field
This patch deals with changes in marc mod templates itself. Note that I can still add a rule like:
Update field 001$a with value XX (But 001 has no subfields.) 
Delete field 001$b 
When moving 022$a to 001, I still have the warning: Both subfields should be filled or empty.
Add field 001 value XX: Not allowed, needs subfield !! 001 has not subfields.
etc.
When you allow rules like this, you can easily generate errors like:
The biblio 1 has not been modified. An error occurred on modifying it. (The error was: Can't use an undefined value as an ARRAY reference at /usr/local/share/perl5/MARC/Field.pm line 453. , see the Koha logfile for more information)
Comment 122 Tomás Cohen Arazi 2015-03-05 15:34:38 UTC
Patches pushed to master.

Nice job Jonathan!