DataTables / Editor-NET

.NET Framework and .NET Core server-side libraries for Editor
Other
15 stars 12 forks source link

Should global validators run AFTER field validators? #26

Open VictorioBerra opened 1 month ago

VictorioBerra commented 1 month ago

Maybe im approaching this wrong but I have a global validator that needs to do some special custom checks.

Because my global validator runs first im duplicating a ton of field validators:

// The entire purpose of this validator is to check if the FieldDefinition already exists associated to the same entities
// For example a field definition for Name cannot exist twice both added to InventoryServers
.Validator((editor, action, data) =>
{
    if (action == DtRequest.RequestTypes.DataTablesGet)
    {
        return string.Empty;
    }

    foreach (var pair in data.Data)
    {
        var values = pair.Value as Dictionary<string, object>;

        if (!values.TryGetValue("FieldDefinition", out var fieldDefinitionData))
        {
            // DUPLICATED FIELD VALIDATOR
            return string.Empty;
        }
        else
        {
            if (action is DtRequest.RequestTypes.EditorEdit
                or DtRequest.RequestTypes.EditorCreate)
            {
                var fieldDefinitionDictionary = fieldDefinitionData as Dictionary<string, object>;
                if (!fieldDefinitionDictionary.TryGetValue("Name", out var fieldDefinitionNameData))
                {
                    // DUPLICATED FIELD VALIDATOR
                    // An edit does not have to result in a name change (they didnt send Name field).
                    // A create Name is required, so another validator will catch this
                    return string.Empty;
                }

                var fieldDefinitionName = fieldDefinitionNameData.ToString();
                if (string.IsNullOrWhiteSpace(fieldDefinitionName))
                {
                    // DUPLICATED FIELD VALIDATOR
                    // Field validator will catch it
                    return string.Empty;
                }

                if (!values.TryGetValue("FieldDefinitionEntity", out var entities))
                {
                    // DUPLICATED FIELD VALIDATOR
                    return string.Empty;
                }

                if (action is DtRequest.RequestTypes.EditorEdit)
                {
                    var existingFieldDefinition = context.WuitFieldDefinition
                        .Include(x => x.Entities)
                        .FirstOrDefault(x => x.Id == Convert.ToInt32(pair.Key, CultureInfo.InvariantCulture));
                    if (fieldDefinitionName == existingFieldDefinition.Name)
                    {
                        // Name did not change, no need to check if it exists
                        continue;
                    }
                }

                var entitiesDict = entities as Dictionary<string, object>;
                var entityIds = entitiesDict.Values
                    .Select(x => x as Dictionary<string, object>)
                    .Select(x => (int)x["Id"])
                    .ToArray();

                if (entityIds.Count == 0)
                {
                    // DUPLICATED FIELD VALIDATOR
                    // Field validator for the MJoin below will check that they provided a min of 1
                    return string.Empty;
                }

                var existingRecord = IsFieldDefinitionInUse(fieldDefinitionName, entityIds);

                if (existingRecord is null)
                {
                    return string.Empty;
                }

                return $"Field Definition already exists for field name {fieldDefinitionName} ({existingRecord.Id}) on entities {string.Join(", ", existingRecord.Entities.Select(x => x.Name))}";
            }
        }
    }

    return string.Empty;
})

Now this would be my code if I could trust that field validators all ran and that it was safe to access my dictionary:

.Validator((editor, action, data) =>
{
    if (action == DtRequest.RequestTypes.DataTablesGet)
    {
        return string.Empty;
    }

    // Create/Edit can accept multiple records at once
    foreach (var pair in data.Data)
    {
        var values = pair.Value as Dictionary<string, object>;

        if (!values.TryGetValue("FieldDefinition", out var fieldDefinitionData))
        {
            return string.Empty;
        }
        else
        {
            if (action is DtRequest.RequestTypes.EditorEdit or DtRequest.RequestTypes.EditorCreate)
            {
                var fieldDefinitionDictionary = fieldDefinitionData as Dictionary<string, object>;
                var fieldDefinitionName = fieldDefinitionDictionary["Name"].ToString();
                var entities = values["FieldDefinitionEntity"] as Dictionary<string, object>;
                var entityIds = entities.Values
                    .Select(x => x as Dictionary<string, object>)
                    .Select(x => (int)x["Id"])
                    .ToArray();

                var existingFieldDefinition = context.WuitFieldDefinition
                    .Include(x => x.Entities)
                    .FirstOrDefault(x => x.Id == Convert.ToInt32(pair.Key, CultureInfo.InvariantCulture) &&
                    x.Name == fieldDefinitionName);

                if (existingFieldDefinition is not null)
                {
                    // Name did not change, no need for further checks
                    continue;
                }

                var existingRecord = IsFieldDefinitionInUse(fieldDefinitionName, entityIds);
                if (existingRecord is null)
                {
                    return string.Empty;
                }

                return $"Field Definition already exists for field name {fieldDefinitionName} ({existingRecord.Id}) on entities {string.Join(", ", existingRecord.Entities.Select(x => x.Name))}";
            }
        }
    }

    return string.Empty;
})

Looking at the code looks like PreX events run before field validation too... so those are not a workaround for me either.

VictorioBerra commented 1 month ago

Thinking on this more, I think this amounts to a feature request:

.PreValidate() and .PostValidate() you could even leave .PreValidate() as .Validate() to be backwards compatible.

Thoughts?

AllanJard commented 1 month ago

Hi - thanks for raising this and illustrating it with an example. I can absolutely see how it might be useful to have the field validation run first, and then stop if there is an error. If I recall correctly, my intention with running the global validation first had been to allow a quick check for access rights.

Perhaps an overload to Editor.Validate() to tell it to run the validator after the fields (or not - default) might be the way to do this, or an Editor.ValidateAfterFields() (ugly!) method.

I'll set a 2.4 target for this as I think it would be a good addition.

VictorioBerra commented 1 month ago

An overload would be awesome! Thanks Allan

On Wed, Jul 17, 2024, 10:22 AM Allan Jardine @.***> wrote:

Hi - thanks for raising this and illustrating it with an example. I can absolutely see how it might be useful to have the field validation run first, and then stop if there is an error. If I recall correctly, my intention with running the global validation first had been to allow a quick check for access rights.

Perhaps an overload to Editor.Validate() to tell it to run the validator after the fields (or not - default) might be the way to do this, or an Editor.ValidateAfterFields() (ugly!) method.

I'll set a 2.4 target for this as I think it would be a good addition.

— Reply to this email directly, view it on GitHub https://github.com/DataTables/Editor-NET/issues/26#issuecomment-2233586088, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWMN2ZQENFZW5MIRBGHBALZM2D4FAVCNFSM6AAAAABLAYJ6L6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZTGU4DMMBYHA . You are receiving this because you authored the thread.Message ID: @.***>