diff --git a/api/handlers/image/imageHandler.go b/api/handlers/image/imageHandler.go index 770dcee..fa82e56 100644 --- a/api/handlers/image/imageHandler.go +++ b/api/handlers/image/imageHandler.go @@ -17,13 +17,15 @@ type Handler struct { CreateUseCase *useCases.CreateImageUseCase GetUseCase *useCases.GetArticleUseCase UpdateUseCase *useCases.UpdateArticleUseCase + DeleteUseCase *useCases.DeleteImageUseCase } -func NewHandler(create *useCases.CreateImageUseCase, get *useCases.GetArticleUseCase, update *useCases.UpdateArticleUseCase) *Handler { +func NewHandler(create *useCases.CreateImageUseCase, get *useCases.GetArticleUseCase, update *useCases.UpdateArticleUseCase, deleteUseCase *useCases.DeleteImageUseCase) *Handler { return &Handler{ CreateUseCase: create, GetUseCase: get, UpdateUseCase: update, + DeleteUseCase: deleteUseCase, } } @@ -78,7 +80,7 @@ func (h *Handler) PostImage(w http.ResponseWriter, r *http.Request) { return } - responseJson, err := json.Marshal(map[string]any{"location": newImage.Path}) + responseJson, err := json.Marshal(map[string]any{"id": newImage.ID, "location": newImage.Path}) if err != nil { writeInternalServerError(w) return @@ -87,3 +89,33 @@ func (h *Handler) PostImage(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusCreated) _, _ = w.Write(responseJson) } + +func (h *Handler) DeleteImage(w http.ResponseWriter, r *http.Request) { + articleId, err := strconv.ParseUint(chi.URLParam(r, "id"), 10, 32) + if err != nil { + http.Error(w, apperr.IdUint32ErrorMessage, http.StatusBadRequest) + return + } + + imageId, err := strconv.ParseUint(chi.URLParam(r, "imageId"), 10, 32) + if err != nil { + http.Error(w, apperr.IdUint32ErrorMessage, http.StatusBadRequest) + return + } + + if _, err := h.GetUseCase.GetArticle(uint32(articleId)); err != nil { + writeArticleError(w, err) + return + } + + if err := h.DeleteUseCase.DeleteImage(uint32(imageId)); err != nil { + if errors.Is(err, domainImage.ErrImageNotFound) { + http.Error(w, apperr.ImageNotFoundErrorMessage, http.StatusNotFound) + return + } + writeInternalServerError(w) + return + } + + w.WriteHeader(http.StatusNoContent) +} diff --git a/api/pkg/apperr/messages.go b/api/pkg/apperr/messages.go index d187ae7..4b5e14f 100644 --- a/api/pkg/apperr/messages.go +++ b/api/pkg/apperr/messages.go @@ -4,6 +4,7 @@ const LogsErrorMessage = "Access to the requested resource is forbidden due to i const BodyErrorMessage = "The request cannot be processed due to a mismatch in the format of the body." const IdUint32ErrorMessage = "The server expects the ID to be in the format of an unsigned 32-bit integer (uint32)." const ArticleNotFoundErrorMessage = "Article not found" +const ImageNotFoundErrorMessage = "Image not found" const ArticleTitleErrorMessage = "Article title must be between 3 and 200 characters." const RegistrationClosedErrorMessage = "You are not allowed to create a user. Log in or reset database." const RegistrationFailedErrorMessage = "Registration failed. Please try again later." diff --git a/api/router.go b/api/router.go index 51a13c7..59327ad 100644 --- a/api/router.go +++ b/api/router.go @@ -33,7 +33,7 @@ func NewArticleRouter(container *UseCases) http.Handler { r := chi.NewRouter() articleHandler := article.NewHandler(container.CreateArticleUseCase, container.GetArticleUseCase, container.ListArticlesUseCase, container.DeleteArticleUseCase, container.UpdateArticleUseCase, container.GetUserUseCase) - imageHandler := image.NewHandler(container.CreateImageUseCase, container.GetArticleUseCase, container.UpdateArticleUseCase) + imageHandler := image.NewHandler(container.CreateImageUseCase, container.GetArticleUseCase, container.UpdateArticleUseCase, container.DeleteImageUseCase) r.Group(func(r chi.Router) { r.Get("/", articleHandler.ListArticles) @@ -49,6 +49,7 @@ func NewArticleRouter(container *UseCases) http.Handler { r.Patch("/{id}/publish", articleHandler.PublishArticle) r.Patch("/{id}/unpublish", articleHandler.UnpublishArticle) r.Post("/{id}/image", imageHandler.PostImage) + r.Delete("/{id}/image/{imageId}", imageHandler.DeleteImage) }) return r } diff --git a/features/editor_image_removal.feature b/features/editor_image_removal.feature new file mode 100644 index 0000000..00347c4 --- /dev/null +++ b/features/editor_image_removal.feature @@ -0,0 +1,148 @@ +Feature: Removing an inline image from the editor + + As a verified RenewCMS owner writing an article + I want to remove an image I've already inserted into the article body, + either through an explicit delete button or by pressing Backspace/Delete + on the selected image, directly from the editor + So that a mistaken or unwanted image doesn't have to stay in the article + forever, and removing it behaves like every other destructive action in + RenewCMS: it asks for confirmation first, however the user starts it + + Background: + Given docs/design/ has no board or state for removing an inline image; + this feature was added at the user's explicit request ahead of design + coverage, reusing the editor's existing visual language rather than + inventing new components: the trash icon and error color already used + by the article "Delete" button, and the ConfirmDialog component + already used by "Delete this article?" (docs/design/screens/editor.md's + "Delete: confirm" state) + And this slice changes ArticleEditor's exposed insertImage(src, alt) to + insertImage(src, alt, imageId), stamping the inserted node with + the image's numeric id as a data-image-id attribute, persisted in the + saved article body HTML; EditorView passes the id it now receives from + a successful upload (features/remove_article_image.feature's change to + POST .../image's 201 body) through to insertImage + And an image already present in an article's body from before this slice + has no data-image-id attribute and so shows no delete affordance and + is not intercepted by Backspace/Delete, until it is removed and + re-added + And selecting an image node that does carry a data-image-id attribute + shows a small delete button anchored to the image's top-right corner, + icon i-lucide-trash-2, color error, matching the article Delete + button's icon and color; deselecting the image (clicking elsewhere) + hides it + And there are two equivalent triggers for the same removal flow: + activating that delete button, and pressing Backspace or Delete while + the image node is selected (a Tiptap NodeSelection on the image). + Tiptap's native node deletion on Backspace/Delete is intercepted + before it reaches the document (e.g. via a keyboard-shortcut/keydown + handler registered on the Image node or at the editor level, checked + first while an image node is selected) so the key press never removes + the node directly; instead it triggers exactly the same flow as the + button + And either trigger emits ArticleEditor's new "request-remove-image" + event with the node's numeric image id; ArticleEditor does not call + the API or show any dialog itself, mirroring how "request-image" + already only opens ImageUploader from EditorView + And EditorView reacts to "request-remove-image" by opening the existing + ConfirmDialog component with title "Remove this image?", description + "This image will be removed from the article. This can't be undone.", + confirmLabel "Remove image", cancelLabel "Cancel", pendingLabel + "Removing…", tone "danger" — the same component and tone already used + for "Delete this article?" + And ConfirmDialog's action calls DELETE /v1/article/{articleId}/image/ + {imageId} through useApi (not the raw XMLHttpRequest used for upload, + since there is no progress to observe) + And on success (204) the action removes the image node from + ArticleEditor's content via a new exposed removeImage(imageId) method, + which moves useArticleDraft's state to "dirty" the same way any other + body edit does; ConfirmDialog then closes and a toast appears: color + success, icon i-lucide-circle-check, title "Image removed", + description "Save to keep this change." + And on any failure (404 for either the article or the image, 500, or a + network failure) the action throws, so ConfirmDialog shows its own + inline error "Couldn't remove the image, so it's still there. Try + again in a moment." and stays open; the image node is not removed and + the draft's dirty state is unchanged + And a 401 is handled entirely by useApi's global handling + (features/api_client_global_error_handling.feature) and is out of + scope for new scenarios here + And once the confirm dialog is open, its cancel/confirm/failure + behaviour (covered below) is identical regardless of which trigger + opened it, so those scenarios are written once, against the button + trigger, and are not duplicated per trigger + + Scenario: Selecting an inserted image reveals its delete button + Given the user is on /article/12/edit with an image in the body carrying + data-image-id "10" + When the user clicks that image + Then a delete button (trash icon, error color) appears at the image's + top-right corner + When the user clicks elsewhere in the editor + Then the delete button is no longer shown + + Scenario Outline: Either trigger opens the confirm dialog and sends no request yet, without letting Tiptap delete the node itself + Given the image with data-image-id "10" is selected on /article/12/edit + When the user + Then a dialog titled "Remove this image?" opens, with description "This + image will be removed from the article. This can't be undone.", + cancel "Cancel" and confirm "Remove image" + And no request has been sent to DELETE /v1/article/12/image/10 + And the image node is still present in the editor's content (Tiptap's + native node deletion did not run) + + Examples: + | triggers-removal | + | activates the image's delete button | + | presses Backspace with the image node selected | + | presses Delete with the image node selected | + + Scenario: Cancelling the confirm dialog changes nothing + Given the confirm dialog is open for image id 10 on /article/12/edit + When the user activates "Cancel" + Then the dialog closes + And no request has been sent to DELETE /v1/article/12/image/10 + And the image is still present in the editor's content + And the draft's dirty state is unchanged + + Scenario: Confirming removes the image, marks the draft dirty, and shows a success toast + Given the confirm dialog is open for image id 10 on /article/12/edit, + the draft in the Saved state + When the user activates "Remove image" + Then DELETE /v1/article/12/image/10 is sent + When that request resolves 204 + Then the dialog closes + And the image is removed from the editor's content + And SaveStatus reads "Unsaved changes" and the Save button becomes + enabled + And a toast appears: color success, icon i-lucide-circle-check, title + "Image removed", description "Save to keep this change." + + Scenario Outline: A failed removal keeps the image and shows the dialog's inline error + Given the confirm dialog is open for image id 10 on /article/12/edit + When the user activates "Remove image" + And DELETE /v1/article/12/image/10 + Then the dialog stays open, showing the inline error "Couldn't remove + the image, so it's still there. Try again in a moment." + And the image is still present in the editor's content + And the draft's dirty state is unchanged + + Examples: + | failure | + | resolves with a 404 response | + | resolves with a 500 response | + | never reaches the server | + + Scenario: Confirming a removal started by Backspace/Delete behaves exactly like confirming one started by the button + Given the confirm dialog is open for image id 10 on /article/12/edit, + opened by pressing Backspace with the image node selected, the draft + in the Saved state + When the user activates "Remove image" + Then DELETE /v1/article/12/image/10 is sent + When that request resolves 204 + Then the dialog closes + And the image is removed from the editor's content + And SaveStatus reads "Unsaved changes" and the Save button becomes + enabled + And a toast appears: color success, icon i-lucide-circle-check, title + "Image removed", description "Save to keep this change." diff --git a/features/editor_image_removal.qa.md b/features/editor_image_removal.qa.md new file mode 100644 index 0000000..5e2fa90 --- /dev/null +++ b/features/editor_image_removal.qa.md @@ -0,0 +1,88 @@ +# QA: Removing an inline image from the editor + +## Automated (Vitest + Vue Test Utils, `task test:web`) + +Mount EditorPage inside DashboardShell at `/article/12/edit` with the +editor_core.qa.md fixture article (id 12, "How we price a bathroom +renovation", is_online false), GET already resolved 200, and a body +containing ``. Mock `useApi`'s `request` for `DELETE /v1/article/12/image/10` +(fetch-based, not XHR — reuse whatever mock +`api_client_global_error_handling.qa.md` already sets up for `request`). + +1. **Selecting the image reveals the delete button** — click the image + node; assert a delete button (trash icon, error color) renders anchored + to it; click elsewhere in the editor; assert the button is gone. +2. **Activating the delete button opens the confirm dialog** — select the + image, activate its delete button; assert a dialog titled "Remove this + image?", description "This image will be removed from the article. This + can't be undone.", cancel "Cancel", confirm "Remove image"; assert no + request was sent yet; assert the image node is still present (Tiptap's + native deletion did not run). +3. **Backspace on the selected image opens the same confirm dialog instead + of deleting the node** — select the image node (NodeSelection), fire a + `keydown` for `Backspace`; assert the same dialog as step 2 opens; + assert no request was sent yet; assert the image node is still present + in the editor's document (i.e. the keydown was intercepted before + Tiptap's default node-deletion ran, not just undone afterwards). +4. **Delete on the selected image opens the same confirm dialog instead of + deleting the node** — same as step 3 but firing `keydown` for `Delete`. +5. **Cancelling changes nothing** — from the open dialog (opened via the + button), click "Cancel"; assert the dialog closes, no request was sent, + the image node is still in the editor's content, and SaveStatus/dirty + state is unchanged. +6. **Confirming removes the image, marks dirty, and toasts (button + trigger)** — from the open dialog (Saved state), click "Remove image"; + assert a DELETE request to `/v1/article/12/image/10` was made; resolve + it 204; assert the dialog closes, the image node is gone from the + editor's content, SaveStatus reads "Unsaved changes" and Save is + enabled, and a toast appears with color success, icon + `i-lucide-circle-check`, title "Image removed", description "Save to + keep this change." +7. **Confirming removes the image the same way when opened via + Backspace/Delete** — repeat step 6's assertions, but open the dialog via + the Backspace keydown from step 3 instead of the button; assert the + outcome (request, node removal, dirty state, toast) is identical. +8. **A failed removal keeps the image and shows the inline error, + Examples-driven** — for a 404 response, a 500 response, and a rejected/ + network-failure request: from the open dialog (button trigger), click + "Remove image"; resolve/reject accordingly; assert the dialog stays open + showing the inline error "Couldn't remove the image, so it's still + there. Try again in a moment."; assert the image node is still present + and the draft's dirty state is unchanged. + +## Visual + +No `docs/design/` board or state exists for this trigger (see the feature's +Background). Per the user's explicit direction this slice ships the SPA +behaviour anyway, but `task visual:check -- editor` has nothing to compare +it against, so no visual capture is added for it — this is an intentional +gap, `skip: no-design-coverage` (distinct from `skip: clock-sensitive`), not +a QA oversight. If a board for this state is added to `docs/design/` later, +it needs its own follow-up slice to add the visual check. + +- [ ] Keyboard/focus: once a delete button is shown, it is reachable by Tab + and activates on Enter/Space; pressing Backspace/Delete with the + image node selected but focus elsewhere in the page does not trigger + removal (only an actual Tiptap NodeSelection on the image should); + ConfirmDialog's existing focus trap and cancel-button-default-focus + behaviour (already covered by the article-delete flow) applies + unchanged regardless of which trigger opened it. + +## Manual verification + +- **Real DELETE round trip against a running server** — Vitest only mocks + `useApi`'s request; confirm a real removal (204/404/500) against + `task dev` with a live SQLite file and the real + `DELETE /v1/article/{id}/image/{imageId}` handler, since + `features/remove_article_image.feature`'s httptest suite covers the + handler in isolation but never through this SPA flow. +- **Real keyboard behaviour** — confirm in a real browser that selecting an + image (click) and pressing Backspace/Delete opens the confirm dialog + rather than deleting the node, including with an IME or on a platform + where Backspace/Delete semantics differ, since jsdom's `keydown` + simulation doesn't exercise Tiptap's real ProseMirror keymap precedence. +- **Design review of the delete-button/ConfirmDialog approach** — since + there is no board for this state, once a board is added, re-verify this + implementation still matches it (or update the implementation to match); + until then this is a best-effort, design-team-unreviewed UI. diff --git a/features/remove_article_image.feature b/features/remove_article_image.feature new file mode 100644 index 0000000..510268e --- /dev/null +++ b/features/remove_article_image.feature @@ -0,0 +1,99 @@ +Feature: Removing an image from an article + + As an authenticated RenewCMS user managing the CMS + I want to delete an already-uploaded image by its id + So that a removed image's file and row are actually gone, and a delete on + an unknown article or an unknown image answers 404 the same way every + other article write does + + Background: + Given internal/infrastructure/useCases.DeleteImageUseCase.DeleteImage(imageId) + already exists and calls image.Repository.Delete(imageId), which returns + image.ErrImageNotFound for an unknown id and, on the real ImageRepository, + also removes the uploaded file from UPLOAD_DIR, per + features/delete_article_cascades_images.feature + And this slice changes POST /v1/article/{id}/image's success response + (api/handlers/image.Handler.PostImage, features/upload_article_image.feature) + to also return the created image's numeric id, so the 201 body becomes + {"id": ..., "location": ...}; this lets a client address the image later + through this slice's DELETE route + And this slice mounts DELETE /v1/article/{articleId}/image/{imageId} in + api/router.go, in the same jwtauth Verifier + Authenticator group as + POST .../image + And this slice adds a DeleteImage method to api/handlers/image.Handler, + grows NewHandler's signature to also take DeleteImageUseCase, and wires + it in api/router.go + And DeleteImage parses articleId and imageId from the URL, answering 400 + with apperr.IdUint32ErrorMessage if either isn't a valid uint32 + And DeleteImage first calls GetArticleUseCase.GetArticle(articleId); + errors.Is(err, article.ErrArticleNotFound) answers 404 with + apperr.ArticleNotFoundErrorMessage; this only confirms the article id + in the URL names a real article, and does not verify that the image id + belongs to that article's Images slice (a shallow existence check on + both ids, not an ownership check) + And DeleteImage then calls DeleteImageUseCase.DeleteImage(imageId); + errors.Is(err, image.ErrImageNotFound) answers 404 with the new + apperr.ImageNotFoundErrorMessage = "Image not found"; any other error + falls back to the existing generic 500 + And on success DeleteImage answers 204 with an empty body, the same shape + as DELETE /v1/article/{id} + + Scenario: Removing an image from an article it belongs to succeeds + Given a valid jwt cookie for a user with id 1 + And the article repository contains an article with id 1 titled + "Backyard Renovation Update" whose Images slice holds image id 10 + And the image repository contains an image with id 10 + When the client sends DELETE /v1/article/1/image/10 with that cookie + Then the response status is 204 + And the response body is empty + And the image repository no longer contains an image with id 10 + + Scenario: Removing an image from an unknown article answers 404 and never touches the image repository + Given a valid jwt cookie for a user with id 1 + And the article repository has no article with id 9999 + And the image repository contains an image with id 10 + When the client sends DELETE /v1/article/9999/image/10 with that cookie + Then the response status is 404 + And the response body is the text "Article not found" + And the image repository's Delete is never called + And the image repository still contains the image with id 10 + + Scenario: Removing an unknown image from an existing article answers 404 + Given a valid jwt cookie for a user with id 1 + And the article repository contains an article with id 1 titled + "Backyard Renovation Update" + And the image repository has no image with id 9999 + When the client sends DELETE /v1/article/1/image/9999 with that cookie + Then the response status is 404 + And the response body is the text "Image not found" + + Scenario: Removing an image id that belongs to a different article still succeeds, because the check is shallow + Given a valid jwt cookie for a user with id 1 + And the article repository contains an article with id 1 titled + "Backyard Renovation Update" whose Images slice is empty + And the article repository contains a second article with id 2 whose + Images slice holds image id 10 + And the image repository contains an image with id 10 + When the client sends DELETE /v1/article/1/image/10 with that cookie + Then the response status is 204 + And the image repository no longer contains an image with id 10 + + Scenario: Removing an image still requires authentication + Given no jwt cookie is sent + And the article repository contains an article with id 1 titled + "Backyard Renovation Update" whose Images slice holds image id 10 + And the image repository contains an image with id 10 + When the client sends DELETE /v1/article/1/image/10 without a cookie + Then the response status is 401 + And the image repository still contains the image with id 10 + + Scenario Outline: A malformed id in the URL answers 400 before either repository is touched + Given a valid jwt cookie for a user with id 1 + When the client sends DELETE /v1/article//image/ with that cookie + Then the response status is 400 + And the response body is the text "The server expects the ID to be in the format of an unsigned 32-bit integer (uint32)." + + Examples: + | article_id | image_id | + | abc | 10 | + | 1 | abc | diff --git a/features/remove_article_image.qa.md b/features/remove_article_image.qa.md new file mode 100644 index 0000000..2933a58 --- /dev/null +++ b/features/remove_article_image.qa.md @@ -0,0 +1,56 @@ +# QA procedure: remove_article_image + +Package: `test` (black-box, extending `article_test.go` / `image_test.go`). + +## Setup shared by every case +1. Call `validator.InitValidator()` once. +2. Reuse `mockArticleRepo` and `mockImageRepo` as-is. +3. Wire `useCases.NewDeleteImageUseCase(imageRepo)` (already used by + `delete_article_cascades_images.qa.md`'s suite) alongside the existing + use cases. +4. Build `image.NewHandler(...)` with its grown constructor including the + delete use case, matching whatever parameter order the coder settles on. +5. Build a test `jwtauth.New("HS256", []byte("test-secret"), nil)`. +6. Mount `DELETE /{id}/image/{imageId}` alongside the existing article + routes under the authenticated group, mirroring `api/router.go` after + this slice. +7. Mint a valid jwt cookie for user id 1 as in `upload_article_image.qa.md`. + +## HTTP checks +8. Seed `articleRepo.articles[1]` with `Images: []*image.Image{{ID: 10}}` + and `imageRepo.images[10] = image.Image{ID: 10}`. Send + `DELETE /v1/article/1/image/10` with the valid cookie. Assert status + 204, empty body; assert `imageRepo.images` no longer has key 10. +9. Empty `articleRepo`; seed `imageRepo.images[10]`. Send + `DELETE /v1/article/9999/image/10`. Assert 404, body + `"Article not found\n"`; assert `imageRepo.images` still has key 10 + (Delete never called — track via a call-count field on the fake, the + same way the existing fakes already track `Create`). +10. Seed `articleRepo.articles[1]`; empty `imageRepo`. Send + `DELETE /v1/article/1/image/9999`. Assert 404, body + `"Image not found\n"`. +11. Seed `articleRepo.articles[1]` (empty Images) and `articleRepo.articles[2]` + with `Images: []*image.Image{{ID: 10}}`; seed `imageRepo.images[10]`. + Send `DELETE /v1/article/1/image/10`. Assert 204; assert + `imageRepo.images` no longer has key 10 — asserts the shallow-check + contract from the feature's fourth scenario. +12. Seed as in step 8. Send `DELETE /v1/article/1/image/10` with **no** + cookie. Assert 401; assert `imageRepo.images` still has key 10. +13. Examples-driven: send `DELETE /v1/article/abc/image/10` and + `DELETE /v1/article/1/image/abc` with the valid cookie. Assert 400, + body `"The server expects the ID to be in the format of an unsigned + 32-bit integer (uint32).\n"` for each. + +## Also touched by this slice: upload_article_image's 201 body +14. Extend `upload_article_image.qa.md`'s existing 201-body assertion to + also decode and check the `id` field equals the newly created image's + `ID` in `imageRepo.images`. + +## Manual verification (not exercised by the Go test) +- Real DELETE against a real SQLite file and real UPLOAD_DIR: confirm the + file is actually removed from disk and the `article_images` join row is + gone, the same way `upload_article_image.qa.md`'s manual item covers the + POST side. +- SPA-visible behaviour: covered separately by + `features/editor_image_removal.qa.md`. +- No SMTP involvement. diff --git a/features/upload_article_image.feature b/features/upload_article_image.feature index ef83d83..fb7a0af 100644 --- a/features/upload_article_image.feature +++ b/features/upload_article_image.feature @@ -37,9 +37,12 @@ Feature: Uploading an image to an article back to a generic 500 And PostImage reads the uploaded file from the multipart form field "file" - And on success PostImage answers 201 with a JSON body {"location": ...} - holding the created image's public path, e.g. - "/static/uploadedImages/" + And on success PostImage answers 201 with a JSON body + {"id": ..., "location": ...} holding the created image's numeric id + and public path, e.g. "/static/uploadedImages/" + And this slice (features/remove_article_image.feature) adds the "id" + field to that body; the id lets a client later address the image + through DELETE /v1/article/{id}/image/{imageId} Scenario: Uploading an allowed image type to an existing article succeeds Given a valid jwt cookie for a user with id 1 @@ -51,6 +54,8 @@ Feature: Uploading an image to an article Then the response status is 201 And the response body is JSON with a "location" field whose value is "/static/uploadedImages/before.png" + And the response body's "id" field equals the new image's id in the + image repository And the image repository contains a new image whose Path is "/static/uploadedImages/before.png" And the article repository's article with id 1 has that new image's id diff --git a/test/remove_article_image_http_test.go b/test/remove_article_image_http_test.go new file mode 100644 index 0000000..a46a42a --- /dev/null +++ b/test/remove_article_image_http_test.go @@ -0,0 +1,196 @@ +package test + +import ( + "RenewCMS/api/handlers/image" + "RenewCMS/api/middleware" + "RenewCMS/api/pkg/apperr" + validator "RenewCMS/api/pkg/validator" + domainArticle "RenewCMS/internal/domain/article" + domainImage "RenewCMS/internal/domain/image" + "RenewCMS/internal/infrastructure/useCases" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/go-chi/jwtauth/v5" +) + +func newRemoveArticleImageRouter(articleRepo *mockArticleRepo, imageRepo *mockImageRepo, token *jwtauth.JWTAuth) *chi.Mux { + userRepo := newVerifiedUserRepo() + getUserUseCase := useCases.NewGetUserUseCase(userRepo) + + imageHandler := image.NewHandler( + useCases.NewCreateImageUseCase(imageRepo), + useCases.NewGetArticleUseCase(articleRepo), + useCases.NewUpdateArticleUseCase(articleRepo), + useCases.NewDeleteImageUseCase(imageRepo), + ) + + r := chi.NewRouter() + r.Route("/v1", func(r chi.Router) { + r.Route("/article", func(r chi.Router) { + r.Group(func(r chi.Router) { + r.Use(jwtauth.Verifier(token)) + r.Group(func(r chi.Router) { + r.Use(jwtauth.Authenticator(token)) + r.Use(middleware.RequireVerifiedUser(getUserUseCase)) + r.Delete("/{id}/image/{imageId}", imageHandler.DeleteImage) + }) + }) + }) + }) + return r +} + +func setRemoveImageJwtCookie(t *testing.T, req *http.Request, token *jwtauth.JWTAuth) { + t.Helper() + _, tokenString, err := token.Encode(map[string]any{"user_id": uint32(1)}) + if err != nil { + t.Fatalf("failed to encode jwt: %v", err) + } + req.AddCookie(&http.Cookie{Name: "jwt", Value: tokenString}) +} + +func TestRemoveArticleImageHttp(t *testing.T) { + validator.InitValidator() + token := jwtauth.New("HS256", []byte("test-secret"), nil) + + t.Run("Removing an image from an article it belongs to succeeds", func(t *testing.T) { + articleRepo := newMockArticleRepo() + articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update", Images: []*domainImage.Image{{ID: 10}}} + imageRepo := newMockImageRepo() + imageRepo.images[10] = domainImage.Image{ID: 10, Path: "/static/uploadedImages/before.png"} + router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) + + req := httptest.NewRequest(http.MethodDelete, "/v1/article/1/image/10", nil) + setRemoveImageJwtCookie(t, req, token) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusNoContent { + t.Fatalf("expected status 204, got %d, body %q", rec.Code, rec.Body.String()) + } + if rec.Body.Len() != 0 { + t.Errorf("expected empty body, got %q", rec.Body.String()) + } + if _, ok := imageRepo.images[10]; ok { + t.Errorf("expected image 10 to have been removed") + } + }) + + t.Run("Removing an image from an unknown article answers 404 and never touches the image repository", func(t *testing.T) { + articleRepo := newMockArticleRepo() + imageRepo := newMockImageRepo() + imageRepo.images[10] = domainImage.Image{ID: 10, Path: "/static/uploadedImages/before.png"} + router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) + + req := httptest.NewRequest(http.MethodDelete, "/v1/article/9999/image/10", nil) + setRemoveImageJwtCookie(t, req, token) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("expected status 404, got %d", rec.Code) + } + if rec.Body.String() != apperr.ArticleNotFoundErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + if len(imageRepo.deleteCalls) != 0 { + t.Errorf("expected Delete to never be called, got %d calls", len(imageRepo.deleteCalls)) + } + if _, ok := imageRepo.images[10]; !ok { + t.Errorf("expected image 10 to still exist") + } + }) + + t.Run("Removing an unknown image from an existing article answers 404", func(t *testing.T) { + articleRepo := newMockArticleRepo() + articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} + imageRepo := newMockImageRepo() + router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) + + req := httptest.NewRequest(http.MethodDelete, "/v1/article/1/image/9999", nil) + setRemoveImageJwtCookie(t, req, token) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("expected status 404, got %d", rec.Code) + } + if rec.Body.String() != apperr.ImageNotFoundErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + + t.Run("Removing an image id that belongs to a different article still succeeds, because the check is shallow", func(t *testing.T) { + articleRepo := newMockArticleRepo() + articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update"} + articleRepo.articles[2] = domainArticle.Article{ID: 2, Images: []*domainImage.Image{{ID: 10}}} + imageRepo := newMockImageRepo() + imageRepo.images[10] = domainImage.Image{ID: 10, Path: "/static/uploadedImages/before.png"} + router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) + + req := httptest.NewRequest(http.MethodDelete, "/v1/article/1/image/10", nil) + setRemoveImageJwtCookie(t, req, token) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusNoContent { + t.Fatalf("expected status 204, got %d, body %q", rec.Code, rec.Body.String()) + } + if _, ok := imageRepo.images[10]; ok { + t.Errorf("expected image 10 to have been removed") + } + }) + + t.Run("Removing an image still requires authentication", func(t *testing.T) { + articleRepo := newMockArticleRepo() + articleRepo.articles[1] = domainArticle.Article{ID: 1, Title: "Backyard Renovation Update", Images: []*domainImage.Image{{ID: 10}}} + imageRepo := newMockImageRepo() + imageRepo.images[10] = domainImage.Image{ID: 10, Path: "/static/uploadedImages/before.png"} + router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) + + req := httptest.NewRequest(http.MethodDelete, "/v1/article/1/image/10", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected status 401, got %d", rec.Code) + } + if _, ok := imageRepo.images[10]; !ok { + t.Errorf("expected image 10 to still exist") + } + }) + + t.Run("A malformed id in the URL answers 400 before either repository is touched", func(t *testing.T) { + testCases := []struct { + name string + articleId string + imageId string + }{ + {"malformed article id", "abc", "10"}, + {"malformed image id", "1", "abc"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + articleRepo := newMockArticleRepo() + imageRepo := newMockImageRepo() + router := newRemoveArticleImageRouter(articleRepo, imageRepo, token) + + req := httptest.NewRequest(http.MethodDelete, "/v1/article/"+tc.articleId+"/image/"+tc.imageId, nil) + setRemoveImageJwtCookie(t, req, token) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected status 400, got %d", rec.Code) + } + if rec.Body.String() != apperr.IdUint32ErrorMessage+"\n" { + t.Errorf("unexpected body: %q", rec.Body.String()) + } + }) + } + }) +} diff --git a/test/upload_article_image_http_test.go b/test/upload_article_image_http_test.go index b908806..735c010 100644 --- a/test/upload_article_image_http_test.go +++ b/test/upload_article_image_http_test.go @@ -29,6 +29,7 @@ func newUploadArticleImageRouter(articleRepo *mockArticleRepo, imageRepo *mockIm useCases.NewCreateImageUseCase(imageRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), + useCases.NewDeleteImageUseCase(imageRepo), ) r := chi.NewRouter() @@ -98,7 +99,7 @@ func TestUploadArticleImageHttp(t *testing.T) { t.Fatalf("expected status 201, got %d, body %q", rec.Code, rec.Body.String()) } - var got map[string]string + var got map[string]any if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { t.Fatalf("failed to decode response: %v", err) } @@ -120,6 +121,10 @@ func TestUploadArticleImageHttp(t *testing.T) { if !found { t.Fatalf("expected the image repository to contain the new image") } + gotId, ok := got["id"].(float64) + if !ok || uint32(gotId) != storedImageID { + t.Errorf("expected id %v, got %v", storedImageID, got["id"]) + } updatedArticle := articleRepo.articles[1] imageAttached := false diff --git a/test/verified_account_required_http_test.go b/test/verified_account_required_http_test.go index 8b066d2..7ae340e 100644 --- a/test/verified_account_required_http_test.go +++ b/test/verified_account_required_http_test.go @@ -35,6 +35,7 @@ func newVerifiedAccountRequiredRouter(articleRepo *mockArticleRepo, userRepo *mo useCases.NewCreateImageUseCase(imageRepo), useCases.NewGetArticleUseCase(articleRepo), useCases.NewUpdateArticleUseCase(articleRepo), + useCases.NewDeleteImageUseCase(imageRepo), ) r := chi.NewRouter() diff --git a/web/bun.lock b/web/bun.lock index acddd03..ac5601d 100644 --- a/web/bun.lock +++ b/web/bun.lock @@ -9,6 +9,9 @@ "@tailwindcss/vite": "^4.1.18", "@tiptap/core": "3.31.3", "@tiptap/extension-drag-handle-vue-3": "^3.31.3", + "@tiptap/extension-image": "3.31.3", + "@tiptap/pm": "3.31.3", + "@tiptap/vue-3": "3.31.3", "@vueuse/core": "14.4.0", "pinia": "^3.0.4", "shiki": "^4.4.3", diff --git a/web/package.json b/web/package.json index d6d4143..29cbc8f 100644 --- a/web/package.json +++ b/web/package.json @@ -20,6 +20,9 @@ "@tailwindcss/vite": "^4.1.18", "@tiptap/core": "3.31.3", "@tiptap/extension-drag-handle-vue-3": "^3.31.3", + "@tiptap/extension-image": "3.31.3", + "@tiptap/pm": "3.31.3", + "@tiptap/vue-3": "3.31.3", "@vueuse/core": "14.4.0", "pinia": "^3.0.4", "shiki": "^4.4.3", diff --git a/web/src/components/ArticleEditor.vue b/web/src/components/ArticleEditor.vue index d4c70c3..031a60c 100644 --- a/web/src/components/ArticleEditor.vue +++ b/web/src/components/ArticleEditor.vue @@ -3,6 +3,7 @@ import { ref } from 'vue' import type { EditorToolbarItem } from '@nuxt/ui' import type { Editor } from '@tiptap/core' import { ArticleIndent } from '@/lib/articleIndent' +import { ArticleImage, findArticleImagePos } from '@/lib/articleImage' const props = withDefaults( defineProps<{ @@ -18,10 +19,16 @@ const props = withDefaults( const emit = defineEmits<{ 'update:modelValue': [value: string] 'request-image': [] + 'request-remove-image': [imageId: number] }>() const editorInstance = ref(null) -const extensions = [ArticleIndent] +const extensions = [ + ArticleIndent, + ArticleImage.configure({ + onRequestRemove: (imageId: number) => emit('request-remove-image', imageId), + }), +] function captureEditor(editor: Editor) { editorInstance.value = editor @@ -198,8 +205,22 @@ defineExpose({ focus() { editorInstance.value?.commands.focus() }, - insertImage(src: string, alt: string) { - editorInstance.value?.commands.setImage({ src, alt }) + insertImage(src: string, alt: string, imageId: number) { + editorInstance.value?.commands.setArticleImage({ src, alt, imageId }) + }, + removeImage(imageId: number) { + const editor = editorInstance.value + if (!editor) { + return + } + const pos = findArticleImagePos(editor, imageId) + if (pos === null) { + return + } + editor + .chain() + .deleteRange({ from: pos, to: pos + 1 }) + .run() }, }) @@ -209,6 +230,7 @@ defineExpose({ v-slot="{ editor }" :model-value="props.modelValue" :extensions="extensions" + :image="false" content-type="html" :editable="!props.disabled" :ui="{ base: 'article-content' }" diff --git a/web/src/components/ArticleEditorImageNode.vue b/web/src/components/ArticleEditorImageNode.vue new file mode 100644 index 0000000..6128743 --- /dev/null +++ b/web/src/components/ArticleEditorImageNode.vue @@ -0,0 +1,53 @@ + + + diff --git a/web/src/components/ImageUploader.vue b/web/src/components/ImageUploader.vue index 3a4458d..6fa5e68 100644 --- a/web/src/components/ImageUploader.vue +++ b/web/src/components/ImageUploader.vue @@ -41,7 +41,7 @@ const props = defineProps<{ }>() const emit = defineEmits<{ - uploaded: [location: string] + uploaded: [location: string, imageId: number] close: [] }>() @@ -167,7 +167,7 @@ async function runUpload(file: File) { const outcome = await start(props.articleId, file) if (outcome.kind === 'success') { succeeded.value = true - emit('uploaded', outcome.location) + emit('uploaded', outcome.location, outcome.id) closeTimer = setTimeout(() => { open.value = false }, 600) diff --git a/web/src/components/__tests__/ArticleEditor.spec.ts b/web/src/components/__tests__/ArticleEditor.spec.ts new file mode 100644 index 0000000..04da746 --- /dev/null +++ b/web/src/components/__tests__/ArticleEditor.spec.ts @@ -0,0 +1,108 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { mount, flushPromises } from '@vue/test-utils' +import { createPinia, setActivePinia } from 'pinia' +import uiPlugin from '@nuxt/ui/vue-plugin' +import router from '@/router' +import { useSessionStore } from '@/stores/session' +import RootComponent from '@/App.vue' +import ArticleEditor from '@/components/ArticleEditor.vue' + +vi.mock('@nuxt/ui/composables', () => ({ + useOverlay: () => ({ create: () => ({ open: () => Promise.resolve(false) }) }), + useToast: () => ({ add: vi.fn() }), +})) + +function mockResponse(status: number, body: unknown = null) { + return { + ok: status >= 200 && status < 300, + status, + headers: { get: () => null }, + json: async () => body, + } as unknown as Response +} + +let currentWrapper: Awaited> | null = null + +async function mountArticleEditor(body = '

Hello

') { + setActivePinia(createPinia()) + const session = useSessionStore() + session.session = { verified: true, username: 'marta' } + session.status = 'ready' + global.fetch = vi.fn(() => + Promise.resolve( + mockResponse(200, { + id: 12, + title: 'T', + body, + images: [], + is_online: false, + created_at: '2026-01-01T00:00:00Z', + updated_at: '2026-01-01T00:00:00Z', + }), + ), + ) as unknown as typeof fetch + await router.push('/article/12/edit') + const wrapper = mount(RootComponent, { + attachTo: document.body, + global: { plugins: [router, uiPlugin] }, + }) + currentWrapper = wrapper + await flushPromises() + return wrapper +} + +afterEach(async () => { + currentWrapper?.unmount() + currentWrapper = null + await router.push('/login') +}) + +describe('insertImage stamps the inserted image with its numeric id', () => { + it('renders data-image-id on the img and emits the updated body', async () => { + const wrapper = await mountArticleEditor() + const editor = wrapper.findComponent(ArticleEditor) + + ;( + editor.vm as unknown as { insertImage: (src: string, alt: string, id: number) => void } + ).insertImage('/static/uploadedImages/photo.png', 'alt text', 10) + await flushPromises() + + expect(editor.props('modelValue')).toContain( + 'alt text', + ) + }) +}) + +describe('removeImage removes only the node whose data-image-id matches', () => { + it('deletes the matching image node and leaves the rest of the content', async () => { + const wrapper = await mountArticleEditor() + const editor = wrapper.findComponent(ArticleEditor) + const vm = editor.vm as unknown as { + insertImage: (src: string, alt: string, id: number) => void + removeImage: (id: number) => void + } + + vm.insertImage('/static/uploadedImages/a.png', '', 10) + await flushPromises() + vm.insertImage('/static/uploadedImages/b.png', '', 11) + await flushPromises() + + vm.removeImage(10) + await flushPromises() + + const body = editor.props('modelValue') as string + expect(body).not.toContain('data-image-id="10"') + expect(body).toContain('data-image-id="11"') + }) + + it('does nothing when no image carries the given id', async () => { + const wrapper = await mountArticleEditor('

Hello

') + const editor = wrapper.findComponent(ArticleEditor) + const vm = editor.vm as unknown as { removeImage: (id: number) => void } + + vm.removeImage(999) + await flushPromises() + + expect(editor.emitted('update:modelValue')).toBeFalsy() + }) +}) diff --git a/web/src/components/__tests__/ImageUploader.spec.ts b/web/src/components/__tests__/ImageUploader.spec.ts index 2917efe..055b40d 100644 --- a/web/src/components/__tests__/ImageUploader.spec.ts +++ b/web/src/components/__tests__/ImageUploader.spec.ts @@ -112,7 +112,7 @@ const Host = defineComponent({ const open = ref(false) return { open, - onUploaded: (location: string) => emit('uploaded', location), + onUploaded: (location: string, imageId: number) => emit('uploaded', location, imageId), onClose: () => emit('close'), } }, @@ -309,10 +309,13 @@ describe('A successful upload emits the location and auto-closes 600 ms later', await chooseRasterFileAndConfirmUpload(fileFixture('north-wall.jpg', 'image/jpeg', 3.9)) const xhr = latestXHR() - xhr.resolve(201, JSON.stringify({ location: '/static/uploadedImages/north-wall.jpg' })) + xhr.resolve( + 201, + JSON.stringify({ id: 7, location: '/static/uploadedImages/north-wall.jpg' }), + ) await vi.advanceTimersByTimeAsync(0) - expect(wrapper.emitted('uploaded')).toEqual([['/static/uploadedImages/north-wall.jpg']]) + expect(wrapper.emitted('uploaded')).toEqual([['/static/uploadedImages/north-wall.jpg', 7]]) expect(document.body.textContent).toContain('Image added') expect(document.querySelector('[role="dialog"]')).toBeTruthy() diff --git a/web/src/composables/useImageUpload.ts b/web/src/composables/useImageUpload.ts index 6271f63..5c94e99 100644 --- a/web/src/composables/useImageUpload.ts +++ b/web/src/composables/useImageUpload.ts @@ -6,7 +6,7 @@ export interface UploadProgress { } export type UploadOutcome = - | { kind: 'success'; location: string } + | { kind: 'success'; location: string; id: number } | { kind: 'rejected'; body: string } | { kind: 'failed' } @@ -32,8 +32,11 @@ export function useImageUpload() { request.addEventListener('load', () => { activeRequest = null if (request.status === 201) { - const body = JSON.parse(request.responseText) as { location: string } - resolve({ kind: 'success', location: body.location }) + const body = JSON.parse(request.responseText) as { + id: number + location: string + } + resolve({ kind: 'success', location: body.location, id: body.id }) return } if (request.status === 400) { diff --git a/web/src/lib/__tests__/articleImage.spec.ts b/web/src/lib/__tests__/articleImage.spec.ts new file mode 100644 index 0000000..ef4035f --- /dev/null +++ b/web/src/lib/__tests__/articleImage.spec.ts @@ -0,0 +1,248 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { defineComponent } from 'vue' +import { mount } from '@vue/test-utils' +import { Editor, EditorContent } from '@tiptap/vue-3' +import StarterKit from '@tiptap/starter-kit' +import { ArticleImage, findArticleImagePos } from '../articleImage' + +function mountHarness(onRequestRemove = vi.fn()) { + let editor: Editor + const Harness = defineComponent({ + components: { EditorContent }, + setup() { + editor = new Editor({ + extensions: [StarterKit, ArticleImage.configure({ onRequestRemove })], + content: '

