fix: align attachment frontend requests with registered OCS routes#8148
Open
Lightshadow02 wants to merge 2 commits into
Open
fix: align attachment frontend requests with registered OCS routes#8148Lightshadow02 wants to merge 2 commits into
Lightshadow02 wants to merge 2 commits into
Conversation
The jest config referenced vue-jest which is not a dependency; the installed transformer is @vue/vue2-jest. Went unnoticed as the repo had no jest tests yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: HUGO LOUREIRO <149155946+Lightshadow02@users.noreply.github.com>
The frontend was migrated to OCS endpoints but kept the old page-route
URL shape (singular /attachment/, composite type:id, POST/GET verbs)
which matches no registered OCS route, so deleting, updating and
restoring attachments silently failed with 404.
- call /cards/{cardId}/attachments/{attachmentId} with a numeric id and
pass the attachment type as a query parameter
- use PUT for restore, keep POST for update and register a POST route
next to PUT since PHP only parses multipart bodies for POST
- default the data parameter of AttachmentOcsController::update() to an
empty string as file uploads do not send it
- unwrap the OCS envelope in updateAttachment()
- add jest regression tests for the attachment API URLs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: HUGO LOUREIRO <149155946+Lightshadow02@users.noreply.github.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
696ca2f migrated the attachment frontend to OCS endpoints, but the frontend kept the old page-route URL shape (singular
/attachment/, composite{type}:{id}ids,POST/GETverbs) while the registered OCS routes use plural/attachments/{attachmentId}withPUT/DELETE. As a result, deleting, updating and restoring an attachment from the card sidebar hit non-existent routes and silently failed with 404 (the Vuex dispatch has no error handling).This PR aligns the frontend with the registered OCS routes and fixes the remaining gaps on the backend side:
AttachmentApi.js: call/cards/{cardId}/attachments/{attachmentId}with a numeric id, pass the attachmenttypeas a query parameter (the OCS controller defaults tofile, sodeck_fileattachments need it explicitly), usePUTfor restore, and unwrap the OCS envelope inupdateAttachment().routes.php: register aPOSTroute forattachment_ocs#updatenext toPUT, since PHP only parsesmultipart/form-databodies (file uploads) forPOSTrequests — mirroring the old page routes which registered both verbs for the same reason.AttachmentOcsController::update(): default$datato an empty string, as the upload form data containsfile/typebut nodatafield.package.jsonpointed atvue-jest, which is not a dependency (the installed transformer is@vue/vue2-jest), so it is fixed in a separate commit.How to test
DELETE .../ocs/v2.php/apps/deck/api/v1.0/cards/{cardId}/attachments/{attachmentId}?type=deck_filerequest should return 200 (previously 404)PUT .../restore)npx jest src/services/AttachmentApi.spec.js→ 5 passingChecklist
🤖 Generated with Claude Code