When editing a MARC modification template action, you have the option of adding a conditional clause ("if/unless") to the action you want to perform. The conditional clause can be further fine-tuned by selecting one of four comparison operators ("exists/doesn't exist/matches/doesn't match") from a drop-down list. There are currently four issues when adding such a clause, namely: 1) When you pick a condition (e.g. "unless") and click on 'Add action' or 'Update action', the jQuery function that validates your input does not check whether you have actually filled in a value for the MARC field (mandatory) or the comparison operator (mandatory). 2) If you pick "matches" or "doesn't match" as a comparison operator, the textbox next to the operator can be left blank when saving the modification action (the textbox _should_ have a value because the operator is meaningless without it). 3) If you check the "RegEx" checkbox and then enter a value in the relevant textbox, when you Add/Update the action the "Action" column in the actions table will display extra spaces between the regex pattern and its delimiters. 4) If you check the "RegEx" checkbox and then enter a value in the relevant textbox, when you later edit that modification action the "m/ /" characters surrounding the regex textbox are not shown anymore.
Created attachment 82482 [details] [review] Bug 21861: The MARC modification template actions editor does not always validate user input The MARC modification template actions editor does not validate user input when adding a conditional clause, making it possible to save a modification action without filling in all necessary fields. This patch fixes that. Test plan: 1) Create a MARC modification template and add a new action, e.g. "Copy All field(s) 200$a to field 600$a". Add a condition ("if" or "unless") but leave the relevant field and subfield textboxes empty. Add "matches" or "doesn't match" as a comparison operator. Check the "RegEx" checkbox, leave the regex textbox empty. 2) Click on "Add action" and notice how it accepts your input. Also notice the extra space in the Action column RegEx. 3) Edit your modification action and notice the missing "m/" before the conditional RegEx. 4) Apply the patch and hit CTRL-F5 to reload the relevant JS. 5) Edit the modification action you added previously (or add a new one). Try leaving the textboxes mentioned above empty and then submit your changes. You should get pop-up windows informing you about missing fields. 6) Now try typing a value in the conditional regex textbox and Update your action. In the Action column, there should be no spaces between the delimiters of the matching operator and the value you just entered. 7) Edit your action: notice how the "m/" before the regular expression and the "/" after it are now showing correctly.
Created attachment 82528 [details] [review] Bug 21861: The MARC modification template actions editor does not always validate user input The MARC modification template actions editor does not validate user input when adding a conditional clause, making it possible to save a modification action without filling in all necessary fields. This patch fixes that. Test plan: 1) Create a MARC modification template and add a new action, e.g. "Copy All field(s) 200$a to field 600$a". Add a condition ("if" or "unless") but leave the relevant field and subfield textboxes empty. Add "matches" or "doesn't match" as a comparison operator. Check the "RegEx" checkbox, leave the regex textbox empty. 2) Click on "Add action" and notice how it accepts your input. Also notice the extra space in the Action column RegEx. 3) Edit your modification action and notice the missing "m/" before the conditional RegEx. 4) Apply the patch and hit CTRL-F5 to reload the relevant JS. 5) Edit the modification action you added previously (or add a new one). Try leaving the textboxes mentioned above empty and then submit your changes. You should get pop-up windows informing you about missing fields. 6) Now try typing a value in the conditional regex textbox and Update your action. In the Action column, there should be no spaces between the delimiters of the matching operator and the value you just entered. 7) Edit your action: notice how the "m/" before the regular expression and the "/" after it are now showing correctly.
+ document.getElementById('conditional_regex').onchange(); Please correct.
(In reply to Marcel de Rooy from comment #3) > + document.getElementById('conditional_regex').onchange(); > > Please correct. Probably needs a dep tree when rebasing..
Created attachment 82581 [details] [review] Bug 21861: (follow-up) replace onchange() with jQuery equivalent
Created attachment 82641 [details] [review] Bug 21861: The MARC modification template actions editor does not always validate user input The MARC modification template actions editor does not validate user input when adding a conditional clause, making it possible to save a modification action without filling in all necessary fields. This patch fixes that. Test plan: 1) Create a MARC modification template and add a new action, e.g. "Copy All field(s) 200$a to field 600$a". Add a condition ("if" or "unless") but leave the relevant field and subfield textboxes empty. Add "matches" or "doesn't match" as a comparison operator. Check the "RegEx" checkbox, leave the regex textbox empty. 2) Click on "Add action" and notice how it accepts your input. Also notice the extra space in the Action column RegEx. 3) Edit your modification action and notice the missing "m/" before the conditional RegEx. 4) Apply the patch and hit CTRL-F5 to reload the relevant JS. 5) Edit the modification action you added previously (or add a new one). Try leaving the textboxes mentioned above empty and then submit your changes. You should get pop-up windows informing you about missing fields. 6) Now try typing a value in the conditional regex textbox and Update your action. In the Action column, there should be no spaces between the delimiters of the matching operator and the value you just entered. 7) Edit your action: notice how the "m/" before the regular expression and the "/" after it are now showing correctly. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 82642 [details] [review] Bug 21861: (follow-up) replace onchange() with jQuery equivalent Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
String changes, heading for 19.05 ;)
(In reply to paxed from comment #2) You forgot a signoff line..
Moving to PQA now since paxed also tested (see above)
Awesome work everyone! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.01
backported to 18.05 for 18.05.07
Pushed to 17.11.x for 17.11.13