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

fix: 어드민 히스토리 기능 수정 #1087

Closed
wants to merge 12 commits into from

Conversation

Soundbar91
Copy link
Contributor

@Soundbar91 Soundbar91 commented Nov 26, 2024

🔥 연관 이슈

🚀 작업 내용

  • 클라이언트 요청으로 작업을 진행했습니다.
    • 반환 날짜 형식을 변경했습니다
    • 조회 과정에서 정렬 옵션을 줄 수 있도록 설정했습니다.
    • 조회 옵션에서 String으로 받는 부분을 enum으로 변경했습니다.
    • 기존 String 필드를 enum으로 변경했습니다.

💬 리뷰 중점사항

@Soundbar91 Soundbar91 added the Team User 유저 팀에서 작업할 이슈입니다 label Nov 26, 2024
@Soundbar91 Soundbar91 self-assigned this Nov 26, 2024
@github-actions github-actions bot added the 버그 정상적으로 동작하지 않는 문제상황입니다. label Nov 26, 2024
Copy link

github-actions bot commented Nov 26, 2024

Unit Test Results

339 tests   338 ✔️  1m 40s ⏱️
  41 suites      1 💤
  41 files        0

Results for commit 0f756c2.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@seongjae6751 seongjae6751 left a comment

Choose a reason for hiding this comment

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

여기서는 딱히 변경 할게 보이지는 않는 것 같네요 고생하셨습니당

Copy link
Contributor

@kwoo28 kwoo28 left a comment

Choose a reason for hiding this comment

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

수고 많으셨습니다!

Comment on lines 55 to 58
public AdminActivityHistory(
Integer domainId, HttpMethodType requestMethod,
DomainType domainName, String requestMessage, Admin admin
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

C

이 부분 현수형이 리뷰해준대로 필드 하나씩 줄바꿈하는거 어떨까요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +30 to +31
(:#{#condition.requestMethod?.name()} IS NULL OR a.requestMethod = :#{#condition.requestMethod}) AND
(:#{#condition.domainName?.name()} IS NULL OR a.domainName = :#{#condition.domainName}) AND
Copy link
Contributor

Choose a reason for hiding this comment

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

A

왼쪽은 ?.name()을 추가했는데, 오른쪽은 추가안한 이유가 먼가요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

domainName과 requestMethod가 null인 경우를 체크하기 위해 ?을 붙여줬습니다. 그렇지 않으면 NPE가 터져버립니다.
오른쪽이 null이면 문장이 true이기 때문에 오른쪽 조건을 검사하지 않습니다.

만약, 왼쪽이 null이 아닌 경우 (즉 false)인 경우는 오른쪽 조건을 검사하기 때문에 ?을 붙이지 않고 값을 그대로 가져갑니다.

@Soundbar91 Soundbar91 closed this Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team User 유저 팀에서 작업할 이슈입니다 버그 정상적으로 동작하지 않는 문제상황입니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants