Skip to content

Conversation

@bjarnef
Copy link
Contributor

@bjarnef bjarnef commented Jul 12, 2022

Testing this in a project, but it doesn't continue in code because ModelState.IsValid is true.

With this change it works as expected and shows the ModelState error when email and verifyEmail fields doesn't match - otherwise it doesn't add the modelstate error.

I wonder if this is possible to archive as well with a custom field and a setting to specify field to compare e.g. using alias email.
My attempt was something like the following, which seems to work as well.

public class ConfirmEmailField : Umbraco.Forms.Core.FieldType
{
    [Setting("Compare Field",
        Description = "Compare field",
        View = "TextField")]
    public string CompareField { get; set; }

    public ConfirmEmailField()
    {
        this.Id = new Guid("79201eb2-71c5-4ee7-8bd4-5fd59fc61553");
        this.Name = "Email Confirm";
        this.Description = "Render input to confirm email.";
        this.Icon = "icon-message";
        this.FieldTypeViewName = "FieldType.Textfield.cshtml";
        this.DataType = FieldDataType.String;
        this.SortOrder = 10;
        this.SupportsRegex = true;
    }

    public override string GetDesignView() =>
        "~/App_Plugins/UmbracoForms/backoffice/Common/FieldTypes/Textfield.html";

    public override IEnumerable<string> ValidateField(Form form, Field field, IEnumerable<object> postedValues, HttpContext context, IPlaceholderParsingService placeholderParsingService)
    {
        var baseValidation = base.ValidateField(form, field, postedValues, context, placeholderParsingService);
        var value = postedValues.FirstOrDefault();

        var compareField = GetPostFieldValue(form, context, CompareField);

        if (compareField == null)
            return baseValidation;

        if (value != null && value.ToString() == compareField)
        {
            return baseValidation;
        }

        var custom = new List<string>();
        custom.AddRange(baseValidation);
        custom.Add("Email does not match.");

        return custom;
    }

    private static string GetPostFieldValue(Form form, HttpContext context, string key)
    {
        Field? field = GetPostField(form, key);
        if (field == null)
        {
            return string.Empty;
        }

        return context.Request.HasFormContentType && context.Request.Form.Keys.Contains(field.Id.ToString())
            ? context.Request.Form[field.Id.ToString()].ToString().Trim()
            : string.Empty;
    }

    private static Field? GetPostField(Form form, string key) => form.AllFields.SingleOrDefault(f => f.Alias == key);
}

image

@bjarnef bjarnef changed the title Return is ModelState is valid Return if ModelState is valid Jul 12, 2022
@eshanrnh eshanrnh requested a review from AndyButland July 13, 2022 08:07
Copy link
Contributor

@AndyButland AndyButland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is correct as is actually. The idea of this sample as I understand it is that if the model state is valid, i.e. there are no existing validation issues, do one further check. If I've understood that correctly, it should exist if the existing model state is invalid (i.e. notification.ModelState.IsValid == false).

@bjarnef
Copy link
Contributor Author

bjarnef commented Jul 13, 2022

@AndyButland okay, I will check again, but I think notification.ModelState.IsValid was false where fields where valid (but I think the reCaptcha) may always have been false as it hasn't been configured yet, so it didn't go further to add this ModelState error.

I wonder which approach is best: A notification handler or a custom field:
#4208 (comment)

In most case I am not sure we can rely on form name as editors may change these, but it could have a statement to check if any of fields in form AllFields match alias veriryEmail, but even then it rely on the editors specify this specific alias.

I wonder if Umbraco Forms has any feature to compare value of two fields - or is it currently only possible via custom code?

@eshanrnh
Copy link
Contributor

eshanrnh commented Jul 15, 2022

Hey @bjarnef 👋

Just checking in - if this PR is still valid. For the CompareField validation, you have created another PR (#4215) , right?

@bjarnef
Copy link
Contributor Author

bjarnef commented Jul 15, 2022

Hi @eshanrnh

I think this was because I had reCaptcha field, which wasn't configured, so ModelState was always invalid.

While the notification handler works it relied on specific field aliases and I think in this case it was a better approach with a custom field type as in #4215 which also allowed us to add client side validation. This could be useful in Umbraco Forms core as it is useful the compare another field - in this case a email confirm, but could be used for other types of values as well.

I will close this one for now.

@bjarnef bjarnef closed this Jul 15, 2022
@bjarnef bjarnef deleted the patch-27 branch July 15, 2022 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants