-
Notifications
You must be signed in to change notification settings - Fork 15
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
AYS-659 | Refactor error code from 404 to 409 when trying to activate an already active user #434
base: main
Are you sure you want to change the base?
AYS-659 | Refactor error code from 404 to 409 when trying to activate an already active user #434
Conversation
@idilalemdar Yanlışlıkla benden review talep edildiğini düşünüyorum. DevOps tarafı ile ilgili bir konu varsa bakabilirim. |
src/main/java/org/ays/auth/service/impl/AysUserUpdateServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/org/ays/auth/exception/AysUserNotPassiveException.java
Outdated
Show resolved
Hide resolved
yok hayır senlik bir durum yok, dediğin gibi yanlışlıkla eklemişim, çok özür dilerim. |
src/main/java/org/ays/auth/exception/AysUserNotPassiveException.java
Outdated
Show resolved
Hide resolved
@@ -118,7 +117,7 @@ public void activate(String id) { | |||
.orElseThrow(() -> new AysUserNotExistByIdException(id)); | |||
|
|||
if (!user.isPassive()) { | |||
throw new AysUserNotPassiveException(AysUserStatus.PASSIVE); | |||
throw new AysUserNotPassiveException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Burada 2 kontrol olması gerekiyor;
- Kullanıcının zaten aktif olduğunu belirtmek için bir kontrol ekleyip, aktif olduğunu belirtmek
- Kullanıcının pasif olmadığına dair bir kontrol ekleyip, pasif olmadığını söylemek
Ek olarak hem unit test'leri hem de bu case'i test eden bir end to end test yazıp hata mesajını ve http status code'unu doğrulayabilir miyiz?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okey şu an !user.isPassiver()'e ek olarak user.isActive() kontrolü var. bunların ilgili unit testleri de var. end-to -end test ne için tam olarak anlayamadım onu. @agitrubard
src/main/java/org/ays/auth/exception/AysUserNotPassiveException.java
Outdated
Show resolved
Hide resolved
src/main/java/org/ays/auth/exception/AysUserNotPassiveException.java
Outdated
Show resolved
Hide resolved
src/main/java/org/ays/auth/exception/AysUserAlreadyActiveException.java
Outdated
Show resolved
Hide resolved
…tion.java Co-authored-by: Egehan Asal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@agitrubard 'in bahsettigi end to end test atlanmis sanirim
@idilalemdar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit Testlere ek olarak 2 senaryo için end-to-end test eklememiz ve response'ta ilgili cevapları doğrulamamız iyi olabilir
src/test/java/org/ays/auth/service/impl/AysUserUpdateServiceImplTest.java
Show resolved
Hide resolved
src/test/java/org/ays/auth/service/impl/AysUserUpdateServiceImplTest.java
Show resolved
Hide resolved
src/test/java/org/ays/auth/service/impl/AysUserUpdateServiceImplTest.java
Outdated
Show resolved
Hide resolved
…n.java Co-authored-by: Egehan Asal <[email protected]>
…plTest.java Co-authored-by: Agit Rubar Demir <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
End-to-End testleri ekledikten sonra mergeleyebiliriz 👍🏼
…ve user
Checklist
Before submitting your pull request, ensure the following:
Title and Branch Naming Conventions:
standard: Pull Request Naming Conventions.
the Branch Naming Conventions.
Local Testing:
Code Quality:
Documentation:
Testing:
Reviewers and Assignees:
Labels and Associations: