JujaLabs / itevents

Resource to subscribe on it events
Apache License 2.0
7 stars 5 forks source link

183: handle registration with existing login #208

Closed IgorMaksymov closed 8 years ago

IgorMaksymov commented 8 years ago

connect to #183

vaa25 commented 8 years ago

@IgorMaksymov MyBatisUserServiceTest.shouldThrowEntityAlreadyExistsServiceExceptionWhenAddExistingSubscriber: remove eq and any @AndriyBaibak 20 min

IgorMaksymov commented 8 years ago

@vaa25 fixed

romach commented 8 years ago

@IgorMaksymov When I try to register with existing login I get:

ERROR: duplicate key value violates unique constraint "user_profile_login_key"
  Подробности: Key (login)=(anakin@email.com) already exists.

This is insecure message. We need something like "Username already exists"

romach commented 8 years ago

@AndriyBaibak 30 minutes spent

IgorMaksymov commented 8 years ago

@romach i tried to fix this, but its not possible for now.

romach commented 8 years ago

@IgorMaksymov what exactly did you try to fix? What's the problem? Is there //Todo and new issue for problem?

IgorMaksymov commented 8 years ago

@romach i tried to catch exception, that is thrown from our DB. i dont see a big problem here. This must be refactored in this task https://github.com/JuniorsJava/itevents/issues/207 and no need to make TODO

romach commented 8 years ago

@IgorMaksymov 1) I think that we need //todo because reviewer will understand that the problem is determined and will be solved in new issue; 2) I think that #207 has bad description. Point out concrete antipatterns in issue description and insert //todos in code with link to issue.

IgorMaksymov commented 8 years ago

1) this is not a problem, this is the only way to handle Postgres Exceptions. It's not looking googd, but its working as it should. 2) to point antipatterns, i need to copy whole article to a description. ok, i will add todo with link to #207

IgorMaksymov commented 8 years ago

@romach added todo

romach commented 8 years ago

@IgorMaksymov you made wrong commits (#196-), but current branch is #183-

romach commented 8 years ago

@AndriyBaibak 30 min