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

fix: Ensure that side effects of readBean do not set hasChanges true (#12181)(CP: 8.0) #12319

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

anssit
Copy link
Contributor

@anssit anssit commented Nov 9, 2021

JavaDoc of hasChanges says:

"Check whether any of the bound fields' have uncommitted changes
since last explicit call to readBean(Object), removeBean(), writeBean(Object)}
or writeBeanIfValid(Object)."

If readBean has converters, they will be run and field values updated accordingly.
Furthermore if fields have value change listeners that will produce further
changes in values, this should be considered according to above as part of
readBean procedure and thus hasChanges still should return false.

Cherry pick from: vaadin/framework#12455

(cherry picked from commit 4e36fd9)

…12181)

JavaDoc of hasChanges says:

"Check whether any of the bound fields' have uncommitted changes
since last explicit call to readBean(Object), removeBean(), writeBean(Object)}
or writeBeanIfValid(Object)."

If readBean has converters, they will be run and field values updated accordingly.
Furthermore if fields have value change listeners that will produce further
changes in values, this should be considered according to above as part of
readBean procedure and thus hasChanges still should return false.

Cherry pick from: vaadin/framework#12455

(cherry picked from commit 4e36fd9)
@vaadin-bot
Copy link
Collaborator

SonarQube analysis reported 6 issues

  1. MAJOR Binder.java#L866: Rename "binding" which hides the field declared at line 820. rule
  2. MAJOR Binder.java#L914: Rename "binding" which hides the field declared at line 820. rule
  3. MINOR Binder.java#L245: "public" is redundant in this context. rule
  4. MINOR Binder.java#L252: "public" is redundant in this context. rule
  5. INFO Binder.java#L2586: Complete the task associated to this TODO comment. rule
  6. INFO Binder.java#L2600: Complete the task associated to this TODO comment. rule

@joheriks joheriks enabled auto-merge (squash) November 9, 2021 14:27
@joheriks joheriks merged commit acad9a8 into 8.0 Nov 9, 2021
@joheriks joheriks deleted the fix-binder-haschanges-8.0 branch November 9, 2021 15:14
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with platform 21.0.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants