Series sales: add validation to ensure that buyer and seller aren't the same #1007

KrivenkoAlexander · 2019-02-21T14:37:52Z

Addressed to #503

mystamps-bot · 2019-02-21T14:49:10Z

	1 Message
📖	@KrivenkoAlexander thank you for the PR! All quality checks have been passed! Next step is to wait when @php-coder will review this code

Generated by 🚫 Danger

php-coder · 2019-02-21T14:49:54Z

src/main/java/ru/mystamps/web/feature/series/sale/AddSeriesSalesForm.java

+//)
+@FieldsMismatch(
+	first = "sellerId",
+	second ="buyerId"


This annotation works.

The reason why it doesn't work for you is that you can't create a participant that is a buyer and a seller at the same time. There 2 possible ways to test it:

modify manually HTML code of the page to be able to choose Eicca twice

modify Eicca's properties directly in database (so she will appear in both select boxes)

KrivenkoAlexander · 2019-02-21T15:01:55Z

There was one doubt moment to me else. I did creation of two new users with the similar names and used them like a buyer and seller . I supposed that we need to check their names to validate from. In this case the validator doesn't work. is it correct test case ? And if so, we need to get the real name of users from DB ( i guess ) and match them. Or i didn't clear catch the task? Thanks

php-coder · 2019-02-21T15:46:51Z

@KrivenkoAlexander I've updated PR's title and body. Also, you can add [WIP] to the PR's title, it means that "Work In Progress" and it isn't ready for a merge.

php-coder · 2019-02-21T15:51:33Z

I did creation of two new users with the similar names and used them like a buyer and seller .

When you said "users" you mean "participants"? Because they're different -- users may login to the site and their information is stored in the "users" table while "participants" aren't real users, they can be "buyers" or "sellers" and their information are stored in the "transaction_participant" table. This task is focused around participants.

I supposed that we need to check their names to validate from. In this case the validator doesn't work. is it correct test case ?

Unfortunately, no, we don't care about their names. The one that is only important is their IDs because only IDs are uniquely identifies them.

Thank you for the questions! Next time I'll put more information to the task descriptions.

php-coder

Yes, that is what I meant! See my comments also.

php-coder · 2019-02-21T20:58:59Z

src/main/java/ru/mystamps/web/feature/series/sale/AddSeriesSalesForm.java

@@ -33,6 +34,10 @@

 @Getter
 @Setter
+@FieldsMatch(


Please, use another validator -- @FieldsMismatch (it also fixes the tests that are broken because you changed the behavior of the @FieldsMatch validator to the opposite).

yes, mixed up the annotation

php-coder · 2019-02-21T21:07:04Z

src/main/java/ru/mystamps/web/feature/series/sale/AddSeriesSalesForm.java

@@ -33,6 +34,10 @@

 @Getter


Please, add a comment before this annotation:

// @todo #503 Add integration test to ensure that seller and buyer are different

This "magick" comment will create the next issue for us, so that we won't forget to add integration tests later.

php-coder · 2019-02-21T21:12:19Z

src/main/java/ru/mystamps/web/feature/series/sale/AddSeriesSalesForm.java

@@ -33,6 +34,10 @@

 @Getter
 @Setter
+@FieldsMatch(
+	first = "sellerId",
+	second = "buyerId"


We need to also add non-default message. Example:

mystamps/src/main/java/ru/mystamps/web/feature/account/ActivateAccountForm.java

Lines 33 to 36 in 4741948

@FieldsMismatch(

first = "login",

second = "password",

message = "{password.login.match}",

mystamps/src/main/resources/ru/mystamps/i18n/ValidationMessages.properties

Line 50 in 4741948

password.login.match = Password and login must be different

mystamps/src/main/resources/ru/mystamps/i18n/ValidationMessages_ru.properties

Line 50 in 4741948

password.login.match = Пароль и логин не должны совпадать

In this case, the message will be

seller.buyer.match = Seller and buyer must be different

codecov · 2019-02-22T13:10:57Z

Codecov Report

Merging #1007 into master will increase coverage by 0.06%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1007      +/-   ##
============================================
+ Coverage     79.94%   80.01%   +0.06%     
- Complexity      488      489       +1     
============================================
  Files            35       35              
  Lines          1466     1466              
  Branches        188      188              
============================================
+ Hits           1172     1173       +1     
  Misses          277      277              
+ Partials         17       16       -1

Impacted Files	Coverage Δ	Complexity Δ
...ture/series/importing/SeriesImportServiceImpl.java	`97.91% <0%> (+1.04%)`	`31% <0%> (+1%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b25cb6...18331c1. Read the comment docs.

codecov · 2019-02-22T13:10:57Z

Codecov Report

Merging #1007 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1007   +/-   ##
=========================================
  Coverage     79.94%   79.94%           
  Complexity      488      488           
=========================================
  Files            35       35           
  Lines          1466     1466           
  Branches        188      188           
=========================================
  Hits           1172     1172           
  Misses          277      277           
  Partials         17       17

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ff7854...f07e6fc. Read the comment docs.

php-coder · 2019-02-22T13:39:40Z

src/main/resources/ru/mystamps/i18n/ValidationMessages_ru.properties

-day.invalid = Некорректный день месяца
-month.invalid = Некорректный номер месяца
-series.too-early-release-year = Год выпуска серии не может быть раньше {value}
+javax.validation.constraints.NotNull.message = \u041F\u043E\u043B\u0435 \u043E\u0431\u044F\u0437\u0430\u0442\u0435\u043B\u044C\u043D\u043E \u0434\u043B\u044F \u0437\u0430\u043F\u043E\u043B\u043D\u0435\u043D\u0438\u044F


Could you please, leave this file in Russian? We have the native2ascii maven plugin which encodes to ascii automatically on every build.

Sorry, but it's not clear to me . I didn't add these lines to this Ru properties ... I added only one string

seller.buyer.match = Продавец и покупатель не должны совпадать
Should i delete it? If i delete it the checkstyle will raise the violation about missing of this line . Is it ok ?
Thanks

As far as i understand you our plugin translates our messages in English , right ?

I didn't add these lines to this Ru properties ... I added only one string

Yes, it wasn't you but it was your IDE. Perhaps, there should be a way to configure it to disable such behavior. Have you enable "Transparent native-to-ascii conversion for properties files"? See https://www.jetbrains.com/help/idea/properties-files.html#encoding

Should i delete it?

No, please, leave it. The issue isn't about you added one line but it's about your IDE convert the content of a whole file to another charset. It shouldn't do that.

I think that everything has set by default ...

What do you have for Default encoding for properties files? UTF-8?

@KrivenkoAlexander I've just checked my IDEA settings and it turned out that "Transparent native-to-ascii conversion" should be turned off! :-) I'll update instruction, thank you!

i have switched off this setting right now

php-coder · 2019-02-22T13:40:19Z

src/main/resources/ru/mystamps/i18n/ValidationMessages.properties

@@ -49,6 +49,8 @@ login.repetition_chars = Login must not contain repetition of hyphen, dot or und
 password.mismatch = Password mismatch
 password.login.match = Password and login must be different

+seller.buyer.match = Seller and buyer must be different
+


I'd rather put it right after password.login.match.

php-coder · 2019-02-22T13:42:03Z

src/main/java/ru/mystamps/web/feature/series/sale/AddSeriesSalesForm.java

+	first = "sellerId",
+	second = "buyerId",
+	message = "{seller.buyer.match}"
+)


As the annotation with all its argument isn't too long, could you write it as a one line, please?

KrivenkoAlexander · 2019-02-23T21:12:17Z

Could you review my solution and merge it if it is ok. And i am ready for new task .
Thank you for sharing of links about validator and spring framework . They help me and i appreciate this.
Thanks.

php-coder · 2019-02-23T22:55:01Z

Could you review my solution and merge it if it is ok. And i am ready for new task .

You've changed the editor settings but the file is still in a bad shape. Could you modify it? ValidationMessages_ru.properties should have the similar small change as ValidationMessages.properties -- just one line of text in Russian.
please, squash the commits into one

php-coder · 2019-02-23T22:58:29Z

And i am ready for new task .

The next task is #504 but it edits the same file and should be based on the changes from this PR.

KrivenkoAlexander · 2019-02-24T08:41:13Z

Let's try again. :) I have changed the settings . I hope it will fix this problem.

php-coder · 2019-02-24T11:15:29Z

Let's try again. :) I have changed the settings . I hope it will fix this problem.

No, it didn't. Check yourself -- open tab "Files changed" and scroll down to the ValidationMessages_ru.properties file -- the whole content was edited.

Let's fix it step-by-step:

turn off "Transparent native-to-ascii conversion" in IDEA
revert the change for that file: git checkout HEAD^ -- src/main/resources/ru/mystamps/i18n/ValidationMessages_ru.properties
open that file in IDEA again and add a line with translation
commit your changes: git commit -a --amend
check your fix: git show src/main/resources/ru/mystamps/i18n/ValidationMessages_ru.properties
push: git push --force-with-lease

…d it just to create new PR no more .

KrivenkoAlexander · 2019-02-24T12:32:33Z

Right now looks good. Am i right that my problem was that i didn't revert back my changes and tried to change the setting of IDEA? I should do reverting every time when i need to change config in IDEA, right?
Thanks.

php-coder · 2019-02-24T17:25:40Z

Merged in 36cf5c3
Thank you!

KrivenkoAlexander requested a review from php-coder as a code owner February 21, 2019 14:37

php-coder reviewed Feb 21, 2019

View reviewed changes

php-coder changed the title ~~First attempt to make improvements of validation : it is draft .~~ [WIP] Series sales: add validation to ensure that buyer and seller aren't the same Feb 21, 2019

KrivenkoAlexander force-pushed the gh503_Series_sales_form_diff_buyer_and_seller branch from a7d77af to 0e50a9b Compare February 21, 2019 20:25

php-coder requested changes Feb 21, 2019

View reviewed changes

php-coder changed the title ~~[WIP] Series sales: add validation to ensure that buyer and seller aren't the same~~ Series sales: add validation to ensure that buyer and seller aren't the same Feb 21, 2019

KrivenkoAlexander force-pushed the gh503_Series_sales_form_diff_buyer_and_seller branch from 0e50a9b to 18331c1 Compare February 22, 2019 13:00

php-coder reviewed Feb 22, 2019

View reviewed changes

KrivenkoAlexander changed the title ~~Series sales: add validation to ensure that buyer and seller aren't the same~~ [WIP]:Series sales: add validation to ensure that buyer and seller aren't the same Feb 24, 2019

KrivenkoAlexander force-pushed the gh503_Series_sales_form_diff_buyer_and_seller branch from bf9475b to d024054 Compare February 24, 2019 08:39

KrivenkoAlexander force-pushed the gh503_Series_sales_form_diff_buyer_and_seller branch from d024054 to 2e23223 Compare February 24, 2019 10:30

php-coder changed the title ~~[WIP]:Series sales: add validation to ensure that buyer and seller aren't the same~~ Series sales: add validation to ensure that buyer and seller aren't the same Feb 24, 2019

new setting for IDEa(Attempt 2): I did one simple addition and delete…

f07e6fc

…d it just to create new PR no more .

KrivenkoAlexander force-pushed the gh503_Series_sales_form_diff_buyer_and_seller branch from 2e23223 to f07e6fc Compare February 24, 2019 12:02

php-coder closed this Feb 24, 2019

php-coder added the resolution/merged label Feb 24, 2019

KrivenkoAlexander deleted the gh503_Series_sales_form_diff_buyer_and_seller branch February 25, 2019 09:49

	@FieldsMismatch(
	first = "login",
	second = "password",
	message = "{password.login.match}",

Series sales: add validation to ensure that buyer and seller aren't the same #1007

Series sales: add validation to ensure that buyer and seller aren't the same #1007

Uh oh!

Conversation

KrivenkoAlexander commented Feb 21, 2019 • edited by php-coder Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mystamps-bot commented Feb 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KrivenkoAlexander commented Feb 21, 2019 via email • edited by php-coder Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

php-coder commented Feb 21, 2019

Uh oh!

php-coder commented Feb 21, 2019

Uh oh!

php-coder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codecov bot commented Feb 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KrivenkoAlexander Feb 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KrivenkoAlexander commented Feb 23, 2019

Uh oh!

php-coder commented Feb 23, 2019

Uh oh!

php-coder commented Feb 23, 2019

Uh oh!

KrivenkoAlexander commented Feb 21, 2019 •

edited by php-coder

Loading

mystamps-bot commented Feb 21, 2019 •

edited

Loading

KrivenkoAlexander commented Feb 21, 2019 via email •

edited by php-coder

Loading

codecov bot commented Feb 22, 2019 •

edited

Loading

codecov bot commented Feb 22, 2019 •

edited

Loading

KrivenkoAlexander Feb 23, 2019 •

edited

Loading

KrivenkoAlexander commented Feb 24, 2019 •

edited

Loading