[25.10.04 / TASK-250] Feature - 주간 뉴스레터 수신거부 기능 추가 #46
Conversation
WalkthroughUser 모델에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant S as Scheduler
participant B as WeeklyNewsletterBatch
participant Q as UserQuery
participant T as TemplateEngine
participant E as EmailService
S->>B: run()
B->>Q: filter(is_active=True, email__isnull=False, newsletter_subscribed=True)
Q-->>B: subscribed_users[]
loop per user
B->>T: _get_newsletter_html(user, is_expired_token_user, weekly_trend_html, user_weekly_trend_html)
Note right of T #e6f7ff: 컨텍스트에 `user` 포함\n푸터에 수신 거부 링크(`user.email`)
T-->>B: html
B->>E: send(email=user.email, html)
E-->>B: result
end
sequenceDiagram
autonumber
participant Admin as Admin User
participant DA as Django Admin
participant DB as Database
Admin->>DA: Action: make_unsubscribed(selected users)
DA->>DB: UPDATE users SET newsletter_subscribed=False WHERE id IN (...)
DB-->>DA: rows updated
DA-->>Admin: Success message (updated count)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
templates/insights/index.html (1)
315-328: 수신 거부 링크 및 이메일 필터링 확인
weekly_newsletter_batch.py(91-95), weekly_user_trend_analysis.py(281-286)에서email__isnull=False, is_active=True, newsletter_subscribed=True로 null 이메일을 걸러냅니다. 빈 문자열도 차단해야 한다면.exclude(email="")추가 또는 DB 제약 검토를 권장합니다.insight/tests/tasks/test_weekly_newsletter_template.py (1)
186-186: 사용되지 않는 mock_logger 매개변수를 제거하는 것을 고려하세요.세 개의 테스트 메서드(
test_get_newsletter_html_success,test_get_newsletter_html_expired_token_user,test_get_newsletter_html_exception)에서mock_logger매개변수가 선언되었지만 사용되지 않습니다. 코드 정리를 위해 제거하는 것이 좋습니다.다음 diff를 적용하세요:
- def test_get_newsletter_html_success( - self, mock_logger, newsletter_batch, user - ): + def test_get_newsletter_html_success(self, newsletter_batch, user):def test_get_newsletter_html_expired_token_user( - self, mock_logger, newsletter_batch, user + self, newsletter_batch, user ):def test_get_newsletter_html_exception( - self, mock_logger, newsletter_batch, user + self, newsletter_batch, user ):Also applies to: 214-214, 245-245
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
insight/schemas.py(1 hunks)insight/tasks/weekly_newsletter_batch.py(4 hunks)insight/tasks/weekly_user_trend_analysis.py(1 hunks)insight/tests/conftest.py(1 hunks)insight/tests/tasks/test_weekly_newsletter_batch.py(2 hunks)insight/tests/tasks/test_weekly_newsletter_template.py(4 hunks)templates/insights/index.html(2 hunks)templates/insights/user_weekly_trend.html(0 hunks)users/admin.py(4 hunks)users/migrations/0014_user_newsletter_subscribed.py(1 hunks)users/models.py(1 hunks)
💤 Files with no reviewable changes (1)
- templates/insights/user_weekly_trend.html
🧰 Additional context used
🧬 Code graph analysis (3)
users/admin.py (3)
users/models.py (1)
User(10-69)insight/tests/conftest.py (1)
user(40-51)users/tests/conftest.py (1)
user(33-42)
insight/tests/tasks/test_weekly_newsletter_template.py (3)
insight/tests/tasks/test_weekly_newsletter_batch.py (1)
newsletter_batch(22-30)insight/tests/conftest.py (1)
user(40-51)insight/tasks/weekly_newsletter_batch.py (1)
_get_newsletter_html(224-250)
insight/tasks/weekly_newsletter_batch.py (1)
insight/tests/conftest.py (1)
user(40-51)
🪛 Ruff (0.13.2)
users/migrations/0014_user_newsletter_subscribed.py
7-9: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
11-19: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
users/admin.py
35-35: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
insight/tests/tasks/test_weekly_newsletter_template.py
186-186: Unused method argument: mock_logger
(ARG002)
214-214: Unused method argument: mock_logger
(ARG002)
245-245: Unused method argument: mock_logger
(ARG002)
🔇 Additional comments (17)
insight/schemas.py (1)
11-11: 변경사항 확인 완료.
NewsletterContext에user: dict필드를 추가하여 사용자별 뉴스레터 렌더링을 지원하도록 개선되었습니다.users/migrations/0014_user_newsletter_subscribed.py (1)
1-19: 마이그레이션 로직 확인 완료.
newsletter_subscribed필드가default=True로 추가되어 기존 사용자들이 자동으로 구독 상태를 유지하도록 구성되었습니다.users/admin.py (3)
24-24: 관리자 화면에 구독 여부 필드 노출 확인.
newsletter_subscribed필드가list_display에 추가되어 관리자 화면에서 사용자의 뉴스레터 구독 여부를 확인할 수 있습니다.
35-35: 구독 해제 액션 등록 확인.
make_unsubscribed액션이actions목록에 추가되어 관리자가 선택된 사용자들의 뉴스레터 구독을 일괄 해제할 수 있습니다.
100-112: 구독 해제 액션 구현 확인.선택된 사용자들의
newsletter_subscribed를False로 업데이트하고, 로그를 남기며, 성공 메시지를 표시하는 로직이 올바르게 구현되었습니다.users/models.py (1)
50-52: User 모델에 구독 여부 필드 추가 확인.
newsletter_subscribed필드가 올바르게 추가되었으며,default=True와null=False설정으로 데이터 무결성이 보장됩니다.insight/tests/conftest.py (1)
50-50: 테스트 픽스처 업데이트 확인.
user픽스처에newsletter_subscribed=True가 추가되어 새로운 모델 필드와 일치하며, 테스트가 구독 상태의 사용자로 실행됩니다.insight/tasks/weekly_user_trend_analysis.py (1)
285-285: 구독자 필터링 로직 추가 확인.대상 사용자 조회 시
newsletter_subscribed=True조건이 추가되어 구독 중인 사용자만 주간 트렌드 분석 대상에 포함됩니다.insight/tests/tasks/test_weekly_newsletter_batch.py (2)
118-122: 구독자 필터 테스트 추가 확인.대상 사용자 조회 테스트에
newsletter_subscribed=True조건이 포함되어, 구독 중인 사용자만 필터링되는지 검증합니다.
205-208: 테스트 어설션 포맷 변경 확인.뉴스레터 제목 검증 로직이 다중 라인 포맷으로 변경되었으나, 검증 내용은 동일합니다.
templates/insights/index.html (1)
137-152: 사용자별 헤더 렌더링 로직 추가 확인.사용자명이 존재하는 경우 "{{user.username}}님의 활동 리포트"를, 그렇지 않은 경우 "활동 리포트"를 표시하여 개인화된 뉴스레터 경험을 제공합니다.
insight/tasks/weekly_newsletter_batch.py (3)
94-94: 뉴스레터 구독자 필터링 로직이 올바르게 적용되었습니다.
newsletter_subscribed=True필터가 추가되어 구독 해지한 사용자를 뉴스레터 발송 대상에서 제외합니다. 이는 PR 목표와 일치합니다.
226-250: 사용자별 렌더링 컨텍스트가 올바르게 추가되었습니다.
user: dict매개변수가 추가되어 뉴스레터 템플릿에서 사용자별 수신 거부 링크 생성이 가능해졌습니다. 매개변수가NewsletterContext를 통해 템플릿으로 올바르게 전달됩니다.
300-300: 호출 사이트가 새로운 시그니처에 맞게 업데이트되었습니다.
_get_newsletter_html호출이user매개변수를 포함하도록 올바르게 업데이트되었습니다.insight/tests/tasks/test_weekly_newsletter_template.py (3)
185-210: 테스트가 새로운 user 매개변수를 올바르게 반영합니다.
_get_newsletter_html호출이user매개변수를 포함하도록 업데이트되었으며, 수신 거부 URL이 사용자 이메일을 포함하는지 검증합니다.
214-241: 토큰 만료 사용자 테스트가 올바르게 업데이트되었습니다.토큰 만료 시나리오에서도
user매개변수가 전달되고 수신 거부 링크가 렌더링되는지 검증합니다.
245-259: 예외 처리 테스트가 올바르게 업데이트되었습니다.렌더링 실패 시나리오에서도
user매개변수가 전달됩니다.
There was a problem hiding this comment.
저는 여전히 user model 을 건드리는건 무섭습니다.. 항상 사용자는 그 영향범위가 너무 너무 크니까요 ㅠ 그래서 대부분 현업에서는 이미 가동되는 서비스 대상으로 user 를 바꿀땐 무조건 table 을 따로 만들어서 처리하려고 합니다.
다만 앞서 우리가 얘기한대로, 볼륨이 작은 만큼, 너무 적절하게 잘 처리해주신 것 같아요. 딱 두개 첨언 드리고 싶은게
- 백오피스에서도 메일 템플릿으로 랜더링 되는데 이 부분에서도 문제가 없는 것이죠~?
- 제 3자가 봤을때
user/newsletter-unsubscribe?email={{user.email}}를 왜 호출하고, 어디에 세팅되어 있는지 F/U이 어려운 것 같아서요! 템플릿 또는 템플릿으로 랜더링하는 배치 등에 이 주석이 있으면 어떨까 해요!
| 활동 리포트 | ||
| {% endif %} | ||
| </h2> | ||
|
|
There was a problem hiding this comment.
개인적으로 궁금해서 님기는 코멘트이니 편하게 답변 부탁드려요!!
해당 부분이 user_weekly_trend.html에서 index.html로 이동한 이유가 궁금합니다!
There was a problem hiding this comment.
지난번에 템플릿 수정이후로 저게 중복되어서 들어가 있더라고요. (index.html 에도, user_weekly_trend.html 에도)
그래서 하나를 삭제해야 했는데, 유저 개인 트렌드가 없어도 토큰만료 박스는 활동 리포트 하단에 있는게 나을 거라고 판단해서 index.html 위치에 있는 것을 살렸습니다!
| user_weekly_trend | ||
| ) | ||
| assert now.strftime("%Y-%m-%d %H:%M") == result | ||
| assert now.strftime("%Y-%m-%d %H:%M:%S") == result |
There was a problem hiding this comment.
혹시 이 부분에 초 단위까지 표시하게 바뀐 이유가 뭘까요??
There was a problem hiding this comment.
지난 현우님 핫픽스의 사이드이펙트입니다~! 테스트가 실패해서 그냥 고쳐두었습니다.
| <h2 | ||
| style=" | ||
| box-sizing: border-box; | ||
| margin-bottom: 24px; | ||
| color: #000000; | ||
| font-size: 24px; | ||
| font-weight: 900; | ||
| letter-spacing: 0; | ||
| " | ||
| > | ||
| {% if user.username %} | ||
| {{user.username}}님의 활동 리포트 | ||
| {% else %} | ||
| 활동 리포트 | ||
| {% endif %} | ||
| </h2> | ||
|
|
There was a problem hiding this comment.
이 부분은 제 기억상 코드를 중첩해서 작성해뒀던걸로 기억하고 있는데, 그걸 정리하려고 이렇게 바꾸신걸까요?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
insight/admin/user_weekly_trend_admin.py (1)
135-151: 코드 중복 제거를 위한 리팩토링 권장사용자 컨텍스트 딕셔너리가 두 번 생성되고 있습니다(lines 135-137, 148-151). 헬퍼 메서드로 추출하여 일관성을 유지하고 중복을 줄일 수 있습니다.
다음 diff를 적용하여 리팩토링할 수 있습니다:
+ def _get_user_context(self, obj: UserWeeklyTrend, include_email: bool = False): + """사용자 컨텍스트 딕셔너리 생성 헬퍼""" + context = { + "username": obj.user.username if obj.user else "N/A" + } + if include_email: + context["email"] = obj.user.email if obj.user else "N/A" + return context + @admin.display(description="뉴스레터 템플릿") def render_full_preview(self, obj: UserWeeklyTrend): if not obj.insight: return "No insight data to preview." try: # 공통 주간 트렌드 데이터 조회 weekly_trend = WeeklyTrend.objects.filter( week_start_date=obj.week_start_date, week_end_date=obj.week_end_date, ).first() if not weekly_trend or not weekly_trend.insight: weekly_trend_html = "<p><strong>경고:</strong> 해당 주차의 공통 WeeklyTrend를 찾을 수 없거나 데이터가 없습니다.</p>" else: weekly_trend_insight = from_dict( WeeklyTrendInsight, weekly_trend.insight ) weekly_trend_html = render_to_string( "insights/weekly_trend.html", {"insight": weekly_trend_insight.to_dict()}, ) # 사용자 주간 트렌드 렌더링 user_weekly_insight = from_dict( WeeklyUserTrendInsight, obj.insight ) user_weekly_trend_html = render_to_string( "insights/user_weekly_trend.html", { - "user": { - "username": obj.user.username if obj.user else "N/A" - }, + "user": self._get_user_context(obj), "insight": user_weekly_insight.to_dict(), }, ) # 최종 뉴스레터 렌더링 final_html = render_to_string( "insights/index.html", { "s_date": obj.week_start_date, "e_date": obj.week_end_date, - "user": { - "username": obj.user.username if obj.user else "N/A", - "email": obj.user.email if obj.user else "N/A", - }, + "user": self._get_user_context(obj, include_email=True), "is_expired_token_user": False, "weekly_trend_html": weekly_trend_html, "user_weekly_trend_html": user_weekly_trend_html, }, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
insight/admin/user_weekly_trend_admin.py(1 hunks)templates/insights/index.html(2 hunks)templates/insights/user_weekly_trend.html(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- templates/insights/index.html
- templates/insights/user_weekly_trend.html
🧰 Additional context used
🧬 Code graph analysis (1)
insight/admin/user_weekly_trend_admin.py (1)
insight/tests/conftest.py (1)
user(40-51)
🔇 Additional comments (1)
insight/admin/user_weekly_trend_admin.py (1)
148-151: 수신 거부 기능을 위한 이메일 컨텍스트 추가 승인뉴스레터 템플릿에서 수신 거부 링크를 생성하기 위해 사용자 이메일을 컨텍스트에 추가한 것은 적절합니다.
"N/A"기본값 처리도 누락된 데이터에 대한 안전한 폴백을 제공합니다.
🔥 변경 사항
주간 뉴스레터 수신 거부 기능을 추가하였습니다.
newsletter_subscribedflag 컬럼 추가수신 거부버튼 추가 및 API와 연결, 이를 위한 메서드 파라미터 수정 (최종 렌더링에서도user를 받도록)🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
수신 거부 버튼 추가
📌 체크리스트
Summary by CodeRabbit
New Features
Chores