feat: editor image removal QOLs
This commit is contained in:
fsylvain committed
1 parent
0fc00ceeff
commit
b29d0ed045
23 files changed
+1398
-151
No files matched your search
@@ -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 <img> 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 <triggers-removal>
|
||||
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 <failure>
|
||||
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."
|
||||
@@ -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 `<img src="/static/uploadedImages/before.png" data-image-id="10"
|
||||
alt="">`. 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.
|
||||
@@ -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/<article_id>/image/<image_id> 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 |
|
||||
@@ -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.
|
||||
@@ -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/<stored-filename>"
|
||||
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/<stored-filename>"
|
||||
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
|
||||
|
||||
Reference in new issue
Block a user