Skip to content

Conversation

marcoacierno
Copy link
Member

No description provided.

Copy link

vercel bot commented Mar 9, 2025

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

Copy link

codecov bot commented Mar 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ 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           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@marcoacierno
Copy link
Member Author

@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?

Copy link
Contributor

claude bot commented Jul 26, 2025

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:

  • Read repository guidelines (CLAUDE.md)
  • Analyze current implementation to identify duplicated pattern
  • Create abstraction method/property
  • Update all instances to use the abstraction
  • Run linting/tests if available
  • 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.

Copy link
Contributor

claude bot commented Jul 27, 2025

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:

  • Read repository guidelines (CLAUDE.md)
  • Analyze current implementation to identify duplicated pattern
  • Create abstraction properties (html_body_content and text_body_content) in SentEmail model
  • Update tasks.py to use new properties (simplified from 8 lines to 2 lines)
  • Update admin/views.py to use new properties (simplified conditional logic)
  • Update test_models.py to use new properties (updated 3 test assertions)
  • Run linting/tests if available
  • 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"

Add html_body_content and text_body_content properties to SentEmail model
to centralize the logic for checking body_file vs body fallback. This
eliminates code duplication across tasks.py, admin/views.py, and tests.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Co-Authored-By: Marco Acierno <[email protected]>
@marcoacierno marcoacierno marked this pull request as ready for review July 29, 2025 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant