merge changes from master #2
Conversation
Gumlet2 and fc bug
There was a problem hiding this comment.
Pull Request Overview
This PR adds S3 cloud storage integration to the attachments plugin, enabling files to be stored in and retrieved from S3-compatible object storage services. It also includes various code quality improvements, bug fixes, and feature enhancements.
- Added comprehensive S3 storage support with automatic file upload, download, and deletion
- Introduced new aspect ratio options (3x4, 1x1) in the image editor and enhanced image processing capabilities
- Improved code quality with better formatting, type hints, and input validation
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Template/Attachments/edit_image.ctp | Added 3x4 and 1x1 aspect ratio buttons to the image cropping interface |
| src/Model/Table/AttachmentsTable.php | Core S3 integration logic, new methods for S3 file operations, code quality improvements |
| src/Model/Entity/Attachment.php | S3 client initialization and automatic file retrieval from S3, added LazyLoadEntityTrait |
| src/Controller/AttachmentsController.php | Enhanced image processing, input validation for image parameters, improved caching headers |
| config/attachments.php | Added S3 configuration parameters |
| config/Migrations/20251013171500_AddSequenceIndexToAttachments.php | New database migration for sequence indexing |
| composer.json | Added dependencies for Imagick extension and AWS SDK |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| $response = $this->response->withType($attachment->filetype) | ||
| ->withFile($attachment->path, ['download' => false, 'name' => $attachment->filename]); | ||
| $file = new File($attachment->filetype); |
There was a problem hiding this comment.
File class constructor expects a file path, not a MIME type. This should be 'new File($attachment->path)' instead of 'new File($attachment->filetype)'.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| } | ||
| $response = $this->response->withType($attachment->filetype) | ||
| ->withFile($attachment->path, ['download' => true, 'name' => $attachment->filename]); | ||
| $file = new File($attachment->filetype); |
There was a problem hiding this comment.
File class constructor expects a file path, not a MIME type. This should be 'new File($attachment->path)' instead of 'new File($attachment->filetype)'.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.