From 25bfc9eaf54b7fa538391e6b4892423de962df02 Mon Sep 17 00:00:00 2001 From: Jess Champion Date: Tue, 8 May 2018 17:02:01 +1200 Subject: [PATCH 1/8] Required fields can have display rules. Hidden required fields are dynamically excluded from server and client side validation. --- client/dist/js/userforms.js | 2 +- client/src/bundles/UserForms.js | 4 + code/Extension/UserFormValidator.php | 2 +- code/Form/UserForm.php | 5 +- code/Form/UserFormsRequiredFields.php | 173 ++++++++++++++++++ code/Model/EditableCustomRule.php | 78 ++++++-- code/Model/EditableFormField.php | 28 +-- code/Model/EditableFormField/Validator.php | 39 +--- lang/en.yml | 3 - lang/eo.yml | 7 +- lang/it.yml | 8 +- lang/sk.yml | 8 +- tests/Form/UserFormsRequiredFieldsTest.php | 149 +++++++++++++++ .../UserFormsCheckboxSetFieldTest.php | 4 +- tests/Model/EditableCustomRuleTest.php | 20 ++ tests/Model/EditableFormFieldTest.php | 5 - tests/UserFormsTest.yml | 107 +++++++++++ 17 files changed, 542 insertions(+), 100 deletions(-) create mode 100644 code/Form/UserFormsRequiredFields.php create mode 100644 tests/Form/UserFormsRequiredFieldsTest.php diff --git a/client/dist/js/userforms.js b/client/dist/js/userforms.js index 2fc19ca..e8c9a4b 100644 --- a/client/dist/js/userforms.js +++ b/client/dist/js/userforms.js @@ -1 +1 @@ -!function(t){function e(n){if(r[n])return r[n].exports;var i=r[n]={i:n,l:!1,exports:{}};return t[n].call(i.exports,i,i.exports,e),i.l=!0,i.exports}var r={};e.m=t,e.c=r,e.i=function(t){return t},e.d=function(t,r,n){e.o(t,r)||Object.defineProperty(t,r,{configurable:!1,enumerable:!0,get:n})},e.n=function(t){var r=t&&t.__esModule?function(){return t.default}:function(){return t};return e.d(r,"a",r),r},e.o=function(t,e){return Object.prototype.hasOwnProperty.call(t,e)},e.p="",e(e.s="./client/src/bundles/bundle.js")}({"./client/src/bundles/UserForms.js":function(t,e,r){"use strict";function n(t){return t&&t.__esModule?t:{default:t}}var i=r(1),s=n(i),o=r(0),a=n(o);(0,s.default)(document).ready(function(t){function e(e){return this.$el=e instanceof t?e:t(e),this.$el.find("h4").text(a.default._t("UserForms.ERROR_CONTAINER_HEADER","Please correct the following errors and try again:")),this}function r(r){var n=this;this.$el=r instanceof t?r:t(r);var i=this.$el.closest(".userform").data("inst");return this.$elButton=t(".step-button-wrapper[data-for='"+this.$el.prop("id")+"']"),this.viewed=!1,this.valid=!1,this.id=null,this.hide(),u.DISPLAY_ERROR_MESSAGES_AT_TOP&&(this.errorContainer=new e(this.$el.find(".error-container")),i.$el.on("userform.form.error",function(e,r){n.$el.is(":visible")&&t.each(r.errorList,function(e,r){n.errorContainer.updateErrorMessage(t(r.element),r.message)})}),i.$el.on("userform.form.valid",function(t,e){n.errorContainer.removeErrorMessage(e)})),this.$elButton.on("userform.field.hide userform.field.show",function(){i.$el.trigger("userform.form.conditionalstep")}),this}function n(e){var r=this;this.$el=e instanceof t?e:t(e),this.$buttons=this.$el.find(".step-button-jump"),this.$jsAlign=this.$el.find(".js-align");var n=this.$el.closest(".userform").data("inst");return this.$buttons.each(function(e,n){t(n).on("click",function(e){e.preventDefault();var n=parseInt(t(e.target).data("step"),10);r.$el.trigger("userform.progress.changestep",n)})}),n.$el.on("userform.form.changestep",function(t,e){r.update(e)}),n.$el.on("userform.form.conditionalstep",function(){var e=r.$buttons.filter(":visible");e.each(function(e,r){t(r).text(e+1)}),r.$el.find(".progress-bar").attr("aria-valuemax",e.length),r.$el.find(".total-step-number").text(e.length)}),this.$jsAlign.each(function(e,n){var i=t(n),s=100/(r.$jsAlign.length-1)*e,o=s+"%",a=i.innerWidth()/2*-1;i.css({left:o,marginLeft:a}),e===r.$jsAlign.length-1?i.css({marginLeft:2*a}):0===e&&i.css({marginLeft:0})}),this}function i(e){var r=this;return this.$el=e instanceof t?e:t(e),this.userformInstance=this.$el.closest(".userform").data("inst"),this.$prevButton=this.$el.find(".step-button-prev"),this.$nextButton=this.$el.find(".step-button-next"),this.$prevButton.parent().attr("aria-hidden",!1).show(),this.$nextButton.parent().attr("aria-hidden",!1).show(),this.$prevButton.on("click",function(t){t.preventDefault(),r.$el.trigger("userform.action.prev")}),this.$nextButton.on("click",function(t){t.preventDefault(),r.$el.trigger("userform.action.next")}),this.userformInstance.$el.on("userform.form.changestep userform.form.conditionalstep",function(){r.update()}),this}function s(r){var n=this;return this.$el=r instanceof t?r:t(r),this.steps=[],this.errorContainer=new e(this.$el.children(".error-container")),this.$el.on("userform.action.prev",function(){n.prevStep()}),this.$el.on("userform.action.next",function(){n.nextStep()}),this.$el.find(".userform-progress").on("userform.progress.changestep",function(t,e){n.jumpToStep(e-1)}),this.$el.on("userform.form.valid",function(t,e){n.errorContainer.removeStepLink(e)}),this.$el.validate(this.validationOptions),this.$el.find(".optionset.requiredField input").each(function(e,r){t(r).rules("add",{required:!0})}),this}function o(o,d){var f=this,c=t(d);if(0!==c.length){u.ENABLE_LIVE_VALIDATION=void 0!==c.data("livevalidation"),u.DISPLAY_ERROR_MESSAGES_AT_TOP=void 0!==c.data("toperrors"),!1===u.ENABLE_LIVE_VALIDATION&&t.extend(s.prototype.validationOptions,{onfocusout:!1}),u.DISPLAY_ERROR_MESSAGES_AT_TOP&&t.extend(s.prototype.validationOptions,{invalidHandler:function(t,e){c.trigger("userform.form.error",[e])},onfocusout:!1}),c.find(".userform-progress, .step-navigation").attr("aria-hidden",!1).show(),t.extend(r.prototype,l),t.extend(e.prototype,l);var h=new s(c);c.data("inst",h),u.HIDE_FIELD_LABELS&&c.find("label.left").each(function(){var e=t(f);t('[name="'+e.attr("for")+'"]').attr("placeholder",e.text()),e.remove()}),h.$el.find(".form-step").each(function(t,e){var n=new r(e);h.addStep(n)}),h.setCurrentStep(h.steps[0]);var p=c.find(".userform-progress");p.length&&new n(p).update(0);var m=c.find(".step-navigation");m.length&&new i(m).update(),t(document).on("click","input.text[data-showcalendar]",function(){var e=t(f);e.ssDatepicker(),e.data("datepicker")&&e.datepicker("show")}),setInterval(function(){t.ajax({url:"UserDefinedFormController/ping"})},18e4),void 0!==c.areYouSure&&c.areYouSure({message:a.default._t("UserForms.LEAVE_CONFIRMATION","You have unsaved changes!")})}}var u={},l={show:function(){this.$el.attr("aria-hidden",!1).show()},hide:function(){this.$el.attr("aria-hidden",!0).hide()}};e.prototype.hasErrors=function(){return this.$el.find(".error-list").children().length>0},e.prototype.removeErrorMessage=function(t){this.$el.find("#"+t+"-top-error").remove(),this.hasErrors()||this.hide()},e.prototype.addStepLink=function(e){var r=this.$el.closest(".userform").data("inst"),n=e.$el.attr("id")+"-error-link",i=this.$el.find("#"+n),s=e.$el.attr("id"),o=e.$el.data("title");i.length||(i=t('
  • '+o+"
    • "),i.on("click",function(t){t.preventDefault(),r.jumpToStep(e.id)}),this.$el.find(".error-list").append(i))},e.prototype.removeStepLink=function(e){var r=t("#"+e).closest(".form-step").attr("id");this.$el.find("#"+r+"-error-link").remove(),this.$el.find(".error-list").is(":empty")&&this.hide()},e.prototype.updateErrorMessage=function(e,r){var n=this,i=e.attr("id"),s="#"+i,o=i+"-top-error",a=t("#"+o),u=e.attr("aria-describedby");if(!r)return void a.addClass("fixed");a.removeClass("fixed"),this.show(),1===a.length?a.show().find("a").html(r):(e.closest(".field[id]").each(function(){s="#"+t(n).attr("id")}),a=t("
    • "),a.attr("id",o).find("a").attr("href",location.pathname+location.search+s).html(r),this.$el.find("ul").append(a),u?u.match(new RegExp("\\b"+o+"\\b"))||(u+=" "+o):u=o,e.attr("aria-describedby",u))},r.prototype.conditionallyHidden=function(){return!this.$elButton.find("button").is(":visible")},n.prototype.update=function(e){var r=t(this.$el.parent(".userform").find(".form-step")[e]),n=0,i=e/(this.$buttons.length-1)*100;this.$buttons.each(function(r,i){return!(r>e||(t(i).is(":visible")&&(n+=1),0))}),this.$el.find(".current-step-number").each(function(e,r){t(r).text(n)}),this.$el.find("[aria-valuenow]").each(function(e,r){t(r).attr("aria-valuenow",n)}),this.$buttons.each(function(e,r){var i=t(r),s=i.parent();if(parseInt(i.data("step"),10)===n&&i.is(":visible"))return s.addClass("current viewed"),void i.removeAttr("disabled");s.removeClass("current")}),this.$el.siblings(".progress-title").text(r.data("title")),i=i?i+"%":"",this.$el.find(".progress-bar").width(i)},i.prototype.update=function(){var t=this.userformInstance.steps.length,e=this.userformInstance.currentStep?this.userformInstance.currentStep.id:0,r=null,n=null;for(this.$el.find(".step-button-prev")[0===e?"hide":"show"](),r=t-1;r>=0;r--)if(n=this.userformInstance.steps[r],!n.conditionallyHidden()){this.$el.find(".step-button-next")[e>=r?"hide":"show"](),this.$el.find(".btn-toolbar")[e>=r?"show":"hide"]();break}},s.prototype.validationOptions={ignore:":hidden,ul",errorClass:"error",errorElement:"span",errorPlacement:function(t,e){t.addClass("message"),e.is(":radio")||e.parents(".checkboxset").length>0?t.appendTo(e.closest(".middleColumn")):e.parents(".checkbox").length>0?t.appendTo(e.closest(".field")):t.insertAfter(e)},invalidHandler:function(t,e){setTimeout(function(){e.currentElements.filter(".error").first().focus()},0)},submitHandler:function(e){var r=!0,n=t(e).closest(".userform").data("inst");n.currentStep&&(n.currentStep.valid=t(e).valid()),t.each(n.steps,function(t,e){e.valid||e.conditionallyHidden()||(r=!1,n.errorContainer.addStepLink(e))}),r?(t(e).removeClass("dirty"),e.submit(),n.$el.trigger("userform.form.submit")):n.errorContainer.show()},success:function(e){var r=t(e).closest(".userform").data("inst"),n=t(e).attr("id"),i=n.substr(0,n.indexOf("-error")).replace(/[\\[\\]]/,"");e.remove(),r.$el.trigger("userform.form.valid",[i])}},s.prototype.addStep=function(t){t instanceof r&&(t.id=this.steps.length,this.steps.push(t))},s.prototype.setCurrentStep=function(t){t instanceof r&&(this.currentStep=t,this.currentStep.show(),this.currentStep.viewed=!0,this.currentStep.$el.addClass("viewed"))},s.prototype.jumpToStep=function(t,e){var r=this.steps[t],n=!1,i=void 0===e||e;if(void 0!==r){if(r.conditionallyHidden())return void(i?this.jumpToStep(t+1):this.jumpToStep(t-1));n=this.$el.valid(),this.currentStep.valid=n,!1===n&&!1===r.viewed||(this.currentStep.hide(),this.setCurrentStep(r),this.$el.trigger("userform.form.changestep",[r.id]))}},s.prototype.nextStep=function(){this.jumpToStep(this.steps.indexOf(this.currentStep)+1,!0)},s.prototype.prevStep=function(){this.jumpToStep(this.steps.indexOf(this.currentStep)-1,!1)},t(".userform").each(o)})},"./client/src/bundles/bundle.js":function(t,e,r){"use strict";r("./client/src/bundles/UserForms.js")},0:function(t,e){t.exports=i18n},1:function(t,e){t.exports=jQuery}}); \ No newline at end of file +!function(t){function e(n){if(r[n])return r[n].exports;var i=r[n]={i:n,l:!1,exports:{}};return t[n].call(i.exports,i,i.exports,e),i.l=!0,i.exports}var r={};e.m=t,e.c=r,e.i=function(t){return t},e.d=function(t,r,n){e.o(t,r)||Object.defineProperty(t,r,{configurable:!1,enumerable:!0,get:n})},e.n=function(t){var r=t&&t.__esModule?function(){return t.default}:function(){return t};return e.d(r,"a",r),r},e.o=function(t,e){return Object.prototype.hasOwnProperty.call(t,e)},e.p="",e(e.s="./client/src/bundles/bundle.js")}({"./client/src/bundles/UserForms.js":function(t,e,r){"use strict";function n(t){return t&&t.__esModule?t:{default:t}}var i=r(1),s=n(i),o=r(0),a=n(o);(0,s.default)(document).ready(function(t){function e(e){return this.$el=e instanceof t?e:t(e),this.$el.find("h4").text(a.default._t("UserForms.ERROR_CONTAINER_HEADER","Please correct the following errors and try again:")),this}function r(r){var n=this;this.$el=r instanceof t?r:t(r);var i=this.$el.closest(".userform").data("inst");return this.$elButton=t(".step-button-wrapper[data-for='"+this.$el.prop("id")+"']"),this.viewed=!1,this.valid=!1,this.id=null,this.hide(),u.DISPLAY_ERROR_MESSAGES_AT_TOP&&(this.errorContainer=new e(this.$el.find(".error-container")),i.$el.on("userform.form.error",function(e,r){n.$el.is(":visible")&&t.each(r.errorList,function(e,r){n.errorContainer.updateErrorMessage(t(r.element),r.message)})}),i.$el.on("userform.form.valid",function(t,e){n.errorContainer.removeErrorMessage(e)})),this.$elButton.on("userform.field.hide userform.field.show",function(){i.$el.trigger("userform.form.conditionalstep")}),this}function n(e){var r=this;this.$el=e instanceof t?e:t(e),this.$buttons=this.$el.find(".step-button-jump"),this.$jsAlign=this.$el.find(".js-align");var n=this.$el.closest(".userform").data("inst");return this.$buttons.each(function(e,n){t(n).on("click",function(e){e.preventDefault();var n=parseInt(t(e.target).data("step"),10);r.$el.trigger("userform.progress.changestep",n)})}),n.$el.on("userform.form.changestep",function(t,e){r.update(e)}),n.$el.on("userform.form.conditionalstep",function(){var e=r.$buttons.filter(":visible");e.each(function(e,r){t(r).text(e+1)}),r.$el.find(".progress-bar").attr("aria-valuemax",e.length),r.$el.find(".total-step-number").text(e.length)}),this.$jsAlign.each(function(e,n){var i=t(n),s=100/(r.$jsAlign.length-1)*e,o=s+"%",a=i.innerWidth()/2*-1;i.css({left:o,marginLeft:a}),e===r.$jsAlign.length-1?i.css({marginLeft:2*a}):0===e&&i.css({marginLeft:0})}),this}function i(e){var r=this;return this.$el=e instanceof t?e:t(e),this.userformInstance=this.$el.closest(".userform").data("inst"),this.$prevButton=this.$el.find(".step-button-prev"),this.$nextButton=this.$el.find(".step-button-next"),this.$prevButton.parent().attr("aria-hidden",!1).show(),this.$nextButton.parent().attr("aria-hidden",!1).show(),this.$prevButton.on("click",function(t){t.preventDefault(),r.$el.trigger("userform.action.prev")}),this.$nextButton.on("click",function(t){t.preventDefault(),r.$el.trigger("userform.action.next")}),this.userformInstance.$el.on("userform.form.changestep userform.form.conditionalstep",function(){r.update()}),this}function s(r){var n=this;return this.$el=r instanceof t?r:t(r),this.steps=[],this.errorContainer=new e(this.$el.children(".error-container")),this.$el.on("userform.action.prev",function(){n.prevStep()}),this.$el.on("userform.action.next",function(){n.nextStep()}),this.$el.find(".userform-progress").on("userform.progress.changestep",function(t,e){n.jumpToStep(e-1)}),this.$el.on("userform.form.valid",function(t,e){n.errorContainer.removeStepLink(e)}),this.$el.validate(this.validationOptions),this.$el.find(".optionset.requiredField input").each(function(e,r){t(r).rules("add",{required:!0})}),this}function o(o,d){var f=this,c=t(d);if(0!==c.length){u.ENABLE_LIVE_VALIDATION=void 0!==c.data("livevalidation"),u.DISPLAY_ERROR_MESSAGES_AT_TOP=void 0!==c.data("toperrors"),!1===u.ENABLE_LIVE_VALIDATION&&t.extend(s.prototype.validationOptions,{onfocusout:!1}),u.DISPLAY_ERROR_MESSAGES_AT_TOP&&t.extend(s.prototype.validationOptions,{invalidHandler:function(t,e){c.trigger("userform.form.error",[e])},onfocusout:!1}),c.find(".userform-progress, .step-navigation").attr("aria-hidden",!1).show(),t.extend(r.prototype,l),t.extend(e.prototype,l);var h=new s(c);c.data("inst",h),u.HIDE_FIELD_LABELS&&c.find("label.left").each(function(){var e=t(f);t('[name="'+e.attr("for")+'"]').attr("placeholder",e.text()),e.remove()}),h.$el.find(".form-step").each(function(t,e){var n=new r(e);h.addStep(n)}),h.setCurrentStep(h.steps[0]);var p=c.find(".userform-progress");p.length&&new n(p).update(0);var m=c.find(".step-navigation");m.length&&new i(m).update(),t(document).on("click","input.text[data-showcalendar]",function(){var e=t(f);e.ssDatepicker(),e.data("datepicker")&&e.datepicker("show")}),setInterval(function(){t.ajax({url:"UserDefinedFormController/ping"})},18e4),void 0!==c.areYouSure&&c.areYouSure({message:a.default._t("UserForms.LEAVE_CONFIRMATION","You have unsaved changes!")})}}var u={},l={show:function(){this.$el.attr("aria-hidden",!1).show()},hide:function(){this.$el.attr("aria-hidden",!0).hide()}};e.prototype.hasErrors=function(){return this.$el.find(".error-list").children().length>0},e.prototype.removeErrorMessage=function(t){this.$el.find("#"+t+"-top-error").remove(),this.hasErrors()||this.hide()},e.prototype.addStepLink=function(e){var r=this.$el.closest(".userform").data("inst"),n=e.$el.attr("id")+"-error-link",i=this.$el.find("#"+n),s=e.$el.attr("id"),o=e.$el.data("title");i.length||(i=t('
  • '+o+"
    • "),i.on("click",function(t){t.preventDefault(),r.jumpToStep(e.id)}),this.$el.find(".error-list").append(i))},e.prototype.removeStepLink=function(e){var r=t("#"+e).closest(".form-step").attr("id");this.$el.find("#"+r+"-error-link").remove(),this.$el.find(".error-list").is(":empty")&&this.hide()},e.prototype.updateErrorMessage=function(e,r){var n=this,i=e.attr("id"),s="#"+i,o=i+"-top-error",a=t("#"+o),u=e.attr("aria-describedby");if(!r)return void a.addClass("fixed");a.removeClass("fixed"),this.show(),1===a.length?a.show().find("a").html(r):(e.closest(".field[id]").each(function(){s="#"+t(n).attr("id")}),a=t("
    • "),a.attr("id",o).find("a").attr("href",location.pathname+location.search+s).html(r),this.$el.find("ul").append(a),u?u.match(new RegExp("\\b"+o+"\\b"))||(u+=" "+o):u=o,e.attr("aria-describedby",u))},r.prototype.conditionallyHidden=function(){return!this.$elButton.find("button").is(":visible")},n.prototype.update=function(e){var r=t(this.$el.parent(".userform").find(".form-step")[e]),n=0,i=e/(this.$buttons.length-1)*100;this.$buttons.each(function(r,i){return!(r>e||(t(i).is(":visible")&&(n+=1),0))}),this.$el.find(".current-step-number").each(function(e,r){t(r).text(n)}),this.$el.find("[aria-valuenow]").each(function(e,r){t(r).attr("aria-valuenow",n)}),this.$buttons.each(function(e,r){var i=t(r),s=i.parent();if(parseInt(i.data("step"),10)===n&&i.is(":visible"))return s.addClass("current viewed"),void i.removeAttr("disabled");s.removeClass("current")}),this.$el.siblings(".progress-title").text(r.data("title")),i=i?i+"%":"",this.$el.find(".progress-bar").width(i)},i.prototype.update=function(){var t=this.userformInstance.steps.length,e=this.userformInstance.currentStep?this.userformInstance.currentStep.id:0,r=null,n=null;for(this.$el.find(".step-button-prev")[0===e?"hide":"show"](),r=t-1;r>=0;r--)if(n=this.userformInstance.steps[r],!n.conditionallyHidden()){this.$el.find(".step-button-next")[e>=r?"hide":"show"](),this.$el.find(".btn-toolbar")[e>=r?"show":"hide"]();break}},s.prototype.validationOptions={ignore:":hidden,ul",errorClass:"error",errorElement:"span",errorPlacement:function(t,e){t.addClass("message"),e.is(":radio")||e.parents(".checkboxset").length>0?t.appendTo(e.closest(".middleColumn")):e.parents(".checkbox").length>0?t.appendTo(e.closest(".field")):t.insertAfter(e)},invalidHandler:function(t,e){setTimeout(function(){e.currentElements.filter(".error").first().focus()},0)},submitHandler:function(e){var r=!0,n=t(e).closest(".userform").data("inst");n.currentStep&&(n.currentStep.valid=t(e).valid()),t.each(n.steps,function(t,e){e.valid||e.conditionallyHidden()||(r=!1,n.errorContainer.addStepLink(e))}),r?(t(e).find(".field.requiredField.hide input").removeAttr("required aria-required data-rule-required").valid(),t(e).removeClass("dirty"),e.submit(),n.$el.trigger("userform.form.submit")):n.errorContainer.show()},success:function(e){var r=t(e).closest(".userform").data("inst"),n=t(e).attr("id"),i=n.substr(0,n.indexOf("-error")).replace(/[\\[\\]]/,"");e.remove(),r.$el.trigger("userform.form.valid",[i])}},s.prototype.addStep=function(t){t instanceof r&&(t.id=this.steps.length,this.steps.push(t))},s.prototype.setCurrentStep=function(t){t instanceof r&&(this.currentStep=t,this.currentStep.show(),this.currentStep.viewed=!0,this.currentStep.$el.addClass("viewed"))},s.prototype.jumpToStep=function(t,e){var r=this.steps[t],n=!1,i=void 0===e||e;if(void 0!==r){if(r.conditionallyHidden())return void(i?this.jumpToStep(t+1):this.jumpToStep(t-1));n=this.$el.valid(),this.currentStep.valid=n,!1===n&&!1===r.viewed||(this.currentStep.hide(),this.setCurrentStep(r),this.$el.trigger("userform.form.changestep",[r.id]))}},s.prototype.nextStep=function(){this.jumpToStep(this.steps.indexOf(this.currentStep)+1,!0)},s.prototype.prevStep=function(){this.jumpToStep(this.steps.indexOf(this.currentStep)-1,!1)},t(".userform").each(o)})},"./client/src/bundles/bundle.js":function(t,e,r){"use strict";r("./client/src/bundles/UserForms.js")},0:function(t,e){t.exports=i18n},1:function(t,e){t.exports=jQuery}}); \ No newline at end of file diff --git a/client/src/bundles/UserForms.js b/client/src/bundles/UserForms.js index a740cdf..6289e9b 100644 --- a/client/src/bundles/UserForms.js +++ b/client/src/bundles/UserForms.js @@ -523,6 +523,10 @@ jQuery(document).ready(($) => { }); if (isValid) { + // Remove required attributes on hidden fields + $(form).find('.field.requiredField.hide input') + .removeAttr('required aria-required data-rule-required').valid(); + // When using the "are you sure?" plugin, ensure the form immediately submits. $(form).removeClass('dirty'); diff --git a/code/Extension/UserFormValidator.php b/code/Extension/UserFormValidator.php index d0306bb..461734d 100644 --- a/code/Extension/UserFormValidator.php +++ b/code/Extension/UserFormValidator.php @@ -31,7 +31,7 @@ class UserFormValidator extends RequiredFields // Page at top level, or after another page is ok if (empty($stack) || (count($stack) === 1 && $stack[0] instanceof EditableFormStep)) { $stack = array($field); - $conditionalStep = $field->EffectiveDisplayRules()->count() > 0; + $conditionalStep = $field->DisplayRules()->count() > 0; continue; } diff --git a/code/Form/UserForm.php b/code/Form/UserForm.php index 6211dda..161b939 100644 --- a/code/Form/UserForm.php +++ b/code/Form/UserForm.php @@ -8,7 +8,6 @@ use SilverStripe\Control\Session; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\FormAction; -use SilverStripe\Forms\RequiredFields; use SilverStripe\UserForms\FormField\UserFormsStepField; use SilverStripe\UserForms\FormField\UserFormsFieldList; @@ -163,7 +162,7 @@ class UserForm extends Form /** * Get the required form fields for this form. * - * @return RequiredFields + * @return UserFormsRequiredFields */ public function getRequiredFields() { @@ -175,7 +174,7 @@ class UserForm extends Form ->filter('Required', true) ->column('Name'); $requiredNames = array_merge($requiredNames, $this->getEmailRecipientRequiredFields()); - $required = new RequiredFields($requiredNames); + $required = new UserFormsRequiredFields($requiredNames); $this->extend('updateRequiredFields', $required); $required->setForm($this); return $required; diff --git a/code/Form/UserFormsRequiredFields.php b/code/Form/UserFormsRequiredFields.php new file mode 100644 index 0000000..02aac42 --- /dev/null +++ b/code/Form/UserFormsRequiredFields.php @@ -0,0 +1,173 @@ +form->Fields(); + + foreach ($fields as $field) { + $valid = ($field->validate($this) && $valid); + } + + if (empty($this->required)) { + return $valid; + } + + foreach ($this->required as $fieldName) { + if (!$fieldName) { + continue; + } + + // get form field + if ($fieldName instanceof FormField) { + $formField = $fieldName; + $fieldName = $fieldName->getName(); + } else { + $formField = $fields->dataFieldByName($fieldName); + } + + // get editable form field - owns display rules for field + $editableFormField = $this->getEditableFormFieldByName($fieldName); + + $error = false; + + // validate if there are no display rules or the field is conditionally visible + if (!$this->hasDisplayRules($editableFormField) || + $this->conditionalFieldEnabled($editableFormField, $data)) { + $error = $this->validateRequired($formField, $data); + } + + // handle error case + if ($formField && $error) { + $this->handleError($formField, $fieldName); + + $valid = false; + } + } + + return $valid; + } + + private function getEditableFormFieldByName($name) + { + return EditableFormField::get()->filter(['name' => $name])->first(); + } + + private function hasDisplayRules($field) + { + return ($field->DisplayRules()->count() > 0); + } + + private function conditionalFieldEnabled($editableFormField, $data) + { + $displayRules = $editableFormField->DisplayRules(); + + $conjunction = $editableFormField->DisplayRulesConjunctionNice(); + + $displayed = ($editableFormField->ShowOnLoadNice() === 'show'); + + // && start with true and find and condition that doesn't satisfy + // || start with false and find and condition that satisfies + $conditionsSatisfied = ($conjunction === '&&'); + + foreach ($displayRules as $rule) { + $controllingField = EditableFormField::get()->byID($rule->ConditionFieldID); + + if ($controllingField->DisplayRules()->count() > 0) { // controllingField is also a conditional field + // recursively check - if any of the dependant fields are hidden, then this field cannot be visible. + if ($this->conditionalFieldEnabled($controllingField, $data)) { + return false; + }; + } + + $ruleSatisfied = $rule->validateAgainstFormData($data); + + if ($conjunction === '||' && $ruleSatisfied) { + $conditionsSatisfied = true; + break; + } + if ($conjunction === '&&' && !$ruleSatisfied) { + $conditionsSatisfied = false; + break; + } + } + + // initially displayed - condition fails || initially hidden, condition passes + return ($displayed xor $conditionsSatisfied); + } + + // logic replicated from php() method of parent class SilverStripe\Forms\RequiredFields + // TODO refactor to share with parent (would require corrosponding change in framework) + private function validateRequired($field, $data) + { + $error = false; + $fieldName = $field->getName(); + // submitted data for file upload fields come back as an array + $value = isset($data[$fieldName]) ? $data[$fieldName] : null; + + if (is_array($value)) { + if ($field instanceof FileField && isset($value['error']) && $value['error']) { + $error = true; + } else { + $error = (count($value)) ? false : true; + } + } else { + // assume a string or integer + $error = (strlen($value)) ? false : true; + } + + return $error; + } + + private function handleError($formField, $fieldName) + { + $errorMessage = _t( + 'SilverStripe\\Forms\\Form.FIELDISREQUIRED', + '{name} is required', + array( + 'name' => strip_tags( + '"' . ($formField->Title() ? $formField->Title() : $fieldName) . '"' + ) + ) + ); + + if ($msg = $formField->getCustomValidationMessage()) { + $errorMessage = $msg; + } + + $this->validationError( + $fieldName, + $errorMessage, + "required" + ); + } +} diff --git a/code/Model/EditableCustomRule.php b/code/Model/EditableCustomRule.php index e3951bf..bd8a0b2 100644 --- a/code/Model/EditableCustomRule.php +++ b/code/Model/EditableCustomRule.php @@ -24,24 +24,24 @@ use SilverStripe\Versioned\Versioned; class EditableCustomRule extends DataObject { private static $condition_options = [ - 'IsBlank' => 'Is blank', - 'IsNotBlank' => 'Is not blank', - 'HasValue' => 'Equals', - 'ValueNot' => 'Doesn\'t equal', - 'ValueLessThan' => 'Less than', - 'ValueLessThanEqual' => 'Less than or equal', - 'ValueGreaterThan' => 'Greater than', + 'IsBlank' => 'Is blank', + 'IsNotBlank' => 'Is not blank', + 'HasValue' => 'Equals', + 'ValueNot' => 'Doesn\'t equal', + 'ValueLessThan' => 'Less than', + 'ValueLessThanEqual' => 'Less than or equal', + 'ValueGreaterThan' => 'Greater than', 'ValueGreaterThanEqual' => 'Greater than or equal' ]; private static $db = [ - 'Display' => 'Enum("Show,Hide")', + 'Display' => 'Enum("Show,Hide")', 'ConditionOption' => 'Enum("IsBlank,IsNotBlank,HasValue,ValueNot,ValueLessThan,ValueLessThanEqual,ValueGreaterThan,ValueGreaterThanEqual")', - 'FieldValue' => 'Varchar(255)' + 'FieldValue' => 'Varchar(255)' ]; private static $has_one = [ - 'Parent' => EditableFormField::class, + 'Parent' => EditableFormField::class, 'ConditionField' => EditableFormField::class ]; @@ -88,7 +88,7 @@ class EditableCustomRule extends DataObject * Return whether a user can create an object of this type * * @param Member $member - * @param array $context Virtual parameter to allow context to be passed in to check + * @param array $context Virtual parameter to allow context to be passed in to check * @return bool */ public function canCreate($member = null, $context = []) @@ -230,6 +230,62 @@ class EditableCustomRule extends DataObject return $result; } + + /** + * Determines whether the rule is satisfied, based on provided form data. + * Used for php validation of required conditional fields + * + * @param array $data Submitted form data + * @return boolean + */ + public function validateAgainstFormData($data) + { + + $controllingField = $this->ConditionField(); + + if (!isset($data[$controllingField->Name])) { + return false; + } + + $valid = false; + + $targetFieldValue = $this->FieldValue; + $actualFieldValue = $data[$controllingField->Name]; + + switch ($this->ConditionOption) { + case 'IsNotBlank': + $valid = ($actualFieldValue !== ''); + break; + case 'IsBlank': + $valid = ($actualFieldValue === ''); + break; + case 'HasValue': + $valid = ($actualFieldValue === $targetFieldValue); + break; + case 'ValueNot': + $valid = ($actualFieldValue !== $targetFieldValue); + break; + case 'ValueLessThan': + $valid = ($actualFieldValue < $targetFieldValue); + break; + case 'ValueLessThanEqual': + $valid = ($actualFieldValue <= $targetFieldValue); + break; + case 'ValueGreaterThan': + $valid = ($actualFieldValue > $targetFieldValue); + break; + case 'ValueGreaterThanEqual': + $valid = ($actualFieldValue >= $targetFieldValue); + break; + default: + throw new LogicException("Unhandled rule {$this->ConditionOption}"); + break; + } + + return $valid; + } + + /** * Returns the opposite visibility function for the value of the initial visibility field, e.g. show/hide. This * will toggle the "hide" class either way, which is handled by CSS. diff --git a/code/Model/EditableFormField.php b/code/Model/EditableFormField.php index 41000a6..1f02ed7 100755 --- a/code/Model/EditableFormField.php +++ b/code/Model/EditableFormField.php @@ -352,19 +352,6 @@ class EditableFormField extends DataObject */ protected function getDisplayRuleFields() { - // Check display rules - if ($this->Required) { - return FieldList::create( - LiteralField::create( - 'DisplayRulesNotEnabled', - '
    ' . _t( - __CLASS__.'.DISPLAY_RULES_DISABLED', - 'Display rules are not enabled for required fields. Please uncheck "Is this field Required?" under "Validation" to re-enable.' - ) . '
    ' - ) - ); - } - $allowedClasses = array_keys($this->getEditableFieldClasses(false)); $editableColumns = new GridFieldEditableColumns(); $editableColumns->setDisplayFields([ @@ -939,19 +926,6 @@ class EditableFormField extends DataObject ->setRecord($this); } - /** - * Determine effective display rules for this field. - * - * @return SS_List - */ - public function EffectiveDisplayRules() - { - if ($this->Required) { - return ArrayList::create(); - } - return $this->DisplayRules(); - } - /** * Extracts info from DisplayRules into array so UserDefinedForm->buildWatchJS can run through it. * @return array|null @@ -972,7 +946,7 @@ class EditableFormField extends DataObject // Check for field dependencies / default /** @var EditableCustomRule $rule */ - foreach ($this->EffectiveDisplayRules() as $rule) { + foreach ($this->DisplayRules() as $rule) { // Get the field which is effected /** @var EditableFormField $formFieldWatch */ $formFieldWatch = DataObject::get_by_id(EditableFormField::class, $rule->ConditionFieldID); diff --git a/code/Model/EditableFormField/Validator.php b/code/Model/EditableFormField/Validator.php index 2836a21..29c9242 100644 --- a/code/Model/EditableFormField/Validator.php +++ b/code/Model/EditableFormField/Validator.php @@ -39,43 +39,6 @@ class Validator extends RequiredFields return false; } - // When the record is unsaved and the classname is not set throw an error - if ((!$this->record || !$this->record->exists()) && (!isset($data['ClassName']) || empty($data['ClassName']))) { - $this->validationError( - 'ClassName', - _t( - __CLASS__ . 'CLASSNAME_ERROR', - 'You need to select a field type before you can create the field' - ) - ); - return false; - } - - // Skip unsaved records - if (!$this->record || !$this->record->exists()) { - return true; - } - - // Skip validation if not required - if (empty($data['Required'])) { - return; - } - - // Skip validation if no rules - $count = EditableCustomRule::get()->filter('ParentID', $this->record->ID)->count(); - if ($count == 0) { - return true; - } - - // Both required = true and rules > 0 should error - $this->validationError( - 'Required_Error', - _t( - __CLASS__.'.REQUIRED_ERROR', - 'Form fields cannot be required and have conditional display rules.' - ), - 'error' - ); - return false; + return true; } } diff --git a/lang/en.yml b/lang/en.yml index ba8e338..5e424ed 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -54,7 +54,6 @@ en: DEFAULT: 'Default value' DEFAULTTOTODAY: 'Default to Today?' DISPLAYIF: 'Toggle visibility when' - DISPLAY_RULES_DISABLED: 'Display rules are not enabled for required fields. Please uncheck "Is this field Required?" under "Validation" to re-enable.' EXTRACLASS_MULTIPLE: 'Separate each CSS class with a single space' EXTRACLASS_SELECT: 'Select from the list of allowed styles' EXTRACLASS_TITLE: 'Extra Styling/Layout' @@ -213,8 +212,6 @@ en: RANGE_TO: to SINGULARNAME: 'Text Field' TEXTLENGTH: 'Allowed text length' - SilverStripe\UserForms\Model\EditableFormField\Validator: - REQUIRED_ERROR: 'Form fields cannot be required and have conditional display rules.' SilverStripe\UserForms\Model\Recipient\EmailRecipient: CUSTOMRULESTAB: 'Custom Rules' EMAILCONTENTTAB: 'Email Content' diff --git a/lang/eo.yml b/lang/eo.yml index f03c97a..9f65d64 100644 --- a/lang/eo.yml +++ b/lang/eo.yml @@ -50,7 +50,10 @@ eo: DEFAULT: 'Apriora valoro' DEFAULTTOTODAY: 'Ĉu apriorie hodiaŭ?' DISPLAYIF: 'Baskuligi videblecon kiam' - DISPLAY_RULES_DISABLED: 'Vidigaj reguloj ne estas enŝaltitaj por bezonataj kampoj. Por reaktivigi ĝin, bonvolu malmarki "Ĉu ĉi tiu kampo estas bezonata?" sub "Validigo".' + DRAG: 'Ŝovi por rearanĝi kampojn' + ENTERQUESTION: 'Enigi demandon' + EXTRACLASSA: 'Ekstra stilado/aranĝado' + EXTRACLASSB: 'Ekstra css-klaso - apartigi opojn per spaceto' EXTRACLASS_MULTIPLE: 'Disigi ĉiun CSS-klason per unu spaceto' EXTRACLASS_SELECT: 'Elekti el la listo de eblaj stiloj' EXTRACLASS_TITLE: 'Ekstra stilado/aranĝado' @@ -208,8 +211,6 @@ eo: RANGE_TO: al SINGULARNAME: 'Teksta kampo' TEXTLENGTH: 'Permesata longo de teksto' - SilverStripe\UserForms\Model\EditableFormField\Validator: - REQUIRED_ERROR: 'Formularaj kampoj ne povas esti nepraj kaj havi kondiĉajn vidigoregulojn.' SilverStripe\UserForms\Model\Recipient\EmailRecipient: CUSTOMRULESTAB: 'Propraj reguloj' EMAILCONTENTTAB: 'Retpoŝta enhavo' diff --git a/lang/it.yml b/lang/it.yml index 3693090..d1f19c2 100644 --- a/lang/it.yml +++ b/lang/it.yml @@ -27,7 +27,11 @@ it: CUSTOMRULES: 'Regole personalizzate' DEFAULT: 'Valore di default' DEFAULTTOTODAY: 'Di default a oggi?' - DISPLAY_RULES_DISABLED: 'Le regole di visualizzazione non sono attivate per i campi richiesti. Per piacere deseleziona "Questo campo è obbligatorio?" sotto "Validazione" per riabilitarle.' + DELETE: Elimina + DRAG: 'Trascina per riordinare l''ordine dei campi' + ENTERQUESTION: 'Inserisci la domanda' + EXTRACLASSA: 'Stile/Layout extra' + EXTRACLASSB: 'Classe CSS extra - separa valori multipli con uno spazio' EXTRACLASS_MULTIPLE: 'Separa ogni classe CSS con uno spazio singolo' EXTRACLASS_SELECT: 'Seleziona da una lista di stili consentiti' EXTRACLASS_TITLE: 'Stile/Layout extra' @@ -116,8 +120,6 @@ it: RANGE_TO: a SINGULARNAME: 'Campo testo' TEXTLENGTH: 'Lunghezza testo consentita' - SilverStripe\UserForms\Model\EditableFormField\Validator: - REQUIRED_ERROR: 'I campi dei form non possono essere obbligatori e avere regole di visualizzazione condizionali.' SilverStripe\UserForms\Model\Recipient\EmailRecipient: CUSTOMRULESTAB: 'Regole personalizzate' EMAILCONTENTTAB: 'Contenuto dell''e-mail' diff --git a/lang/sk.yml b/lang/sk.yml index c27db30..c6e279f 100644 --- a/lang/sk.yml +++ b/lang/sk.yml @@ -27,7 +27,11 @@ sk: CUSTOMRULES: 'Vlastné pravidlá' DEFAULT: 'Predvolená hodnota' DEFAULTTOTODAY: 'Zobraziť v predvolenom režime na dnešný dátum?' - DISPLAY_RULES_DISABLED: 'Pravidlá zobrazenia nie sú povolené pre povinné/vyžadované polia. Prosím odškrtnite "Je pole povinné/vyžadované?" na záložke "Validácia".' + DELETE: Vymazať + DRAG: 'Ťahaním preskupiť poradie polí' + ENTERQUESTION: 'Zadajte otázku' + EXTRACLASSA: 'Extra úprava/vzhľad' + EXTRACLASSB: 'Extra CSS triedy - viaceré triedy oddeľujte medzerou' EXTRACLASS_MULTIPLE: 'Jednotlivé CSS triedy oddeľte jednou medzerou' EXTRACLASS_SELECT: 'Vyberte si zo zoznamu povolených štýlov' EXTRACLASS_TITLE: 'Extra úprava/vzhľad' @@ -116,8 +120,6 @@ sk: RANGE_TO: do SINGULARNAME: 'Textové pole' TEXTLENGTH: 'Povolená dĺžka textu' - SilverStripe\UserForms\Model\EditableFormField\Validator: - REQUIRED_ERROR: 'Formulárové polia nemôžu byť vyžadované, ak majú podmienené pravidlá zobrazenia.' SilverStripe\UserForms\Model\Recipient\EmailRecipient: CUSTOMRULESTAB: 'Vlastné pravidlá' EMAILCONTENTTAB: 'Obsah e-mailu' diff --git a/tests/Form/UserFormsRequiredFieldsTest.php b/tests/Form/UserFormsRequiredFieldsTest.php new file mode 100644 index 0000000..6688de3 --- /dev/null +++ b/tests/Form/UserFormsRequiredFieldsTest.php @@ -0,0 +1,149 @@ +getValidator(); + } + + public function testUsesUserFormsRequiredFieldsValidator() + { + $page = $this->objFromFixture(UserDefinedForm::class, 'required-custom-rules-form'); + $this->assertEquals(3, $page->Fields()->count()); + $validator = $this->getValidatorFromPage($page); + $this->assertNotNull($validator); + $this->assertInstanceOf(UserFormsRequiredFields::class, $validator, 'Uses UserFormsRequiredFields validator'); + } + + public function testValidationOfConditionalRequiredFields() + { + $page = $this->objFromFixture(UserDefinedForm::class, 'required-custom-rules-form'); + $validator = $this->getValidatorFromPage($page); + $this->assertNotNull($validator); + + $this->assertFalse( + $validator->php([]), + 'Fails when non-conditional required field is empty' + ); + + $this->assertTrue( + $validator->php( + [ + 'required-text-field-2' => 'some text', + 'radio-option-2' => 'N', + 'conditional-required-text' => '' + ] + ), + 'Passes when non-conditional required field has a value' + ); + + $this->assertFalse( + $validator->php( + [ + 'required-text-field-2' => 'some text', + 'radio-option-2' => 'Y', + 'conditional-required-text' => '' + ] + ), + 'Fails when conditional required is displayed but not completed' + ); + + $this->assertTrue( + $validator->php( + [ + 'required-text-field-2' => 'some text', + 'radio-option-2' => 'Y', + 'conditional-required-text' => 'some more text' + ] + ), + 'Passes when conditional required field has a value' + ); + } + + public function testValidationOfNestedConditionalRequiredFields() + { + $page = $this->objFromFixture(UserDefinedForm::class, 'required-nested-custom-rules-form'); + $this->assertEquals(4, $page->Fields()->count()); + $validator = $this->getValidatorFromPage($page); + $this->assertNotNull($validator); + + $this->assertFalse( + $validator->php([]), + 'Fails when non-conditional required field is empty' + ); + + $this->assertTrue( + $validator->php( + [ + 'required-text-field-3' => 'some text', + 'radio-option-2' => 'N', + 'conditional-required-text-2' => '', + 'conditional-required-text-3' => '' + ] + ), + 'Passes when non-conditional required field has a value' + ); + + $this->assertFalse( + $validator->php( + [ + 'required-text-field-3' => 'some text', + 'radio-option-2' => 'Y', + 'conditional-required-text-2' => '', + 'conditional-required-text-3' => '' + ] + ), + 'Fails when conditional required is displayed but not completed' + ); + + $this->assertTrue( + $validator->php( + [ + 'required-text-field-3' => 'some text', + 'radio-option-3' => 'Y', + 'conditional-required-text-2' => 'this text', + 'conditional-required-text-3' => '' + ] + ), + 'Passes when non-conditional required field has a value' + ); + + $this->assertFalse( + $validator->php( + [ + 'required-text-field-3' => 'some text', + 'radio-option-3' => 'Y', + 'conditional-required-text-2' => 'Show more', + 'conditional-required-text-3' => '' + ] + ), + 'Fails when nested conditional required is displayed but not completed' + ); + + $this->assertTrue( + $validator->php( + [ + 'required-text-field-3' => 'some text', + 'radio-option-3' => 'Y', + 'conditional-required-text-2' => 'Show more', + 'conditional-required-text-3' => 'more text' + ] + ), + 'Passes when nested conditional required field has a value' + ); + } +} diff --git a/tests/FormField/UserFormsCheckboxSetFieldTest.php b/tests/FormField/UserFormsCheckboxSetFieldTest.php index 58aefed..dba5332 100644 --- a/tests/FormField/UserFormsCheckboxSetFieldTest.php +++ b/tests/FormField/UserFormsCheckboxSetFieldTest.php @@ -3,7 +3,7 @@ namespace SilverStripe\UserForms\Tests\FormField; use SilverStripe\Dev\SapphireTest; -use SilverStripe\Forms\RequiredFields; +use SilverStripe\UserForms\Form\UserFormsRequiredFields; use SilverStripe\UserForms\FormField\UserFormsCheckboxSetField; class UserFormsCheckboxSetFieldTest extends SapphireTest @@ -11,7 +11,7 @@ class UserFormsCheckboxSetFieldTest extends SapphireTest public function testValidate() { $field = new UserFormsCheckboxSetField('Field', 'My field', ['One' => 'One', 'Two' => 'Two']); - $validator = new RequiredFields(); + $validator = new UserFormsRequiredFields(); // String values $field->setValue('One'); diff --git a/tests/Model/EditableCustomRuleTest.php b/tests/Model/EditableCustomRuleTest.php index 3d08d9a..1a70a56 100644 --- a/tests/Model/EditableCustomRuleTest.php +++ b/tests/Model/EditableCustomRuleTest.php @@ -60,4 +60,24 @@ class EditableCustomRuleTest extends SapphireTest $this->assertSame('userform.field.show', $rule1->toggleDisplayEvent('show', true)); $this->assertSame('userform.field.hide', $rule1->toggleDisplayEvent('hide', true)); } + + /** + * Test that methods are returned for manipulating the presence of the "hide" CSS class depending + * on whether the field should be hidden or shown + */ + public function testValidateAgainstFormData() + { + $rule1 = $this->objFromFixture(EditableCustomRule::class, 'rule1'); + + $this->assertFalse($rule1->validateAgainstFormData([])); + $this->assertFalse($rule1->validateAgainstFormData(['CountrySelection' => 'AU'])); + $this->assertTrue($rule1->validateAgainstFormData(['CountrySelection' => 'NZ'])); + + $rule2 = $this->objFromFixture(EditableCustomRule::class, 'rule2'); + + $this->assertFalse($rule2->validateAgainstFormData([])); + $this->assertFalse($rule2->validateAgainstFormData(['CountryTextSelection' => 0])); + $this->assertFalse($rule2->validateAgainstFormData(['CountryTextSelection' => 1])); + $this->assertTrue($rule2->validateAgainstFormData(['CountryTextSelection' => 2])); + } } diff --git a/tests/Model/EditableFormFieldTest.php b/tests/Model/EditableFormFieldTest.php index 595a340..c1580de 100644 --- a/tests/Model/EditableFormFieldTest.php +++ b/tests/Model/EditableFormFieldTest.php @@ -72,7 +72,6 @@ class EditableFormFieldTest extends FunctionalTest // form has 2 fields - a checkbox and a text field // it has 1 rule - when ticked the checkbox hides the text field $this->assertEquals(1, $rules->Count()); - $this->assertEquals($rules, $checkbox->EffectiveDisplayRules()); $checkboxRule = $rules->First(); $checkboxRule->ConditionFieldID = $field->ID; @@ -80,10 +79,6 @@ class EditableFormFieldTest extends FunctionalTest $this->assertEquals($checkboxRule->Display, 'Hide'); $this->assertEquals($checkboxRule->ConditionOption, 'HasValue'); $this->assertEquals($checkboxRule->FieldValue, '6'); - - // If field is required then all custom rules are disabled - $checkbox->Required = true; - $this->assertEquals(0, $checkbox->EffectiveDisplayRules()->count()); } public function testEditableOptionEmptyValue() diff --git a/tests/UserFormsTest.yml b/tests/UserFormsTest.yml index 9098845..d715aec 100644 --- a/tests/UserFormsTest.yml +++ b/tests/UserFormsTest.yml @@ -61,6 +61,26 @@ SilverStripe\UserForms\Model\EditableFormField\EditableOption: Name: Option9 Title: Green + option-y: + Name: option-y + Title: Yes + Value: Y + + option-n: + Name: option-n + Title: No + Value: N + + option-y-2: + Name: option-y-2 + Title: Yes + Value: Y + + option-n-2: + Name: option-n-2 + Title: No + Value: N + SilverStripe\UserForms\Model\EditableFormField\EditableTextField: basic-text: Name: basic_text_name @@ -92,6 +112,38 @@ SilverStripe\UserForms\Model\EditableFormField\EditableTextField: CustomErrorMessage: Custom Error Message Required: true + required-text-2: + Name: required-text-field-2 + Title: Required Text Field 2 + Required: true + + required-text-3: + Name: required-text-field-3 + Title: Required Text Field 3 + Required: true + + conditional-required-text: + Name: conditional-required-text + Title: Conditional Required Text Field + CustomErrorMessage: Custom Error Message + Required: true + DisplayRulesConjunction: Or + ShowOnLoad: false + + conditional-required-text-2: + Name: conditional-required-text-2 + Title: Conditional Required Text Field 2 + Required: true + DisplayRulesConjunction: Or + ShowOnLoad: false + + conditional-required-text-3: + Name: conditional-required-text-3 + Title: Conditional Required Text Field 3 + Required: true + DisplayRulesConjunction: Or + ShowOnLoad: false + field-1: Name: Field1 @@ -141,6 +193,14 @@ SilverStripe\UserForms\Model\EditableFormField\EditableCheckbox: Name: checkbox-1 Title: Checkbox 1 + checkbox-3: + Name: checkbox-3 + Title: Checkbox 3 + + checkbox-4: + Name: checkbox-4 + Title: Checkbox 4 + SilverStripe\UserForms\Model\EditableFormField\EditableCheckboxGroupField: checkbox-group: Name: check-box-group @@ -174,6 +234,18 @@ SilverStripe\UserForms\Model\EditableFormField\EditableRadioField: Options: - =>SilverStripe\UserForms\Model\EditableFormField\EditableOption.option-5 - =>SilverStripe\UserForms\Model\EditableFormField\EditableOption.option-6 + radio-field-2: + Name: radio-option-2 + Title: Radio Option 2 + Options: + - =>SilverStripe\UserForms\Model\EditableFormField\EditableOption.option-y + - =>SilverStripe\UserForms\Model\EditableFormField\EditableOption.option-n + radio-field-3: + Name: radio-option-2 + Title: Radio Option 3 + Options: + - =>SilverStripe\UserForms\Model\EditableFormField\EditableOption.option-y-2 + - =>SilverStripe\UserForms\Model\EditableFormField\EditableOption.option-n-2 SilverStripe\UserForms\Model\EditableFormField\EditableFieldGroupEnd: group1end: @@ -308,6 +380,21 @@ SilverStripe\UserForms\Model\UserDefinedForm: - =>SilverStripe\UserForms\Model\EditableFormField\EditableCheckbox.checkbox-2 - =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text-2 + required-custom-rules-form: + Title: Required Custom Rules Form + Fields: + - =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.required-text-2 + - =>SilverStripe\UserForms\Model\EditableFormField\EditableRadioField.radio-field-2 + - =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.conditional-required-text + + required-nested-custom-rules-form: + Title: Required Nested Custom Rules Form + Fields: + - =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.required-text-3 + - =>SilverStripe\UserForms\Model\EditableFormField\EditableRadioField.radio-field-3 + - =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.conditional-required-text-2 + - =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.conditional-required-text-3 + summary-rules-form: Title: Summary Fields Form Fields: @@ -349,3 +436,23 @@ SilverStripe\UserForms\Model\UserDefinedForm: Title: Form with MultipleOption fields Fields: - =>SilverStripe\UserForms\Model\EditableFormField\EditableDropdown.basic-dropdown + +SilverStripe\UserForms\Model\EditableCustomRule: + rule1: + Display: Show + ConditionOption: HasValue + FieldValue: 'Y' + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableRadioField.radio-field-2 + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.conditional-required-text + rule2: + Display: Show + ConditionOption: HasValue + FieldValue: 'Y' + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableRadioField.radio-field-3 + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.conditional-required-text-2 + rule3: + Display: Show + ConditionOption: HasValue + FieldValue: 'Show more' + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.conditional-required-text-2 + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.conditional-required-text-3 From b53619477c02e005097b6ac49eb4befd1953627f Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Fri, 10 Jan 2020 17:34:43 +1300 Subject: [PATCH 2/8] Revert the return type for UserForm::getRequiredFields --- code/Form/UserForm.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/Form/UserForm.php b/code/Form/UserForm.php index 161b939..d6a2c1b 100644 --- a/code/Form/UserForm.php +++ b/code/Form/UserForm.php @@ -162,7 +162,7 @@ class UserForm extends Form /** * Get the required form fields for this form. * - * @return UserFormsRequiredFields + * @return RequiredFields */ public function getRequiredFields() { From a0cedaeb384393dd083b7ea5d4288ea1b46dc70b Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Fri, 10 Jan 2020 17:36:04 +1300 Subject: [PATCH 3/8] Move conditionalFieldEnabled to EditableFormField as isDisplayed --- code/Form/UserFormsRequiredFields.php | 81 ++++++--------- code/Model/EditableFormField.php | 44 +++++++- tests/Model/EditableFormFieldTest.php | 54 ++++++++++ tests/Model/EditableFormFieldTest.yml | 139 +++++++++++++++++++++++++- 4 files changed, 263 insertions(+), 55 deletions(-) diff --git a/code/Form/UserFormsRequiredFields.php b/code/Form/UserFormsRequiredFields.php index 02aac42..bce0beb 100644 --- a/code/Form/UserFormsRequiredFields.php +++ b/code/Form/UserFormsRequiredFields.php @@ -2,6 +2,7 @@ namespace SilverStripe\UserForms\Form; +use InvalidArgumentException; use SilverStripe\Dev\Debug; use SilverStripe\Forms\FileField; use SilverStripe\Forms\FormField; @@ -27,7 +28,7 @@ class UserFormsRequiredFields extends RequiredFields * * @param array $data * - * @return boolean + * @return bool */ public function php($data) { @@ -58,18 +59,14 @@ class UserFormsRequiredFields extends RequiredFields // get editable form field - owns display rules for field $editableFormField = $this->getEditableFormFieldByName($fieldName); - $error = false; - - // validate if there are no display rules or the field is conditionally visible - if (!$this->hasDisplayRules($editableFormField) || - $this->conditionalFieldEnabled($editableFormField, $data)) { - $error = $this->validateRequired($formField, $data); - } + // Validate if the field is displayed + $error = + $editableFormField->isDisplayed($data) && + $this->validateRequired($formField, $data); // handle error case if ($formField && $error) { $this->handleError($formField, $fieldName); - $valid = false; } } @@ -77,57 +74,35 @@ class UserFormsRequiredFields extends RequiredFields return $valid; } + /** + * Retrieve an Editable Form field by its name. + * @param string $name + * @return EditableFormField + */ private function getEditableFormFieldByName($name) { - return EditableFormField::get()->filter(['name' => $name])->first(); - } + $field = EditableFormField::get()->filter(['Name' => $name])->first(); - private function hasDisplayRules($field) - { - return ($field->DisplayRules()->count() > 0); - } - - private function conditionalFieldEnabled($editableFormField, $data) - { - $displayRules = $editableFormField->DisplayRules(); - - $conjunction = $editableFormField->DisplayRulesConjunctionNice(); - - $displayed = ($editableFormField->ShowOnLoadNice() === 'show'); - - // && start with true and find and condition that doesn't satisfy - // || start with false and find and condition that satisfies - $conditionsSatisfied = ($conjunction === '&&'); - - foreach ($displayRules as $rule) { - $controllingField = EditableFormField::get()->byID($rule->ConditionFieldID); - - if ($controllingField->DisplayRules()->count() > 0) { // controllingField is also a conditional field - // recursively check - if any of the dependant fields are hidden, then this field cannot be visible. - if ($this->conditionalFieldEnabled($controllingField, $data)) { - return false; - }; - } - - $ruleSatisfied = $rule->validateAgainstFormData($data); - - if ($conjunction === '||' && $ruleSatisfied) { - $conditionsSatisfied = true; - break; - } - if ($conjunction === '&&' && !$ruleSatisfied) { - $conditionsSatisfied = false; - break; - } + if ($field) { + return $field; } - // initially displayed - condition fails || initially hidden, condition passes - return ($displayed xor $conditionsSatisfied); + // This should happen if form field data got corrupted + throw new InvalidArgumentException(sprintf( + 'Could not find EditableFormField with name `%s`', + $name + )); } - // logic replicated from php() method of parent class SilverStripe\Forms\RequiredFields - // TODO refactor to share with parent (would require corrosponding change in framework) - private function validateRequired($field, $data) + /** + * Check if the validation rules for the specified field are met by the provided data. + * + * @note Logic replicated from php() method of parent class `SilverStripe\Forms\RequiredFields` + * @param EditableFormField $field + * @param array $data + * @return bool + */ + private function validateRequired(FormField $field, array $data) { $error = false; $fieldName = $field->getName(); diff --git a/code/Model/EditableFormField.php b/code/Model/EditableFormField.php index 1f02ed7..2ec4f8f 100755 --- a/code/Model/EditableFormField.php +++ b/code/Model/EditableFormField.php @@ -57,7 +57,7 @@ use Symbiote\GridFieldExtensions\GridFieldEditableColumns; * @property boolean $ShowOnLoad * @property string $DisplayRulesConjunction * @method UserDefinedForm Parent() Parent page - * @method DataList DisplayRules() List of EditableCustomRule objects + * @method DataList|EditableCustomRule[] DisplayRules() List of EditableCustomRule objects * @mixin Versioned */ class EditableFormField extends DataObject @@ -977,6 +977,48 @@ class EditableFormField extends DataObject return (count($result['selectors'])) ? $result : null; } + /** + * Check if this EditableFormField is displayed based on its DisplayRules and the provided data. + * @param array $data + * @return bool + */ + public function isDisplayed(array $data) + { + $displayRules = $this->DisplayRules(); + + if ($displayRules->count() === 0) { + // If no display rule have been defined, isDisplayed equals the ShowOnLoad property + return $this->ShowOnLoad; + } + + $conjunction = $this->DisplayRulesConjunctionNice(); + + // && start with true and find and condition that doesn't satisfy + // || start with false and find and condition that satisfies + $conditionsSatisfied = ($conjunction === '&&'); + + foreach ($displayRules as $rule) { + $controllingField = $rule->ConditionField(); + + // recursively check - if any of the dependant fields are hidden, assume the rule can not be satisfied + $ruleSatisfied = $controllingField->isDisplayed($data) && $rule->validateAgainstFormData($data); + + if ($conjunction === '||' && $ruleSatisfied) { + $conditionsSatisfied = true; + break; + } + if ($conjunction === '&&' && !$ruleSatisfied) { + $conditionsSatisfied = false; + break; + } + } + + // initially displayed - condition fails || initially hidden, condition passes + $startDisplayed = $this->ShowOnLoad; + return ($startDisplayed xor $conditionsSatisfied); + } + + /** * Replaces the set DisplayRulesConjunction with their JS logical operators * @return string diff --git a/tests/Model/EditableFormFieldTest.php b/tests/Model/EditableFormFieldTest.php index c1580de..80de25b 100644 --- a/tests/Model/EditableFormFieldTest.php +++ b/tests/Model/EditableFormFieldTest.php @@ -232,4 +232,58 @@ class EditableFormFieldTest extends FunctionalTest $this->assertContains('/images/editabletextfield.png', $field->getIcon()); } + + public function displayedProvider() + { + $one = ['basic_text_name' => 'foobar']; + $two = array_merge($one, ['basic_text_name_2' => 'foobar']); + + return [ + 'no display rule AND' => ['alwaysVisible', [], true], + 'no display rule OR' => ['alwaysVisibleOr', [], true], + + 'no display rule hidden AND' => ['neverVisible', [], false], + 'no display rule hidden OR' => ['neverVisibleOr', [], false], + + '1 unmet display rule AND' => ['singleDisplayRule', [], false], + '1 met display rule AND' => ['singleDisplayRule', $one, true], + '1 unmet display rule OR' => ['singleDisplayRuleOr', [], false], + '1 met display rule OR' => ['singleDisplayRuleOr', $one, true], + + '1 unmet hide rule AND' => ['singleHiddingRule', [], true], + '1 met hide rule AND' => ['singleHiddingRule', $one, false], + '1 unmet hide rule OR' => ['singleHiddingRuleOr', [], true], + '1 met hide rule OR' => ['singleHiddingRuleOr', $one, false], + + 'multi display rule AND none met' => ['multiDisplayRule', [], false], + 'multi display rule AND partially met' => ['multiDisplayRule', $one, false], + 'multi display rule AND all met' => ['multiDisplayRule', $two, true], + + 'multi display rule OR none met' => ['multiDisplayRuleOr', [], false], + 'multi display rule OR partially met' => ['multiDisplayRuleOr', $one, true], + 'multi display rule OR all met' => ['multiDisplayRuleOr', $two, true], + + 'multi hide rule AND none met' => ['multiHiddingRule', [], true], + 'multi hide rule AND partially met' => ['multiHiddingRule', $one, true], + 'multi hide rule AND all met' => ['multiHiddingRule', $two, false], + + 'multi hide rule OR none met' => ['multiHiddingRuleOr', [], true], + 'multi hide rule OR partially met' => ['multiHiddingRuleOr', $one, false], + 'multi hide rule OR all met' => ['multiHiddingRuleOr', $two, false], + ]; + } + + /** + * @param $fieldName + * @param $data + * @param $expected + * @dataProvider displayedProvider + */ + public function testIsDisplayed($fieldName, $data, bool $expected) + { + /** @var EditableFormField $field */ + $field = $this->objFromFixture(EditableTextField::class, $fieldName); + $this->assertEquals($expected, $field->isDisplayed($data)); + } + } diff --git a/tests/Model/EditableFormFieldTest.yml b/tests/Model/EditableFormFieldTest.yml index b514964..d5f8695 100644 --- a/tests/Model/EditableFormFieldTest.yml +++ b/tests/Model/EditableFormFieldTest.yml @@ -4,7 +4,7 @@ SilverStripe\UserForms\Model\EditableFormField\EditableTextField: Title: Basic Text Field basic-text-2: - Name: basic_text_name + Name: basic_text_name_2 Title: Basic Text Field required-text: @@ -23,6 +23,83 @@ SilverStripe\UserForms\Model\EditableFormField\EditableTextField: DisplayRulesConjunction: And ShowOnLoad: false + # No rule + alwaysVisible: + Name: AlwaysVisible + Title: "This field is always visible" + ShowOnLoad: true + DisplayRulesConjunction: And + + alwaysVisibleOr: + Name: AlwaysVisibleOr + Title: "This field is always visible" + ShowOnLoad: true + DisplayRulesConjunction: Or + + neverVisible: + Name: NeverVisible + Title: "This field is never visible" + ShowOnLoad: false + DisplayRulesConjunction: And + + neverVisibleOr: + Name: NeverVisibleOr + Title: "This field is never visible" + ShowOnLoad: false + DisplayRulesConjunction: Or + + # Single rule + + singleDisplayRule: + Name: SingleDisplayRule + Title: "This field will be displayed if the display rule is tripped" + ShowOnLoad: false + DisplayRulesConjunction: And + + singleDisplayRuleOr: + Name: SingleDisplayRuleOr + Title: "This field will be displayed if the display rule is tripped" + ShowOnLoad: false + DisplayRulesConjunction: Or + + singleHiddingRule: + Name: SingleHiddingRule + Title: "This field will be hidden if the display rule is tripped" + ShowOnLoad: true + DisplayRulesConjunction: And + + singleHiddingRuleOr: + Name: SingleHiddingRuleOr + Title: "This field will be hidden if the display rule is tripped" + ShowOnLoad: true + DisplayRulesConjunction: Or + + # Multi rule + multiDisplayRule: + Name: MultiDisplayRule + Title: "This field will be displayed if displayed if all the rule are met" + ShowOnLoad: false + DisplayRulesConjunction: And + + multiDisplayRuleOr: + Name: MultiDisplayRuleOr + Title: "This field will be displayed if at least one rule is met" + ShowOnLoad: false + DisplayRulesConjunction: Or + + multiHiddingRule: + Name: MultiHiddingRule + Title: "This field will be hidden if all the rule are met" + ShowOnLoad: true + DisplayRulesConjunction: And + + multiHiddingRuleOr: + Name: MultiHiddingRuleOr + Title: "This field will be hidden if one rule is met" + ShowOnLoad: true + DisplayRulesConjunction: Or + + SilverStripe\UserForms\Model\EditableCustomRule: rule1: Display: Show @@ -35,6 +112,66 @@ SilverStripe\UserForms\Model\EditableCustomRule: ConditionOption: HasValue FieldValue: 6 + # Single rules + ruleSingleDisplay: + Display: Show + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.singleDisplayRule + ruleSingleDisplayOr: + Display: Show + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.singleDisplayRuleOr + ruleSingleHidding: + Display: Show + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.singleHiddingRule + ruleSingleHiddingOr: + Display: Show + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.singleHiddingRuleOr + + # Multi rules + ruleMultiDisplay1: + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.multiDisplayRule + ruleMultiDisplay2: + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text-2 + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.multiDisplayRule + + ruleMultiDisplayOr1: + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.multiDisplayRuleOr + ruleMultiDisplayOr2: + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text-2 + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.multiDisplayRuleOr + + + ruleMultiHidding1: + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.multiHiddingRule + ruleMultiHidding2: + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text-2 + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.multiHiddingRule + + ruleMultiHiddingOr1: + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.multiHiddingRuleOr + ruleMultiHiddingOr2: + ConditionOption: IsNotBlank + ConditionField: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.basic-text-2 + Parent: =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.multiHiddingRuleOr + SilverStripe\UserForms\Model\EditableFormField\EditableOption: option-1: Name: Option1 From 793f43728947b7a90284e47f82efd2df5464c914 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Mon, 13 Jan 2020 12:12:14 +1300 Subject: [PATCH 4/8] API Mark EditableFormField::EffectiveDisplayRules() for deprecation --- .upgrade.yml | 6 ++++++ code/Model/EditableFormField.php | 14 ++++++++++++++ tests/Model/EditableFormFieldTest.php | 8 +++++++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/.upgrade.yml b/.upgrade.yml index 7ae535e..1f5fd04 100644 --- a/.upgrade.yml +++ b/.upgrade.yml @@ -55,3 +55,9 @@ mappings: EditableLiteralFieldTest: SilverStripe\UserForms\Tests\Model\EditableFormField\EditableLiteralFieldTest SecureEditableFileFieldTest: SilverStripe\UserForms\Tests\Model\EditableFormField\SecureEditableFileFieldTest UserDefinedForm_EmailRecipientConditionTest: SilverStripe\UserForms\Tests\Model\Recipient\EmailRecipientConditionTest + +warnings: + methods: + 'SilverStripe\UserForms\Model\EditableFormField->EffectiveDisplayRules()': + message: 'EffectiveDisplayRules deprecated because of new support for conditional required field' + url: 'https://github.com/silverstripe/silverstripe-userforms/pull/770/' diff --git a/code/Model/EditableFormField.php b/code/Model/EditableFormField.php index 2ec4f8f..b85c67d 100755 --- a/code/Model/EditableFormField.php +++ b/code/Model/EditableFormField.php @@ -926,6 +926,20 @@ class EditableFormField extends DataObject ->setRecord($this); } + /** + * Determine effective display rules for this field. + * + * @return SS_List + * @deprecated 5.6 No longer needed because of support for conditional required field. + */ + public function EffectiveDisplayRules() + { + if ($this->Required) { + return ArrayList::create(); + } + return $this->DisplayRules(); + } + /** * Extracts info from DisplayRules into array so UserDefinedForm->buildWatchJS can run through it. * @return array|null diff --git a/tests/Model/EditableFormFieldTest.php b/tests/Model/EditableFormFieldTest.php index 80de25b..227c8e8 100644 --- a/tests/Model/EditableFormFieldTest.php +++ b/tests/Model/EditableFormFieldTest.php @@ -73,12 +73,19 @@ class EditableFormFieldTest extends FunctionalTest // it has 1 rule - when ticked the checkbox hides the text field $this->assertEquals(1, $rules->Count()); + // EffectiveDisplayRules rule has been deprecated + $this->assertEquals($rules, $checkbox->EffectiveDisplayRules()); + $checkboxRule = $rules->First(); $checkboxRule->ConditionFieldID = $field->ID; $this->assertEquals($checkboxRule->Display, 'Hide'); $this->assertEquals($checkboxRule->ConditionOption, 'HasValue'); $this->assertEquals($checkboxRule->FieldValue, '6'); + + // If field is required then all custom rules are disabled + $checkbox->Required = true; + $this->assertEquals(0, $checkbox->EffectiveDisplayRules()->count()); } public function testEditableOptionEmptyValue() @@ -285,5 +292,4 @@ class EditableFormFieldTest extends FunctionalTest $field = $this->objFromFixture(EditableTextField::class, $fieldName); $this->assertEquals($expected, $field->isDisplayed($data)); } - } From 8d0a5dd093d96acf27e714dd8e57dea5b2de0be5 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Mon, 13 Jan 2020 12:14:11 +1300 Subject: [PATCH 5/8] Add some missing comments --- code/Form/UserFormsRequiredFields.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/code/Form/UserFormsRequiredFields.php b/code/Form/UserFormsRequiredFields.php index bce0beb..79356f5 100644 --- a/code/Form/UserFormsRequiredFields.php +++ b/code/Form/UserFormsRequiredFields.php @@ -123,7 +123,13 @@ class UserFormsRequiredFields extends RequiredFields return $error; } - private function handleError($formField, $fieldName) + /** + * Register an error for the provided field. + * @param FormField $formField + * @param string $fieldName + * @return void + */ + private function handleError(FormField $formField, $fieldName) { $errorMessage = _t( 'SilverStripe\\Forms\\Form.FIELDISREQUIRED', From d280c5486084e33209f7503403bc263338a9cfe8 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Mon, 13 Jan 2020 12:15:34 +1300 Subject: [PATCH 6/8] Patch and reorganise some test --- code/Model/EditableCustomRule.php | 2 + tests/Form/UserFormsRequiredFieldsTest.php | 194 ++++++++++----------- tests/Model/EditableCustomRuleTest.php | 80 ++++++++- tests/UserFormsTest.yml | 2 +- 4 files changed, 170 insertions(+), 108 deletions(-) diff --git a/code/Model/EditableCustomRule.php b/code/Model/EditableCustomRule.php index bd8a0b2..7778455 100644 --- a/code/Model/EditableCustomRule.php +++ b/code/Model/EditableCustomRule.php @@ -2,6 +2,7 @@ namespace SilverStripe\UserForms\Model; +use InvalidArgumentException; use LogicException; use SilverStripe\CMS\Controllers\CMSMain; use SilverStripe\Control\Controller; @@ -237,6 +238,7 @@ class EditableCustomRule extends DataObject * * @param array $data Submitted form data * @return boolean + * @throws LogicException Invalid ConditionOption is set for this rule. */ public function validateAgainstFormData($data) { diff --git a/tests/Form/UserFormsRequiredFieldsTest.php b/tests/Form/UserFormsRequiredFieldsTest.php index 6688de3..9aae7be 100644 --- a/tests/Form/UserFormsRequiredFieldsTest.php +++ b/tests/Form/UserFormsRequiredFieldsTest.php @@ -29,7 +29,42 @@ class UserFormsRequiredFieldsTest extends SapphireTest $this->assertInstanceOf(UserFormsRequiredFields::class, $validator, 'Uses UserFormsRequiredFields validator'); } - public function testValidationOfConditionalRequiredFields() + public function dataProviderValidationOfConditionalRequiredFields() + { + return [ + 'Passes when non-conditional required field has a value' => [ + [ + 'required-text-field-2' => 'some text', + 'radio-option-2' => 'N', + 'conditional-required-text' => '' + ], + true + ], + 'Fails when conditional required is displayed but not completed' => [ + [ + 'required-text-field-2' => 'some text', + 'radio-option-2' => 'Y', + 'conditional-required-text' => '' + ], + false + ], + 'Passes when conditional required field has a value' => [ + [ + 'required-text-field-2' => 'some text', + 'radio-option-2' => 'Y', + 'conditional-required-text' => 'some more text' + ], + true + ] + ]; + } + + /** + * @param $data + * @param $expected + * @dataProvider dataProviderValidationOfConditionalRequiredFields + */ + public function testValidationOfConditionalRequiredFields($data, $expected) { $page = $this->objFromFixture(UserDefinedForm::class, 'required-custom-rules-form'); $validator = $this->getValidatorFromPage($page); @@ -40,110 +75,73 @@ class UserFormsRequiredFieldsTest extends SapphireTest 'Fails when non-conditional required field is empty' ); - $this->assertTrue( - $validator->php( - [ - 'required-text-field-2' => 'some text', - 'radio-option-2' => 'N', - 'conditional-required-text' => '' - ] - ), - 'Passes when non-conditional required field has a value' - ); - - $this->assertFalse( - $validator->php( - [ - 'required-text-field-2' => 'some text', - 'radio-option-2' => 'Y', - 'conditional-required-text' => '' - ] - ), - 'Fails when conditional required is displayed but not completed' - ); - - $this->assertTrue( - $validator->php( - [ - 'required-text-field-2' => 'some text', - 'radio-option-2' => 'Y', - 'conditional-required-text' => 'some more text' - ] - ), - 'Passes when conditional required field has a value' - ); + $this->assertEquals($expected, $validator->php($data)); } - public function testValidationOfNestedConditionalRequiredFields() + public function dataProviderValidationOfNestedConditionalRequiredFields() + { + return [ + 'Fails when non-conditional required field is empty' => [[], false], + 'Passes when non-conditional required field has a value' => [ + [ + 'required-text-field-3' => 'some text', + 'radio-option-3' => 'N', + 'conditional-required-text-2' => '', + 'conditional-required-text-3' => '' + ], + true + ], + 'Fails when conditional required is displayed but not completed' => [ + [ + 'required-text-field-3' => 'some text', + 'radio-option-3' => 'Y', + 'conditional-required-text-2' => '', + 'conditional-required-text-3' => '' + ], + false + ], + 'Passes when non-conditional required field has a value' => [ + [ + 'required-text-field-3' => 'some text', + 'radio-option-3' => 'Y', + 'conditional-required-text-2' => 'this text', + 'conditional-required-text-3' => '' + ], + true + ], + 'Fails when nested conditional required is displayed but not completed' => [ + [ + 'required-text-field-3' => 'some text', + 'radio-option-3' => 'Y', + 'conditional-required-text-2' => 'Show more', + 'conditional-required-text-3' => '' + ], + false + ], + 'Passes when nested conditional required field has a value' => [ + [ + 'required-text-field-3' => 'some text', + 'radio-option-3' => 'Y', + 'conditional-required-text-2' => 'Show more', + 'conditional-required-text-3' => 'more text' + ], + true + ] + ]; + } + + /** + * @param string $data + * @param array $expected + * @dataProvider dataProviderValidationOfNestedConditionalRequiredFields + */ + public function testValidationOfNestedConditionalRequiredFields($data, $expected) { $page = $this->objFromFixture(UserDefinedForm::class, 'required-nested-custom-rules-form'); $this->assertEquals(4, $page->Fields()->count()); $validator = $this->getValidatorFromPage($page); $this->assertNotNull($validator); - $this->assertFalse( - $validator->php([]), - 'Fails when non-conditional required field is empty' - ); - - $this->assertTrue( - $validator->php( - [ - 'required-text-field-3' => 'some text', - 'radio-option-2' => 'N', - 'conditional-required-text-2' => '', - 'conditional-required-text-3' => '' - ] - ), - 'Passes when non-conditional required field has a value' - ); - - $this->assertFalse( - $validator->php( - [ - 'required-text-field-3' => 'some text', - 'radio-option-2' => 'Y', - 'conditional-required-text-2' => '', - 'conditional-required-text-3' => '' - ] - ), - 'Fails when conditional required is displayed but not completed' - ); - - $this->assertTrue( - $validator->php( - [ - 'required-text-field-3' => 'some text', - 'radio-option-3' => 'Y', - 'conditional-required-text-2' => 'this text', - 'conditional-required-text-3' => '' - ] - ), - 'Passes when non-conditional required field has a value' - ); - - $this->assertFalse( - $validator->php( - [ - 'required-text-field-3' => 'some text', - 'radio-option-3' => 'Y', - 'conditional-required-text-2' => 'Show more', - 'conditional-required-text-3' => '' - ] - ), - 'Fails when nested conditional required is displayed but not completed' - ); - - $this->assertTrue( - $validator->php( - [ - 'required-text-field-3' => 'some text', - 'radio-option-3' => 'Y', - 'conditional-required-text-2' => 'Show more', - 'conditional-required-text-3' => 'more text' - ] - ), - 'Passes when nested conditional required field has a value' - ); + $this->assertEquals($expected, $validator->php($data)); } } diff --git a/tests/Model/EditableCustomRuleTest.php b/tests/Model/EditableCustomRuleTest.php index 1a70a56..c87b949 100644 --- a/tests/Model/EditableCustomRuleTest.php +++ b/tests/Model/EditableCustomRuleTest.php @@ -61,23 +61,85 @@ class EditableCustomRuleTest extends SapphireTest $this->assertSame('userform.field.hide', $rule1->toggleDisplayEvent('hide', true)); } + public function dataProviderValidateAgainstFormData() + { + return [ + 'IsNotBlank with blank value' => + ['IsNotBlank', '', '', false], + 'IsNotBlank with nopn-blank value' => + ['IsNotBlank', '', 'something', true], + 'IsBlank with blank value' => + ['IsBlank', '', '', true], + 'IsBlank with nopn-blank value' => + ['IsBlank', '', 'something', false], + 'HasValue with blank value' => + ['HasValue', 'NZ', '', false], + 'HasValue with correct value' => + ['HasValue', 'NZ', 'NZ', true], + 'HasValue with incorrect value' => + ['HasValue', 'NZ', 'UK', false], + 'ValueNot with blank value' => + ['ValueNot', 'NZ', '', true], + 'ValueNot with targeted value' => + ['ValueNot', 'NZ', 'NZ', false], + 'ValueNot with non-targeted value' => + ['ValueNot', 'NZ', 'UK', true], + 'ValueLessThan with value below target' => + ['ValueLessThan', '0', '-0.00001', true], + 'ValueLessThan with value equal to target' => + ['ValueLessThan', '0', '0', false], + 'ValueLessThan with value greater to target' => + ['ValueLessThan', '0', '0.0001', false], + 'ValueLessThanEqual with value below target' => + ['ValueLessThanEqual', '0', '-0.00001', true], + 'ValueLessThanEqual with value equal to target' => + ['ValueLessThanEqual', '0', '0', true], + 'ValueLessThanEqual with value greater to target' => + ['ValueLessThanEqual', '0', '0.0001', false], + 'ValueGreaterThan with value below target' => + ['ValueGreaterThan', '0', '-0.00001', false], + 'ValueGreaterThan with value equal to target' => + ['ValueGreaterThan', '0', '0', false], + 'ValueGreaterThan with value greater to target' => + ['ValueGreaterThan', '0', '0.0001', true], + 'ValueGreaterThanEqual with value below target' => + ['ValueGreaterThanEqual', '0', '-0.00001', false], + 'ValueGreaterThanEqual with value equal to target' => + ['ValueGreaterThanEqual', '0', '0', true], + 'ValueGreaterThanEqual with value greater to target' => + ['ValueGreaterThanEqual', '0', '0.0001', true], + ]; + } + /** * Test that methods are returned for manipulating the presence of the "hide" CSS class depending * on whether the field should be hidden or shown + * @dataProvider dataProviderValidateAgainstFormData */ - public function testValidateAgainstFormData() + public function testValidateAgainstFormData($condition, $targetValue, $value, $expected) { $rule1 = $this->objFromFixture(EditableCustomRule::class, 'rule1'); + $rule1->ConditionOption = $condition; + $rule1->FieldValue = $targetValue; - $this->assertFalse($rule1->validateAgainstFormData([])); - $this->assertFalse($rule1->validateAgainstFormData(['CountrySelection' => 'AU'])); - $this->assertTrue($rule1->validateAgainstFormData(['CountrySelection' => 'NZ'])); + $this->assertFalse( + $rule1->validateAgainstFormData([]), + 'Unset value always returns false no matter the rule' + ); - $rule2 = $this->objFromFixture(EditableCustomRule::class, 'rule2'); + $this->assertEquals( + $expected, + $rule1->validateAgainstFormData(['CountrySelection' => $value]) + ); + } - $this->assertFalse($rule2->validateAgainstFormData([])); - $this->assertFalse($rule2->validateAgainstFormData(['CountryTextSelection' => 0])); - $this->assertFalse($rule2->validateAgainstFormData(['CountryTextSelection' => 1])); - $this->assertTrue($rule2->validateAgainstFormData(['CountryTextSelection' => 2])); + /** + * @expectedException LogicException + */ + public function testValidateAgainstFormDataWithNonSenseRule() + { + $rule1 = $this->objFromFixture(EditableCustomRule::class, 'rule1'); + $rule1->ConditionOption = 'NonSenseRule'; + $rule1->validateAgainstFormData(['CountrySelection' => 'booya']); } } diff --git a/tests/UserFormsTest.yml b/tests/UserFormsTest.yml index d715aec..f0d6871 100644 --- a/tests/UserFormsTest.yml +++ b/tests/UserFormsTest.yml @@ -241,7 +241,7 @@ SilverStripe\UserForms\Model\EditableFormField\EditableRadioField: - =>SilverStripe\UserForms\Model\EditableFormField\EditableOption.option-y - =>SilverStripe\UserForms\Model\EditableFormField\EditableOption.option-n radio-field-3: - Name: radio-option-2 + Name: radio-option-3 Title: Radio Option 3 Options: - =>SilverStripe\UserForms\Model\EditableFormField\EditableOption.option-y-2 From 39ee08cff961d3c57123262e31cd4b01112bd741 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Mon, 13 Jan 2020 12:25:47 +1300 Subject: [PATCH 7/8] Remove bool explicit type definition to please PHP 5.6 --- tests/Model/EditableFormFieldTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Model/EditableFormFieldTest.php b/tests/Model/EditableFormFieldTest.php index 227c8e8..a159384 100644 --- a/tests/Model/EditableFormFieldTest.php +++ b/tests/Model/EditableFormFieldTest.php @@ -286,7 +286,7 @@ class EditableFormFieldTest extends FunctionalTest * @param $expected * @dataProvider displayedProvider */ - public function testIsDisplayed($fieldName, $data, bool $expected) + public function testIsDisplayed($fieldName, $data, $expected) { /** @var EditableFormField $field */ $field = $this->objFromFixture(EditableTextField::class, $fieldName); From e2c05d5a5502bfd1a1712aa9c056c954cabc4590 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Tue, 14 Jan 2020 12:28:04 +1300 Subject: [PATCH 8/8] Minor linting adjustment. --- code/Form/UserFormsRequiredFields.php | 10 +++------- code/Model/EditableCustomRule.php | 22 +++++++++++----------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/code/Form/UserFormsRequiredFields.php b/code/Form/UserFormsRequiredFields.php index 79356f5..70b13ff 100644 --- a/code/Form/UserFormsRequiredFields.php +++ b/code/Form/UserFormsRequiredFields.php @@ -134,21 +134,17 @@ class UserFormsRequiredFields extends RequiredFields $errorMessage = _t( 'SilverStripe\\Forms\\Form.FIELDISREQUIRED', '{name} is required', - array( + [ 'name' => strip_tags( '"' . ($formField->Title() ? $formField->Title() : $fieldName) . '"' ) - ) + ] ); if ($msg = $formField->getCustomValidationMessage()) { $errorMessage = $msg; } - $this->validationError( - $fieldName, - $errorMessage, - "required" - ); + $this->validationError($fieldName, $errorMessage, "required"); } } diff --git a/code/Model/EditableCustomRule.php b/code/Model/EditableCustomRule.php index 7778455..c2fd544 100644 --- a/code/Model/EditableCustomRule.php +++ b/code/Model/EditableCustomRule.php @@ -25,24 +25,24 @@ use SilverStripe\Versioned\Versioned; class EditableCustomRule extends DataObject { private static $condition_options = [ - 'IsBlank' => 'Is blank', - 'IsNotBlank' => 'Is not blank', - 'HasValue' => 'Equals', - 'ValueNot' => 'Doesn\'t equal', - 'ValueLessThan' => 'Less than', - 'ValueLessThanEqual' => 'Less than or equal', - 'ValueGreaterThan' => 'Greater than', + 'IsBlank' => 'Is blank', + 'IsNotBlank' => 'Is not blank', + 'HasValue' => 'Equals', + 'ValueNot' => 'Doesn\'t equal', + 'ValueLessThan' => 'Less than', + 'ValueLessThanEqual' => 'Less than or equal', + 'ValueGreaterThan' => 'Greater than', 'ValueGreaterThanEqual' => 'Greater than or equal' ]; private static $db = [ - 'Display' => 'Enum("Show,Hide")', + 'Display' => 'Enum("Show,Hide")', 'ConditionOption' => 'Enum("IsBlank,IsNotBlank,HasValue,ValueNot,ValueLessThan,ValueLessThanEqual,ValueGreaterThan,ValueGreaterThanEqual")', - 'FieldValue' => 'Varchar(255)' + 'FieldValue' => 'Varchar(255)' ]; private static $has_one = [ - 'Parent' => EditableFormField::class, + 'Parent' => EditableFormField::class, 'ConditionField' => EditableFormField::class ]; @@ -240,7 +240,7 @@ class EditableCustomRule extends DataObject * @return boolean * @throws LogicException Invalid ConditionOption is set for this rule. */ - public function validateAgainstFormData($data) + public function validateAgainstFormData(array $data) { $controllingField = $this->ConditionField();