-
Notifications
You must be signed in to change notification settings - Fork 0
[25.07.09 / TASK-215] Feature - 뉴스레터 배치 이슈 대응 & 리마인더 추가 #34
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
Conversation
""" Walkthrough이 변경사항은 뉴스레터 배치 프로세스와 관련된 환경 변수, 데이터 스키마, 템플릿 렌더링 로직을 확장 및 리팩토링합니다. AWS 및 이메일 발송 환경 변수 설정이 추가되었고, 사용자 주간 통계 및 리마인더 데이터 처리가 개선되었으며, 관련 템플릿에서 조건부 렌더링과 스타일이 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WeeklyNewsletterBatch
participant DB
participant Template
User->>WeeklyNewsletterBatch: 뉴스레터 배치 실행
WeeklyNewsletterBatch->>DB: 사용자 주간 통계 및 리마인더 데이터 조회
WeeklyNewsletterBatch->>Template: 개인화 뉴스레터 HTML 렌더링 (user, weekly_trend_html, user_weekly_trend_html, 만료 토큰 여부)
Template-->>WeeklyNewsletterBatch: 렌더링된 HTML 반환
WeeklyNewsletterBatch-->>User: 뉴스레터 발송
Possibly related PRs
Suggested reviewers
Poem
""" 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
insight/tasks/weekly_newsletter_batch.py (2)
170-207
: 대용량 데이터 처리 시 성능을 고려하세요.두 개의 별도 쿼리로 인해 성능이 저하될 수 있습니다. 사용자가 많을 경우 쿼리 최적화를 고려하세요.
다음과 같은 최적화 방안을 고려해보세요:
- 두 날짜의 데이터를 한 번의 쿼리로 가져오기
- 데이터베이스 인덱스 추가 (user_id, date)
- 쿼리 결과 캐싱
238-241
: 에러 처리 시 부분적 실패를 고려하세요.현재 에러 발생 시 모든 사용자의 통계가 누락됩니다. 개별 사용자 단위로 에러를 처리하는 것이 더 안전합니다.
except Exception as e: # 개인 통계 조회 실패 시에도 계속 진행 logger.error(f"Failed to get users weekly stats: {e}") - return {}, set() + # 부분적으로 성공한 데이터라도 반환 + return users_weekly_stats_dict, expired_user_ids
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backoffice/settings/base.py
(1 hunks)insight/schemas.py
(2 hunks)insight/tasks/weekly_newsletter_batch.py
(11 hunks)templates/insights/index.html
(1 hunks)templates/insights/user_weekly_trend.html
(2 hunks)templates/insights/weekly_trend.html
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
backoffice/settings/base.py (1)
Learnt from: Nuung
PR: Check-Data-Out/velog-dashboard-v2-back-office#24
File: backoffice/settings/base.py:19-22
Timestamp: 2025-03-25T15:31:20.445Z
Learning: Sentry SDK 설정 시 `environment` 파라미터를 사용하면 로컬 개발 환경과 운영 환경에서 발생한 에러를 구분할 수 있어 디버깅이 더 효율적입니다.
insight/tasks/weekly_newsletter_batch.py (1)
Learnt from: ooheunda
PR: Check-Data-Out/velog-dashboard-v2-back-office#30
File: modules/mail/schemas.py:21-25
Timestamp: 2025-06-08T15:02:11.518Z
Learning: AWSSESCredentials dataclass에서 clear_credentials 메서드는 SESClient의 싱글톤 + lazy init 패턴과 충돌하여 실용적이지 않음. credentials가 reset_client() 후 재초기화에 필요하기 때문.
🧬 Code Graph Analysis (2)
insight/schemas.py (1)
insight/models.py (1)
WeeklyTrendInsight
(31-33)
insight/tasks/weekly_newsletter_batch.py (5)
posts/models.py (1)
PostDailyStatistics
(50-88)utils/utils.py (3)
get_local_now
(17-24)to_dict
(68-77)parse_json
(27-36)insight/tests/conftest.py (1)
user_weekly_trend
(117-138)insight/models.py (1)
UserWeeklyTrend
(72-112)insight/schemas.py (1)
UserWeeklyTrendContext
(25-31)
🔇 Additional comments (12)
backoffice/settings/base.py (1)
29-34
: AWS 및 이메일 설정이 올바르게 구성되었습니다.AWS SES 통합을 위한 설정이 적절하게 추가되었습니다. 기본 리전이 한국에 적합하고, 기본 이메일 주소도 no-reply 패턴을 따라 좋은 관례입니다.
templates/insights/index.html (1)
30-30
: 만료된 토큰 경고 메시지의 색상이 개선되었습니다.텍스트 색상이 더 어두운 색상으로 변경되어 가독성이 향상되었습니다.
templates/insights/weekly_trend.html (1)
2-2
: 조건부 렌더링 로직이 개선되었습니다.제목이
trending_summary
또는trend_analysis
중 하나가 존재할 때 표시되도록 변경되어 더 유연한 콘텐츠 표시가 가능합니다.insight/schemas.py (2)
13-13
:weekly_trend_html
필드가 필수로 변경되었습니다.이 변경사항이 의도된 것인지 확인해주세요. 만약 글로벌 트렌드가 항상 표시되어야 한다면 적절한 변경이지만, 데이터가 없는 경우를 고려해야 할 수도 있습니다.
27-31
: 리마인더 기능을 위한 스키마 변경이 잘 구성되었습니다.
user_weekly_stats
를 선택적으로 만들고reminder
필드를 추가한 것은 PR 목표와 일치합니다. 토큰 만료 사용자와 일반 사용자를 다르게 처리할 수 있도록 유연성이 제공됩니다.templates/insights/user_weekly_trend.html (3)
2-2
: 조건부 렌더링이 리마인더 기능을 포함하도록 확장되었습니다.
reminder
조건이 추가되어 리마인더가 있는 경우에도 활동 리포트 섹션이 표시되도록 개선되었습니다.
16-38
: 사용자 통계 블록의 스타일과 텍스트가 개선되었습니다."이번 주에는"에서 "저번 주에는"으로 변경된 것은 주간 통계의 의미상 더 정확합니다. 스타일링 개선(box-shadow, 여백 조정 등)도 UI를 향상시킵니다.
40-53
: 리마인더 기능이 적절하게 구현되었습니다.새로운 리마인더 블록이 잘 디자인되었고, 사용자에게 도움이 되는 정보(마지막 글 제목, 경과 일수, 리더보드 링크 등)를 제공합니다. 스타일링도 일관성 있게 적용되었습니다.
insight/tasks/weekly_newsletter_batch.py (4)
184-187
: 토큰 만료 감지 로직의 정확성을 검증하세요.오늘자 통계가 없다고 해서 항상 토큰이 만료된 것은 아닙니다. 사용자가 단순히 오늘 글을 작성하지 않았을 수도 있습니다.
토큰 만료 여부를 더 정확하게 판단할 수 있는 방법이 있는지 확인해주세요. 예를 들어:
- User 모델에 토큰 만료 필드가 있는지
- 토큰 유효성을 직접 검증하는 API가 있는지
17-17
: 환경 변수 처리가 Django settings로 일관되게 변경되었습니다.
environ.Env()
에서django.conf.settings
로의 변경이 파일 전체에 일관되게 적용되었습니다.Also applies to: 411-411, 567-568, 637-639
322-336
: 리마인더 기능이 PR 목표에 따라 올바르게 구현되었습니다.토큰이 만료되지 않은 사용자 중 주간 트렌드가 없는 경우에만 리마인더를 제공하는 로직이 적절히 구현되었습니다.
Also applies to: 375-381, 389-394
292-294
: 타임존 일관성 검증 완료get_local_now()가 Django의 timezone.localtime을 사용해 timezone-aware datetime을 반환하며,
released_at
도 USE_TZ=True 환경에서 ORM이 timezone-aware datetime으로 로드하기 때문에 두 값 간 뺄셈으로 올바른 일수 차이가 계산됩니다. 추가 조치가 필요 없습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋았던 점
- 전체적으로 디버깅을 위한 로깅, 계층형, 함수로 분리해서 최대한 클린하게 만드시려고 한게 이해가 되어서 너무 좋았습니다!
아쉬운 점
- ORM 쿼리의 최적화 (n+1), 디테일한 부분에서의 약간 코멘트가 있습니다! 확인해주세요 :)
aws_credentials = AWSSESCredentials( | ||
aws_access_key_id=env("AWS_ACCESS_KEY_ID"), | ||
aws_secret_access_key=env("AWS_SECRET_ACCESS_KEY"), | ||
aws_region_name=env("AWS_REGION"), | ||
aws_access_key_id=settings.AWS_ACCESS_KEY_ID, | ||
aws_secret_access_key=settings.AWS_SECRET_ACCESS_KEY, | ||
aws_region_name=settings.AWS_REGION, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bbb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 어떤 목적으로 남기신 코멘트일까요..? @Nuung
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅋㅋㅋㅋ 참고로 bb = 👍 이겁니다! 환경변수를 런타임에서 중복해서 메모리에 올리는 이슈가 있었는데 이게 해결되었어요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 잘 읽었습니다!
현우님이 리뷰를 잘 남겨주셔서 제가 별도로 확인한 이슈는 크게 없습니다.
좋았던 점
- 이전에 있었던 뉴스레터 관련 문제 (모든 글을 이번 주에 작성한 것으로 인식하는 문제) 해결을 위해 넣으신 것 같은데, 괜찮은 방식인 것 같아 좋네요.
- env 형태 변경과 같은 사소한 수정들에 눈에 띕니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 잘 읽었습니다 고생하셨습니다!!
발생했던 이슈 모두 꼼꼼히 적용해주신 것 같습니다!
좋았던 점
- None 처리를 세심하게 명시해주신 부분이 좋았습니다!
- 뉴스레터 항상 예쁘게 개선해주시는 부분이 좋은 것 같습니다!👍
- 플로우차트 정말 꼼꼼하게 그려주신 것 같아요! 고민하신 흔적이 보이네요 저도 배워갑니다:)
PostDailyStatistics로부터의 3중 조인 -> Post로부터의 2중 조인
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
insight/tasks/weekly_newsletter_batch.py (1)
164-242
: 주간 통계 계산 로직 대폭 개선기존 누적 방식에서 차이값 계산 방식으로 변경한 것이 정확한 주간 변동량 계산을 위한 올바른 접근입니다. 토큰 만료 사용자 감지 기능도 잘 통합되었습니다.
다음 최적화를 고려해보세요:
# 성능 최적화를 위해 select_related나 prefetch_related 사용 고려 stats = ( Post.objects.filter( user_id__in=user_ids, daily_statistics__date__in=[ self.today, self.before_a_week, ], is_active=True, ) + .select_related('daily_statistics') .values("user_id", "daily_statistics__date") .annotate( posts=Count("id", distinct=True), views=Sum("daily_statistics__daily_view_count"), likes=Sum("daily_statistics__daily_like_count"), ) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
insight/tasks/weekly_newsletter_batch.py
(14 hunks)
🧰 Additional context used
🧠 Learnings (1)
insight/tasks/weekly_newsletter_batch.py (1)
Learnt from: ooheunda
PR: Check-Data-Out/velog-dashboard-v2-back-office#30
File: modules/mail/schemas.py:21-25
Timestamp: 2025-06-08T15:02:11.518Z
Learning: AWSSESCredentials dataclass에서 clear_credentials 메서드는 SESClient의 싱글톤 + lazy init 패턴과 충돌하여 실용적이지 않음. credentials가 reset_client() 후 재초기화에 필요하기 때문.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-ci
🔇 Additional comments (7)
insight/tasks/weekly_newsletter_batch.py (7)
16-16
: Django settings 사용으로 환경변수 접근 방식 개선
environ.Env()
에서 Djangosettings
사용으로 변경한 것이 좋습니다. 이는 Django 프로젝트에서 일관성 있는 설정 관리를 위한 모범 사례입니다.Also applies to: 31-31, 34-34
69-70
: 날짜 초기화 로직 개선
get_local_date()
사용으로 일관된 로컬 날짜 처리가 가능해졌습니다. 이전 timezone 이슈를 해결하는 좋은 개선사항입니다.
309-363
: 렌더링 로직 분리가 코드 구조 개선에 기여기존 단일 메서드에서
_get_user_weekly_trend_html
과_get_newsletter_html
로 분리한 것이 책임 분리 원칙에 부합하고 가독성을 향상시켰습니다.
377-420
: 배치 빌드 로직의 토큰 만료 처리 개선토큰 만료 사용자와 리마인더 기능을 통합한 로직이 잘 구현되었습니다. 조건부 렌더링으로 성능도 고려되었습니다.
428-428
: Django settings 활용 일관성 확보
DEFAULT_FROM_EMAIL
,DEBUG
등 Django settings 직접 사용으로 환경변수 접근 방식이 일관되게 개선되었습니다.Also applies to: 552-553, 585-585
655-658
: AWS 자격증명 설정 방식 개선Environment variables에서 Django settings로 변경하여 설정 관리가 일관되게 개선되었습니다. 과거 학습 내용과 일치하는 AWSSESCredentials 사용법입니다.
275-307
: 리마인더 기능 추가 잘 구현됨새로운 리마인더 기능이 잘 구현되었지만, 성능 최적화를 위한 개선이 필요합니다.
# N+1 쿼리 방지를 위한 최적화 reminders = ( Post.objects.filter( user_id__in=user_ids, is_active=True, ) + .select_related('user') .values("user_id", "title", "released_at") .order_by("user_id", "-released_at") .distinct("user_id") )또한
get_local_now()
호출을 루프 밖으로 이동하는 것이 좋습니다:+ now = get_local_now() users_reminder_dict = { reminder["user_id"]: { "title": reminder["title"], - "days_ago": (get_local_now() - reminder["released_at"]).days, + "days_ago": (now - reminder["released_at"]).days, } for reminder in reminders }Likely an incorrect or invalid review comment.
is_expired_token_user = ( | ||
user["id"] in expired_token_user_ids | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이게 사실 for loop 랑 같아서요
DP 는 어떠실까요? 메모이제이션이 넉넉하다면,
expired_token_user_ids[유저ID의 index] = 0 또는 1 / 또는 boolean
is_expired_token_user = bool(expired_token_user_ids[user["id"]])
제 과정이 정답이 아니라서요, 메모이제이션 vs 반복 for loop 중 고민을 해주시면 더 좋을 것 같아요! 아니면 hash 로 접근하는 것도 좋구요. -> 마스킹
좋은 알고리즘 시간..!
모든 데이터 세팅은 분석 배치에서 실행됨 이에 따른 주간 통계 / 리마인더 함수 삭제 및 관련 스키마, 로직 수정
🔥 변경 사항
주간 통계 이슈 픽스일주일 간의 누적값 전부 Sum -> 오늘 통계/일주일 전 통계 누적값을 각각 구한뒤 차이 계산오늘자 통계를 확인해 판단했던 토큰 만료 확인 메서드(_get_expired_token_user_ids
)를 주간 통계 함수(_get_users_weekly_stats_chunk
)로 통합글 미작성 유저에 대한 리마인더 추가개인 트렌드
/개인 통계 요약
/리마인더
셋 중 하나만 있어도 개인 리포트 표시토큰이 만료된 유저이면, 셋 다 있어도 skip됩니다! (아래 차트 참고)25.07.13 회의 후 수정된 내용
유저별 포함 내용 차트
🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
(*위 통계 요약 박스와는 무관)
user_weekly_trend 데이터가 없는 사용자인 경우 위 리마인더 박스가 표시됩니다!
📌 체크리스트
Summary by CodeRabbit
Summary by CodeRabbit
신규 기능
버그 수정
스타일
기타