[25.08.06 / TASK-238] Refactor - 뉴스레터 메일 템플릿 개선 #40
Conversation
Walkthrough이번 변경에서는 Velog 대시보드 주간 리포트 관련 HTML 템플릿 3종이 전면적으로 리디자인 및 리팩토링되었습니다. 전반적으로 색상, 폰트, 레이아웃, 조건부 렌더링 로직, 한글화 등이 적용되었으며, 토큰 만료 안내 및 사용자별 트렌드 섹션의 조건 처리도 개선되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EmailTemplate
participant DataEngine
User->EmailTemplate: 이메일 열람
EmailTemplate->DataEngine: 주간 트렌드/사용자 데이터 조회
DataEngine-->>EmailTemplate: 데이터 반환 (트렌드, 사용자, 토큰 상태 등)
alt 토큰 만료
EmailTemplate->User: 토큰 만료 안내 및 재로그인 링크 표시
else 토큰 유효
EmailTemplate->User: 주간 트렌드 및 사용자별 리포트 표시
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
templates/insights/index.html (1)
8-20:@import웹폰트는 다수 메일 클라이언트에서 차단됩니다Gmail·Outlook Web 등 주요 클라이언트는
<style>내@import를 무시하거나 CSS 자체를 제거할 수 있습니다.
– 브랜드 폰트 유지가 중요하다면<link rel="stylesheet">로 교체하거나,
– 최소한 시스템 폰트("Apple SD Gothic Neo", "맑은 고딕", sans-serif)를 첫 번째 폴백으로 지정해 주세요.templates/insights/weekly_trend.html (2)
67-74: 외부 링크에rel="noopener noreferrer"추가 권장메일에서 열리는 새 탭은 JS 접근 위험은 낮지만, 일관된 보안을 위해
target="_blank"사용 시엔 항상rel="noopener noreferrer"를 함께 넣어주세요.- <a - href="https://velog.io/@{{item.username}}/{{item.slug}}?utm_source=velog_dashboard_email&utm_medium=weekly_analysis" - target="_blank" - style="color: #000000; text-decoration: none" - > + <a + href="https://velog.io/@{{item.username}}/{{item.slug}}?utm_source=velog_dashboard_email&utm_medium=weekly_analysis" + target="_blank" + rel="noopener noreferrer" + style="color: #000000; text-decoration: none" + >
1-10: 상위<div>에 이미color지정이 있으므로 중첩 스타일은 중복입니다헤더·본문 곳곳에 반복적으로
color: #000000이 선언되어 있는데, 상위 컨테이너에서 기본값이 같다면 삭제해도 결과는 동일하며 템플릿 용량이 줄어듭니다.templates/insights/user_weekly_trend.html (1)
186-193:target="_blank"링크 동일 보안 보강 필요weekly_trend.html 과 동일하게
rel="noopener noreferrer"속성을 추가해 주세요.- <a - href="https://velog.io/@{{item.username}}/{{item.slug}}?utm_source=velog_dashboard_email&utm_medium=weekly_analysis" - target="_blank" - style="color: #000000; text-decoration: none" - > + <a + href="https://velog.io/@{{item.username}}/{{item.slug}}?utm_source=velog_dashboard_email&utm_medium=weekly_analysis" + target="_blank" + rel="noopener noreferrer" + style="color: #000000; text-decoration: none" + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
templates/insights/index.html(1 hunks)templates/insights/user_weekly_trend.html(1 hunks)templates/insights/weekly_trend.html(1 hunks)
🔇 Additional comments (1)
templates/insights/user_weekly_trend.html (1)
64-99: 메일 TIP URL에utm_campaign파라미터 누락 확인다른 CTA 링크에는
utm_campaign이 포함되어 있으나, 리마인더 구간의 리더보드 링크엔 빠져 있습니다.
캠페인별 효과 측정이 필요하다면 동일한 UTM 파라미터 세트를 유지해 주세요.
There was a problem hiding this comment.
고생하셨습니다! 👍 🔥 의견 몇 개 드립니다.
좋았던 점
- CTA 버튼이랑 푸터가 확실히 전보다 예뻐졌네요!
- 각 글별 키워드를 따로 두지 않고 작성자와 같은 레벨에 두는게 깔끔해보이네요!
아쉬웠던 점
- 글꼴이 기본적으로 굵어서 가독성이 떨어지는 것 같아요.
- 키워드를 작성자와 같은 레벨에 두는 건 좋은데, 눈에 잘 안 띄는게 아쉬워요! 그리고 이모지가 하나엔 있고 하나엔 없네요.
일단 저는 글꼴은 꼭 바뀌었으면 좋겠어요...🥲🙏
그리고 지금 있는 테스트는 직접 렌더링하고 템플릿 체크하는 테스트여서, 수정하시면서 테스트도 같이 돌려보면 좋을 것 같네요!
templates/insights/index.html
Outdated
| <style> | ||
| @import url("https://fonts.googleapis.com/css2?family=Noto+Sans+KR:wght@300;400;500;700;900&display=swap"); | ||
| * { | ||
| margin: 0; | ||
| padding: 0; | ||
| box-sizing: border-box; | ||
| } | ||
| body { | ||
| font-family: "Noto Sans KR", sans-serif; | ||
| background-color: #ebebeb; | ||
| margin: 0; | ||
| padding: 0; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
메일로 보내는 HTML에 스타일은 보통 인라인으로만 넣는다고 합니다.
메일 클라이언트에서 인식을 못하는 경우가 종종 있어서라는데... 현재에 얼마나 경우가 많을지는 모르겠지만, 적용해보는게 어떨까요?
There was a problem hiding this comment.
너무 고생 많으셨습니다! 👏
항상 예쁘게 레이아웃 구성해주셔서 정말 감사드려요 🙇♀️
개인적인 의견이긴 하지만, 아래 두 부분은 살짝 아쉬운 점이 있어 공유드려요:)
좋았던 점
- 전체적으로 디자인을 통일화 해서 개선해주신 부분이 좋았습니다!
- 트렌딩 글의 키워드가 제목 바로 하단에 위치하도록 수정해주신 부분이 좋았습니다!
아쉬운 점
- 글꼴이 다소 굵게 느껴져서, 가독성이 조금 떨어지는 느낌이 있었어요 🥲 조금 더 얇은 서체로 테스트해보는 것도 괜찮을 것 같아요!
- 상단의 'Velog Dashboard' 색상이 기존 연두색일 때 저희 서비스 느낌이 더 잘 살아났던 것 같아요. 기준님은 어떤 색상이 더 눈에 잘 들어오셨는지 궁금합니다!
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
insight/tests/conftest.py (1)
163-166: 플레인 텍스트 본문 가독성 보강 제안이메일 클라이언트에 따라 text_body가 표시될 수 있으니 줄바꿈/구분선 등을 넣어 가독성을 높이는 것을 권장합니다. HTML과 주요 메시지가 분리되지 않도록 핵심 CTA/제목도 포함해 주세요.
예:
text_body = ( "벨로그 대시보드 주간 뉴스레터 #1\n" "Weekly Report Test content\n\n" "대시보드 보러가기: https://velog-dashboard.kro.kr/\n" )insight/tests/tasks/test_weekly_newsletter_batch.py (1)
199-201: 제목 검증을 더 명확한 패턴으로 강화하세요현재는 부분 문자열만 검사합니다. 가능하면 newsletter_id까지 포함한 포맷을 검증하도록 사전 상태 세팅 후 정합성을 체크하세요.
예:
# 사전 준비 newsletter_batch.weekly_info["newsletter_id"] = 42 # 검증 subject = newsletters[0].email_message.subject assert subject.startswith("벨로그 대시보드 주간 뉴스레터 #42")insight/tests/tasks/test_weekly_newsletter_template.py (1)
150-160: 조건부 기대값 분기는 OK, 하지만 테스트를 분리하는 편이 명확합니다days_ago 유무 케이스를 하나의 테스트 내 조건문으로 분기하기보다, 파라미터화/별도 테스트로 분리하면 의도가 선명해지고 실패 시 원인 파악이 쉬워집니다.
templates/insights/index.html (3)
68-77:user.username컨텍스트 미전달로 사용자명 헤더가 항상 일반 문구로 출력됩니다
_get_newsletter_html컨텍스트에는user가 없으므로{% if user.username %}가 항상 거짓으로 평가되어 “활동 리포트”만 노출됩니다. 만료 안내에 사용자명을 노출하려면 username을 별도 키로 넘기거나user를 전달하세요.예(파이썬 쪽 보완):
# insight/tasks/weekly_newsletter_batch.py 내 _get_newsletter_html 서명/호출부에 username 전달 def _get_newsletter_html(..., username: str | None = None) -> str: ctx = to_dict(NewsletterContext(...)) if username: ctx["username"] = username return render_to_string("insights/index.html", ctx) # 호출부(_build_newsletters) html_body = self._get_newsletter_html( is_expired_token_user=is_expired_token_user, weekly_trend_html=weekly_trend_html, user_weekly_trend_html=user_weekly_trend_html, username=user.get("username"), )그리고 템플릿에서는
user.username대신username사용:{% if username %} {{ username }}님의 활동 리포트 {% else %} 활동 리포트 {% endif %}
46-55: CTA 링크 속성 보강 제안이메일 클라이언트 호환에 큰 영향은 없지만, 보안/사용성 측면에서
target="_blank"와rel="noopener noreferrer"추가를 권장합니다.- <a + <a target="_blank" rel="noopener noreferrer"
<title>은 영문, 본문은 한글입니다. 미리보기/접근성 측면에서 한글 제목(예: “벨로그 대시보드 주간 뉴스레터”)로 맞추는 것을 권장합니다.
6-6: 문서 타이틀과 본문 언어 일관성
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
insight/tasks/weekly_newsletter_batch.py(1 hunks)insight/tests/conftest.py(1 hunks)insight/tests/tasks/test_weekly_newsletter_batch.py(1 hunks)insight/tests/tasks/test_weekly_newsletter_template.py(5 hunks)templates/insights/index.html(1 hunks)templates/insights/insight_preview.html(3 hunks)templates/insights/user_weekly_trend.html(1 hunks)templates/insights/weekly_trend.html(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- templates/insights/insight_preview.html
🚧 Files skipped from review as they are similar to previous changes (2)
- templates/insights/weekly_trend.html
- templates/insights/user_weekly_trend.html
🔇 Additional comments (4)
insight/tests/tasks/test_weekly_newsletter_template.py (3)
65-67: 업데이트된 헤더 문구 검증 LGTM새 템플릿 카피에 맞는 기대값으로 잘 갱신되었습니다.
128-133: 표기 일관성(‘이번 주’ vs ‘이번주’) 확인 요청본 테스트는 “이번주에 작성한 글”, 위쪽 테스트는 “이번 주의 트렌딩 글”을 사용합니다. 의도된 차이가 아니라면 템플릿/테스트에서 띄어쓰기를 일관화해 주세요.
218-224: 만료 토큰 안내 문구 검증 LGTM새 문구와 조건부 렌더링 의도에 부합합니다.
templates/insights/index.html (1)
102-154: 푸터 SVG 호환성 점검 필요여러 메일 클라이언트(특히 Gmail/Outlook)에서 인라인 SVG가 표시되지 않을 수 있습니다. PNG 대체(또는 서버 호스팅된 이미지)로 교체/백업을 고려하세요.
테스트 가이드:
- Gmail Web/모바일, Outlook(Windows/웹), iOS Mail에서 수신 확인
- 보이지 않을 경우
<img alt="Velog Dashboard" src="...png">로 대체
| {% if weekly_trend_html %} | ||
| {{weekly_trend_html}} | ||
| {% endif %} | ||
|
|
||
| {% if not is_expired_token_user and user_weekly_trend_html %} | ||
| {{user_weekly_trend_html}} | ||
| {% endif %} |
There was a problem hiding this comment.
크리티컬: 사전 렌더된 HTML을 안전하게 삽입하기 위해 |safe 필터가 필요합니다
현재 {{weekly_trend_html}}, {{user_weekly_trend_html}}는 이스케이프되어 태그가 그대로 노출될 수 있습니다. 이메일 레이아웃이 붕괴하고 text_body 추출에도 악영향을 줍니다. 반드시 |safe를 적용하세요.
다음 변경을 적용해 주세요:
- {% if weekly_trend_html %}
- {{weekly_trend_html}}
+ {% if weekly_trend_html %}
+ {{weekly_trend_html|safe}}
{% endif %}
- {% if not is_expired_token_user and user_weekly_trend_html %}
- {{user_weekly_trend_html}}
+ {% if not is_expired_token_user and user_weekly_trend_html %}
+ {{user_weekly_trend_html|safe}}
{% endif %} 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {% if weekly_trend_html %} | |
| {{weekly_trend_html}} | |
| {% endif %} | |
| {% if not is_expired_token_user and user_weekly_trend_html %} | |
| {{user_weekly_trend_html}} | |
| {% endif %} | |
| {% if weekly_trend_html %} | |
| {{ weekly_trend_html|safe }} | |
| {% endif %} | |
| {% if not is_expired_token_user and user_weekly_trend_html %} | |
| {{ user_weekly_trend_html|safe }} | |
| {% endif %} |
🤖 Prompt for AI Agents
In templates/insights/index.html around lines 60 to 66, the variables
weekly_trend_html and user_weekly_trend_html are rendered without the |safe
filter, causing their HTML content to be escaped and displayed as plain text. To
fix this, add the |safe filter to both variables in the template expressions to
ensure the pre-rendered HTML is inserted safely and correctly without escaping.
There was a problem hiding this comment.
(퀵하게 닫기위해 리뷰잉 형식을 단일화 했어요!)
- 밀도있게 디자인 톤앤 매너, 기본 폰트까지 전반적으로 너무 고생많으셨습니다. 사실 in/out data 들때문에 실제 적용하시는데 읽는 비용이 많으셨을 것 같아요! 너무 고생하셨어요~
safe는 사실 무조건 넣긴 합니다만, 이 경우는 우리가 직접 잘 만들어진 html 으로 우리가 직접 원하는 위치에 넣기 때문에 필수는 아닌 것으로 보입니다~- 테스트 코드에서 언급한 "문자열만을 기반으로 한 테스팅" 은 지향하는게 맞으나, 이는 기준님 선택에 맡기도록 하겠습니다!
- 앞으로 메일링 보내보면서 다양한 output, 출력 형식에 대해 더블 체크하며 점진적 업데이트를 하면 좋을 것 같아요!
🔥 변경 사항
전체적인 뉴스레터 메일 템플릿을 수정하였습니다!
우선은 현재 나온 디자인으로 진행했는데, 여기서 더 개선할 곳이 있을까요?
🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
📌 체크리스트
Summary by CodeRabbit
신규 디자인
스타일 개선
조건부 안내 강화
테스트 및 검증 강화