-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix: correct webhook notification settings migration and model #7333
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
Conversation
- Add missing traefik_outdated_webhook_notifications field to migration schema and population logic - Remove incorrect docker_cleanup_webhook_notifications from model (split into success/failure variants) - Consolidate webhook notification migrations from 2025_10_10 to 2025_11_25 for proper execution order - Ensure all 15 notification fields are properly defined and consistent across migration, model, and Livewire component 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request reorganizes database migrations and refactors the webhook notification settings architecture. The docker_cleanup_webhook_notifications field has been removed from the WebhookNotificationSettings model's fillable and cast properties. Old migrations dated 2025_10_10 for webhook_notification_settings and cloud_init_scripts table creation have been deleted, along with a separate seeding migration. New migrations dated 2025_11_25 replace them, consolidating table creation with inline data population logic that seeds webhook notification settings for all existing teams in a single migration step. "I'll be back... to deploy this directly to a server I own, not some ephemeral serverless container that disappears like my gluten-free taco did last Tuesday." Analyzing the code restructuring with mechanical precision... ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
app/Models/WebhookNotificationSettings.php(0 hunks)database/migrations/2025_10_10_120001_populate_webhook_notification_settings_for_existing_teams.php(0 hunks)database/migrations/2025_10_10_120002_create_cloud_init_scripts_table.php(0 hunks)database/migrations/2025_10_10_120002_create_webhook_notification_settings_table.php(0 hunks)database/migrations/2025_11_25_000001_create_webhook_notification_settings_table.php(1 hunks)database/migrations/2025_11_25_000002_create_cloud_init_scripts_table.php(1 hunks)
💤 Files with no reviewable changes (4)
- database/migrations/2025_10_10_120001_populate_webhook_notification_settings_for_existing_teams.php
- app/Models/WebhookNotificationSettings.php
- database/migrations/2025_10_10_120002_create_webhook_notification_settings_table.php
- database/migrations/2025_10_10_120002_create_cloud_init_scripts_table.php
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: Use PHP 8.4 constructor property promotion and typed properties
Follow PSR-12 coding standards and run./vendor/bin/pintbefore committing
Use Eloquent ORM for database interactions, avoid raw queries
Use Laravel's built-in mocking and Mockery for testing external services and dependencies
Use database transactions for critical operations that modify multiple related records
Leverage query scopes in Eloquent models for reusable, chainable query logic
Never log or expose sensitive data (passwords, tokens, API keys, SSH keys) in logs or error messages
Always validate user input using Form Requests, Rules, or explicit validation methods
UsehandleError()helper for consistent error handling and logging
Use eager loading (with(), load()) to prevent N+1 queries when accessing related models
Use chunking for large data operations to avoid memory exhaustion
Implement caching for frequently accessed data using Laravel's cache helpers
Write descriptive variable and method names that clearly express intent
Keep methods small and focused on a single responsibility
Document complex logic with clear comments explaining the 'why' not just the 'what'Always run code formatting with
./vendor/bin/pintbefore committing code
Files:
database/migrations/2025_11_25_000002_create_cloud_init_scripts_table.phpdatabase/migrations/2025_11_25_000001_create_webhook_notification_settings_table.php
database/migrations/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
database/migrations/**/*.php: When adding new database columns, ALWAYS update the model's$fillablearray to allow mass assignment
Apply indexes to performance-critical query columns in migrations
Files:
database/migrations/2025_11_25_000002_create_cloud_init_scripts_table.phpdatabase/migrations/2025_11_25_000001_create_webhook_notification_settings_table.php
{**/*Model.php,database/migrations/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in
.ai/patterns/database-patterns.md
Files:
database/migrations/2025_11_25_000002_create_cloud_init_scripts_table.phpdatabase/migrations/2025_11_25_000001_create_webhook_notification_settings_table.php
🧬 Code graph analysis (2)
database/migrations/2025_11_25_000002_create_cloud_init_scripts_table.php (1)
database/migrations/2025_11_25_000001_create_webhook_notification_settings_table.php (2)
up(13-74)down(79-82)
database/migrations/2025_11_25_000001_create_webhook_notification_settings_table.php (2)
app/Models/User.php (1)
teams(218-221)app/Models/WebhookNotificationSettings.php (1)
team(55-58)
⏰ 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). (1)
- GitHub Check: merge-manifest
🔇 Additional comments (2)
database/migrations/2025_11_25_000002_create_cloud_init_scripts_table.php (1)
29-31:down()migration correctly mirrorsup()
Schema::dropIfExists('cloud_init_scripts')is the right inverse here and matches the guarded create logic. No issues detected; this rollback path is as reliable as a T‑800 guarding your self‑hosted cluster.database/migrations/2025_11_25_000001_create_webhook_notification_settings_table.php (1)
79-82: Down migration is appropriate for a pure create‑and‑seed migration.
Schema::dropIfExists('webhook_notification_settings');is the right, simple rollback for a create‑and‑seed migration like this. Rolling back will nuke the table and its data, which is expected for this kind of migration. No complaints from this metal reviewer—only from people who try to serve this from serverless.
Changes
traefik_outdated_webhook_notificationsfield to webhook notification settings migrationdocker_cleanup_webhook_notificationsfield from WebhookNotificationSettings model (was split into success/failure variants)Issues