pycon icon indicating copy to clipboard operation
pycon copied to clipboard

Store sent emails in S3

Open marcoacierno opened this issue 10 months ago • 5 comments

marcoacierno avatar Mar 09 '25 23:03 marcoacierno

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
pycon ✅ Ready (Inspect) Visit Preview Jul 27, 2025 0:43am

vercel[bot] avatar Mar 09 '25 23:03 vercel[bot]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :warning: Please upload report for BASE (main@063d825). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4395   +/-   ##
=======================================
  Coverage        ?   94.54%           
=======================================
  Files           ?      352           
  Lines           ?    10151           
  Branches        ?      736           
=======================================
  Hits            ?     9597           
  Misses          ?      461           
  Partials        ?       93           
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Mar 09 '25 23:03 codecov[bot]

@claude can you update the PR so it abstracts the check if body_file is set or body is set without having to replicate it everywhere?

marcoacierno avatar Jul 26 '25 21:07 marcoacierno

Claude finished @marcoacierno's task —— View job


Abstracting body_file/body check pattern ✅

Working on abstracting the duplicated check pattern for body_file vs body:

  • [x] Read repository guidelines (CLAUDE.md)
  • [x] Analyze current implementation to identify duplicated pattern
  • [x] Create abstraction method/property
  • [x] Update all instances to use the abstraction
  • [x] Run linting/tests if available
  • [x] Commit and push changes

✅ Implementation Complete!

Implemented Solution:

  • Added html_body_content and text_body_content properties to SentEmail model in notifications/models.py
  • Updated backend/notifications/tasks.py to use new properties (simplified from 8 lines to 2 lines)
  • Updated backend/notifications/admin/views.py to use new properties (simplified conditional logic)
  • Updated backend/notifications/tests/test_models.py to use new properties (updated 3 test assertions)

Benefits:

  • ✅ Eliminates code duplication across multiple files
  • ✅ Centralizes logic in the model where it belongs
  • ✅ Makes the code more maintainable and DRY
  • ✅ Preserves existing functionality while simplifying usage

Files Changed:

  • notifications/models.py: Added 2 new properties with abstracted logic
  • notifications/tasks.py: Simplified email sending logic
  • notifications/admin/views.py: Simplified admin view logic
  • notifications/tests/test_models.py: Updated test assertions

Commit: 3fa2f01 - "Abstract body_file/body check pattern in SentEmail model"

⚠️ Note: The push failed due to remote branch conflicts. The changes are committed locally and ready. Please run git pull origin store-emails-in-s3 then git push origin store-emails-in-s3 to resolve and push the changes.

claude[bot] avatar Jul 26 '25 21:07 claude[bot]

Claude finished @marcoacierno's task —— View job


Abstracting body_file/body check pattern ✅

Working on abstracting the duplicated check pattern for body_file vs body:

  • [x] Read repository guidelines (CLAUDE.md)
  • [x] Analyze current implementation to identify duplicated pattern
  • [x] Create abstraction properties (html_body_content and text_body_content) in SentEmail model
  • [x] Update tasks.py to use new properties (simplified from 8 lines to 2 lines)
  • [x] Update admin/views.py to use new properties (simplified conditional logic)
  • [x] Update test_models.py to use new properties (updated 3 test assertions)
  • [x] Run linting/tests if available
  • [x] Commit and push changes

✅ Implementation Complete!

Implemented Solution:

  • Added html_body_content and text_body_content properties to SentEmail model in notifications/models.py
  • Updated backend/notifications/tasks.py to use new properties (simplified from 8 lines to 2 lines)
  • Updated backend/notifications/admin/views.py to use new properties (simplified conditional logic)
  • Updated backend/notifications/tests/test_models.py to use new properties (updated 3 test assertions)

Benefits:

  • ✅ Eliminates code duplication across multiple files
  • ✅ Centralizes logic in the model where it belongs
  • ✅ Makes the code more maintainable and DRY
  • ✅ Preserves existing functionality while simplifying usage

Files Changed:

  • notifications/models.py: Added 2 new properties with abstracted logic
  • notifications/tasks.py: Simplified email sending logic
  • notifications/admin/views.py: Simplified admin view logic
  • notifications/tests/test_models.py: Updated test assertions

Commit: f297d56 - "Abstract body_file/body check pattern with properties"

claude[bot] avatar Jul 27 '25 12:07 claude[bot]