Conversation
Summary of ChangesHello @lingard1234, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 마케팅 동의를 위한 새로운 약관을 도입하고 알림 설정 메커니즘을 리팩토링합니다. NotificationSetting 엔티티는 일반적인 type 및 channel 열거형 대신 appPushEnabled 및 emailEnabled 플래그를 사용하여 더 세분화된 설정을 가능하게 합니다. AuthService는 회원가입 과정에서 사용자의 동의 여부에 따라 이러한 새로운 알림 설정을 자동으로 구성하도록 수정되어, 마케팅 알림이 적절하게 관리되도록 합니다. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 Pull Request는 새로운 약관 MARKETING_CONSENT를 추가하고, NotificationSetting 엔티티와 관련 리포지토리를 단순화하도록 리팩토링했습니다. 또한 AuthService에 마케팅 동의 여부에 따라 회원가입 시 알림 설정을 처리하는 로직이 추가되었습니다. NotificationSetting을 사용자와의 일대다 관계에서 일대일 관계로 리팩토링한 점은 모델을 더 단순하고 직관적으로 만들어주는 좋은 개선입니다.
리뷰에는 AuthService에 대한 한 가지 'high' 심각도의 제안이 포함되어 있습니다. 현재 알림 설정을 처리하는 구현에는 사용자가 마케팅에 동의하는 경우만 처리하는 논리적 결함이 있으며, 실행 불가능한 코드를 포함하고 있습니다. 모든 사용자가 회원가입 시 알림 설정 레코드를 갖도록 보장하고 마케팅 동의 여부를 정확히 반영하도록 handleNotificationSettings 메서드를 리팩토링할 것을 제안했습니다. 이를 통해 코드의 명확성과 유지보수성이 향상될 것입니다.
| private void handleNotificationSettings(User user, List<SignupCompleteRequest.TermAgreementDto> terms) { | ||
| // MARKETING_NOTIFICATION 동의 여부 확인 | ||
| boolean marketingNotificationAgreed = terms.stream() | ||
| .anyMatch(term -> term.type() == TermName.MARKETING_NOTIFICATION && term.agreed()); | ||
|
|
||
| if (marketingNotificationAgreed) { | ||
| // 기존 알림 설정 조회 또는 새로 생성 | ||
| NotificationSetting notificationSetting = notificationSettingRepository | ||
| .findByUserId(user.getId()) | ||
| .orElseGet(() -> NotificationSetting.builder() | ||
| .user(user) | ||
| .appPushEnabled(false) | ||
| .emailEnabled(false) | ||
| .build()); | ||
|
|
||
| // 동의 여부에 따라 알림 설정 업데이트 | ||
| if (marketingNotificationAgreed) { | ||
| notificationSetting.update(true, true); | ||
| } else { | ||
| notificationSetting.update(false, false); | ||
| } | ||
|
|
||
| notificationSettingRepository.save(notificationSetting); | ||
| } | ||
| } |
There was a problem hiding this comment.
현재 handleNotificationSettings 메서드의 로직은 마케팅 알림에 동의한 경우에만 알림 설정을 생성하거나 업데이트합니다. 동의하지 않은 경우에는 아무런 처리도 하지 않아, 해당 유저의 알림 설정 레코드가 생성되지 않을 수 있습니다. 또한, marketingNotificationAgreed가 true인 블록 내에 불필요한 if/else 조건문이 있어 else 구문이 실행될 수 없는 구조입니다.
모든 유저에 대해 회원가입 시점에 알림 설정 레코드를 생성하고, 마케팅 동의 여부에 따라 설정을 업데이트하도록 로직을 개선하는 것을 제안합니다. 이렇게 하면 로직이 더 명확해지고, 향후 알림 설정을 관리하기 용이해집니다.
private void handleNotificationSettings(User user, List<SignupCompleteRequest.TermAgreementDto> terms) {
// MARKETING_NOTIFICATION 동의 여부 확인
boolean marketingNotificationAgreed = terms.stream()
.anyMatch(term -> term.type() == TermName.MARKETING_NOTIFICATION && term.agreed());
// 기존 알림 설정이 없으면 새로 생성
NotificationSetting notificationSetting = notificationSettingRepository.findByUserId(user.getId())
.orElseGet(() -> NotificationSetting.builder()
.user(user)
.build());
// 동의 여부에 따라 알림 설정 업데이트
notificationSetting.update(marketingNotificationAgreed, marketingNotificationAgreed);
notificationSettingRepository.save(notificationSetting);
}1 participant
Summary
TermName 추가 및 handleNotificationSettings 설정
Changes
TermName, AuthService
Type of Change
Related Issues
Closes #335
참고 사항