Skip to content

Conversation

@mdkaifansari04
Copy link
Contributor

@mdkaifansari04 mdkaifansari04 commented Nov 12, 2025

Summary

FIxes: OWASP-BLT/Sizzle#1

This PR implements Issue OWASP-BLT/Sizzle#1 - converting the Sizzle daily check-in and time tracking feature into a standalone, pluggable Django app. This is part of the larger initiative (#4709) to modularize OWASP BLT into reusable components.[1][2]

Demo Video

Uploading 2025-11-12 18-13-12.mov…

What Changed

Core Separation

  • Created new sizzle/ Django app with complete isolation
  • Moved 3 models: DailyStatusReport, TimeLog, ReminderSettings
  • Moved 9 views from website/views/organization.py to sizzle/views.py
  • Moved all Sizzle templates to sizzle/templates/sizzle/
  • Moved static assets to sizzle/static/sizzle/
  • Moved 4 management commands with dynamic model loading

Pluggability Features

  • Settings-based configuration - follows Django's AUTH_USER_MODEL pattern[2][1]
  • Dynamic model loading using apps.get_model() for external dependencies[1]
  • Standalone base template - works out-of-the-box without BLT's base.html[2]
  • Optional dependencies - Slack integration is truly optional[2]
  • Zero breaking changes - BLT works exactly as before with sensible defaults

File Structure

sizzle/
├── __init__.py
├── apps.py                    # AppConfig with proper naming
├── models.py                  # DailyStatusReport, TimeLog, ReminderSettings
├── views.py                   # All sizzle views
├── urls.py                    # URL patterns with namespace
├── admin.py                   # Admin configurations
├── forms.py                   # ReminderSettingsForm
├── conf.py                    # Settings configuration (NEW)
├── utils/
├── tests.py                   # Under progress
├── management/
│   └── commands/
│       ├── slack_daily_timelogs.py
│       ├── send_daily_reminders.py
│       ├── daily_checkin_reminder.py
│       └── cron_send_reminders.py
├── templates/
│   └── sizzle/
│       ├── base.html          # Standalone base template (NEW)
│       ├── sizzle.html
│       ├── check_in.html
│       └── ...
├── static/
│   └── sizzle/
│       └── imgages/
├── migrations/
│   ├── 0001_initial.py
│   └── 0002_data_migration.py # Migrates existing data

Technical Implementation

1. Settings-Based Configuration (sizzle/conf.py)

Following Django best practices, external dependencies are configurable via settings:[1][2]

# Swappable model references (like AUTH_USER_MODEL)
SIZZLE_SLACK_INTEGRATION_MODEL = getattr(
    settings, 
    'SIZZLE_SLACK_INTEGRATION_MODEL',
    'website.SlackIntegration'  # Default for BLT
)

SIZZLE_ORGANIZATION_MODEL = getattr(
    settings,
    'SIZZLE_ORGANIZATION_MODEL', 
    'website.Organization'
)

# Feature toggles
SIZZLE_SLACK_ENABLED = getattr(settings, 'SIZZLE_SLACK_ENABLED', True)
SIZZLE_EMAIL_REMINDERS_ENABLED = getattr(settings, 'SIZZLE_EMAIL_REMINDERS_ENABLED', True)

Why this approach?

  • Standard Django pattern used by AUTH_USER_MODEL[1]
  • BLT works with zero configuration changes
  • Other projects can customize or disable features
  • Graceful degradation when dependencies missing

2. Dynamic Model Loading (sizzle/utils/model_loader.py)

External models are loaded dynamically to avoid hardcoded imports:[1]

def get_slack_integration_model():
    """Get SlackIntegration model from settings."""
    from sizzle.conf import SIZZLE_SLACK_INTEGRATION_MODEL
    
    if not SIZZLE_SLACK_INTEGRATION_MODEL:
        return None
    
    try:
        app_label, model_name = SIZZLE_SLACK_INTEGRATION_MODEL.split('.')
        return apps.get_model(app_label, model_name)
    except (ValueError, LookupError) as e:
        raise ImproperlyConfigured(f'Invalid model reference: {e}')

Benefits:

  • No hardcoded from website.models import SlackIntegration
  • Works with any project's models via settings
  • Proper error handling with clear messages[1]

3. Template Independence

Created sizzle/templates/sizzle/base.html as a minimal standalone base:[2]

