-
Notifications
You must be signed in to change notification settings - Fork 344
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
fix local files in docker #338
fix local files in docker #338
Conversation
Response::make diesnt exist anymore and the nginx.conf didnt allow the needed paths
WalkthroughThe recent updates involve enhancements to the web server configuration and API routing. Specifically, two new Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- docker/nginx.conf (1 hunks)
- routes/api.php (1 hunks)
Additional comments: 3
docker/nginx.conf (2)
- 28-31: The addition of the
location
block for/local/temp/
is correctly configured to handle requests to this path. By setting$original_uri
and using thetry_files
directive, it ensures that requests are properly directed, either serving the requested file or falling back toindex.php
with query parameters if needed. This setup is appropriate for serving temporary files while maintaining the flexibility to handle requests dynamically.- 33-36: Similarly, the
location
block for/forms/assets/
is well-configured to handle requests to form-related assets. The configuration mirrors that of/local/temp/
, ensuring consistency in handling file requests across different parts of the application. This addition improves the application's robustness by ensuring essential assets are accessible.routes/api.php (1)
- 231-231: The update to use
response()->file()
for serving files is a positive change, aligning with modern Laravel practices. This method simplifies the response creation process and enhances readability. It's important to ensure that theStorage::path($path)
andStorage::mimeType($path)
calls are secure and do not expose the application to directory traversal attacks. Given the use of signed URLs, this risk is mitigated, but it's always good practice to validate or sanitize input paths as an additional layer of security.
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.
Thank you for your contribution!
i fixed the import so the test should pass now |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- routes/api.php (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- routes/api.php
Response::make
doesn't exist anymore and the nginx.conf didnt allow the needed paths (/local/temp/
,/forms/assets/
)Summary by CodeRabbit