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

AYS-660 | Change Error Code and Message for Already Passive Users #436

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

egehanasal
Copy link
Contributor

Checklist

Before submitting your pull request, ensure the following:

  • Title and Branch Naming Conventions:

  • Local Testing:

    • I have tested my changes locally on Postman, and they are working as expected.
  • Code Quality:

    • The code is formatted according to the project's coding guidelines and style.
    • The code has been reviewed to ensure its quality.
    • The code does not contain any issues flagged by SonarLint.
  • Documentation:

    • Necessary documentation has been added or existing documentation has been updated, specifically detailing changes made in Postman.
  • Testing:

    • Relevant unit tests have been written and included.
    • Relevant integration tests have been written and included.
  • Reviewers and Assignees:

    • Default reviewers have been assigned to this pull request.
    • Assignees have been added if necessary.
  • Labels and Associations:

    • No specific actions are required in the Labels and Associations section for this pull request.

idilalemdar
idilalemdar previously approved these changes Jan 23, 2025
Assertions.assertEquals(AysUserStatus.PASSIVE, userFromDatabase.get().getStatus());
Assertions.assertNull(userFromDatabase.get().getUpdatedUser());
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diğer senaryo için de end-to-end test ekleyebilir miyiz?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ayrıca bu senaryoda da en sondaki assertnull satırının doğru çalıştığından emin miyiz? findbyid metodu updated alanı null ise created ile aynı alanı dönüyor gözlemlediğim kadarıyla, bu sebeple bende şu an assertnull hata veriyor. @agitrubard @egehanasal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Evet bu test basari ile geciyor. Testin basinda kullanici olusturdugumuz ve sonrasinda bir degisiklik yapmadigimiz icin, userFromDatabase'de ilgili alanlar bu sekilde geliyorlar
image
Kullanicinin state'inin degismedigini gozlemlemek adina testin son satirina o kontrolu eklemenin onemli oldugunu dusundum

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@egehanasal buraya updatedAt alanını da ekleyebiliriz ✅

Assertions.assertEquals(AysUserStatus.PASSIVE, userFromDatabase.get().getStatus());
Assertions.assertNull(userFromDatabase.get().getUpdatedUser());
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@egehanasal buraya updatedAt alanını da ekleyebiliriz ✅

Comment on lines +650 to +654
@ValueSource(strings = {
"NOT_VERIFIED",
"DELETED"
})
void givenInactiveUserId_whenTryToPassivate_thenReturnUserNotActiveError(String inactiveStatus) throws Exception {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@ValueSource(strings = {
"NOT_VERIFIED",
"DELETED"
})
void givenInactiveUserId_whenTryToPassivate_thenReturnUserNotActiveError(String inactiveStatus) throws Exception {
@EnumSource(value = AysUserStatus.class, names = {
"NOT_VERIFIED",
"DELETED"
})
void givenInactiveUserId_whenTryToPassivate_thenReturnUserNotActiveError(AysUserStatus status) throws Exception {

Comment on lines +1008 to +1012
@ValueSource(strings = {
"NOT_VERIFIED",
"DELETED"
})
void givenValidId_whenUserIsNotActive_thenThrowUserNotActiveException(String inactiveStatus) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    @EnumSource(value = AysUserStatus.class, names = {
            "NOT_VERIFIED",
            "DELETED"
    })
    void givenValidId_whenUserIsNotActive_thenThrowUserNotActiveException(AysUserStatus mockStatus) {

Assertions.assertTrue(userFromDatabase.isPresent());
Assertions.assertEquals(userFromDatabase.get().getId(), user.getId());
Assertions.assertEquals(AysUserStatus.valueOf(inactiveStatus), userFromDatabase.get().getStatus());
Assertions.assertNull(userFromDatabase.get().getUpdatedUser());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buraya updatedAt alanını da ekleyebiliriz

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

Successfully merging this pull request may close these issues.

3 participants