MARC modification templates does not allow an action with move, copy nor copy and replace when both from and to fields are control fields. This should be allowed. Looks like there just some work to do on RegExp use since it calls $field->subfields.
Created attachment 85209 [details] [review] Bug 22245: Allow copy/move from and to control fields The JS conditions were wrong, we want to allow copy and move between control fields. Test plan: Create a new MARC modification template action using control fields. Confirm that you are allowed to copy/move a control field to another one
See also commit 1079f971e3bc659f3053d9608dbde0b856531111 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
1) With this patch, if I set "from" with control field (ie 003) and "to" with a regular field (ie 998$e) : I get message : MSG_MMT_SUBFIELDS_MATCH = _("Both subfield values should be filled or empty."); I think this message is only for regular fields in "from" and "to", so you don't move a entire field to a subfield for example. 2) This does not allow copy/move of a control field in the subfield of a regular field, ie 005 => 998$d. This should be allowed ? What is surely not allowed is : 005 => 998
(In reply to Fridolin SOMERS from comment #3) > 1) > With this patch, if I set "from" with control field (ie 003) and "to" with a > regular field (ie 998$e) : > > I get message : > MSG_MMT_SUBFIELDS_MATCH = _("Both subfield values should be filled or > empty."); > > I think this message is only for regular fields in "from" and "to", so you > don't move a entire field to a subfield for example. I would not add complexity to the JS code, the message is correct even if it could be more exact. You can provide a follow-up if you think it's really needed, I personally think it's not that important. > 2) > This does not allow copy/move of a control field in the subfield of a > regular field, ie 005 => 998$d. > This should be allowed ? > What is surely not allowed is : 005 => 998 Maybe, maybe not. This limitation has been added by commit 1079f971e3bc "Raise an alert if control field is used". Added on bug 11395 comment 36 without explanations. I cannot remember why, maybe you can find it on the BibLibre bug tracker? Anyway it's for another bug report.
> > 2) > > This does not allow copy/move of a control field in the subfield of a > > regular field, ie 005 => 998$d. > > This should be allowed ? A very common use case would be 001 > 035$a + Adding a prefix in MARC21. So it would be nice if that worked. > > What is surely not allowed is : 005 => 998 I agree, in this case the 998 should have a subfield. Not sure the standard has a say about this, but a lot of code I think assumed only <10 have no subfields.
Ah OK, actually control field can not be used with the subfield of a regular field. I see is not implemented in Koha::SimpleMARC : https://gitlab.com/koha-community/Koha/blob/f355788b340b19912891fd41c3168bac9c951aff/Koha/SimpleMARC.pm#L130 So this patch is good. Just a problem : When using 2 control fields, looks like move or copy with RexExp does not work. I think its from : https://gitlab.com/koha-community/Koha/blob/f355788b340b19912891fd41c3168bac9c951aff/Koha/SimpleMARC.pm#L570
22436(In reply to Katrin Fischer from comment #5) > > > 2) > > > This does not allow copy/move of a control field in the subfield of a > > > regular field, ie 005 => 998$d. > > > This should be allowed ? > > A very common use case would be 001 > 035$a + Adding a prefix in MARC21. So > it would be nice if that worked. > > > > What is surely not allowed is : 005 => 998 > > I agree, in this case the 998 should have a subfield. Not sure the standard > has a say about this, but a lot of code I think assumed only <10 have no > subfields. I have opened Bug 22436 for that.
Created attachment 97847 [details] [review] Bug 22245: Allow copy/move from and to control fields The JS conditions were wrong, we want to allow copy and move between control fields. Test plan: Create a new MARC modification template action using control fields. Confirm that you are allowed to copy/move a control field to another one Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Created attachment 99044 [details] [review] Bug 22245: Allow copy/move from and to control fields The JS conditions were wrong, we want to allow copy and move between control fields. Test plan: Create a new MARC modification template action using control fields. Confirm that you are allowed to copy/move a control field to another one Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz> Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Created attachment 99218 [details] [review] Bug 22245: Allow copy/move from and to control fields The JS conditions were wrong, we want to allow copy and move between control fields. Test plan: Create a new MARC modification template action using control fields. Confirm that you are allowed to copy/move a control field to another one Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz> Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
No regressions found and bug opened for followup work I see.. QA scripts are happy and I believe this patch moves us forward. Passing QA
Nice work everyone! Pushed to master for 20.05
Pushed to 19.11.x branch for 19.11.04
backported to 19.05.x for 19.05.09