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.
Created attachment 23502 [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.
Created attachment 23503 [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.
Created attachment 23504 [details] [review] Bug 11395: Fix return for ModifyRecordWithTemplate Make sure the ModifyRecordWithTemplate routine returns undef. This patch also removes a warning if GetModificationTemplates is called without parameter. Verify prove t/db_dependent/MarcModificationTemplates.t returns green.
Created attachment 23505 [details] [review] Bug 11395: DB: Add permission tools_records_batchmod
Created attachment 23506 [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.
Created attachment 23507 [details] form screen
Created attachment 23508 [details] list screen
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.
Created attachment 23592 [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.
Created attachment 24634 [details] [review] Bug 11395: DB: Add permission tools_records_batchmod
Created attachment 26072 [details] [review] Bug 11395: DB: Add permission tools_records_batchmod
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"]') ..."
(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?
(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.
Created attachment 26981 [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.
None of these patches appear to be signed off, change status to "Needs Signoff".
<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.
Created attachment 28978 [details] [review] Bug 11395: DB: Add permission tools_records_batchmod
Created attachment 28999 [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.
Created attachment 29000 [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.
Created attachment 29001 [details] [review] Bug 11395: Fix return for ModifyRecordWithTemplate Make sure the ModifyRecordWithTemplate routine returns undef. This patch also removes a warning if GetModificationTemplates is called without parameter. Verify prove t/db_dependent/MarcModificationTemplates.t returns green.
Created attachment 29002 [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.
Created attachment 29003 [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.
Created attachment 29004 [details] [review] Bug 11395: DB: Add permission tools_records_batchmod
I added the dependency on bug 11413 since this enhancement is dangerous without fixing the current issues.
Created attachment 29056 [details] [review] Bug 11395: Fix field_numbers This fix is a global fix for the MarcModificationTemplate feature. Some unit tests were missing and some behaviors were wrong. For instance, if you tried to update a non existent field, the script crashed. The following line was completely stupid: if $from_field ne $to_subfield The field_number equals 1 if the user wants to update the first field and 0 for all fields. The field_numbers (note the s) variable contains the field numbers to update. This array is filled if a condition exists (field exists or field equals).
(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.
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.
* 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.
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
Created attachment 29509 [details] [review] Bug 11395: Fix path for the checkboxes jQuery plugin This patch fixes a bad resolution conflict (bug 12107).
(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.
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.
(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).
Created attachment 29569 [details] [review] Bug 11395: prevent processing no record If no record is selected, the modification should not be launched.
Created attachment 31344 [details] [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
(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?
(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.
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.
It should work. Please try refreshing completely (ctrl+r / ctrl+f5) your cache. Did you get a JS error (using Firebug)?
(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.
Created attachment 32181 [details] screen-1 screen-1: top of the "double" page shown by koha after having selected "Batch record modification" on Tools menu
Created attachment 32182 [details] screen-2 screen-2: middle part of the same "double" page.
Created attachment 32184 [details] [review] Bug 11395: exit should be done after displaying the output
(In reply to Paola Rossi from comment #41) Good catch! Should be fixed now!
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
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.
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
Created attachment 32239 [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>
Created attachment 32240 [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>
Created attachment 32241 [details] [review] Bug 11395: Fix return for ModifyRecordWithTemplate Make sure the ModifyRecordWithTemplate routine returns undef. This patch also removes a warning if GetModificationTemplates is called without parameter. Verify prove t/db_dependent/MarcModificationTemplates.t returns green. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 32242 [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>
Created attachment 32243 [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>
Created attachment 32244 [details] [review] Bug 11395: DB: Add permission tools_records_batchmod Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 32245 [details] [review] Bug 11395: Fix field_numbers This fix is a global fix for the MarcModificationTemplate feature. Some unit tests were missing and some behaviors were wrong. For instance, if you tried to update a non existent field, the script crashed. The following line was completely stupid: if $from_field ne $to_subfield The field_number equals 1 if the user wants to update the first field and 0 for all fields. The field_numbers (note the s) variable contains the field numbers to update. This array is filled if a condition exists (field exists or field equals). Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 32246 [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>
Created attachment 32247 [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>
Created attachment 32248 [details] [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 Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 32249 [details] [review] Bug 11395: exit should be done after displaying the output Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Please set back to Signed off when bug 11413 comes back from FQA
Created attachment 33145 [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>
Created attachment 33146 [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>
Created attachment 33147 [details] [review] Bug 11395: Fix return for ModifyRecordWithTemplate Make sure the ModifyRecordWithTemplate routine returns undef. This patch also removes a warning if GetModificationTemplates is called without parameter. Verify prove t/db_dependent/MarcModificationTemplates.t returns green. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 33148 [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>
Created attachment 33149 [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>
Created attachment 33150 [details] [review] Bug 11395: DB: Add permission tools_records_batchmod Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 33151 [details] [review] Bug 11395: Fix field_numbers This fix is a global fix for the MarcModificationTemplate feature. Some unit tests were missing and some behaviors were wrong. For instance, if you tried to update a non existent field, the script crashed. The following line was completely stupid: if $from_field ne $to_subfield The field_number equals 1 if the user wants to update the first field and 0 for all fields. The field_numbers (note the s) variable contains the field numbers to update. This array is filled if a condition exists (field exists or field equals). Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 33152 [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>
Created attachment 33153 [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>
Created attachment 33154 [details] [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 Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 33155 [details] [review] Bug 11395: exit should be done after displaying the output Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Patches rebased.
Comment on attachment 33151 [details] [review] Bug 11395: Fix field_numbers This should be on bug 11413.
Comment on attachment 33147 [details] [review] Bug 11395: Fix return for ModifyRecordWithTemplate This should be on bug 11413.
Created attachment 34393 [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>
Created attachment 34394 [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>
Created attachment 34395 [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>
Created attachment 34396 [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>
Created attachment 34397 [details] [review] Bug 11395: DB: Add permission tools_records_batchmod Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 34398 [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>
Created attachment 34399 [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>
Created attachment 34400 [details] [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 Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Created attachment 34401 [details] [review] Bug 11395: exit should be done after displaying the output Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Patches rebased
Created attachment 34402 [details] [review] 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!
Created attachment 34552 [details] [review] [PASSED QA] 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>
Created attachment 34553 [details] [review] [PASSED QA] 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>
Created attachment 34554 [details] [review] [PASSED QA] 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>
Created attachment 34555 [details] [review] [PASSED QA] 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>
Created attachment 34556 [details] [review] [PASSED QA] 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>
Created attachment 34557 [details] [review] [PASSED QA] 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>
Created attachment 34558 [details] [review] [PASSED QA] 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>
Created attachment 34559 [details] [review] [PASSED QA] 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>
Created attachment 34560 [details] [review] [PASSED 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>
Created attachment 34561 [details] [review] [PASSED QA] 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>
Created attachment 34562 [details] [review] [PASSED 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>
This is limited to 500, see bug 13606 for more information.
Created attachment 35783 [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>
Created attachment 35784 [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>
Created attachment 35785 [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>
Created attachment 35786 [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>
Created attachment 35787 [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>
Created attachment 35788 [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>
Created attachment 35789 [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>
Created attachment 35790 [details] [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 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>
Created attachment 35791 [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>
Created attachment 35792 [details] [review] 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>
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.
(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.
QA: Picking this up again now.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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)
Patches pushed to master. Nice job Jonathan!