-
Notifications
You must be signed in to change notification settings - Fork 0
Closed
Description
This issue tracks refactoring improvements for the SecretAttachment API endpoints, as deferred from PR #177 to maintain "One PR = One Topic" principle.
Copilot Review Comments to Address
From PR #177 (6 comments deferred):
- ✅ Thread PRRT_kwDOQJgcLc5iPRK2 (store method): Use API Resources instead of manual JSON array building
- ✅ Thread PRRT_kwDOQJgcLc5iPRKm (index method): Use API Resources for consistent response structure
- ✅ Thread PRRT_kwDOQJgcLc5iPRLg (store validation): Move validation rules to Form Request class
- ✅ Thread PRRT_kwDOQJgcLc5iPRLc (download accessor): Use property accessor pattern instead of getter method
- ❌ Thread PRRT_kwDOQJgcLc5iPRKv (named routes):
Add route names for better maintainabilityNOT IMPLEMENTED - ❌ Thread PRRT_kwDOQJgcLc5iPRKf (route() helper):
Use route() instead of url()NOT IMPLEMENTED
Architecture Decision: Named Routes
Decision: NOT implementing named routes (items 5-6 above).
Rationale:
- Project uses OpenAPI contracts (
/contracts/docs/openapi.yaml) - contract-first architecture - Contracts define exact URL paths (e.g.,
/v1/attachments/{id}/download) - Frontend consumes these contract paths directly
- Named routes are a Laravel backend abstraction that doesn't align with contract-first design
- Zero named routes exist in 20+ existing routes in
routes/api.php - All URL generation consistently uses
url()helper, notroute()
This architectural pattern ensures URLs match OpenAPI specifications exactly, maintaining contract integrity.
Implementation Tasks
- Create
SecretAttachmentResourcefor JSON transformation - Create
StoreSecretAttachmentRequestfor validation - Update controller to use Resource and Form Request
- Fix property accessor usage (
filename_plainvs getter) - Add PHPStan type hints for Level Max compliance
- Add comprehensive test coverage
- Consolidate test helpers in
tests/Pest.php
Completed Work
Changes Implemented
-
API Resource (
SecretAttachmentResource):- Consistent JSON transformation for attachments
- ISO 8601 date formatting
- Filename decryption via accessor
- Used in
store()andindex()endpoints
-
Form Request (
StoreSecretAttachmentRequest):- Centralized validation rules
- Config-based file size/type validation
- Fixed byte-to-KB conversion bug
- Custom error messages
-
Controller Refactoring:
- Removed inline validation
- Removed manual JSON array building
- Uses property accessor pattern
-
Test Coverage:
- 4 Resource transformation tests
- 7 Form Request validation tests
- Helper functions moved to
tests/Pest.php - All 342 tests passing (1085 assertions)
-
Quality Gates:
- PHPStan Level Max: PASSING (fixed 12 type errors)
- Laravel Pint: PASSING (PSR-12 compliant)
- REUSE 3.3: Compliant
Related
- Phase 2: File Attachments API (Upload/Download/Encryption) #175 - File Attachments API (Phase 2)
- feat: File Attachments API (Phase 2) #175 #177 - File Attachments Implementation PR
- refactor: API Resources and Form Requests for SecretAttachment (#180) #181 - This refactoring PR
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
Status
✅ Done