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

@Compare don't display error message for left property in certain cases #634

Closed
3 tasks done
yoshikawaa opened this issue Nov 15, 2016 · 4 comments
Closed
3 tasks done

Comments

@yoshikawaa
Copy link
Contributor

yoshikawaa commented Nov 15, 2016

Description

For example set as follows, if use @Compare with attribute requiredBoth is true and node is PROPERTY (default).

@Compare(left = "email", right = "confirmEmail",
    operator = Compare.Operator.EQUAL,
    requireBoth = true)
public class UserRegisterForm {
    private String email;
    private String confirmEmail;
}

When either left property or right property is null, error message is not desplayed.
Because error message is desplayed as ROOT_BEAN message unintentionally.

Possible Solutions

https://github.com/terasolunaorg/terasoluna-gfw/blob/master/terasoluna-gfw-common-libraries/terasoluna-gfw-validator/src/main/java/org/terasoluna/gfw/common/validator/constraintvalidators/CompareValidator.java#L100
Also in the case of that (leftValue == null && rightValue == null) return false, it should be called CompareValidator#constructValidationMessage.

Affects Version/s

  • 5.1.0.RELEASE and later

Fix Version/s

  • 5.3.0
  • 5.2.1
  • 5.1.2

Issue Links

  • #XXX
@yoshikawaa yoshikawaa added this to the 2017 winter milestone Nov 15, 2016
@sasakitsy sasakitsy added ready and removed ready labels Nov 16, 2016
yoshikawaa added a commit that referenced this issue Dec 12, 2016
Modify error message output node in @Compare requireBoth check.#634
yoshikawaa pushed a commit that referenced this issue Dec 12, 2016
yoshikawaa added a commit that referenced this issue Dec 12, 2016
…_for_5.2.x

[5.2.x]Modify error message output node in @Compare requireBoth check.#634
yoshikawaa pushed a commit that referenced this issue Dec 12, 2016
yoshikawaa added a commit that referenced this issue Dec 12, 2016
…_for_5.1.x

[5.1.x]Modify error message output node in @Compare requireBoth check.#634
@kazuki43zoo
Copy link
Contributor

Hi @btyoshikawaa , Are there not need a modification on the integration test ?

@kazuki43zoo
Copy link
Contributor

@yoshikawaa

ping for about above question.

@yoshikawaa
Copy link
Contributor Author

@kazuki43zoo
I think that the E2E test is unnecessary, because unit test can ensure sufficient code quality for this problem.

@kazuki43zoo
Copy link
Contributor

@yoshikawaa

OK !

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

No branches or pull requests

4 participants