Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binder.validate() no longer fails with "bean level validators have been configured but no bean is currently set" when in buffered mode #18121

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mvysny
Copy link
Member

@mvysny mvysny commented Nov 24, 2023

Description

This PR makes the Binder.validate() implementation in line with its javadoc. Namely, Binder.validate() no longer fails with "bean level validators have been configured but no bean is currently set" when in buffered mode, which is according to the current javadoc: "Bean level validators are ignored if there is no bound bean or if any field level validator fails."

Fixes #18120

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

Copy link

github-actions bot commented Nov 24, 2023

Test Results

1 092 files  ± 0  1 092 suites  ±0   1h 21m 52s ⏱️ -42s
6 821 tests ± 0  6 775 ✅ ± 0  46 💤 ±0  0 ❌ ±0 
7 133 runs   - 37  7 075 ✅  - 37  58 💤 ±0  0 ❌ ±0 

Results for commit 1a5d08d. ± Comparison against base commit dc68b28.

This pull request removes 3 and adds 3 tests. Note that renamed tests count towards both.
com.vaadin.flow.data.binder.BinderTest ‑ beanvalidation_isValid_throws_with_readBean
com.vaadin.flow.data.binder.BinderTest ‑ beanvalidation_validate_throws_with_readBean
com.vaadin.flow.data.binder.BinderTest ‑ isValidTest_unbound_binder_throws_with_bean_level_validation
com.vaadin.flow.data.binder.BinderTest ‑ beanvalidation_isValid_passes_with_readBean
com.vaadin.flow.data.binder.BinderTest ‑ beanvalidation_validate_passes_with_readBean
com.vaadin.flow.data.binder.BinderTest ‑ isValidTest_unbound_binder_passes_with_bean_level_validation

♻️ This comment has been updated with latest results.

@mcollovati mcollovati added the Contribution PRs coming from the community or external to the team label Nov 29, 2023
Copy link

sonarcloud bot commented Dec 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mshabarov
Copy link
Contributor

This should be postponed until major release of Flow (25), as this is a behaviour change.

@mshabarov mshabarov closed this Dec 15, 2023
@tepi tepi reopened this Jan 29, 2024
@tepi tepi marked this pull request as draft January 29, 2024 12:58
Copy link

sonarcloud bot commented Jan 29, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution PRs coming from the community or external to the team planned-in-25.0 +1.0.0
Projects
None yet
6 participants