refac: deleting an article also deletes its images, and deleting a missing article returns 404
This commit is contained in:
1 parent
c16fdbd26d
commit
8a8a24434a
13 files changed
+414
-18
No files matched your search
@@ -0,0 +1,84 @@
|
||||
Feature: Deleting an article also deletes its images
|
||||
|
||||
As an authenticated RenewCMS user managing the CMS
|
||||
I want deleting an article to also delete every image attached to it
|
||||
So that removing an article doesn't leave orphaned image rows and files behind,
|
||||
and so that an id that doesn't exist still answers 404 the same way every other
|
||||
article write does
|
||||
|
||||
Background:
|
||||
Given internal/domain/article.Article carries Images []*image.Image, populated by
|
||||
ArticleRepository.Get via Preload("Images")
|
||||
And internal/domain/image.Repository already exposes Delete(id uint32) error, which
|
||||
on the real ImageRepository also removes the uploaded file from UPLOAD_DIR
|
||||
And before this slice, DeleteArticleUseCase.DeleteArticle only called
|
||||
articleRepository.Delete(id) and never touched image.Repository, so an article's
|
||||
images were never deleted along with it
|
||||
And this slice changes NewDeleteArticleUseCase to also take an image.Repository, and
|
||||
changes DeleteArticleUseCase.DeleteArticle(id) to look up the article, delete every
|
||||
image in its Images slice through image.Repository.Delete, and only then delete the
|
||||
article itself through article.Repository.Delete
|
||||
And this slice rewires api/dependecyInjection.go so
|
||||
useCases.NewDeleteArticleUseCase(repos.ArticleRepo, repos.ImageRepo) supplies both
|
||||
repositories
|
||||
And DELETE /v1/article/{id} keeps sitting behind the jwtauth Verifier and
|
||||
Authenticator group in api/router.go, unchanged from
|
||||
features/write_not_found_errors.feature
|
||||
And DELETE /v1/article/{id} keeps answering 204 with an empty body on success and 404
|
||||
with body apperr.ArticleNotFoundErrorMessage for an id absent from the article
|
||||
repository, backed by errors.Is(err, article.ErrArticleNotFound), unchanged from
|
||||
features/write_not_found_errors.feature
|
||||
And no image HTTP endpoint is mounted in api/router.go as of this slice, so cascading
|
||||
image deletion is only observable through the image repository's state, not through
|
||||
a separate HTTP call
|
||||
And the use case deletes images in the order they appear in the article's Images
|
||||
slice, and aborts on the first image.Repository.Delete error, leaving the article
|
||||
and any not-yet-deleted images untouched, and answering the same generic 500 the
|
||||
DeleteArticle handler already falls back to on any non-not-found error
|
||||
|
||||
Scenario: Deleting an article with images removes the article and every one of its images
|
||||
Given a valid jwt cookie for a user with id 1
|
||||
And the article repository contains an article with id 1 whose Images slice holds
|
||||
image id 10 and image id 11
|
||||
And the image repository contains images with id 10 and id 11
|
||||
When the client sends DELETE /v1/article/1 with that cookie
|
||||
Then the response status is 204
|
||||
And the article repository no longer contains an article with id 1
|
||||
And the image repository no longer contains an image with id 10 or id 11
|
||||
|
||||
Scenario: Deleting an article with no images still succeeds
|
||||
Given a valid jwt cookie for a user with id 1
|
||||
And the article repository contains an article with id 2 whose Images slice is empty
|
||||
When the client sends DELETE /v1/article/2 with that cookie
|
||||
Then the response status is 204
|
||||
And the article repository no longer contains an article with id 2
|
||||
And no call reaches the image repository's Delete method
|
||||
|
||||
Scenario: Deleting an unknown article still answers 404 and deletes no images
|
||||
Given a valid jwt cookie for a user with id 1
|
||||
And the article repository has no article with id 9999
|
||||
When the client sends DELETE /v1/article/9999 with that cookie
|
||||
Then the response status is 404
|
||||
And the response body is the text "Article not found"
|
||||
And no call reaches the image repository's Delete method
|
||||
|
||||
Scenario: Deleting an article still requires authentication
|
||||
Given no jwt cookie is sent
|
||||
And the article repository contains an article with id 1 whose Images slice holds
|
||||
image id 10
|
||||
When the client sends DELETE /v1/article/1 without a cookie
|
||||
Then the response status is 401
|
||||
And the article repository still contains the article with id 1
|
||||
And the image repository still contains the image with id 10
|
||||
|
||||
Scenario: A failure deleting one of an article's images aborts the whole delete
|
||||
Given a valid jwt cookie for a user with id 1
|
||||
And the article repository contains an article with id 3 whose Images slice holds
|
||||
image id 20 followed by image id 21
|
||||
And the image repository contains images with id 20 and id 21
|
||||
And the image repository's Delete fails for image id 20
|
||||
When the client sends DELETE /v1/article/3 with that cookie
|
||||
Then the response status is 500
|
||||
And the response body is the text "Failed to delete article"
|
||||
And the article repository still contains the article with id 3
|
||||
And the image repository still contains the image with id 21
|
||||
@@ -0,0 +1,67 @@
|
||||
# QA procedure: delete_article_cascades_images
|
||||
|
||||
Package: `test` (black-box, extending `article_test.go`).
|
||||
|
||||
## Setup shared by every case
|
||||
1. Call `validator.InitValidator()` once.
|
||||
2. Reuse `mockArticleRepo` (`test/article_test.go`) and `mockImageRepo`
|
||||
(`test/image_test.go`) as-is. Extend `mockArticleRepo` seeding so a seeded article's
|
||||
`Images []*image.Image` can hold pointers into the same `mockImageRepo.images` map used
|
||||
by the test, so deleting through the article side and asserting through the image side
|
||||
observe the same state.
|
||||
3. Add a call counter (or reuse the existing `failSwitch`/call-tracking pattern already in
|
||||
`test/`) on `mockImageRepo.Delete` so a scenario can assert it was never called, and reuse
|
||||
`mockImageRepo.fail` (the existing `failSwitch`) to make `Delete` fail for a specific
|
||||
image id.
|
||||
4. Wire the real use cases over the fakes: `useCases.NewCreateArticleUseCase`,
|
||||
`useCases.NewGetArticleUseCase`, `useCases.NewListArticlesUseCase`,
|
||||
`useCases.NewUpdateArticleUseCase`, and the new two-argument
|
||||
`useCases.NewDeleteArticleUseCase(articleRepo, imageRepo)`. Build `article.NewHandler(...)`
|
||||
with the same constructor shape it already has (the use case's internal dependency count
|
||||
changes, not the handler's).
|
||||
5. Build a test `jwtauth.New("HS256", []byte("test-secret"), nil)`.
|
||||
6. Mount the routes on a fresh `chi.NewRouter()` mirroring `api/router.go`'s current shape:
|
||||
`GET /` under `jwtauth.Verifier` only; `GET /{id}`, `POST /`, `DELETE /{id}`,
|
||||
`PATCH /{id}`, `PATCH /{id}/publish`, `PATCH /{id}/unpublish` under `jwtauth.Verifier` +
|
||||
`jwtauth.Authenticator`.
|
||||
7. Mint a valid jwt cookie for user id 1: `token.Encode(map[string]any{"user_id": uint32(1)})`,
|
||||
set as the `jwt` cookie on the relevant `httptest.NewRequest`.
|
||||
|
||||
## HTTP checks
|
||||
8. Seed `imageRepo.images[10]` and `imageRepo.images[11]`, then seed
|
||||
`articleRepo.articles[1] = article.Article{ID: 1, Title: "Backyard Renovation Update",
|
||||
Images: []*image.Image{imageRepo.images[10], imageRepo.images[11]}}` (pointers into the
|
||||
same map). Send `DELETE /v1/article/1` with the valid cookie -> status `204`; assert
|
||||
`articleRepo.articles` no longer has key `1`; assert `imageRepo.images` no longer has
|
||||
keys `10` or `11`.
|
||||
9. Seed `articleRepo.articles[2] = article.Article{ID: 2, Title: "Draft: Kitchen Remodel"}`
|
||||
with a nil/empty `Images` slice. Send `DELETE /v1/article/2` with the valid cookie ->
|
||||
status `204`; assert `articleRepo.articles` no longer has key `2`; assert the image-delete
|
||||
call counter is `0`.
|
||||
10. Empty `articleRepo`, empty `imageRepo`. Send `DELETE /v1/article/9999` with the valid
|
||||
cookie -> status `404`, body `"Article not found\n"`; assert the image-delete call
|
||||
counter is `0`.
|
||||
11. Seed `imageRepo.images[10]` and `articleRepo.articles[1]` with `Images` holding image id
|
||||
10, per step 8's pattern. Send `DELETE /v1/article/1` with **no** cookie -> status `401`;
|
||||
assert `articleRepo.articles[1]` is still present and `imageRepo.images[10]` is still
|
||||
present.
|
||||
12. Seed `imageRepo.images[20]` and `imageRepo.images[21]`, then seed
|
||||
`articleRepo.articles[3] = article.Article{ID: 3, Title: "Kitchen Progress Photos",
|
||||
Images: []*image.Image{imageRepo.images[20], imageRepo.images[21]}}` (image 20 before
|
||||
image 21 in the slice). Arm `imageRepo.fail` so `Delete` returns an error only when
|
||||
called with id `20` (leave id `21` unarmed). Send `DELETE /v1/article/3` with the valid
|
||||
cookie -> status `500`, body `"Failed to delete article\n"`; assert
|
||||
`articleRepo.articles[3]` is still present; assert `imageRepo.images[21]` is still
|
||||
present (never reached because the delete aborted on image `20` first).
|
||||
|
||||
## Manual verification (not exercised by the Go test)
|
||||
- Real cascading delete against a real SQLite file and real `UPLOAD_DIR`: confirm that after
|
||||
`DELETE /v1/article/{id}` on a real running instance, the uploaded image files under
|
||||
`UPLOAD_DIR` for that article are gone from disk and the `images` table no longer holds
|
||||
those rows, and that no orphaned rows are left behind in the `article_images` join table.
|
||||
Not exercised by `task test` (fakes only); requires `task dev` or a built `task build`
|
||||
binary with a scratch DB/upload dir.
|
||||
- SPA-visible behaviour: confirm the back-office article deletion flow (if/when one exists)
|
||||
reflects that any images shown for that article are gone too. No such UI exists yet as of
|
||||
this slice.
|
||||
- No SMTP involvement.
|
||||
Reference in new issue
Block a user