<!DOCTYPE html>
<html>
<head>
    <title>{% block title %}Sizzle{% endblock %}</title>
    <link rel="stylesheet" href="{% static 'sizzle/css/sizzle.css' %}">
</head>
<body>
    {% block content %}{% endblock %}
</body>
</html>

All Sizzle templates now extend sizzle/base.html instead of base.html.[2]

Users can override by creating their_project/templates/sizzle/base.html to match their site design.[2]

4. Optional Slack Integration

Management commands check for Slack availability gracefully:

# sizzle/management/commands/slack_daily_timelogs.py
def handle(self, *args, **options):
    # Check if enabled
    if not SIZZLE_SLACK_ENABLED:
        self.stdout.write(self.style.WARNING('Slack disabled'))
        return
    
    # Check if slack-bolt installed
    try:
        from slack_bolt import App
    except ImportError:
        self.stdout.write(self.style.ERROR(
            'Install with: pip install slack-bolt'
        ))
        return
    
    # Load model dynamically
    SlackIntegration = get_slack_integration_model()
    if SlackIntegration is None:
        self.stdout.write(self.style.WARNING('No Slack model configured'))
        return
    
    # Proceed with Slack logic...

🗄️ Database Migration Strategy

Phase 1: Create New Tables

python manage.py makemigrations sizzle
python manage.py migrate sizzle

Creates:

  • sizzle_dailystatusreport
  • sizzle_timelog
  • sizzle_remindersettings

Phase 2: Data Migration

Created sizzle/migrations/0002_data_migration.py to copy existing data:

def copy_data_forward(apps, schema_editor):
    """Copy data from website app to sizzle app."""
    # Copy DailyStatusReport
    OldReport = apps.get_model('website', 'DailyStatusReport')
    NewReport = apps.get_model('sizzle', 'DailyStatusReport')
    
    for old in OldReport.objects.all():
        NewReport.objects.create(
            id=old.id,
            user_id=old.user_id,
            content=old.content,
            created_at=old.created_at,
            # ... copy all fields
        )
    
    # Copy TimeLog...
    # Copy ReminderSettings...

Safety: Data migration is reversible with a copy_data_backward function.

Phase 3: Cleanup (Future PR)

After verifying data integrity, old tables can be dropped.

🔗 Dependencies Handled

Required (Auto-installed)

  • Django >= 4.0
  • pytz >= 2023.3

Optional (User choice)

  • slack-bolt >= 1.18.0 (for Slack features)

External Models (Referenced, not included)

  • django.contrib.auth.models.User (standard Django)
  • website.Organization (configurable via settings)
  • website.SlackIntegration (configurable via settings)
  • website.UserProfile (for streak tracking)

Note: DailyStats remains in website app as it's used for voting system too (not just Sizzle).[2]

Documentation Added

README.md (sizzle/README.md)

Complete documentation including:

  • Installation instructions
  • Configuration options
  • Template customization guide
  • API reference
  • Troubleshooting section
  • Usage examples

Configuration Guide

Added to BLT docs explaining:

  • How to enable/disable features
  • How to customize models
  • How to override templates
  • Slack integration setup

Testing Checklist

Functionality Tests

  • Daily check-ins work in BLT
  • Time logging works in BLT
  • Leaderboards display correctly
  • Streak tracking updates properly
  • Admin panel accessible
  • All URLs resolve correctly
  • Static files load properly

Migration Tests

  • Fresh database: migrations apply cleanly
  • Existing data: data migrates without loss
  • Rollback: reverse migration works

Configuration Tests

  • Default settings work (BLT integration)
  • Slack disabled: app works without Slack
  • Custom base template: overrides work
  • Missing dependencies: graceful error messages

Management Commands

  • send_daily_reminders works
  • slack_daily_timelogs works with Slack enabled
  • slack_daily_timelogs fails gracefully when Slack disabled
  • daily_checkin_reminder works
  • cron_send_reminders work

Backward Compatibility

