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

관심 없음 기능 추가 #47

Merged
merged 6 commits into from
Jan 27, 2024
Merged

관심 없음 기능 추가 #47

merged 6 commits into from
Jan 27, 2024

Conversation

jacobhboy
Copy link
Member

@jacobhboy jacobhboy commented Jan 26, 2024

🎫 관련 이슈

close #46


🔨 작업 내용

  • 관심없음 기능을 개발하였습니다.

🏁 확인 사항

  • 테스트를 완료했나요?
  • API 문서를 작성했나요?
  • 코드 컨벤션을 준수했나요?
  • 불필요한 로그, 주석, import 등을 삭제했나요?

🙋🏻 덧붙일 말

@jacobhboy jacobhboy linked an issue Jan 26, 2024 that may be closed by this pull request
1 task
Copy link
Contributor

@Woongbin06 Woongbin06 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 +21 to +23

Long userId;
Long partyId;
Copy link
Contributor

Choose a reason for hiding this comment

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

Id로 간접매핑을 한 이유가 있나요??

Copy link
Member Author

Choose a reason for hiding this comment

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

관심없음과 관련해서는 graphQL 복잡한 쿼리만 날아가는데 그럴 때에는 ID만 매핑하고 쓰는게 훨씬 편하더라구요. 사실 이것보다 저거 두개 복합키로 쓰려고 했는데 따로 클래스 빼자고 하니, 필드가 ID하나인 객체가 만들어져서 일단 저렇게 했습니다 😃

}

public void shouldNotAlreadyNotInterested(User user, Long partyId, Type partyType) {
boolean isAlreadyExist = notInterestedRepository.existsByUserIdAndPartyIdAndPartyType(user.getId(), partyId,
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
Member Author

Choose a reason for hiding this comment

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

읭 어떻게요??

Copy link
Contributor

Choose a reason for hiding this comment

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

저거 레파지토리의 메소드 네이밍 말한거였어요!!

Comment on lines +22 to +24
private final CommandInterestService commandInterestService;
private final AuthReader authReader;

Copy link
Contributor

Choose a reason for hiding this comment

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

생각을 해보니 Reader는 서비스에서 사용하는 구현체라 프레젠테이션에서 바로 쓰지 말고 서비스를 구현해서 사용하는 게 어떤가요????

Copy link
Member Author

Choose a reason for hiding this comment

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

이건 나중에 몰아서 개선하죠!

@jacobhboy jacobhboy merged commit f115bfa into master Jan 27, 2024
2 checks passed
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.

관심 없음 기능 추가
2 participants