Skip to content

✨ Add verified MVC validation support for OptionalValue<T> with the package OptionalValues.Mvc#33

Merged
desjoerd merged 14 commits into
mainfrom
copilot/fix-controller-action-error
May 27, 2026
Merged

✨ Add verified MVC validation support for OptionalValue<T> with the package OptionalValues.Mvc#33
desjoerd merged 14 commits into
mainfrom
copilot/fix-controller-action-error

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 27, 2026

This pull request introduces a new package, OptionalValues.Mvc, which adds ASP.NET Core MVC validation metadata support for the OptionalValue<T> type.

Fixes #31

Copilot AI linked an issue May 27, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix error in controller action with OptionalValues Skip MVC validation for OptionalValue<T>.SpecifiedValue May 27, 2026
Copilot AI requested a review from desjoerd May 27, 2026 14:28
@desjoerd
Copy link
Copy Markdown
Owner

@copilot

This should not be implemented in the main OptionalValues project.

Move the MVC-specific validation behavior into a separate OptionalValues.Mvc package.

Required changes:

  • Remove the MVC validation attribute from OptionalValue<T> / SpecifiedValue
  • Add a new OptionalValues.Mvc project/package
  • Move the MVC-specific tests out of the core test project
  • Add MVC-specific tests for this behavior in an OptionalValues.Mvc.Tests project
  • Implement this through MVC metadata, not through annotations on the core type

The MVC package should provide an IValidationMetadataProvider / metadata details provider that detects OptionalValue<> and disables child validation for it.

The important behavior is:

context.ValidationMetadata.ValidateChildren = false;

for OptionalValue<>.

This should then be registered through MVC options, for example via an extension method on MvcOptions:

options.ModelMetadataDetailsProviders.Add(
    new OptionalValueValidationMetadataProvider());

or a package-specific helper such as:

options.AddOptionalValuesMvc();

The core OptionalValues package should remain framework-agnostic and should not reference ASP.NET Core MVC abstractions.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 27, 2026

@copilot

This should not be implemented in the main OptionalValues project.
...

Addressed in ec7910d.

The core package no longer contains MVC-specific annotations or references. MVC child-validation suppression now lives in a new OptionalValues.Mvc package via OptionalValueValidationMetadataProvider and MvcOptions.AddOptionalValuesMvc(), with dedicated tests in OptionalValues.Mvc.Tests.

Copilot AI changed the title Skip MVC validation for OptionalValue<T>.SpecifiedValue Move MVC validation support for OptionalValue<T> into OptionalValues.Mvc May 27, 2026
Copy link
Copy Markdown
Owner

@desjoerd desjoerd left a comment

Choose a reason for hiding this comment

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

@copilot please look at the comments

/// Adds validation metadata support for <see cref="OptionalValue{T}"/> so MVC does not validate its child properties.
/// </summary>
/// <param name="options">The MVC options to configure.</param>
public static void AddOptionalValuesMvc(this MvcOptions options)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

rename to AddOptionalValueSupport to align with the other extension methods.

Comment thread test/OptionalValues.Mvc.Tests/MvcControllerValidationTest.cs
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 27, 2026

@copilot please look at the comments

Addressed in 3431762. The MVC helper is now AddOptionalValueSupport, and OptionalValues.Mvc.Tests now includes controller-integration coverage for both unspecified and specified OptionalValue<T> payloads.

Comment thread test/OptionalValues.Mvc.Tests/MvcControllerValidationTest.cs Outdated
Comment thread src/OptionalValues.Mvc/MvcOptionsExtensions.cs Outdated
Comment thread src/OptionalValues.Mvc/OptionalValues.Mvc.csproj
Comment thread test/OptionalValues.Mvc.Tests/MvcControllerValidationTest.cs Outdated
Comment thread test/OptionalValues.Mvc.Tests/MvcControllerValidationTest.cs Outdated
Comment thread test/OptionalValues.Mvc.Tests/OptionalValues.Mvc.Tests.csproj
@desjoerd
Copy link
Copy Markdown
Owner

@copilot please look at the comments

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 27, 2026

@copilot please look at the comments

Addressed in 6ce7ab3.

Comment thread src/OptionalValues.Mvc/OptionalValueSpecifiedValueModelValidator.cs Outdated
Comment thread src/OptionalValues.Mvc/OptionalValues.Mvc.csproj
Comment thread test/OptionalValues.Mvc.Tests/MvcControllerValidationTest.cs Outdated
Comment thread test/OptionalValues.Mvc.Tests/OptionalValues.Mvc.Tests.csproj
Copilot AI requested a review from desjoerd May 27, 2026 18:42
Comment thread test/OptionalValues.Mvc.Tests/MvcControllerValidationTest.cs Outdated
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 27, 2026

@copilot please look at the comments

Addressed in 4804140.

Copilot AI requested a review from desjoerd May 27, 2026 19:24
@desjoerd desjoerd marked this pull request as ready for review May 27, 2026 21:14
@desjoerd desjoerd changed the title Move MVC validation support for OptionalValue<T> into OptionalValues.Mvc ✨ Add verified MVC validation support for OptionalValue<T> with the package OptionalValues.Mvc May 27, 2026
@desjoerd desjoerd merged commit 41e4686 into main May 27, 2026
1 check passed
@desjoerd desjoerd deleted the copilot/fix-controller-action-error branch May 27, 2026 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Does not seem to work on a controller action

2 participants