Zero Breaking Changes for BLT

  • All existing URLs work (/sizzle/* routes)
  • All views function identically
  • Templates render the same
  • Database queries unchanged (table names migrate seamlessly)
  • Admin interface unchanged
  • Management commands work as before

Migration Path

Existing BLT installations need only:

  1. Run migrations: python manage.py migrate sizzle
  2. Add to INSTALLED_APPS: 'sizzle' (optional: can keep in existing setup)

No code changes required.

Benefits

For BLT Project

  • Modular architecture - easier to maintain and test
  • Clear boundaries - sizzle concerns isolated
  • No impact - works exactly as before with defaults

For Other Projects

  • Easy integration - pip install django-sizzle (future)
  • Customizable - override templates, disable features via settings
  • Optional dependencies - don't need Slack if not using it
  • Documented - comprehensive README with examples

Summary by CodeRabbit

  • New Features

    • Added Sizzle plugin (daily check‑ins, time logging, reminders, streaks, leaderboards, Slack summaries); accessible at /sizzle/.
  • Documentation

    • New Sizzle docs and Featured Plugins section; expanded Quick Start with Docker and Poetry install flows, prerequisites, and troubleshooting.
  • Admin / UI

    • Admin registrations and templates updated for configurable base layout and optional sidenav; UI pages for reports, timelogs, and docs.
  • Chores

    • New daily runner, reminder and Slack tasks; added models, utilities, config flags, and context integration.

mdkaifansari04 and others added 30 commits November 8, 2025 11:59
Replace custom OAuth callback implementations with allauth's built-in
provider callback views. This ensures proper OAuth2 flow handling
and follows django-allauth best practices.

- Use github_views.oauth2_callback, google_views.oauth2_callback,
  facebook_views.oauth2_callback directly in URL patterns
- Remove unused custom callback view classes
- Remove unused OAuth2CallbackView import
- Remove unused safe_redirect_allowed and urllib imports
- Format imports according to isort and Ruff requirements

fixes: OWASP-BLT#4753
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (3)
sizzle/management/commands/cron_send_reminders.py (2)

176-191: Guard domain lookup for reminder links.

Direct access to settings.PRODUCTION_DOMAIN will raise AttributeError in projects that don't define this BLT-specific setting, breaking the pluggable app promise.


201-229: Domain references in HTML template need configuration.

The HTML email template also hardcodes settings.PRODUCTION_DOMAIN, which is part of the same pluggability issue.

sizzle/views.py (1)

296-296: Remove debug print statement.

A print() statement remains in production code and should be replaced with proper logging.

🧹 Nitpick comments (3)
sizzle/management/commands/slack_daily_timelogs.py (1)

103-108: Consider reusing the utility function.

A similar format_timedelta function exists in sizzle/utils/__init__.py (lines 20-27). While the output formats differ slightly ("Xh Ym Zs" vs "X hours, Y minutes, Z seconds"), you could standardize on one format and eliminate this duplication.

sizzle/utils/model_loader.py (2)

101-139: Consider exception chaining for clearer tracebacks.

The core model loaders raise ImproperlyConfigured when models aren't found, but don't chain the original LookupError. Adding raise ... from e would preserve the full exception context for debugging.

Example for one function:

     except LookupError as e:
-        raise ImproperlyConfigured(
+        raise ImproperlyConfigured(
             f"Could not load ReminderSettings model from sizzle app. "
             f"Make sure sizzle migrations have been run. Error: {e}"
-        )
+        ) from e

142-153: Remove unnecessary noqa directives.

The # noqa comments on lines 148-149 are unnecessary since the imports are actually used (they verify the modules can be imported).

Apply this diff:

     try:
-        from slack_bolt import App  # noqa
-        from slack_sdk.web import WebClient  # noqa
+        from slack_bolt import App
+        from slack_sdk.web import WebClient
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between ce7eea4 and ddc9c26.

📒 Files selected for processing (9)
  • sizzle/admin.py (1 hunks)
  • sizzle/context_processors.py (1 hunks)
  • sizzle/management/commands/cron_send_reminders.py (1 hunks)
  • sizzle/management/commands/slack_daily_timelogs.py (1 hunks)
  • sizzle/migrations/0001_initial.py (1 hunks)
  • sizzle/models.py (1 hunks)
  • sizzle/utils/__init__.py (1 hunks)
  • sizzle/utils/model_loader.py (1 hunks)
  • sizzle/views.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • sizzle/admin.py
🧰 Additional context used
🪛 Ruff (0.14.4)
sizzle/context_processors.py

7-7: Unused function argument: request

(ARG001)

sizzle/utils/__init__.py

11-11: Probable use of requests call without timeout

(S113)


15-15: Consider moving this statement to an else block

(TRY300)


16-16: Do not catch blind exception: Exception

(BLE001)

sizzle/utils/model_loader.py

109-112: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


109-112: Avoid specifying long messages outside the exception class

(TRY003)


123-125: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


123-125: Avoid specifying long messages outside the exception class

(TRY003)


136-139: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


136-139: Avoid specifying long messages outside the exception class

(TRY003)


148-148: Unused blanket noqa directive

Remove unused noqa directive

(RUF100)


149-149: Unused blanket noqa directive

Remove unused noqa directive

(RUF100)


151-151: Consider moving this statement to an else block

(TRY300)

sizzle/management/commands/cron_send_reminders.py

32-32: Unused method argument: args

(ARG002)


32-32: Unused method argument: options

(ARG002)


107-107: Do not catch blind exception: Exception

(BLE001)


132-132: Do not catch blind exception: Exception

(BLE001)


133-133: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


133-133: Use explicit conversion flag

Replace with conversion flag

(RUF010)


150-150: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)


154-154: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)


242-242: Do not catch blind exception: Exception

(BLE001)


244-244: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


244-244: Use explicit conversion flag

Replace with conversion flag

(RUF010)


256-256: Consider moving this statement to an else block

(TRY300)


258-258: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


258-258: Use explicit conversion flag

Replace with conversion flag

(RUF010)

sizzle/views.py

224-224: Loop control variable date not used within loop body

Rename unused date to _date

(B007)


259-259: Unpacked variable created is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


316-316: Local variable e is assigned to but never used

Remove assignment to unused variable e

(F841)


338-338: Loop control variable date not used within loop body

Rename unused date to _date

(B007)

sizzle/migrations/0001_initial.py

12-14: Avoid specifying long messages outside the exception class

(TRY003)


28-116: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

sizzle/models.py

52-52: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


53-55: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


76-79: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

sizzle/management/commands/slack_daily_timelogs.py

12-12: Unused method argument: args

(ARG002)


12-12: Unused method argument: kwargs

(ARG002)


46-46: Do not catch blind exception: Exception

(BLE001)


97-97: Do not catch blind exception: Exception

(BLE001)


120-120: Do not catch blind exception: Exception

(BLE001)

⏰ 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). (2)
  • GitHub Check: Run Tests
  • GitHub Check: docker-test
🔇 Additional comments (14)
sizzle/context_processors.py (1)

7-29: LGTM! Context processor correctly implements feature toggle.

The implementation now properly honors the SIZZLE_SHOW_SIDENAV setting before checking template existence, addressing the previous review feedback. The unused request parameter is required by Django's context processor signature.

sizzle/utils/__init__.py (1)

20-27: LGTM! Duration formatting is correct.

The implementation correctly converts timedelta to a human-readable format using appropriate time division.

sizzle/management/commands/cron_send_reminders.py (1)

56-81: LGTM! Midnight window wrap-around correctly implemented.

The time window filtering now properly handles cases where the reminder window crosses midnight using Django Q objects, addressing the previous review feedback.

sizzle/management/commands/slack_daily_timelogs.py (1)

40-41: LGTM! Timezone-aware timestamp correctly implemented.

The command now uses timezone.now() instead of naive datetime.utcnow(), fixing the previous timezone comparison issue.

sizzle/utils/model_loader.py (1)

156-195: LGTM! Validation logic correctly checks model availability.

The validation function properly checks core and optional models based on feature flags, providing useful diagnostic information.

sizzle/models.py (3)

12-36: LGTM! TimeLog model correctly uses swappable references.

The model now properly uses settings.AUTH_USER_MODEL and configurable SIZZLE_ORGANIZATION_MODEL, addressing previous review feedback.


38-59: LGTM! DailyStatusReport model uses correct user reference.

The model now consistently uses settings.AUTH_USER_MODEL instead of a direct User import, fixing the previous inconsistency.


61-100: LGTM! ReminderSettings model uses correct user reference.

The OneToOneField now properly references settings.AUTH_USER_MODEL, and the timezone conversion logic in the save method is correctly implemented.

sizzle/views.py (4)

17-18: LGTM! Imports now use dynamic model loading.

The file correctly imports from sizzle.utils and sizzle.utils.model_loader, addressing the previous hardcoded dependency on website.models.


175-191: LGTM! Authentication and authorization properly implemented.

The view now has the @login_required decorator and includes authorization logic to restrict access to report owners and staff, addressing previous review feedback.


316-319: LGTM! Exception handling properly narrowed.

The exception handling now catches specific Django exceptions rather than bare Exception, and uses logger.exception for proper traceback logging, addressing previous review feedback.


347-349: LGTM! Null check for end_time properly implemented.

The code now checks if end_time is None before calling strftime(), preventing the AttributeError that was flagged in the previous review.

sizzle/migrations/0001_initial.py (2)

9-20: LGTM! Migration now uses swappable organization model.

The migration correctly uses SIZZLE_ORGANIZATION_MODEL with swappable dependencies and fails fast with a clear error message if not configured, addressing the previous review feedback about hardcoded dependencies.


28-116: LGTM! Migration operations correctly reference swappable models.

All model operations properly use settings.AUTH_USER_MODEL and SIZZLE_ORGANIZATION_MODEL, ensuring the migration is compatible with different project configurations.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
sizzle/views.py (1)

297-297: Replace debug print with structured logging

That bare print(...) (previously flagged) still writes to stdout in production. Please switch to the module logger so the message respects configured handlers.

-            print(previous_work, next_plan, blockers, goal_accomplished, current_mood)
+            logger.debug(
+                "Creating daily status report: previous_work=%s, next_plan=%s, blockers=%s, goal_accomplished=%s, current_mood=%s",
+                previous_work,
+                next_plan,
+                blockers,
+                goal_accomplished,
+                current_mood,
+            )
🧹 Nitpick comments (1)
sizzle/context_processors.py (1)

7-29: Consider enhancing documentation for integrators.

The context processor logic is sound, but the docstring could be more helpful. Consider documenting:

  • The returned context keys (sizzle_has_sidenav, parent_base) and their purposes
  • That includes/sidenav.html (line 16) is expected to be provided by the integrating project, not by the Sizzle app itself
  • Configuration settings referenced (SIZZLE_SHOW_SIDENAV, SIZZLE_PARENT_BASE)

Example enhancement:

 def sizzle_context(request):
-    """Provide sizzle-specific template context for seamless integration"""
+    """
+    Provide sizzle-specific template context for seamless integration.
+    
+    Returns:
+        dict: Context with:
+            - sizzle_has_sidenav (bool): Whether sidenav template is available.
+              Controlled by SIZZLE_SHOW_SIDENAV setting and presence of
+              includes/sidenav.html template (provided by integrating project).
+            - parent_base (str|None): Base template path from SIZZLE_PARENT_BASE
+              setting, allowing Sizzle templates to extend project's base layout.
+    """
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between ddc9c26 and dd803de.

📒 Files selected for processing (7)
  • sizzle/context_processors.py (1 hunks)
  • sizzle/management/commands/cron_send_reminders.py (1 hunks)
  • sizzle/management/commands/slack_daily_timelogs.py (1 hunks)
  • sizzle/migrations/0001_initial.py (1 hunks)
  • sizzle/models.py (1 hunks)
  • sizzle/utils/__init__.py (1 hunks)
  • sizzle/views.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.4)
sizzle/management/commands/cron_send_reminders.py

32-32: Unused method argument: args

(ARG002)


32-32: Unused method argument: options

(ARG002)


106-106: Do not catch blind exception: Exception

(BLE001)


131-131: Do not catch blind exception: Exception

(BLE001)


132-132: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


132-132: Use explicit conversion flag

Replace with conversion flag

(RUF010)


149-149: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)


153-153: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)


255-255: Do not catch blind exception: Exception

(BLE001)


257-257: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


257-257: Use explicit conversion flag

Replace with conversion flag

(RUF010)


269-269: Consider moving this statement to an else block

(TRY300)


271-271: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


271-271: Use explicit conversion flag

Replace with conversion flag

(RUF010)

sizzle/context_processors.py

7-7: Unused function argument: request

(ARG001)

sizzle/migrations/0001_initial.py

12-12: Avoid specifying long messages outside the exception class

(TRY003)


26-114: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

sizzle/views.py

225-225: Loop control variable date not used within loop body

Rename unused date to _date

(B007)


260-260: Unpacked variable created is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


317-317: Local variable e is assigned to but never used

Remove assignment to unused variable e

(F841)


339-339: Loop control variable date not used within loop body

Rename unused date to _date

(B007)

sizzle/management/commands/slack_daily_timelogs.py

13-13: Unused method argument: args

(ARG002)


13-13: Unused method argument: kwargs

(ARG002)


47-47: Do not catch blind exception: Exception

(BLE001)


98-98: Do not catch blind exception: Exception

(BLE001)


121-121: Do not catch blind exception: Exception

(BLE001)

sizzle/models.py

54-54: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


55-57: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


80-83: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

sizzle/utils/__init__.py

15-15: Consider moving this statement to an else block

(TRY300)


16-16: Do not catch blind exception: Exception

(BLE001)

⏰ 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). (2)
  • GitHub Check: Run Tests
  • GitHub Check: docker-test
🔇 Additional comments (3)
sizzle/context_processors.py (1)

7-7: Unused request parameter is required by Django.

The Ruff warning about the unused request parameter is a false positive. Django context processors must be callables that accept a single argument — the current HttpRequest, making the parameter mandatory by framework design. Optionally, you could prefix it with an underscore (_request) to signal intentional non-use, though leaving it as request is standard Django practice.

sizzle/migrations/0001_initial.py (2)

15-18: LGTM once the configuration check is moved here.

The dynamic dependency generation is implemented correctly and supports swappable models as intended.


26-114: Safely ignore the RUF012 static analysis warning.

The Ruff warning about operations needing ClassVar annotation is a false positive—Django migrations use these as ordinary class attributes by design.

The model definitions are correct:

  • Proper swappable model references
  • Appropriate field types, constraints, and indexes
  • Good related_name choices to avoid conflicts

@mdkaifansari04
Copy link
Contributor Author

@coderabbitai

I was getting this ci/cd error, as it was good prior to this, can you help with it.

 File "/home/runner/work/BLT/BLT/manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/commands/test.py", line 24, in run_from_argv
    super().run_from_argv(argv)
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/base.py", line 413, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/base.py", line 459, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/commands/test.py", line 63, in handle
    failures = test_runner.run_tests(test_labels)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/test/runner.py", line 1066, in run_tests
    old_config = self.setup_databases(
                 ^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/test/runner.py", line 964, in setup_databases
    return _setup_databases(
           ^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/test/utils.py", line 206, in setup_databases
    connection.creation.create_test_db(
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/backends/base/creation.py", line 78, in create_test_db
    call_command(
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/__init__.py", line 194, in call_command
    return command.execute(*args, **defaults)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/base.py", line 459, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/base.py", line 107, in wrapper
    res = handle_func(*args, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/core/management/commands/migrate.py", line 357, in handle
    post_migrate_state = executor.migrate(
                         ^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/migrations/executor.py", line 135, in migrate
    state = self._migrate_all_forwards(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/migrations/executor.py", line 167, in _migrate_all_forwards
    state = self.apply_migration(
            ^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/migrations/executor.py", line 255, in apply_migration
    state = migration.apply(state, schema_editor)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/migrations/migration.py", line 132, in apply
    operation.database_forwards(
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/migrations/operations/models.py", line 97, in database_forwards
    schema_editor.create_model(model)
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 502, in create_model
    sql, params = self.table_sql(model)
                  ^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 219, in table_sql
    definition, extra_params = self.column_sql(model, field)
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 376, in column_sql
    field_db_params = field.db_parameters(connection=self.connection)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/models/fields/related.py", line 1182, in db_parameters
    target_db_parameters = self.target_field.db_parameters(connection)
                           ^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/models/fields/related.py", line 1072, in target_field
    return self.foreign_related_fields[0]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/utils/functional.py", line 47, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
                                         ^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/models/fields/related.py", line 744, in foreign_related_fields
    rhs_field for lhs_field, rhs_field in self.related_fields if rhs_field
                                          ^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/utils/functional.py", line 47, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
                                         ^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/models/fields/related.py", line 731, in related_fields
    return self.resolve_related_fields()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/models/fields/related.py", line 1099, in resolve_related_fields
    related_fields = super().resolve_related_fields()
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/blt-yuw0N2NF-py3.11/lib/python3.11/site-packages/django/db/models/fields/related.py", line 711, in resolve_related_fields
    raise ValueError(
ValueError: Related model 'website.organization' cannot be resolved

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Check the actual migration file for how Organization is referenced
echo "=== Checking sizzle/migrations/0001_initial.py for Organization references ==="
cat sizzle/migrations/0001_initial.py | grep -i organization

echo -e "\n=== Checking if Organization model exists in website/models.py ==="
rg -n "class Organization" website/models.py

echo -e "\n=== Checking INSTALLED_APPS order in blt/settings.py ==="
sed -n '/INSTALLED_APPS/,/]/p' blt/settings.py | head -30

echo -e "\n=== Checking for any hardcoded lowercase 'website.organization' references ==="
rg -n "website\.organization" sizzle/

Length of output: 1500


🏁 Script executed:

#!/bin/bash

# Check where sizzle is in INSTALLED_APPS - need to see the full list
echo "=== Full INSTALLED_APPS including sizzle ==="
sed -n '/INSTALLED_APPS = (/,/^)/p' blt/settings.py

echo -e "\n=== Check sizzle/conf.py for SIZZLE_ORGANIZATION_MODEL default ==="
cat sizzle/conf.py | grep -A2 -B2 SIZZLE_ORGANIZATION_MODEL

echo -e "\n=== Check sizzle/models.py for Organization reference ==="
rg -n "SIZZLE_ORGANIZATION_MODEL|organization" sizzle/models.py | head -20

echo -e "\n=== Check the exact _dependencies function in migration ==="
sed -n '/def _dependencies/,/return deps/p' sizzle/migrations/0001_initial.py

Length of output: 1580


🏁 Script executed:

#!/bin/bash

# Check the exact TimeLog field definition in migration
echo "=== TimeLog CreateModel operation in migration ==="
sed -n '/migrations.CreateModel/,/^            \],$/p' sizzle/migrations/0001_initial.py | grep -A50 "name=\"TimeLog\""

echo -e "\n=== Check how settings.SIZZLE_ORGANIZATION_MODEL is used in models.py ==="
sed -n '/class TimeLog/,/class /p' sizzle/models.py | head -30

Length of output: 3764


@mdkaifansari04 I found the issue! The problem is in your migration file sizzle/migrations/0001_initial.py.

Root Cause

The migration is using a bare variable SIZZLE_ORGANIZATION_MODEL imported from sizzle.conf, but Django's migration framework expects swappable model references to use settings.SETTING_NAME format (like how AUTH_USER_MODEL is handled).

Current (incorrect):

from sizzle.conf import SIZZLE_ORGANIZATION_MODEL
# ...
to=SIZZLE_ORGANIZATION_MODEL,  # ❌ Wrong

Should be:

to=settings.SIZZLE_ORGANIZATION_MODEL,  # ✅ Correct

The Fix

Update sizzle/migrations/0001_initial.py:

1. Remove the import from sizzle.conf:

# Remove this line:
from sizzle.conf import SIZZLE_ORGANIZATION_MODEL

2. Update the _dependencies function:

def _dependencies():
    from django.conf import settings
    
    deps = [migrations.swappable_dependency(settings.AUTH_USER_MODEL)]
    
    # Check if SIZZLE_ORGANIZATION_MODEL is configured
    org_model = getattr(settings, 'SIZZLE_ORGANIZATION_MODEL', None)
    if not org_model:
        raise ImproperlyConfigured(
            "SIZZLE_ORGANIZATION_MODEL must be configured before running sizzle migrations."
        )
    deps.append(migrations.swappable_dependency(org_model))
    return deps

3. Update the TimeLog CreateModel operation:

migrations.CreateModel(
    name="TimeLog",
    fields=[
        # ... other fields ...
        (
            "organization",
            models.ForeignKey(
                blank=True,
                null=True,
                on_delete=django.db.models.deletion.CASCADE,
                related_name="sizzle_time_logs",
                to=settings.SIZZLE_ORGANIZATION_MODEL,  # ✅ Changed from SIZZLE_ORGANIZATION_MODEL
            ),
        ),
        # ... rest of fields ...
    ],
)

Why This Fixes It

Django's migration framework needs swappable model references to be accessed through settings at migration runtime, not pre-imported as variables. This allows Django to properly resolve the model path regardless of when/how the migration is executed.

After making these changes, the migration should work correctly in your CI/CD environment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (3)
sizzle/migrations/0001_initial.py (2)

11-12: Critical: Remove the duplicate module-level configuration check.

The module-level raise on lines 11-12 will crash when Django imports this file to discover migrations (e.g., showmigrations, migrate --list), even though _dependencies() already performs the same check at runtime. This was flagged in a previous review but the module-level check was not removed.

Apply this diff to remove the duplicate check:

 from sizzle.conf import SIZZLE_ORGANIZATION_MODEL
 
-if not SIZZLE_ORGANIZATION_MODEL:
-    raise ImproperlyConfigured("SIZZLE_ORGANIZATION_MODEL must be configured before running sizzle migrations.")
-
 
 def _dependencies():
     from django.conf import settings

The check inside _dependencies() (lines 21-26) will still enforce the requirement when the migration actually runs.


15-27: Add the missing return statement in _dependencies() function.

The function builds a dependencies list but lacks an explicit return statement. Since the function is called on line 33 and assigned to the Migration class's dependencies attribute, it will currently return None instead of the list, causing Django migration processing to fail.

     deps.append(migrations.swappable_dependency(org_model))
+    return deps
sizzle/views.py (1)

297-297: Remove the debug print statement.

A print() statement remains in production code on line 297. This was previously flagged for replacement with proper logging but appears to still be present.

Apply this diff to replace it with structured logging:

-            print(previous_work, next_plan, blockers, goal_accomplished, current_mood)
+            logger.debug(
+                "Creating daily status report: previous_work=%s, next_plan=%s, blockers=%s, "
+                "goal_accomplished=%s, current_mood=%s",
+                previous_work, next_plan, blockers, goal_accomplished, current_mood
+            )
🧹 Nitpick comments (5)
sizzle/views.py (5)

225-225: Unused loop variable date.

The loop variable date is not used in the loop body; rename it to _date to indicate it's intentionally unused.

-    for date, logs in grouped_logs.items():
+    for _date, logs in grouped_logs.items():

260-260: Unused variable created.

The created variable from Token.objects.get_or_create() is never used; prefix it with an underscore.

-    token, created = Token.objects.get_or_create(user=request.user)
+    token, _created = Token.objects.get_or_create(user=request.user)

262-264: Clarify Organization model availability check.

The check if Organization: on line 262 verifies the model class exists (in case get_organization_model() returns None when SIZZLE_ORGANIZATION_MODEL is not configured). While this handles optional organization support correctly, consider adding a brief inline comment to clarify the intent for maintainability.

     organizations_list = []
-    if Organization:
+    if Organization:  # Organization model is optional; may be None if not configured
         organizations_list_queryset = Organization.objects.all().values("url", "name")
         organizations_list = list(organizations_list_queryset)

317-320: Unused exception variable e.

The exception variable e is captured but never used in the handler; remove it or use it in logging.

-    except (ValidationError, IntegrityError) as e:
+    except (ValidationError, IntegrityError):
         logger.exception("Error creating daily status report")
         messages.error(request, "An error occurred while submitting your report. Please try again.")
         return redirect("sizzle")

340-340: Unused loop variable date.

The loop variable date is not used in the loop body; rename it to _date to indicate it's intentionally unused.

-    for date, logs in grouped_logs.items():
+    for _date, logs in grouped_logs.items():
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between dd803de and 715266d.

📒 Files selected for processing (2)
  • sizzle/migrations/0001_initial.py (1 hunks)
  • sizzle/views.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.4)
sizzle/migrations/0001_initial.py

12-12: Avoid specifying long messages outside the exception class

(TRY003)


23-25: Avoid specifying long messages outside the exception class

(TRY003)


35-123: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

sizzle/views.py

225-225: Loop control variable date not used within loop body

Rename unused date to _date

(B007)


260-260: Unpacked variable created is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


317-317: Local variable e is assigned to but never used

Remove assignment to unused variable e

(F841)


340-340: Loop control variable date not used within loop body

Rename unused date to _date

(B007)

⏰ 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). (2)
  • GitHub Check: Run Tests
  • GitHub Check: docker-test

Copy link
Collaborator

@DonnieBLT DonnieBLT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please apply this to the new sizzle repo and for the blt repo the change will be adding the sizzle to requirements and deleting the code so it migrates seamlessly

@DonnieBLT DonnieBLT added the d15 label Nov 16, 2025
@mdkaifansari04
Copy link
Contributor Author

Sure @DonnieBLT, I'll make the updates for BLT as well as for sizzle. Thanks for the feedback.

@github-actions github-actions bot added the unresolved-conversations: 0 PR has 0 unresolved conversations label Nov 16, 2025
@DonnieBLT DonnieBLT removed the d15 label Nov 17, 2025
@DonnieBLT
Copy link
Collaborator

follow up on the Sizzle repo

@mdkaifansari04
Copy link
Contributor Author

Follwing up with other pr in sizzle repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unresolved-conversations: 0 PR has 0 unresolved conversations

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Sizzle / Daily Check-in: Develop as a pluggable Django app.

4 participants