Skip to content

DataGridRowValidator : Mark the Model property as virtual#7771

Merged
ScarletKuro merged 1 commit intoMudBlazor:devfrom
iDerrien:feature/datagridrowvalidator_model_override
Mar 7, 2024
Merged

DataGridRowValidator : Mark the Model property as virtual#7771
ScarletKuro merged 1 commit intoMudBlazor:devfrom
iDerrien:feature/datagridrowvalidator_model_override

Conversation

@iDerrien
Copy link
Contributor

@iDerrien iDerrien commented Nov 16, 2023

Fixes: #7794

Marked the Model property on the DataGridRowValidator as virtual so, that it can be overriden. Existing DatagridRowValidator.Model throws the NotImplementedexception when accessed. See open discussion for more details: #7767

Description

I would like to use the built in mudblazor validation (For) in the MudDataGrid in combination with the FluentValidator. That could potentially be done quite nicely in a similar clean way as it's already possible with other controls like MudTextField.

Here is the problem:
MudDataGrid.razor.cs contains the validator property:
public Interfaces.IForm Validator { get; set; } = new DataGridRowValidator();

There is currently no working implementation in place for the DataGrid and trying to use the mentioned validator leads to a crash because of the following implementation in the DataGridRowValidator :
public object? Model { get => throw new System.NotImplementedException(); set => throw new System.NotImplementedException(); }

The MudFormComponent would start the validation using "For" and crashes because of Form?.Model

protected virtual async Task ValidateModelWithFullPathOfMember(Func<object, string, Task<IEnumerable<string?>>> func, List<string> errors)
{
    try
    {
        if (Form?.Model is null)
        {
            return;
        }

The implementation is just not in place yet, which is perfectly fine.
Unfortunately it is currently impossible to extend or override the functionality because of the Model Property implementation (or rather the fact that it's missing and throws an exception) on the DataGridRowValidator. It's also impossible to provide a completely new implementation for the IForm since some of te properties are marked as internal.

    public interface IForm
    {
        public bool IsValid { get; }
        public string[] Errors { get; }
#nullable enable
        public object? Model { get; set; }
        public void FieldChanged(IFormComponent formControl, object? newValue);
#nullable disable
        internal void Add(IFormComponent formControl);
        internal void Remove(IFormComponent formControl);
        internal void Update(IFormComponent formControl);
    }

The easiest solution would probably be to mark the Model property on the DataGridRowValidator as virtual
The alternative would be to allow setting of Model on DataGridRowValidator. If you would prefer that let me know and I'll provide a new PR.

How Has This Been Tested?

I've created a small application to check whether the change would suffice for what I was intending to do. (override model and provide my own implementation. Honestly a normal get/set instead of an exception would have probably already be enough but I just wanted to have as little of an impact as possible.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

…that it can be overriden. Existing DatagridRowValidator.Model throws the NotImplementedexception when accessed.
@github-actions github-actions bot added enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library PR: needs review labels Nov 16, 2023
@ScarletKuro
Copy link
Member

Hi.
Thanks for the PR.
@tjscience I will accept this PR as compromise, because as it was said it's not possible to provide fully custom IForm implementation since some things are internal.
It's interesting to me that the interface has some internal methods but it doesn't provide at least default implementation for them.
I find it an anti pattern if the interface is public and contains internal things, but it is what it is.

@ScarletKuro ScarletKuro changed the title Marked the Model property on the DataGridRowValidator as virtual DataGridRowValidator : Mark the Model property as virtual Mar 7, 2024
@ScarletKuro ScarletKuro merged commit 7d0a4b1 into MudBlazor:dev Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MudDataGrid validation with DataGridRowValidator

2 participants