Bug 8015 is pushed and I plan to take over bug 5725. The idea will be to provide a batch modification tools for records (biblios and authorities). But before, I have to prepare the ground: Koha::SimpleMARC does not split clearly fields and subfields manipulations. The API should provide routines like copy_field and copy_subfield for example. Later the UI will have to reflect this change.
Created attachment 23206 [details] [review] Bug 11319: POC Marc modification templates improvements This patch is a POC. Please give me your feedback on the implementation of the fields/subfields split. This patch only implements the delete and the read routines. See new UT at the end of t/SimpleMARC.t
Kyle and Galen, I really would like to have your opinion on this patch please.
As a POC, it looks pretty good to me! The only suggestion I would have would be not keep the export for read_field, and modify that sub to check for a value in the subfield value. That is, instead of having read, read_field, and read_subfield, keep all the code in read_field and just do an if/then/else. That way the exported name is still read_field, and thus doesn't change the API. It's entirely possible for SimpleMARC to be used in custom scripts, so adding to the API is fine, but removing/renaming api subs should be avoided. I know that such an occurence is unlikely at this point, but even in the future it will confuse users to have update_field, copy_field, move_field, read, and delete. It just breaks the naming convention. The other option would be to remove "_field" from all the subroutines. I have no problem with either choice. Let's just keep the convention standardized! Other than that, it looks good to me! Kyle (In reply to Jonathan Druart from comment #2) > Kyle and Galen, I really would like to have your opinion on this patch > please.
(In reply to Kyle M Hall from comment #3) Kyle, Thank you for your feedback. > As a POC, it looks pretty good to me! The only suggestion I would have would > be not keep the export for read_field, and modify that sub to check for a > value in the subfield value. That is, instead of having read, read_field, > and read_subfield, keep all the code in read_field and just do an > if/then/else. That way the exported name is still read_field, and thus > doesn't change the API. This way, I am afraid that routines like copy_field and update_field will be really big. It's why I think it would be better to split them. > It's entirely possible for SimpleMARC to be used in custom scripts, so > adding to the API is fine, but removing/renaming api subs should be avoided. > I know that such an occurence is unlikely at this point, but even in the > future it will confuse users to have update_field, copy_field, move_field, > read, and delete. It just breaks the naming convention. Yes of course, but as I said, this patch only implements the read and delete routines. The goal is to do the same work for all other subroutines in the SimpleMarc module (update_field, copy_field, move_field). > The other option would be to remove "_field" from all the subroutines. I > have no problem with either choice. Let's just keep the convention > standardized! Maybe should we only keep a call to delete_field which will call either the private _delete_field or _delete_subfield routines depending on the subfield value. Like that, we would not have to change the API.
Created attachment 23414 [details] [review] Bug 11319: Template modifications This patch add template modifications for the restrictions: - the source field is always mandatory - on move and copy, the source and destination subfield should be both filled or blank. - on move and copy, the destination subfield should be filled. - on update, the subfield value should be filled. Test plan: Verify you are not able to create/modify template actions according to these restrictions.
Created attachment 23415 [details] [review] Bug 11319: Koha::SimpleMARC should take a hashref for parameters In order to avoid a long list of parameters, it should be better to pass all of them into a hashref. This patch does not add or modify a behavior. Test plan: Verify the unit tests still pass - prove t/SimpleMARC.t - prove t/db_dependent/MarcModificationTemplates.t
Created attachment 23416 [details] [review] Bug 11319: Add UT for the fields management
Created attachment 23417 [details] [review] Bug 11319: Add the field management for Koha::SimpleMARC Currently the Koha::SimpleMARC module call a "field" a "subfield". And the way to manage field is not implemented for all routines. This patch does not modify the API. The routine's names are kept. It just creates 2 privates routines for each action (e.g. delete_field will call _delete_field if the action affects field and _delete_subfield if the action affects subfields). Before this patch the move action was authorised by the interface but caused an error if executed. Note: I don't see the meaning for the add/update action if no subfield is given. So the call without subfield raises an error. Test plan: - apply all patches - create or modify an existent template - try at least the correct behavior for the following actions: * delete subfield and field * add new subfield to an existing field * add new subfield to an nonexisting field * move a subfield * move an entire field * copy a subfield * copy an entire field - import a biblio and use this template - verify the imported biblio matches actions defined.
Created attachment 23418 [details] [review] Bug 11319: Add specific UT for nonexistent field/subfield This patch only adds unit tests for the copy and move actions. They test if the action does not create a field/subfield if the source did not exist. Also it adds a unit tests for the existing behavior (in order not to lost it): we can use the '^' and the '$' character in regex for substituing. For example: Copy 245$a to 245$a with the regex s/^/BEGIN / This will add the string "BEGIN " at the beginning of the 245$a fields. To test: prove t/SimpleMARC.t
Created attachment 23419 [details] [review] Bug 11319: Template modifications This patch add template modifications for the restrictions: - the source field is always mandatory - on move and copy, the source and destination subfield should be both filled or blank. - on move and copy, the destination subfield should be filled. - on update, the subfield value should be filled. Test plan: Verify you are not able to create/modify template actions according to these restrictions.
The sandbox you've requested is not ready. Some problems occurred applying patches from bug 11319: <h1>Something went wrong !</h1>Applying: Bug 11319: Koha::SimpleMARC should take a hashref for parameters Applying: Bug 11319: Add UT for the fields management Applying: Bug 11319: Add the field management for Koha::SimpleMARC Applying: Bug 11319: Add specific UT for nonexistent field/subfield Applying: Bug 11319: Template modifications Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt Failed to merge in the changes. Patch failed at 0001 Bug 11319: Template modifications When you have resolved this problem run git bz apply --continue. If you would prefer to skip this patch, instead run git bz apply --skip. To restore the original branch and stop patching run git bz apply --abort. Bug 11319 - Marc modification templates improvements 23415 - Bug 11319: Koha::SimpleMARC should take a hashref for parameters 23416 - Bug 11319: Add UT for the fields management 23417 - Bug 11319: Add the field management for Koha::SimpleMARC 23418 - Bug 11319: Add specific UT for nonexistent field/subfield 23419 - Bug 11319: Template modifications Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-11319-Template-modifications-Yo0tWr.patch
Created attachment 24650 [details] [review] Bug 11319: Template modifications This patch add template modifications for the restrictions: - the source field is always mandatory - on move and copy, the source and destination subfield should be both filled or blank. - on move and copy, the destination subfield should be filled. - on update, the subfield value should be filled. Test plan: Verify you are not able to create/modify template actions according to these restrictions.
Fix conflict with bug 11414.
Created attachment 25751 [details] [review] Bug 11319: Koha::SimpleMARC should take a hashref for parameters In order to avoid a long list of parameters, it should be better to pass all of them into a hashref. This patch does not add or modify a behavior. Test plan: Verify the unit tests still pass - prove t/SimpleMARC.t - prove t/db_dependent/MarcModificationTemplates.t
Created attachment 25752 [details] [review] Bug 11319: Add UT for the fields management
Created attachment 25753 [details] [review] Bug 11319: Add the field management for Koha::SimpleMARC Currently the Koha::SimpleMARC module call a "field" a "subfield". And the way to manage field is not implemented for all routines. This patch does not modify the API. The routine's names are kept. It just creates 2 privates routines for each action (e.g. delete_field will call _delete_field if the action affects field and _delete_subfield if the action affects subfields). Before this patch the move action was authorised by the interface but caused an error if executed. Note: I don't see the meaning for the add/update action if no subfield is given. So the call without subfield raises an error. Test plan: - apply all patches - create or modify an existent template - try at least the correct behavior for the following actions: * delete subfield and field * add new subfield to an existing field * add new subfield to an nonexisting field * move a subfield * move an entire field * copy a subfield * copy an entire field - import a biblio and use this template - verify the imported biblio matches actions defined.
Created attachment 25754 [details] [review] Bug 11319: Add specific UT for nonexistent field/subfield This patch only adds unit tests for the copy and move actions. They test if the action does not create a field/subfield if the source did not exist. Also it adds a unit tests for the existing behavior (in order not to lost it): we can use the '^' and the '$' character in regex for substituing. For example: Copy 245$a to 245$a with the regex s/^/BEGIN / This will add the string "BEGIN " at the beginning of the 245$a fields. To test: prove t/SimpleMARC.t
Created attachment 25755 [details] [review] Bug 11319: Template modifications This patch add template modifications for the restrictions: - the source field is always mandatory - on move and copy, the source and destination subfield should be both filled or blank. - on move and copy, the destination subfield should be filled. - on update, the subfield value should be filled. Test plan: Verify you are not able to create/modify template actions according to these restrictions.
Fix conflict with bug 11478.
Created attachment 28988 [details] [review] Bug 11319: Koha::SimpleMARC should take a hashref for parameters In order to avoid a long list of parameters, it should be better to pass all of them into a hashref. This patch does not add or modify a behavior. Test plan: Verify the unit tests still pass - prove t/SimpleMARC.t - prove t/db_dependent/MarcModificationTemplates.t
Created attachment 28989 [details] [review] Bug 11319: Add UT for the fields management
Created attachment 28990 [details] [review] Bug 11319: Add the field management for Koha::SimpleMARC Currently the Koha::SimpleMARC module call a "field" a "subfield". And the way to manage field is not implemented for all routines. This patch does not modify the API. The routine's names are kept. It just creates 2 privates routines for each action (e.g. delete_field will call _delete_field if the action affects field and _delete_subfield if the action affects subfields). Before this patch the move action was authorised by the interface but caused an error if executed. Note: I don't see the meaning for the add/update action if no subfield is given. So the call without subfield raises an error. Test plan: - apply all patches - create or modify an existent template - try at least the correct behavior for the following actions: * delete subfield and field * add new subfield to an existing field * add new subfield to an nonexisting field * move a subfield * move an entire field * copy a subfield * copy an entire field - import a biblio and use this template - verify the imported biblio matches actions defined.
Created attachment 28991 [details] [review] Bug 11319: Add specific UT for nonexistent field/subfield This patch only adds unit tests for the copy and move actions. They test if the action does not create a field/subfield if the source did not exist. Also it adds a unit tests for the existing behavior (in order not to lost it): we can use the '^' and the '$' character in regex for substituing. For example: Copy 245$a to 245$a with the regex s/^/BEGIN / This will add the string "BEGIN " at the beginning of the 245$a fields. To test: prove t/SimpleMARC.t
Created attachment 28992 [details] [review] Bug 11319: Template modifications This patch add template modifications for the restrictions: - the source field is always mandatory - on move and copy, the source and destination subfield should be both filled or blank. - on move and copy, the destination subfield should be filled. - on update, the subfield value should be filled. Test plan: Verify you are not able to create/modify template actions according to these restrictions.
Last patch set fixes silly error introduced by previous rebase (c/p error).
Created attachment 29497 [details] [review] Bug 11319: Koha::SimpleMARC should take a hashref for parameters In order to avoid a long list of parameters, it should be better to pass all of them into a hashref. This patch does not add or modify a behavior. Test plan: Verify the unit tests still pass - prove t/SimpleMARC.t - prove t/db_dependent/MarcModificationTemplates.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 29498 [details] [review] Bug 11319: Add UT for the fields management Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 29499 [details] [review] Bug 11319: Add the field management for Koha::SimpleMARC Currently the Koha::SimpleMARC module call a "field" a "subfield". And the way to manage field is not implemented for all routines. This patch does not modify the API. The routine's names are kept. It just creates 2 privates routines for each action (e.g. delete_field will call _delete_field if the action affects field and _delete_subfield if the action affects subfields). Before this patch the move action was authorised by the interface but caused an error if executed. Note: I don't see the meaning for the add/update action if no subfield is given. So the call without subfield raises an error. Test plan: - apply all patches - create or modify an existent template - try at least the correct behavior for the following actions: * delete subfield and field * add new subfield to an existing field * add new subfield to an nonexisting field * move a subfield * move an entire field * copy a subfield * copy an entire field - import a biblio and use this template - verify the imported biblio matches actions defined. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 29500 [details] [review] Bug 11319: Add specific UT for nonexistent field/subfield This patch only adds unit tests for the copy and move actions. They test if the action does not create a field/subfield if the source did not exist. Also it adds a unit tests for the existing behavior (in order not to lost it): we can use the '^' and the '$' character in regex for substituing. For example: Copy 245$a to 245$a with the regex s/^/BEGIN / This will add the string "BEGIN " at the beginning of the 245$a fields. To test: prove t/SimpleMARC.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 29501 [details] [review] Bug 11319: Template modifications This patch add template modifications for the restrictions: - the source field is always mandatory - on move and copy, the source and destination subfield should be both filled or blank. - on move and copy, the destination subfield should be filled. - on update, the subfield value should be filled. Test plan: Verify you are not able to create/modify template actions according to these restrictions. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Planning QA for this week
Planning to QA this after 11319
(In reply to M. de Rooy from comment #32) > Planning to QA this after 11319 Somehow a backspace got in between :)
Started QA on this one; will have some comments tomorrow.
QA Comment: In general these patches look good to me. I have the impression that this feature still needs some work however. Here are some comments to start with: [1] The last patch (template mods) adds the restriction: A move/copy/update should have a subfield. If I want to update control field 003 to some text, it should be possible. There is no subfield. (Blocker.) It also says: move/copy source and dest should have either a subfield or not. I agree that it is normal behavior. But suppose that I want to copy some field to a controlfield? This is somewhat unlikely, but it would not be wrong. (No blocker since I do not have a real application for it at hand.) [2] Some code in the third patch for update_field: + if ( not $subfieldName or $subfieldName eq '' ) { + # FIXME I'm not sure the actual implementation is correct. + die "This action is not implemented yet"; + #_update_field({ record => $record, field => $fieldName, values => \@values }); This actually is the situation I was referring to (I want to update a control field without subfield). At this moment you are blocking an update without subfield in the interface. This makes it hard to reach this die statement.. [3] Just looking at some code in SimpleMarc.pm, I notice the following: field_equals just picks the first field value and runs the regex. If I do not pass a subfield, it does not run the regex over the whole field just one subfield. If I pass a subfield and the record contains repeated subfields, it just checks the first one. This is not the behavior that I would (intuitively) expect from the interface. Note that your patch does not really touch that part of code, but you do touch the routine. Is improving that functionality not even more important than passing a hashref? [4] BTW See some warnings in the log: [Wed Jul 30 16:37:51 2014] stage-marc-import.pl: Filehandle STDOUT reopened as FH only for input at Template/Provider.pm line 964., referer: /cgi-bin/koha/tools/stage-marc-import.pl [Wed Jul 30 16:37:51 2014] stage-marc-import.pl: Filehandle STDOUT reopened as FH only for input at Template/Provider.pm line 964., referer: /cgi-bin/koha/tools/stage-marc-import.pl [Wed Jul 30 16:53:18 2014] marc_modification_templates.pl: Use of uninitialized value in string eq at tools/marc_modification_templates.pl line 70., referer: /cgi-bin/koha/tools/marc_modification_templates.pl?template_id=3&op=delete_action&mmta_id=8 [Wed Jul 30 16:53:31 2014] marc_modification_templates.pl: Use of uninitialized value $template_id in string eq at C4/MarcModificationTemplates.pm line 84., referer: /cgi-bin/koha/tools/marc_modification_templates.pl Setting status to Failed QA
(In reply to M. de Rooy from comment #35) > QA Comment: Marcel, Thanks a lot to had a look at this. > In general these patches look good to me. I have the impression that this > feature still needs some work however. > Here are some comments to start with: > > [1] The last patch (template mods) adds the restriction: A move/copy/update > should have a subfield. > [...] > [2] > At this moment you are blocking an update without subfield in the interface. > This makes it hard to reach this die statement.. It is more or less intended. All changes done on these patches (and linked) are not trivial and I don't want to add complexity for now. This limitation will exist until the entire feature will be pushed. (Not enough time at the moment to work on it without knowing when it will be pushed...). > [3] Just looking at some code in SimpleMarc.pm, I notice the following: > field_equals just picks the first field value and runs the regex. If I do > not pass a subfield, it does not run the regex over the whole field just one > subfield. If I pass a subfield and the record contains repeated subfields, > it just checks the first one. This is not the behavior that I would > (intuitively) expect from the interface. Note that your patch does not > really touch that part of code, but you do touch the routine. Is improving > that functionality not even more important than passing a hashref? Could you please retest with *all* patches applied? Actually I split patches into different bug reports but all patches should be QA at once (patches from bug 11413 and bug 11395). I think this is fixed by the patch "Bug 11413: Change the field number logic". > [4] BTW See some warnings in the log: > [Wed Jul 30 16:37:51 2014] stage-marc-import.pl: Filehandle STDOUT reopened > as FH only for input at Template/Provider.pm line 964., referer: > /cgi-bin/koha/tools/stage-marc-import.pl > [Wed Jul 30 16:37:51 2014] stage-marc-import.pl: Filehandle STDOUT reopened > as FH only for input at Template/Provider.pm line 964., referer: > /cgi-bin/koha/tools/stage-marc-import.pl > [Wed Jul 30 16:53:18 2014] marc_modification_templates.pl: Use of > uninitialized value in string eq at tools/marc_modification_templates.pl > line 70., referer: > /cgi-bin/koha/tools/marc_modification_templates. > pl?template_id=3&op=delete_action&mmta_id=8 > [Wed Jul 30 16:53:31 2014] marc_modification_templates.pl: Use of > uninitialized value $template_id in string eq at > C4/MarcModificationTemplates.pm line 84., referer: > /cgi-bin/koha/tools/marc_modification_templates.pl These warnings existed before this patch set isn't it? I can open another bug report to get rid of warnings if they still exist after applying all patches.
(In reply to Jonathan Druart from comment #36) > It is more or less intended. > All changes done on these patches (and linked) are not trivial and I don't > want to add complexity for now. > This limitation will exist until the entire feature will be pushed. > (Not enough time at the moment to work on it without knowing when it will be > pushed...). > Could you please retest with *all* patches applied? > Actually I split patches into different bug reports but all patches should > be QA at once (patches from bug 11413 and bug 11395). > I think this is fixed by the patch "Bug 11413: Change the field number > logic". I applied 11319, 11413 and 11395 together now. The limitation seems to not be the same anymore. But it is still not possible to move from a control field to a regular subfield. If I move from a control field to a regular field, the marc modification will not work without warning the user. Say from 001 to 029. Only a error like this one could be found in the koha logs: stage-marc-import.pl: ERROR occurred in SetUTF8Flag Field 029 must have indicators (use ' ' for empty indicators)at /usr/share/koha/testclone/C4/Charset.pm line 146. Note that the field 001 is cleared, but the contents does not reach 029.
(In reply to M. de Rooy from comment #37) > I applied 11319, 11413 and 11395 together now. > The limitation seems to not be the same anymore. > But it is still not possible to move from a control field to a regular > subfield. If I move from a control field to a regular field, the marc > modification will not work without warning the user. What do you expect? You mean this should not be possible to create this rule, that's it?
Created attachment 31343 [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
Marcel, I submitted a patch on bug 11395 comment 39. Could you confirm it's what you expected?
(In reply to Jonathan Druart from comment #40) > Marcel, I submitted a patch on bug 11395 comment 39. Could you confirm it's > what you expected? It is bug 11395 comment 36.
(In reply to Jonathan Druart from comment #41) > (In reply to Jonathan Druart from comment #40) > > Marcel, I submitted a patch on bug 11395 comment 39. Could you confirm it's > > what you expected? > > It is bug 11395 comment 36. Will have a look soon.
(In reply to M. de Rooy from comment #42) > (In reply to Jonathan Druart from comment #41) > > (In reply to Jonathan Druart from comment #40) > > > Marcel, I submitted a patch on bug 11395 comment 39. Could you confirm it's > > > what you expected? > > > > It is bug 11395 comment 36. > > Will have a look soon. Soon was kind of relative. Try to get back at this now..
Created attachment 33029 [details] [review] Bug 11319: [QA follow-up] Add test message to MarcModificationTemplates.t The last test (#74) did not print anything. It now does..
Created attachment 33030 [details] [review] Bug 11319: [QA follow-up] Remove a warning from GetModificationTemplates Removes this warning: Use of uninitialized value $template_id in string eq at C4/MarcModificationTemplates.pm line 84. GetModificationTemplates has no template_id if called from marc_modification_templates.pl without operation (first click from interface) and from tools/stage-marc-import.pl. Slightly adjusted the POD lines accordingly. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
QA Comment bug 11319: Generally looks good. The current implementation in master is not error free. This will not resolve all bugs. 5 patches, no koha-qa complaints, added 2 qa follow-ups. I pass QA on this patch (taking into account the amount of work), but we are not ready yet. Discussion continues on follow-up reports.. From patch 3 (as well as the restrictions mentioned in patch 5) -- Actually, I repeat myself here: > Note: I don't see the meaning for the add/update action if no subfield is given. So the call without subfield raises an error. Moving 001 to 029a should be possible. But is not allowed now. Moving 001 to 029 should not be possible (since 001 has no subfields, and 029 has). This is possible, but will generate an invalid MARC record. I will not block this patch however for that reason, because current master does not implement it correctly either. Current master moves the 001 to <subfield code="001"></subfield> in field 029. Also invalid MARC ! No blocker, but a welcome improvement: Fields are inserted at the end. I would rather insert the fields in the same group of fields 0XX, 1XX, etc. You could use insert_grouped_field for that. Still see some related warnings (resolved another one): marc_modification_templates.pl: Use of uninitialized value in string eq at marc_modification_templates.pl line 70. stage-marc-import.pl: Filehandle STDOUT reopened as FH only for input at /usr/local/lib64/perl5/Template/Provider.pm line 964. Passed QA
Created attachment 33038 [details] [review] Bug 11319: Koha::SimpleMARC should take a hashref for parameters In order to avoid a long list of parameters, it should be better to pass all of them into a hashref. This patch does not add or modify a behavior. Test plan: Verify the unit tests still pass - prove t/SimpleMARC.t - prove t/db_dependent/MarcModificationTemplates.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 33039 [details] [review] Bug 11319: Add UT for the fields management Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 33040 [details] [review] Bug 11319: Add the field management for Koha::SimpleMARC Currently the Koha::SimpleMARC module call a "field" a "subfield". And the way to manage field is not implemented for all routines. This patch does not modify the API. The routine's names are kept. It just creates 2 privates routines for each action (e.g. delete_field will call _delete_field if the action affects field and _delete_subfield if the action affects subfields). Before this patch the move action was authorised by the interface but caused an error if executed. Note: I don't see the meaning for the add/update action if no subfield is given. So the call without subfield raises an error. Test plan: - apply all patches - create or modify an existent template - try at least the correct behavior for the following actions: * delete subfield and field * add new subfield to an existing field * add new subfield to an nonexisting field * move a subfield * move an entire field * copy a subfield * copy an entire field - import a biblio and use this template - verify the imported biblio matches actions defined. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 33041 [details] [review] Bug 11319: Add specific UT for nonexistent field/subfield This patch only adds unit tests for the copy and move actions. They test if the action does not create a field/subfield if the source did not exist. Also it adds a unit tests for the existing behavior (in order not to lost it): we can use the '^' and the '$' character in regex for substituing. For example: Copy 245$a to 245$a with the regex s/^/BEGIN / This will add the string "BEGIN " at the beginning of the 245$a fields. To test: prove t/SimpleMARC.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 33042 [details] [review] Bug 11319: Template modifications This patch add template modifications for the restrictions: - the source field is always mandatory - on move and copy, the source and destination subfield should be both filled or blank. - on move and copy, the destination subfield should be filled. - on update, the subfield value should be filled. Test plan: Verify you are not able to create/modify template actions according to these restrictions. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 33043 [details] [review] Bug 11319: [QA follow-up] Add test message to MarcModificationTemplates.t The last test (#74) did not print anything. It now does.. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 33044 [details] [review] Bug 11319: [QA follow-up] Remove a warning from GetModificationTemplates Removes this warning: Use of uninitialized value $template_id in string eq at C4/MarcModificationTemplates.pm line 84. GetModificationTemplates has no template_id if called from marc_modification_templates.pl without operation (first click from interface) and from tools/stage-marc-import.pl. Slightly adjusted the POD lines accordingly. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Patches pushed to master. Thanks Jonathan and Marcel!