Skip to content

fix(ValidateForm): wait all field validate when submit form#7454

Merged
ArgoZhang merged 6 commits intomainfrom
fix-card-upload
Dec 31, 2025
Merged

fix(ValidateForm): wait all field validate when submit form#7454
ArgoZhang merged 6 commits intomainfrom
fix-card-upload

Conversation

@ArgoZhang
Copy link
Copy Markdown
Member

@ArgoZhang ArgoZhang commented Dec 31, 2025

Link issues

fixes #7440

Summary By Copilot

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Merge the latest code from the main branch

Summary by Sourcery

Ensure form submission waits for completion of all field validations before determining overall validity.

Bug Fixes:

  • Fix form validation flow so the overall result is set only after all field validators complete, preventing premature success or failure on submit.

Enhancements:

  • Simplify validation state tracking by removing the internal invalid flag and relying on aggregated validation results.
  • Streamline synchronous validation API to delegate directly to the underlying validator without extra state management.

Copilot AI review requested due to automatic review settings December 31, 2025 08:10
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Dec 31, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts ValidateForm’s validation flow so that form submission waits for all field validations to complete, centralizing success/failure signaling in ValidateObject and simplifying the Validate() API.

Sequence diagram for updated ValidateForm submission and validation flow

sequenceDiagram
    actor User
    participant EditForm
    participant ValidateForm
    participant ValidatorCollection as Validators
    participant Validator as IValidateComponent
    participant Tcs as TaskCompletionSource_bool_

    User->>EditForm: Submit()
    EditForm->>ValidateForm: OnValidSubmitForm(EditContext)
    ValidateForm->>ValidateForm: ValidateObject(ValidationContext, results)
    ValidateForm->>Tcs: _tcs = new TaskCompletionSource_bool_()
    ValidateForm->>Validators: Enumerate validators
    loop For each field validator
        ValidateForm->>Validator: ValidateAsync(validator, context)
        alt Built_in_validation
            Validator-->>ValidateForm: property value
            ValidateForm->>ValidateForm: ValidateDataAnnotations(...)
        else Custom_validation
            Validator-->>ValidateForm: property value
            ValidateForm->>Validator: ValidatePropertyAsync(propertyValue, context, messages)
        end
        ValidateForm->>Validator: ToggleMessage(messages)
    end
    ValidateForm->>ValidateForm: Aggregate results
    ValidateForm->>Tcs: _tcs.TrySetResult(results.Count == 0)
    EditForm-->>ValidateForm: Await _tcs.Task (all fields done)
    alt All fields valid
        ValidateForm-->>EditForm: Validation success
        EditForm-->>User: Invoke success handler
    else Some fields invalid
        ValidateForm-->>EditForm: Validation failed
        EditForm-->>User: Show validation errors
    end
Loading

Updated class diagram for ValidateForm validation state handling

classDiagram
    class ValidateForm {
        - TaskCompletionSource_bool_ _tcs
        - List_ButtonBase_ AsyncSubmitButtons
        + Task ValidateObject(ValidationContext context, List_ValidationResult_ results)
        - Task ValidateAsync(IValidateComponent validator, ValidationContext context)
        + bool Validate()
    }

    class IValidateComponent {
        <<interface>>
        + Task ValidatePropertyAsync(object propertyValue, ValidationContext context, List_ValidationResult_ messages)
        + Task ToggleMessage(List_ValidationResult_ messages)
    }

    class ValidationContext
    class ValidationResult
    class ButtonBase

    ValidateForm --> IValidateComponent : uses
    ValidateForm --> ValidationContext : uses
    ValidateForm --> ValidationResult : uses
    ValidateForm --> ButtonBase : manages async submit buttons

    IValidateComponent --> ValidationContext : uses
    IValidateComponent --> ValidationResult : produces messages
Loading

File-Level Changes

Change Details Files
Make form-level validation wait on all field validations and determine overall validity in ValidateObject instead of per-field.
  • Initialize a TaskCompletionSource at the start of ValidateObject to represent the overall validation completion/result.
  • Clear previous validation results and run validation for all properties or specific properties as before.
  • After all validators run, complete the TaskCompletionSource with a success value based on whether there are any ValidationResult entries.
  • Remove the old logic that set the _invalid flag when there were no validation components and per-validator code that set TaskCompletionSource based on individual field messages.
