-
Notifications
You must be signed in to change notification settings - Fork 3
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
Revert "feat: add beehiiv integration" #238
Conversation
WalkthroughThis pull request involves a comprehensive removal of Beehiiv integration from the project. The changes systematically eliminate all Beehiiv-related configurations, environment variables, tasks, models, and client implementations across multiple files. The modifications include deleting the Beehiiv client, removing subscriber synchronization tasks, dropping Beehiiv-specific model fields, and cleaning up configuration files related to Celery and environment settings. Changes
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 1
🔭 Outside diff range comments (1)
README.md (1)
Line range hint
1-211
: Document the removal of Beehiiv environment variables.The environment variables section should be updated to remove references to the deleted Beehiiv configuration. Consider adding a note about the removal of Beehiiv integration to help users understand this architectural change.
Add this section under the environment variables documentation:
```env DJANGO_SETTINGS_MODULE='core.config.local' #for Dev environment # Sqlite3 database config SECRET_KEY='paste db.sqlite3 key here' + # Note: Beehiiv integration was removed in PR #238. + # The following environment variables are no longer needed: + # - BEEHIIV_ENABLED + # - BEEHIIV_API_KEY + # - BEEHIIV_PUBLICATION_ID + # - BEEHIIV_SYNC_BATCH_SIZE + # Production-Only Env Database config
🧹 Nitpick comments (1)
server/apps/newsletter/tasks.py (1)
37-37
: Consider using proper logging instead of print statements.Replace print statements with proper logging to maintain better observability and consistency with production best practices.
-print(f"Error sending email to {subscriber.email}: {e}") +logger.error(f"Error sending email to {subscriber.email}: {e}") -print(f'Newsletter sent to {subscriber_count} subscribers') +logger.info(f'Newsletter sent to {subscriber_count} subscribers')Also applies to: 46-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
README.md
(1 hunks)server/.env.example
(1 hunks)server/apps/newsletter/beehiiv.py
(0 hunks)server/apps/newsletter/migrations/0012_subscriber_beehiiv_id_subscriber_sync_error_and_more.py
(0 hunks)server/apps/newsletter/models.py
(0 hunks)server/apps/newsletter/tasks.py
(3 hunks)server/apps/research/urls.py
(1 hunks)server/core/celery.py
(1 hunks)server/core/config/beehiiv.py
(0 hunks)server/core/config/celery_config.py
(0 hunks)
💤 Files with no reviewable changes (5)
- server/apps/newsletter/models.py
- server/apps/newsletter/migrations/0012_subscriber_beehiiv_id_subscriber_sync_error_and_more.py
- server/core/config/beehiiv.py
- server/apps/newsletter/beehiiv.py
- server/core/config/celery_config.py
🔇 Additional comments (7)
server/core/celery.py (2)
30-30
: LGTM! Schedule configuration is properly maintained.The remaining Celery beat tasks for article publishing and newsletter sending are correctly configured with their minute-based schedules.
Also applies to: 35-35
43-43
: LGTM! Debug task formatting is correct.The debug print statement is properly formatted.
server/apps/research/urls.py (3)
15-15
: LGTM! Comments improve code clarity.The added comments clearly explain the purpose of each URL pattern.
Also applies to: 17-18
21-24
: LGTM! Article retrieval endpoint is well-defined.The endpoint for retrieving articles by identifier (slug or UUID) is properly implemented with clear comments.
25-29
: LGTM! New endpoints enhance functionality.The new endpoints for category-based article retrieval, TinyMCE image upload, and RSS feed are well-structured and properly documented.
server/apps/newsletter/tasks.py (1)
19-24
: LGTM! Unsubscribe link generation is correct.The unsubscribe link is properly generated using the site URL and subscriber email.
server/.env.example (1)
32-32
: LGTM! Environment configuration is properly maintained.The removal of Beehiiv-related environment variables is consistent with the PR objective, and the remaining configuration is well-documented.
Let's verify that all Beehiiv references have been removed from the codebase:
@@ -213,7 +207,7 @@ Before running the application, ensure you have the following: | |||
python3 manage.py migrate # To migrate the changes in Database | |||
python3 manage.py runserver # To run the API server | |||
redis-server # to start redis-server | |||
celery -A core worker -Q default,beehiiv -l info # to run celery with beehiiv queue | |||
celery -A core worker -l info # to run celery |
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.
💡 Codebase verification
Incomplete removal of Beehiiv integration detected
The Celery worker command change in README.md is correct, but there are remaining Beehiiv references that need to be removed:
server/apps/newsletter/views.py
:- Remove the import
from .tasks import sync_subscriber_to_beehiiv
- Remove the task call
sync_subscriber_to_beehiiv.delay(subscriber.id)
- Remove the related log message
- Remove the import
🔗 Analysis chain
Verify Celery worker configuration.
The Celery worker command has been simplified by removing the -Q default,beehiiv
queue configuration, which aligns with the removal of Beehiiv integration.
Run this script to ensure no remaining Beehiiv-related Celery tasks exist:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining Beehiiv-related Celery tasks
# that might need the beehiiv queue.
# Search for any remaining Beehiiv-related task definitions
rg -i "beehiiv.*task"
# Search for any remaining queue configurations
rg -i "queue.*beehiiv"
Length of output: 48
Script:
#!/bin/bash
# Broader search for any Beehiiv references
echo "=== Searching for any Beehiiv references ==="
rg -i "beehiiv"
# Look for Celery configuration files and task definitions
echo -e "\n=== Checking Celery configurations ==="
fd -e py -e ini -e conf -e cfg | grep -i "celery"
# Search for queue configurations in Python files
echo -e "\n=== Checking queue configurations ==="
rg "queue.*=" "\.py$"
Length of output: 770
Script:
#!/bin/bash
# Check the content of newsletter views and tasks
echo "=== Newsletter Views Content ==="
cat server/apps/newsletter/views.py
echo -e "\n=== Celery Configuration Files ==="
echo "--- core/celery.py ---"
cat server/core/celery.py
echo -e "\n--- core/config/celery_config.py ---"
cat server/core/config/celery_config.py
Length of output: 3677
Reverts #235
Summary by CodeRabbit
Refactor
Documentation
Chores