Hello

', + }) + return { editor } + }, + template: '', + }) + const wrapper = mount(Harness, { attachTo: document.body }) + return { wrapper, onRequestRemove, getEditor: () => editor! } +} + +function deleteButton() { + return document.body.querySelector('[data-testid="remove-image-button"]') +} + +function flush() { + return new Promise((resolve) => setTimeout(resolve, 20)) +} + +function dispatchKeydown(editor: Editor, key: string) { + editor.view.dom.dispatchEvent( + new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true }), + ) +} + +let activeWrapper: ReturnType['wrapper'] | null = null + +afterEach(() => { + activeWrapper?.unmount() + activeWrapper = null +}) + +describe('Selecting an inserted image reveals its delete button', () => { + it('shows the delete button when selected and hides it once deselected', async () => { + const { wrapper, getEditor } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setArticleImage({ src: 'photo.png', alt: '', imageId: 10 }) + const pos = findArticleImagePos(editor, 10) as number + + editor.commands.setNodeSelection(pos) + await flush() + expect(deleteButton()).toBeTruthy() + + editor.commands.setTextSelection(0) + await flush() + expect(deleteButton()).toBeFalsy() + }) +}) + +describe('An image with no data-image-id gets no delete affordance', () => { + it('renders no delete button even when the node is selected', async () => { + const { wrapper, getEditor } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setImage({ src: 'legacy.png', alt: '' }) + let pos: number | null = null + editor.state.doc.descendants((node, nodePos) => { + if (node.type.name === 'image') { + pos = nodePos + } + }) + + editor.commands.setNodeSelection(pos as number) + await flush() + + expect(deleteButton()).toBeFalsy() + }) +}) + +describe('Activating the delete button triggers removal without deleting the node', () => { + it('calls onRequestRemove with the image id and leaves the node in place', async () => { + const { wrapper, getEditor, onRequestRemove } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setArticleImage({ src: 'photo.png', alt: '', imageId: 10 }) + const pos = findArticleImagePos(editor, 10) as number + editor.commands.setNodeSelection(pos) + await flush() + + deleteButton()?.click() + + expect(onRequestRemove).toHaveBeenCalledWith(10) + expect(findArticleImagePos(editor, 10)).not.toBeNull() + }) +}) + +describe('Pressing Backspace on a selected image triggers removal without deleting the node', () => { + it('calls onRequestRemove and leaves the image node in the document', () => { + const { wrapper, getEditor, onRequestRemove } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setArticleImage({ src: 'photo.png', alt: '', imageId: 10 }) + const pos = findArticleImagePos(editor, 10) as number + editor.commands.setNodeSelection(pos) + + dispatchKeydown(editor, 'Backspace') + + expect(onRequestRemove).toHaveBeenCalledWith(10) + expect(findArticleImagePos(editor, 10)).not.toBeNull() + }) +}) + +describe('Pressing Delete on a selected image triggers removal without deleting the node', () => { + it('calls onRequestRemove and leaves the image node in the document', () => { + const { wrapper, getEditor, onRequestRemove } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setArticleImage({ src: 'photo.png', alt: '', imageId: 10 }) + const pos = findArticleImagePos(editor, 10) as number + editor.commands.setNodeSelection(pos) + + dispatchKeydown(editor, 'Delete') + + expect(onRequestRemove).toHaveBeenCalledWith(10) + expect(findArticleImagePos(editor, 10)).not.toBeNull() + }) +}) + +describe('Backspace on a pre-existing image with no data-image-id is not intercepted', () => { + it('does not call onRequestRemove, letting native deletion proceed', () => { + const { wrapper, getEditor, onRequestRemove } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setImage({ src: 'legacy.png', alt: '' }) + let pos: number | null = null + editor.state.doc.descendants((node, nodePos) => { + if (node.type.name === 'image') { + pos = nodePos + } + }) + editor.commands.setNodeSelection(pos as number) + + dispatchKeydown(editor, 'Backspace') + + expect(onRequestRemove).not.toHaveBeenCalled() + }) +}) + +describe('The custom image node view renders as a block-level element', () => { + it('uses a div wrapper so consecutive images stack vertically', async () => { + const { wrapper, getEditor } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setArticleImage({ src: 'photo.png', alt: '', imageId: 10 }) + await flush() + + const node = document.body.querySelector( + '[data-testid="article-editor-image"]', + ) + expect(node?.tagName).toBe('DIV') + expect(node ? getComputedStyle(node).display : null).toBe('block') + }) +}) + +describe('Selecting an image shows a visible selection outline', () => { + it('applies the selected outline classes to the wrapper when the node is selected', async () => { + const { wrapper, getEditor } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setArticleImage({ src: 'photo.png', alt: '', imageId: 10 }) + const pos = findArticleImagePos(editor, 10) as number + + editor.commands.setNodeSelection(pos) + await flush() + + const node = document.body.querySelector( + '[data-testid="article-editor-image"]', + ) + expect(node?.classList.contains('outline-primary')).toBe(true) + }) +}) + +describe('The custom image node view preserves width, height and title attributes', () => { + it('renders width, height and title on the img element', async () => { + const { wrapper, getEditor } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.insertContent({ + type: 'image', + attrs: { + src: 'photo.png', + alt: 'alt text', + title: 'a title', + width: 320, + height: 240, + 'data-image-id': 10, + }, + }) + await flush() + + const img = document.body.querySelector( + '[data-testid="article-editor-image"] img', + ) + expect(img?.getAttribute('width')).toBe('320') + expect(img?.getAttribute('height')).toBe('240') + expect(img?.getAttribute('title')).toBe('a title') + }) +}) + +describe('The image node is not natively draggable', () => { + it('disables native dragging on the img element', async () => { + const { wrapper, getEditor } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setArticleImage({ src: 'photo.png', alt: '', imageId: 10 }) + await flush() + + const node = document.body.querySelector( + '[data-testid="article-editor-image"]', + ) + const img = node?.querySelector('img') + expect(img?.getAttribute('draggable')).toBe('false') + }) +}) + +describe('removeImage removes the matching image node from the document', () => { + it('deletes only the node whose data-image-id matches', () => { + const { wrapper, getEditor } = mountHarness() + activeWrapper = wrapper + const editor = getEditor() + editor.commands.setArticleImage({ src: 'a.png', alt: '', imageId: 10 }) + editor.commands.setArticleImage({ src: 'b.png', alt: '', imageId: 11 }) + + const pos = findArticleImagePos(editor, 10) as number + editor + .chain() + .focus() + .deleteRange({ from: pos, to: pos + 1 }) + .run() + + expect(findArticleImagePos(editor, 10)).toBeNull() + expect(findArticleImagePos(editor, 11)).not.toBeNull() + }) +}) diff --git a/web/src/lib/articleImage.ts b/web/src/lib/articleImage.ts new file mode 100644 index 0000000..ad6942d --- /dev/null +++ b/web/src/lib/articleImage.ts @@ -0,0 +1,120 @@ +import type { Node as ProseMirrorNode } from '@tiptap/pm/model' +import Image from '@tiptap/extension-image' +import { NodeSelection } from '@tiptap/pm/state' +import { VueNodeViewRenderer } from '@tiptap/vue-3' +import ArticleEditorImageNode from '@/components/ArticleEditorImageNode.vue' + +export interface ArticleImageOptions { + onRequestRemove: (imageId: number) => void +} + +export interface ArticleImageEditorLike { + state: { + selection: unknown + doc: ProseMirrorNode + } +} + +declare module '@tiptap/core' { + interface Commands { + articleImage: { + setArticleImage: (options: { src: string; alt?: string; imageId: number }) => ReturnType + } + } +} + +export function articleImageId(node: { attrs: Record }): number | null { + const raw = node.attrs['data-image-id'] + if (raw === null || raw === undefined || raw === '') { + return null + } + const id = Number(raw) + return Number.isNaN(id) ? null : id +} + +export function selectedArticleImageId(editor: ArticleImageEditorLike): number | null { + const selection = editor.state.selection + if (!(selection instanceof NodeSelection)) { + return null + } + if (selection.node.type.name !== 'image') { + return null + } + return articleImageId(selection.node) +} + +export function findArticleImagePos( + editor: ArticleImageEditorLike, + imageId: number, +): number | null { + let targetPos: number | null = null + editor.state.doc.descendants((node, pos) => { + if (targetPos !== null) { + return false + } + if (node.type.name === 'image' && articleImageId(node) === imageId) { + targetPos = pos + return false + } + return true + }) + return targetPos +} + +export const ArticleImage = Image.extend({ + addOptions() { + return { + ...this.parent?.(), + onRequestRemove: () => {}, + } + }, + addAttributes() { + return { + ...this.parent?.(), + 'data-image-id': { + default: null, + parseHTML: (element) => element.getAttribute('data-image-id'), + renderHTML: (attributes) => { + if (!attributes['data-image-id']) { + return {} + } + return { 'data-image-id': attributes['data-image-id'] } + }, + }, + } + }, + addNodeView() { + return VueNodeViewRenderer(ArticleEditorImageNode) + }, + addCommands() { + return { + ...this.parent?.(), + setArticleImage: + (options: { src: string; alt?: string; imageId: number }) => + ({ commands }) => { + return commands.insertContent({ + type: this.name, + attrs: { + src: options.src, + alt: options.alt, + 'data-image-id': options.imageId, + }, + }) + }, + } + }, + addKeyboardShortcuts() { + const triggerRemoval = () => { + const imageId = selectedArticleImageId(this.editor) + if (imageId === null) { + return false + } + this.options.onRequestRemove(imageId) + return true + } + return { + Backspace: triggerRemoval, + Delete: triggerRemoval, + } + }, +}) diff --git a/web/src/views/EditorView.vue b/web/src/views/EditorView.vue index 30731c2..1bdb90e 100644 --- a/web/src/views/EditorView.vue +++ b/web/src/views/EditorView.vue @@ -221,8 +221,8 @@ function onEditorDrop(event: DragEvent) { openImageUploader(file) } -function onImageUploaded(location: string) { - editorRef.value?.insertImage(location, '') +function onImageUploaded(location: string, imageId: number) { + editorRef.value?.insertImage(location, '', imageId) toast.add({ color: 'success', icon: 'i-lucide-circle-check', @@ -231,6 +231,37 @@ function onImageUploaded(location: string) { }) } +async function performRemoveImage(imageId: number) { + try { + await request(`/v1/article/${articleId}/image/${imageId}`, { method: 'DELETE' }) + } catch { + throw new Error("Couldn't remove the image, so it's still there. Try again in a moment.") + } + editorRef.value?.removeImage(imageId) + toast.add({ + color: 'success', + icon: 'i-lucide-circle-check', + title: 'Image removed', + description: 'Save to keep this change.', + }) +} + +async function onRequestRemoveImage(imageId: number) { + await overlay + .create(ConfirmDialog, { + props: { + title: 'Remove this image?', + description: "This image will be removed from the article. This can't be undone.", + confirmLabel: 'Remove image', + cancelLabel: 'Cancel', + pendingLabel: 'Removing…', + tone: 'danger', + action: () => performRemoveImage(imageId), + }, + }) + .open() +} + function onImageUploaderClosed() { imageUploaderFile.value = null } @@ -426,6 +457,7 @@ onBeforeRouteLeave(async () => { :article-id="articleId" @update:model-value="draft.edit" @request-image="onRequestImage" + @request-remove-image="onRequestRemoveImage" >