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

상세 조회에서 다른 사람의 비공개 템플릿 확인 시 예외 처리 #852

Merged
merged 8 commits into from
Oct 23, 2024

Conversation

kyum-q
Copy link
Contributor

@kyum-q kyum-q commented Oct 22, 2024

⚡️ 관련 이슈

#739

📍주요 변경 사항

  • 단건 조회 시 다른 사람의 비공개 템플릿 조회일 경우 예외 발생
    • ErrorCode.FORBIDDEN_ACCESS, "해당 템플릿은 비공개 템플릿입니다."
  • DB에서 템플릿 기본 값을 PUBLIC으로 설정

🎸기타

고려해야 하는 내용을 작성해 주세요.

🍗 PR 첫 리뷰 마감 기한

10/23 10:00

해당 PR은 내일 릴리즈 전에 올라가야할 것 같습니다~ 빠른 리뷰 부탁드려요 !

@kyum-q kyum-q added refactor 요구사항이 바뀌지 않은 변경사항 zap 리뷰 우선순위가 높은 사항 BE 백엔드 labels Oct 22, 2024
@kyum-q kyum-q added this to the 6차 스프린트 🦴 milestone Oct 22, 2024
@kyum-q kyum-q self-assigned this Oct 22, 2024
Copy link
Contributor

@zeus6768 zeus6768 left a comment

Choose a reason for hiding this comment

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

켬미 작업 너무 빨라~~ RC 남겼으니 확인 바라요~~

Comment on lines 84 to 98
public void validateAuthorization(Member member) {
if (!getMember().equals(member)) {
if (!matchMember(member)) {
throw new CodeZapException(ErrorCode.FORBIDDEN_ACCESS, "해당 템플릿에 대한 권한이 없습니다.");
}
}

public boolean matchMember(Member member) {
return getMember().equals(member);
}

public void ensureNotPrivateTemplate() {
if (visibility == Visibility.PRIVATE) {
throw new CodeZapException(ErrorCode.FORBIDDEN_ACCESS, "해당 템플릿은 비공개 템플릿입니다.");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ErrorCode는 뷰 로직 같은데, 도메인에서 의존하면 안 되지 않을까요?

Template에서는 예외를 직접 발생시키기보다 boolean을 반환하면 더 좋다고 생각하는데, 켬미는 어떤가요?

다음과 같이 제안해요. 메서드명도 바꿔 봤어요!

public boolean ownedBy(Member member) {
    return getMember().equals(member);
}

public boolean isPrivate() {
    return visibility.isPrivate();
}

public boolean isPublic() {
    return visibility.isPublic();
}

@kyum-q kyum-q added zap 리뷰 우선순위가 높은 사항 and removed zap 리뷰 우선순위가 높은 사항 labels Oct 22, 2024
@kyum-q kyum-q changed the base branch from dev/be to main October 22, 2024 06:19
@kyum-q kyum-q added zap 리뷰 우선순위가 높은 사항 and removed zap 리뷰 우선순위가 높은 사항 labels Oct 22, 2024
@kyum-q kyum-q closed this Oct 22, 2024
@kyum-q kyum-q reopened this Oct 22, 2024
@kyum-q kyum-q changed the base branch from main to dev/be October 22, 2024 06:20
Copy link
Contributor

@jminkkk jminkkk 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 74 to 76
if (!template.matchMember(loginMember) && template.isPrivate()) {
throw new CodeZapException(ErrorCode.FORBIDDEN_ACCESS, "해당 템플릿은 비공개 템플릿입니다.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

메서드 추출해보면 어떨까요~~ 위에 것도!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commit에 반영 했습니다~

Copy link
Contributor

@HoeSeong123 HoeSeong123 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 78 to 80
public boolean matchMember(Member member) {
return getMember().equals(member);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

일부러 getMember 사용하신건가요?

Suggested change
public boolean matchMember(Member member) {
return getMember().equals(member);
}
public boolean matchMember(Member member) {
return this.member.equals(member);
}

Copy link
Contributor Author

@kyum-q kyum-q Oct 23, 2024

Choose a reason for hiding this comment

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

Commit에 반영했습니다~

zeus6768
zeus6768 previously approved these changes Oct 23, 2024
@zeus6768 zeus6768 merged commit d89ee41 into dev/be Oct 23, 2024
8 checks passed
@zeus6768 zeus6768 deleted the refactor/739-find-private branch October 23, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 refactor 요구사항이 바뀌지 않은 변경사항 zap 리뷰 우선순위가 높은 사항
Projects
Status: Weekend Done
Development

Successfully merging this pull request may close these issues.

5 participants