src/BootstrapBlazor/Components/ValidateForm/ValidateForm.razor.cs
Remove the _invalid flag and simplify the public Validate() API to delegate solely to the underlying Validator.
  • Delete the _invalid backing field and all assignments to it during validation.
  • Change the public Validate() method to a simple expression-bodied method that returns Validator.Validate() without extra state checks.
src/BootstrapBlazor/Components/ValidateForm/ValidateForm.razor.cs

Assessment against linked issues

Issue Objective Addressed Explanation
#7440 Ensure ValidateForm correctly waits for all (including asynchronous, e.g., CardUpload) field validations to complete before determining overall form validity on submit, so dialogs do not close when there are validation errors.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@bb-auto bb-auto Bot added the chore This are tasks or bot action label Dec 31, 2025
@bb-auto bb-auto Bot added this to the v10.1.0 milestone Dec 31, 2025
@ArgoZhang ArgoZhang merged commit 168864f into main Dec 31, 2025
6 of 8 checks passed
@ArgoZhang ArgoZhang deleted the fix-card-upload branch December 31, 2025 08:10
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • Re-evaluate using a shared _tcs field for coordination: if ValidateObject can be called concurrently, reassigning _tcs per call can lead to race conditions or tasks that are never completed for earlier callers; consider scoping the TaskCompletionSource to the call or otherwise synchronizing access.
  • Now that _tcs is only completed at the end of ValidateObject, ensure that all code paths (including early returns and exception paths) either set a result or fault the TaskCompletionSource, so no awaiting code can hang indefinitely.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Re-evaluate using a shared `_tcs` field for coordination: if `ValidateObject` can be called concurrently, reassigning `_tcs` per call can lead to race conditions or tasks that are never completed for earlier callers; consider scoping the `TaskCompletionSource` to the call or otherwise synchronizing access.
- Now that `_tcs` is only completed at the end of `ValidateObject`, ensure that all code paths (including early returns and exception paths) either set a result or fault the `TaskCompletionSource`, so no awaiting code can hang indefinitely.

## Individual Comments

### Comment 1
<location> `src/BootstrapBlazor/Components/ValidateForm/ValidateForm.razor.cs:273-276` </location>
<code_context>
     /// <param name="results"></param>
     internal async Task ValidateObject(ValidationContext context, List<ValidationResult> results)
     {
+        _tcs = new TaskCompletionSource<bool>();
         _validateResults.Clear();

</code_context>

<issue_to_address>
**issue (bug_risk):** TaskCompletionSource may remain uncompleted if validation throws before reaching TrySetResult

Since `_tcs` is set at the start of `ValidateObject` and only completed at the end, any exception in the method (e.g. from `validator.ToggleMessage` or inside the loop) will leave it incomplete and hang awaiting callers. Please ensure `_tcs` is always completed (e.g. via a `try`/`finally` that calls `TrySetResult(false)` or `TrySetCanceled` on error).
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (30f3eb5) to head (62b0a1e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #7454   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          749       749           
  Lines        32919     32908   -11     
  Branches      4574      4572    -2     
=========================================
- Hits         32919     32908   -11     
Flag Coverage Δ
BB 100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes an issue where form validation wasn't properly waiting for all field validations to complete before determining the form's validity. The fix refactors the TaskCompletionSource logic to ensure synchronization across all field validations.

Key changes:

  • Moved TaskCompletionSource initialization to the beginning of ValidateObject method to track the entire validation process
  • Removed the _invalid field and related logic that was causing incorrect validation state tracking
  • Simplified the Validate() method by removing unnecessary _invalid flag manipulation

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/BootstrapBlazor/Components/ValidateForm/ValidateForm.razor.cs Refactored validation logic to properly wait for all field validations by moving TaskCompletionSource creation to the start of ValidateObject and setting the result only after all fields are validated; removed the problematic _invalid field
src/BootstrapBlazor/BootstrapBlazor.csproj Incremented version from 10.2.1-beta02 to 10.2.1-beta03 for this bug fix release

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

/// <param name="results"></param>
internal async Task ValidateObject(ValidationContext context, List<ValidationResult> results)
{
_tcs = new TaskCompletionSource<bool>();
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

There is a potential concurrency issue with the TaskCompletionSource field. If ValidateObject is called multiple times before the previous validation completes (e.g., rapid form submissions), the _tcs field could be overwritten, causing the previous await in OnValidSubmitForm to wait on the wrong task. Consider using a local variable instead of a field, or implementing proper synchronization to prevent concurrent validations.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore This are tasks or bot action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(ValidateForm): 异步验证导致验证结果不正确

2 participants