-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix: envs added to the right place in dockerfiles #7123
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
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe PR refactors Dockerfile ARG insertion logic in ApplicationDeploymentJob by extracting a helper method to find FROM instructions and reorganizing ARG placement to occur after each FROM line. It introduces comprehensive unit tests for this functionality, updates CSS base styling from neutral-50 to gray-50, applies minor UI/formatting refinements across authentication views, and substantially restructures base.blade.php with DOMPurify hook integration and theme initialization reorganization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/Jobs/ApplicationDeploymentJob.php(6 hunks)resources/css/app.css(1 hunks)resources/views/auth/login.blade.php(2 hunks)resources/views/auth/register.blade.php(3 hunks)resources/views/auth/reset-password.blade.php(2 hunks)resources/views/auth/two-factor-challenge.blade.php(1 hunks)resources/views/layouts/base.blade.php(3 hunks)tests/Unit/DockerfileArgInsertionTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
resources/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Store frontend assets and views under resources
Files:
resources/css/app.cssresources/views/auth/two-factor-challenge.blade.phpresources/views/auth/login.blade.phpresources/views/auth/register.blade.phpresources/views/auth/reset-password.blade.phpresources/views/layouts/base.blade.php
resources/**/*.css
📄 CodeRabbit inference engine (.cursor/rules/laravel-boost.mdc)
In Tailwind v4, import Tailwind via @import "tailwindcss" instead of @tailwind directives
Files:
resources/css/app.css
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
**/*.php: Follow PSR-12 coding standards for all PHP code
Format PHP code with Laravel Pint configuration
Run static analysis with PHPStan to ensure type safety in PHP code
Document complex methods with PHPDoc blocks including parameters, return types, and thrown exceptions
**/*.php: Follow PSR-12 PHP coding standards across the codebase
Prefer eager loading and query optimization to prevent N+1 issues in database interactions
Use Laravel best practices for structure, services, and policies
**/*.php: Always use curly braces for control structures in PHP, even for single-line bodies
Use PHP 8 constructor property promotion in __construct() and avoid empty constructors
Always declare explicit return types for functions and methods; add appropriate parameter type hints
Prefer PHPDoc blocks over inline comments; only add inline code comments for very complex logic
When documenting arrays, add useful array shape types in PHPDoc where appropriate
Enum case names should be TitleCase (e.g., FavoritePerson, Monthly)
Files:
resources/views/auth/two-factor-challenge.blade.phptests/Unit/DockerfileArgInsertionTest.phpresources/views/auth/login.blade.phpresources/views/auth/register.blade.phpresources/views/auth/reset-password.blade.phpresources/views/layouts/base.blade.phpapp/Jobs/ApplicationDeploymentJob.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
Use semantic Tailwind CSS classes and consistent spacing in Blade templates
resources/views/**/*.blade.php: In Blade views, prefer using x-forms components with canGate and :canResource instead of wrapping elements in @can/@else blocks
Remove legacy @can/@else blocks around individual form inputs/buttons and migrate to the single-line component pattern
Choose gates consistently in views: use update for configuration changes, deploy for operational actions, view for read-only access, and delete for destructive actions
Always pass the specific resource to :canResource (e.g., :canResource="$application", "$service", "$server"), and use team context for creation permissions (e.g., :canResource="auth()->user()->currentTeam" with canGate="createAnyResource")
Only set autoDisable="false" when also providing explicit :disabled logic on the component
Use single-line authorized components for forms (inputs, selects, checkboxes, buttons) to reduce duplication (e.g., <x-forms.input canGate="update" :canResource="$resource" ... />)
resources/views/**/*.blade.php: In Blade views, prefer x-forms.* components with canGate and :canResource for authorization (autoDisable defaults to true) instead of manual permission blocks.
Do not wrap x-forms.* components in @can/@else to toggle disabled state; use component-level canGate/:canResource instead.
For custom interactive components not using x-forms.* (e.g., Alpine.js widgets), wrap with @can('gate', $resource) and provide a disabled/readonly fallback in @else with opacity-50 and cursor-not-allowed styling.
resources/views/**/*.blade.php: Prefer named routes and the route() helper when generating links in views
Dispatch events using $this->dispatch() (not emit/dispatchBrowserEvent) in Livewire 3 templates/components
Do not manually include Alpine.js; Livewire 3 bundles Alpine with required plugins
Files:
resources/views/auth/two-factor-challenge.blade.phpresources/views/auth/login.blade.phpresources/views/auth/register.blade.phpresources/views/auth/reset-password.blade.phpresources/views/layouts/base.blade.php
resources/**/*.{blade.php,vue}
📄 CodeRabbit inference engine (.cursor/rules/laravel-boost.mdc)
Use Tailwind gap utilities for spacing lists instead of margins when listing items
Files:
resources/views/auth/two-factor-challenge.blade.phpresources/views/auth/login.blade.phpresources/views/auth/register.blade.phpresources/views/auth/reset-password.blade.phpresources/views/layouts/base.blade.php
tests/Unit/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Place unit tests under tests/Unit and keep them focused on individual classes/methods
tests/Unit/**/*.php: Unit tests (tests/Unit) must not use database connections or model factories
Unit tests must use mocking for models and external dependencies
Files:
tests/Unit/DockerfileArgInsertionTest.php
tests/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
Write feature tests for API endpoints using Pest; mock external services and assert JSON responses and side effects
Add tests validating that components respect authorization (e.g., unauthorized users see disabled inputs; checkbox instantSave becomes false)
Include security tests covering SQL injection resistance, XSS validation, and enforcement of team isolation (403 on cross-team access).
Write tests using Pest (feature/unit) and include Laravel Dusk for browser scenarios as needed
Prefer Pest PHP as the primary testing framework for writing tests
Files:
tests/Unit/DockerfileArgInsertionTest.php
tests/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Keep automated tests (Pest/Dusk) under tests
Files:
tests/Unit/DockerfileArgInsertionTest.php
tests/{Feature,Unit}/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/laravel-boost.mdc)
tests/{Feature,Unit}/**/*.php: All tests must be written using Pest and live under tests/Feature or tests/Unit
Use specific response assertion helpers (e.g., assertForbidden, assertNotFound) instead of generic assertStatus
Use datasets in Pest to reduce duplication where suitable (e.g., validation tests)
Write or update unit/feature tests for every change and run the affected tests
Files:
tests/Unit/DockerfileArgInsertionTest.php
app/Jobs/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Queueable background work should be implemented as Jobs under app/Jobs; Actions may expose asJob to dispatch
app/Jobs/**/*.php: Queue jobs should implement ShouldQueue and define retry strategy (tries, maxExceptions, backoff)
Queue jobs must handle success/failure state updates and broadcast corresponding events; implement failed(Throwable) for final failure handlingQueue jobs should update status, wrap external calls in try/catch, set failure status on exceptions, and rethrow
Files:
app/Jobs/ApplicationDeploymentJob.php
app/Jobs/*.php
📄 CodeRabbit inference engine (.cursor/rules/deployment-architecture.mdc)
Implement background deployment, monitoring, backup, and notification work as Laravel Job classes under app/Jobs
Files:
app/Jobs/ApplicationDeploymentJob.php
app/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
Use database transactions to group related write operations for consistency in services/controllers/jobs
Files:
app/Jobs/ApplicationDeploymentJob.php
app/Jobs/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Implement background jobs under app/Jobs
Files:
app/Jobs/ApplicationDeploymentJob.php
🧠 Learnings (9)
📚 Learning: 2025-10-15T09:16:25.875Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/laravel-boost.mdc:0-0
Timestamp: 2025-10-15T09:16:25.875Z
Learning: Applies to resources/**/*.css : In Tailwind v4, import Tailwind via import "tailwindcss" instead of tailwind directives
Applied to files:
resources/css/app.css
📚 Learning: 2025-10-15T09:17:35.563Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/testing-patterns.mdc:0-0
Timestamp: 2025-10-15T09:17:35.563Z
Learning: Feature tests must run inside a Docker container
Applied to files:
tests/Unit/DockerfileArgInsertionTest.php
📚 Learning: 2025-08-27T15:02:37.804Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/form-components.mdc:0-0
Timestamp: 2025-08-27T15:02:37.804Z
Learning: Applies to resources/views/**/*.blade.php : Use single-line authorized components for forms (inputs, selects, checkboxes, buttons) to reduce duplication (e.g., <x-forms.input canGate="update" :canResource="$resource" ... />)
Applied to files:
resources/views/auth/login.blade.phpresources/views/auth/register.blade.php
📚 Learning: 2025-08-27T15:02:37.804Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/form-components.mdc:0-0
Timestamp: 2025-08-27T15:02:37.804Z
Learning: Applies to resources/views/**/*.blade.php : Remove legacy can/else blocks around individual form inputs/buttons and migrate to the single-line component pattern
Applied to files:
resources/views/auth/login.blade.phpresources/views/auth/register.blade.phpresources/views/auth/reset-password.blade.phpresources/views/layouts/base.blade.php
📚 Learning: 2025-08-27T15:01:58.710Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/development-workflow.mdc:0-0
Timestamp: 2025-08-27T15:01:58.710Z
Learning: Applies to resources/views/**/*.blade.php : Use semantic Tailwind CSS classes and consistent spacing in Blade templates
Applied to files:
resources/views/layouts/base.blade.php
📚 Learning: 2025-10-15T09:16:25.875Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/laravel-boost.mdc:0-0
Timestamp: 2025-10-15T09:16:25.875Z
Learning: Applies to resources/views/**/*.blade.php : Do not manually include Alpine.js; Livewire 3 bundles Alpine with required plugins
Applied to files:
resources/views/layouts/base.blade.php
📚 Learning: 2025-08-27T15:04:22.911Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/project-overview.mdc:0-0
Timestamp: 2025-08-27T15:04:22.911Z
Learning: Use Laravel for the backend with Livewire for the frontend
Applied to files:
resources/views/layouts/base.blade.php
📚 Learning: 2025-08-27T14:59:11.004Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/application-architecture.mdc:0-0
Timestamp: 2025-08-27T14:59:11.004Z
Learning: Applies to app/Livewire/**/*.php : Define Livewire components in app/Livewire and keep HTTP/UI concerns there
Applied to files:
resources/views/layouts/base.blade.php
📚 Learning: 2025-08-27T15:06:41.387Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/technology-stack.mdc:0-0
Timestamp: 2025-08-27T15:06:41.387Z
Learning: Applies to app/Livewire/**/*.php : Place Livewire components under app/Livewire/ (e.g., Dashboard.php, ActivityMonitor.php, MonacoEditor.php)
Applied to files:
resources/views/layouts/base.blade.php
🔇 Additional comments (1)
tests/Unit/DockerfileArgInsertionTest.php (1)
9-21: Tests crash: protected method is inaccessible
findFromInstructionLinesisprotected, but the tests call it directly on the partial mock. WithoutshouldAllowMockingProtectedMethods()(or an exposed proxy), PHP throws “Call to protected method”. The suite never reaches its assertions.Please expose the helper via a test seam (e.g., subclass wrapper or reflection) before calling it, or switch to exercising the public API that uses this helper.
⛔ Skipped due to learnings
Learnt from: CR Repo: coollabsio/coolify PR: 0 File: .cursor/rules/testing-patterns.mdc:0-0 Timestamp: 2025-10-15T09:17:35.563Z Learning: Applies to tests/Unit/**/*.php : Unit tests must use mocking for models and external dependencies
Summary by CodeRabbit
Style
Chores