fix(ValidateBase): do not add validate state when ErrorMessage is null#7463
Merged
fix(ValidateBase): do not add validate state when ErrorMessage is null#7463
Conversation
Contributor
Reviewer's GuideAdjusts ValidateBase parsing logic to avoid adding validation state when no error message is produced, aligns Select parsing to use null error messages, and adds unit tests for InputNumber and DateTimePicker to verify validation behavior with invalid input that does not change the model value. Sequence diagram for ValidateBase parsing and validation state updatesequenceDiagram
actor User
participant InputComponent as InputComponent
participant ValidateBase as ValidateBase
participant FieldParser as FieldParser
participant EditContext as EditContext
User->>InputComponent: change input value
InputComponent->>ValidateBase: set CurrentValueAsString(newValue)
ValidateBase->>FieldParser: TryParseValueFromString(newValue, out parsedValue, out validationErrorMessage)
alt parsing succeeds (validationErrorMessage is not null or empty)
FieldParser-->>ValidateBase: parsedValue, validationErrorMessage
ValidateBase->>ValidateBase: PreviousParsingAttemptFailed = true
ValidateBase->>ValidateBase: PreviousErrorMessage = validationErrorMessage
ValidateBase->>ValidateBase: add message to _parsingValidationMessages
ValidateBase->>EditContext: NotifyFieldChanged(FieldIdentifier)
else parsing fails but validationErrorMessage is null or empty
FieldParser-->>ValidateBase: parsedValue (unchanged), validationErrorMessage = null
ValidateBase->>ValidateBase: PreviousParsingAttemptFailed = false
ValidateBase-->>EditContext: no validation state added
end
Updated class diagram for ValidateBase and Select parsing behaviorclassDiagram
class ValidateBase~TValue~ {
- bool PreviousParsingAttemptFailed
- string PreviousErrorMessage
- IDictionary FieldIdentifierToMessages _parsingValidationMessages
- FieldIdentifier~TValue~? FieldIdentifier
- EditContext EditContext
+ string CurrentValueAsString
+ void SetCurrentValueAsString(string value)
}
class Select~TValue~ {
+ object SelectedItem
+ bool TryParseSelectItem(string value, out TValue result, out string validationErrorMessage)
}
class EditContext {
+ void NotifyFieldChanged(FieldIdentifier fieldIdentifier)
}
class FieldIdentifier~TValue~ {
+ object Model
+ string FieldName
}
ValidateBase~TValue~ o-- EditContext : uses
ValidateBase~TValue~ o-- FieldIdentifier~TValue~ : identifies
ValidateBase~TValue~ ..> Select~TValue~ : consumes parsing result
Select~TValue~ ..> ValidateBase~TValue~ : supplies validationErrorMessage
note for Select~TValue~ "When SelectedItem is not null and parsing succeeds:
- result is cast from SelectedItem
- validationErrorMessage is null (no validation state added by ValidateBase)"
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In ValidateBase.CurrentValueAsString setter, the new branch uses string.IsNullOrEmpty(validationErrorMessage) but the comment and behavior distinguish only the null case (not empty); consider tightening this to
validationErrorMessage == nullor updating the comment/semantics so empty-string errors are handled consistently and intentionally.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In ValidateBase.CurrentValueAsString setter, the new branch uses string.IsNullOrEmpty(validationErrorMessage) but the comment and behavior distinguish only the null case (not empty); consider tightening this to `validationErrorMessage == null` or updating the comment/semantics so empty-string errors are handled consistently and intentionally.
## Individual Comments
### Comment 1
<location> `test/UnitTest/Components/InputNumberTest.cs:219-228` </location>
<code_context>
+ [Fact]
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen validation assertions to verify field-level validation state, not only the overall form validity flag.
In `Validate_Ok`, consider also asserting the field-level validation messages on the `EditContext` (e.g., via `EditContext.GetValidationMessages(FieldIdentifier)` or a helper) after each change. That will verify that invalid input actually adds an error and valid input clears it, catching regressions in validation state even when the overall `Validate()` bool still returns the expected value.
Suggested implementation:
```csharp
[Fact]
public async Task Validate_Ok()
{
var model = new Foo() { Count = 1 };
FieldIdentifier countField = new(model, nameof(Foo.Count));
IReadOnlyList<string> GetCountFieldMessages(EditContext editContext)
=> editContext.GetValidationMessages(countField).ToList();
var cut = Context.Render<ValidateForm>(pb =>
{
pb.Add(a => a.Model, model);
pb.AddChildContent<BootstrapInputNumber<int>>(builder =>
{
builder.Add(a => a.Value, model.Count);
builder.Add(a => a.ValueChanged, EventCallback.Factory.Create<int>(this, v =>
```
To fully implement the suggestion, update the rest of `Validate_Ok` to:
1. Obtain the `EditContext` from the rendered `ValidateForm` instance (e.g., `var editContext = cut.Instance.EditContext;` or via the contained `EditForm` depending on how `ValidateForm` is implemented).
2. After each interaction that changes `Foo.Count` and triggers validation (for both invalid and valid values), add assertions like:
- `Assert.NotEmpty(GetCountFieldMessages(editContext));` and assert expected message text for invalid input.
- `Assert.Empty(GetCountFieldMessages(editContext));` once the input is corrected and validation passes.
If `FieldIdentifier`, `EditContext`, or `ToList()` are not already in scope, ensure the file has `using Microsoft.AspNetCore.Components.Forms;` and `using System.Linq;` at the top.
</issue_to_address>
### Comment 2
<location> `test/UnitTest/Components/DateTimePickerTest.cs:1258-1267` </location>
<code_context>
+ [Fact]
</code_context>
<issue_to_address>
**suggestion (testing):** Extend the DateTimePicker validation test to assert the absence of validation messages for the field after invalid editable input.
In `ValidateForm_IsEditable_Ok`, you already confirm the model value is unchanged and `Validate()` returns true after an invalid editable value. To better cover the change that skips adding validation state when the error message is null, please also assert that `EditContext.GetValidationMessages` for the DateTime field is empty. This verifies the field itself is not marked invalid despite the parse failure and strengthens future validation regression coverage.
Suggested implementation:
```csharp
[Fact]
public async Task ValidateForm_IsEditable_Ok()
{
var model = new Foo() { DateTime = new DateTime(2024, 2, 15) };
var cut = Context.Render<ValidateForm>(pb =>
{
pb.Add(a => a.Model, model);
pb.AddChildContent<DateTimePicker<DateTime?>>(builder =>
{
builder.Add(a => a.IsEditable, true);
builder.Add(a => a.ViewMode, DatePickerViewMode.Date);
```
To implement the suggestion, inside `ValidateForm_IsEditable_Ok` after you simulate an invalid editable input, assert the model value is unchanged, and call `Validate()` (or `cut.Instance.EditContext.Validate()`) and assert it returns `true`, add an assertion that the `EditContext` has no validation messages for the `DateTime` field.
Concretely, right after your existing assertion that validation succeeds, add something along these lines:
```csharp
// existing code (example)
var editContext = cut.Instance.EditContext;
var isValid = editContext.Validate();
Assert.True(isValid);
Assert.Equal(new DateTime(2024, 2, 15), model.DateTime); // existing assertion
// new assertion: no validation messages for the DateTime field
var messages = editContext.GetValidationMessages(
FieldIdentifier.Create(() => model.DateTime)
);
Assert.Empty(messages);
```
If you are using the lambda-based overload of `GetValidationMessages`, you can also write:
```csharp
Assert.Empty(editContext.GetValidationMessages(() => model.DateTime));
```
Make sure `editContext` is the same `EditContext` instance you already use for `Validate()`, and keep this new assertion after the invalid editable input has been processed to ensure the field itself is not marked invalid despite the parse failure.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1258
to
+1267
| [Fact] | ||
| public async Task ValidateForm_IsEditable_Ok() | ||
| { | ||
| var model = new Foo() { DateTime = new DateTime(2024, 2, 15) }; | ||
| var cut = Context.Render<ValidateForm>(pb => | ||
| { | ||
| pb.Add(a => a.Model, model); | ||
| pb.AddChildContent<DateTimePicker<DateTime?>>(builder => | ||
| { | ||
| builder.Add(a => a.IsEditable, true); |
Contributor
There was a problem hiding this comment.
suggestion (testing): Extend the DateTimePicker validation test to assert the absence of validation messages for the field after invalid editable input.
In ValidateForm_IsEditable_Ok, you already confirm the model value is unchanged and Validate() returns true after an invalid editable value. To better cover the change that skips adding validation state when the error message is null, please also assert that EditContext.GetValidationMessages for the DateTime field is empty. This verifies the field itself is not marked invalid despite the parse failure and strengthens future validation regression coverage.
Suggested implementation:
[Fact]
public async Task ValidateForm_IsEditable_Ok()
{
var model = new Foo() { DateTime = new DateTime(2024, 2, 15) };
var cut = Context.Render<ValidateForm>(pb =>
{
pb.Add(a => a.Model, model);
pb.AddChildContent<DateTimePicker<DateTime?>>(builder =>
{
builder.Add(a => a.IsEditable, true);
builder.Add(a => a.ViewMode, DatePickerViewMode.Date);To implement the suggestion, inside ValidateForm_IsEditable_Ok after you simulate an invalid editable input, assert the model value is unchanged, and call Validate() (or cut.Instance.EditContext.Validate()) and assert it returns true, add an assertion that the EditContext has no validation messages for the DateTime field.
Concretely, right after your existing assertion that validation succeeds, add something along these lines:
// existing code (example)
var editContext = cut.Instance.EditContext;
var isValid = editContext.Validate();
Assert.True(isValid);
Assert.Equal(new DateTime(2024, 2, 15), model.DateTime); // existing assertion
// new assertion: no validation messages for the DateTime field
var messages = editContext.GetValidationMessages(
FieldIdentifier.Create(() => model.DateTime)
);
Assert.Empty(messages);If you are using the lambda-based overload of GetValidationMessages, you can also write:
Assert.Empty(editContext.GetValidationMessages(() => model.DateTime));Make sure editContext is the same EditContext instance you already use for Validate(), and keep this new assertion after the invalid editable input has been processed to ensure the field itself is not marked invalid despite the parse failure.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7463 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 32908 32912 +4
Branches 4572 4573 +1
=========================================
+ Hits 32908 32912 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #7455 by preventing validation error states from being added when components fail to parse input but intentionally return a null error message. This allows components like DateTimePicker and Select to silently reject invalid input while maintaining the previous valid value, rather than triggering form validation errors.
Key Changes:
- Added conditional logic in ValidateBase to treat null/empty validation error messages as non-failures
- Updated Select component to return null instead of empty string for validation error messages when item lookup fails
- Bumped version to 10.2.1-beta04
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Validate/ValidateBase.cs | Added logic to handle null validation error messages by setting PreviousParsingAttemptFailed to false, preventing validation state changes when components intentionally suppress errors |
| src/BootstrapBlazor/Components/Select/Select.razor.cs | Changed validationErrorMessage from empty string to null to signal parsing failure without validation error |
| test/UnitTest/Components/InputNumberTest.cs | Added tests to verify validation behavior with invalid input and ensure parsing failures are properly handled |
| test/UnitTest/Components/DateTimePickerTest.cs | Added test to verify DateTimePicker with IsEditable correctly handles invalid input without triggering validation errors |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Version bump to 10.2.1-beta04 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Jan 7, 2026
This was referenced Jan 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #7455
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Adjust validation behavior so failed parsing without an error message does not mark fields invalid, and ensure form validation reflects unchanged values correctly.
Bug Fixes:
Tests: