refac: article reads, story delete, no leaks, stored SEO status, rate limit, cleanup (%site%, article repo split, old lang repos, one default per site rule to domain)

This commit is contained in:
Florian Sylvain committed 2026-11-03 16:01:52 +01:00
1 parent 9d284e65a9
commit 7c15206170
85 files changed
+1788 -593

No files matched your search

+15 -1
View File
@@ -122,7 +122,8 @@ Feature: Editorial article list by story and language
Given the "Journal" dataset and an owner session
When the client sends GET /v1/article/?group=story
Then the coverage of story <story> is <coverage>
And each entry has id, lang, title, is_online, language_enabled and capabilities, like an entry of "translations" in features/article_story_lifecycle.feature
And each entry has id, lang, title, is_online, language_enabled, excluded_from_public, source_article_id and capabilities, like an entry of "translations" in features/article_story_lifecycle.feature
And each row has excluded_from_public, the flag of its representative version
And missing_langs of story <story> is <missing>
Examples:
@@ -160,6 +161,13 @@ Feature: Editorial article list by story and language
Then its row has lang "", coverage with its single version, and missing_langs []
And it never appears in the "not in" view of any language
Scenario: A mixed story is listed under Missing
Given story 31 holds article 31 (lang "") and article 32 (en), and story 33 holds article 33 (lang "") and article 34 (fr)
When the owner sends GET /v1/article/?group=story&tab=missing
Then stories 31 and 33 are listed, with the enabled languages they lack in missing_langs
And the version without language fills no language
And a story whose readable versions all lack a language is never listed
Rule: Language selected means one row per version of that language
Scenario: French selected
@@ -268,6 +276,12 @@ Feature: Editorial article list by story and language
When the owner sends GET /v1/article/?group=story&seo=suggestions
Then story 5 is listed once, with its own representative 501
Scenario: The SEO status is stored, not recomputed from every body
Given 40,001 versions whose SEO status was computed once
When the owner requests any page of ?group=story&seo=suggestions, with or without q
Then the status filter and counts.suggestions are evaluated in SQL on the stored status
And no body of a visible version is read except the representatives of the page
Scenario: All filters combine with paging and stay stable
Given 12 stories match
When the owner requests pages 1, 2 and 3 of ?group=story&size=5 with the same filters
+3 -2
View File
@@ -6,7 +6,8 @@ Order of work is part of the procedure: the repository query and its real-databa
1. Seed the "Journal" dataset (8 stories, 17 versions, bylines, updated_at as in the feature) and the "hidden-version" variant through the real `ArticleRepository` on a temp SQLite file with `database.Migrate`. Drive the new grouped query through `article.Repository` with the new filter fields (group, lang, tab, author) and assert, table-driven over the feature's Examples: story ids per page, `total`, representative per story, coverage order, `missing_langs`, `story_updated_at`, `story_authors`, counts.
2. Access filter inside the query: for a caller with scope `own`, `all`-online and `online only`, assert the hidden-version dataset yields the same ids, totals and counts as a database where the hidden articles do not exist. Also check page boundaries (a hidden version must not shift a page: 5 per page of readable stories).
3. Filter semantics: the "single readable version satisfies all version-level filters" cases (status+author, status+q, author+seo), the "story_authors is the union" case, the "missing" cases (no readable version, site not enabling the language, disabled language, unassigned articles) and accent-insensitive q in grouped, language and not-in modes.
3. Filter semantics: the "single readable version satisfies all version-level filters" cases (status+author, status+q, author+seo), the "story_authors is the union" case, the "missing" cases (no readable version, site not enabling the language, disabled language, unassigned-only stories never listed, mixed stories listed: TestArticleListLocalization* "Missing membership and counts agree with the missing_langs of the rows") and accent-insensitive q in grouped, language and not-in modes.
3b. Stored SEO status: TestArticleStoryListSEOFilterAtScaleSQLite runs the startup backfill once over 40,001 versions, then asserts per request zero stale versions read, zero FindPage calls and one title search with q. TestArticleSEOIndex* (SQLite, MySQL, PostgreSQL) asserts that each write path clears the stored status and the refresh recomputes it, that a status computed before a concurrent write is not stored, and that the site description counts for the default language only and follows a default change.
4. Sort whitelist and stability: each sort of the feature, ties by story_id descending, no overlap or gap between pages 1 to 3 on a 12-story set.
## Automated, real databases (task test:mysql, task test:postgres)
@@ -21,7 +22,7 @@ Repeat items 1 to 4 against MySQL 8.4, MariaDB 11 and PostgreSQL. Compare ids, t
6. `article.NewHandler(...)` over the real use cases and fakes, a fresh `chi.NewRouter()` mirroring `NewArticleRouter` in `api/router.go` (GET / under `jwtauth.Verifier` only), `validator.InitValidator()` first, `httptest`, jwt cookies signed with a test `jwtauth.New("HS256", ...)` secret (owner, reader, restricted writer, unverified user, expired, none).
7. "Public lists are unchanged": bare request and a public request keep the flat keys exactly (compare the key set and values with a golden produced before the slice); `lang=fr` and `author` narrow the flat list; `group=story` for the three public callers and every invalid query in the Examples answer 400 with `apperr.BodyErrorMessage` and the fake repository records no call.
8. Grouped response shape: decode and assert `items[]` keys (flat item keys plus `story_updated_at`, `story_authors`, `coverage`, `missing_langs`, `capabilities`), `total`, `page`, `size`, `counts` keys (all, online, draft, suggestions, missing), `items` `[]` not null, snake_case, errors plain text. Repository failure and count failure answer 500 "Failed to retrieve articles". Docs: `docs/` gains the grouped contract (parameters, shape, counts, errors) so the SPA mocks `/v1` from it.
8. Grouped response shape: decode and assert `items[]` keys (flat item keys plus `story_updated_at`, `story_authors`, `coverage`, `missing_langs`, `excluded_from_public`, `capabilities`; coverage entries carry `excluded_from_public` and `source_article_id`), `total`, `page`, `size`, `counts` keys (all, online, draft, suggestions, missing), `items` `[]` not null, snake_case, errors plain text. Repository failure and count failure answer 500 "Failed to retrieve articles". Docs: `docs/` gains the grouped contract (parameters, shape, counts, errors) so the SPA mocks `/v1` from it.
## Automated, SPA (Vitest + Vue Test Utils, task test:web)
+1 -1
View File
@@ -4,7 +4,7 @@ Backend only. Go tests in test/, black-box. No design states. No new endpoints.
## Automated, no server (task test)
1. Language domain: table-driven over "Language tag normalization" and "Suggestion for a site's default language". Assert the sentinel errors with errors.Is.
2. Use case: with fake repositories, run "Creating an article before the default is confirmed" and "after the default is confirmed". Assert that CreateArticle runs inside the Transactor, that the starting language is obtained through LockStartingLanguage of the site language repository (under the site-row lock) and passed to Create as Lang, and that the story is created by the article repository inside the same transaction. Existing create tests stay green.
2. Use case: with fake repositories, run "Creating an article before the default is confirmed" and "after the default is confirmed". Assert that CreateArticle runs inside the Transactor, that it locks the site row (site.Repository.LockSite) and then reads the starting language through the localization port (localization.Repository.Rows, localization.ConfirmedDefaultTag) and passes it to Create as Lang, and that the story is created by the article repository inside the same transaction. The former language.Repository, its SiteLanguageRepository adapter and the unwired ConfirmDefaultLanguageUseCase are retired; the confirmation tests drive ApplyLanguageSetupUseCase. Existing create tests stay green.
3. HTTP: NewHandler over use cases with in-memory fakes, a fresh chi.NewRouter mirroring api/router.go, validator.InitValidator(), httptest. Cover "Article reads expose lang and story_id" for GET /v1/article/{id} with a jwt cookie signed by JWT_SECRET (the test secret), the list GET /v1/article/, and the public read of an online article. Assert that old keys are unchanged, that lang and story_id are the added keys, and that "lang" or "story_id" in a PATCH body is ignored. POST /v1/article/ honours lang, story_id, source_article_id and copy only as defined in features/article_translation_create.feature, and a POST without them behaves as before. Errors stay plain text.
4. Regression: run the full existing article, pagination, publishing and SEO tests untouched.
+81 -3
View File
@@ -1,5 +1,5 @@
Feature: Capabilities, translations, publish rules, delete and story move
An article read tells the editor what the caller may do with the version and which readable sibling versions exist. Publishing respects disabled languages. Deleting the last version removes its story. A story moves to another site as a whole, atomically, never split.
An article read tells the editor what the caller may do with the version and which readable sibling versions exist. Publishing respects disabled languages. Deleting the last version removes its story, and a whole article can be deleted at once, atomically. A story moves to another site as a whole, atomically, never split. Nothing reveals a version the caller cannot read.
Background:
Given error responses are plain text, with extra data in headers
@@ -9,6 +9,8 @@ Feature: Capabilities, translations, publish rules, delete and story move
And "owner" holds every content permission at scope all
And "writer1" (user 1) holds content.create, content.read, content.update, content.publish and content.delete at scope own, and content.read at scope all for online articles only
And "reader" holds content.read at scope all only
And "restricted" holds content.create at scope all and content.read at scope own
And "deleter" holds content.delete at scope all and content.read at scope own
Scenario: Capabilities of the owner
When the owner sends GET /v1/article/12
@@ -30,8 +32,8 @@ Feature: Capabilities, translations, publish rules, delete and story move
Scenario: Translations list readable versions only
When writer1 sends GET /v1/article/10
Then "translations" has entries for 10, 12 and 13 and none for 11
And each entry has id, lang, title, is_online, language_enabled and capabilities
And no count, placeholder or total names the hidden version
And each entry has id, lang, title, is_online, language_enabled, excluded_from_public, source_article_id and capabilities
And no entry, count, placeholder or total names the hidden version
Scenario: Translations are ordered default language first, then by tag
When the owner sends GET /v1/article/12
@@ -52,6 +54,7 @@ Feature: Capabilities, translations, publish rules, delete and story move
When an anonymous client sends GET /v1/article/10
Then "translations" is []
And "capabilities" is ["read"]
And the response has no "missing_langs", "excluded_from_public", "source_unavailable" or "partial_view"
And the public shape is otherwise unchanged
Scenario: The source on the read
@@ -62,6 +65,47 @@ Feature: Capabilities, translations, publish rules, delete and story move
Then "source" is null
And a version created empty has "source": null
Scenario Outline: Missing languages are the enabled languages without a readable version
When <caller> sends GET /v1/article/10
Then "missing_langs" is <missing>
Examples:
| caller | missing |
| owner | [] |
| restricted | ["de","fr"] |
| reader | [] |
Scenario: A version in a disabled language or without language is left out of public surfaces
Given article 14 (es, offline, created by restricted) is in story 5 and es is disabled on journal
And article 30 has lang "" and its own story
When the owner sends GET /v1/article/14
Then "excluded_from_public" is true
And the translations entry of 14 has "excluded_from_public": true and the entry of 10 has false
And GET /v1/article/30 has "excluded_from_public": true
And GET /v1/article/10 has "excluded_from_public": false
Scenario: A deleted source and an unreadable source read the same
Given article 14 tracks article 12 at content revision 2
When the owner sends GET /v1/article/14
Then "source" is {"article_id":12,"revision":2} and "source_unavailable" is false
When the restricted user, who cannot read 12, sends GET /v1/article/14
Then "source" is null and "source_unavailable" is true
When article 12 is deleted and the owner sends GET /v1/article/14
Then "source" is null and "source_unavailable" is true
Scenario: Translations name the source they track only when it is readable
Given article 14 tracks article 12
When the owner sends GET /v1/article/10
Then the translations entry of 14 has "source_article_id": 12 and the entry of 10 has null
When the restricted user sends GET /v1/article/14
Then the translations entry of 14 has "source_article_id": null
Scenario: Partial view follows the read scope of the caller only
When the owner sends GET /v1/article/10
Then "partial_view" is false
When the restricted user sends GET /v1/article/13
Then "partial_view" is true, whether or not the story holds a version the restricted user cannot read
Scenario Outline: Publishing respects the language configuration
Given a valid jwt cookie for a user holding content.publish
When the client sends PATCH /v1/article/<id>/publish
@@ -136,6 +180,40 @@ Feature: Capabilities, translations, publish rules, delete and story move
Then the story remains
And nothing in the response mentions the other version
Scenario: Deleting an article removes every version and the article
When the owner sends DELETE /v1/story/5
Then the response status is 204 with an empty body
And articles 10, 11, 12 and 13 and story 5 are gone
And one article.delete event per version is recorded with params.lang and params.story_id 5
Scenario: Versions the caller cannot read are deleted with the article
When the deleter, who reads 10 and 13 only, sends DELETE /v1/story/5
Then the response status is 204
And articles 11 and 12 are deleted too
Scenario: Deleting an article needs delete permission on every version
When writer1, whose content.delete is at scope own, sends DELETE /v1/story/5
Then the response status is 403, the body is "missing_permission" and X-Missing-Permission is "content.delete"
And every version remains
And the answer is the same whether or not the story holds a version writer1 cannot read
Scenario: A failed delete removes nothing
Given the database fails while the versions are deleted
When the owner sends DELETE /v1/story/5
Then the response status is 500 and the body is "Failed to delete article"
And every version and story 5 remain
Scenario: An unknown or unreadable article answers the same not found
When the owner sends DELETE /v1/story/9999
Then the response status is 404 and the body is "Article not found"
When the deleter sends DELETE /v1/story/77, whose only version is a draft by another user
Then the response status is 404 and the body is "Article not found"
And that version remains
Scenario: An invalid id is refused
When the owner sends DELETE /v1/story/abc
Then the response status is 400 with the uint32 id message
Scenario: Move a story
When the owner sends POST /v1/story/5/move with {"site":"atelier"}
Then the response status is 200
+3 -1
View File
@@ -5,12 +5,14 @@ Backend only. Go tests in test/, black-box. No design states.
## Automated, no server (task test)
1. Use cases over fakes (article, site, language, localization, role grants) and a fake Transactor: the capabilities and translations mapping, the publish rule, deleting the last version and MoveStory. Table-driven over every Examples table; assert the sentinels with errors.Is. MoveStory runs the permission check, the language check, the slug check and all writes inside one Transactor.Run, and a failing write rolls back all fake rows.
2. HTTP via the article NewHandler and the new story handler, a chi router mirroring api/router.go (/article and /story mounts, authtoken group), validator.InitValidator(), httptest and jwt cookies for owner, writer1, reader and none. Assert exact bodies and the headers X-Existing-Article-Id (absent where not allowed), X-Current-Revision, X-Conflicting-Articles and X-Missing-Languages. Hidden versions appear in no body, header or count.
3. Anonymous reads: the key set is the slice 1 set plus revision, capabilities, translations and source.
3. Anonymous reads: the key set is the slice 1 set plus revision, capabilities, translations and source. Signed-in reads add missing_langs, excluded_from_public, source_unavailable and partial_view (TestArticleEditorialContextHTTP; TestArticleCreatedByHttp compares the two shapes).
3b. Whole-article delete: TestStoryDeleteHTTP covers every scenario of the "Deleting a whole article" rule over the story world (fake transactor with rollback, DeleteStory failure injected through the fail switch).
4. Activity: a fake recorder receives params.lang on publish, unpublish, edit, delete and per-version move events.
5. Regression: the article_publishing and article_title_rules tests pass with the updated pair text.
## Automated, SQLite file (task test)
6. Deleting the last version removes the story row, deleting one keeps it. Re-creating fr after deleting fr works. The site_languages in-use count still includes a deleted fr version (slice 2 TestLanguageInUseCountsSoftDeletedVersions stays green).
6b. DeleteStory: TestArticleSEOIndex* "Deleting an article removes every version and the article row" (live versions 0, story row gone, slugs free, a second call is not found), on SQLite and on the real databases.
7. Move: all site_id values change together. A unique (site_id, lang, slug) violation at the destination rolls back every row. An injected write failure on the third row leaves all rows unchanged.
8. PATCH site_id counts readable versions only: with one readable version in a multi-version story it runs the full story move (every version, hidden ones too, moves in one transaction; language and slug refusals roll back and name readable versions only), which needs content.update on every version (403 missing_permission otherwise); with two readable versions it is 409 story_move_required.
+1 -1
View File
@@ -1,5 +1,5 @@
Feature: Create a translation of an article version
A user who can create articles adds a version of an existing article in another language. The version joins the story of the version it is created from, in the same site, as an offline draft. Two entry points share one implementation: POST /v1/article/{id}/translations and POST /v1/article/ with story_id. Hidden versions are never named, counted or hinted at.
A user who can create articles adds a version of an existing article in another language. The version joins the story of the version it is created from, in the same site, as an offline draft. Two entry points share one implementation: POST /v1/article/{id}/translations and POST /v1/article/ with story_id. The story_id entry point is deprecated: it keeps working with the same results, and new clients use POST /v1/article/{id}/translations. Hidden versions are never named, counted or hinted at.
Background:
Given the language rules of features/article_localization_storage.feature apply
@@ -303,6 +303,14 @@ Feature: Localized public API, SEO output and the existing surfaces that show la
Then fallbacks.site_text_skipped is true
And this is the documented limitation of treating the site text as written in the current default language
Scenario: The SEO check and the list status follow the same language rule
Given journal's default_description is 108 characters long and versions 10 (en, default) and 12 (fr) have no description, excerpt or body sentence
When the owner sends POST /v1/article/10/seo/check and POST /v1/article/12/seo/check with {}
Then only the fr check reports desc_short
And the seo_suggestions of 12 in GET /v1/article/ is one more than that of 10
When the owner makes fr the default language of journal
Then the next GET /v1/article/ reports one more suggestion for 10 than for 12, without any edit of the articles
Scenario: Brand text and images stay shared
Then title_template and site_name apply to every language
And the image chain (social, cover, site default image) is unchanged and each version's cover_image_alt stays its own
@@ -326,6 +334,15 @@ Feature: Localized public API, SEO output and the existing surfaces that show la
When an anonymous caller sends GET /v1/site/journal/sitemap
Then the url of version 20 is "https://www.journal.example/spring" as before this slice
Rule: The public routes and the sitemap are rate limited per client
Scenario: The public article routes and the sitemap share one budget per client
Given one client address has sent 120 requests within a minute to /v1/public/... and /v1/site/journal/sitemap together
When it sends one more request to either
Then the response status is 429 with Retry-After and the body "Too many requests. Please try again later."
And another client address is still answered
And X-Real-IP is used as the client address only when the connection comes from TRUSTED_PROXIES
Rule: Media usage names the language and counts a story once
Background:
@@ -400,6 +417,7 @@ Feature: Localized public API, SEO output and the existing surfaces that show la
And the tool descriptions no longer say that a title cannot be changed
And get_article, list_articles and get_sitemap return the JSON they already return, which now carries lang and story_id
And no tool is added for the public routes
And the MCP server keeps calling the authenticated HTTP routes with its API key rather than the use cases, for the reasons in docs/mcp.md
Scenario: The llms documents mention languages
Then docs/llms.md, docs/llm/ and the llms-full output describe lang, story_id, alternates, the two public routes and the fallback rule
@@ -15,6 +15,8 @@ Fixture: the Background of the feature (journal with en default, fr fr_FR, de, a
5. **SEO object**: key sets (additive `lang`, `alternates`, `og.locale_alternates`, `fallbacks.site_text_skipped`); canonical table (pattern, override wins, no pattern keeps legacy, unassigned keeps legacy, `{lang}` keeps `pt-BR`); `fallbacks.canonical` values; og:locale Examples table including `en` with Site.Locale `en` and `null` (empty string, alternates `[]`); `og.locale_alternates`; the description/title fallback tables and `site_text_skipped`; the default-change limitation scenario. Existing `test/seo_output_*` specs are amended only where they assert the exact key set, the `og.locale` of an assigned article, or the canonical of a site that has a pattern (unassigned and pattern-less cases must pass unchanged).
6. **Sitemap**: eligible versions only, `lang` key, localized urls with a pattern and legacy urls without, `X-Total-Count`, paging invariants unchanged.
7. **Languages read**: `social_locale_state` `derived` with `en_GB` for bare `en` and Site.Locale `en-GB`, `not_set` for bare Site.Locale; amend `test/site_languages_*` accordingly.
7b. **Rate limit**: TestPublicReadRateLimit sends 120 requests alternating the exact lookup and the sitemap from one address, then asserts 429 with Retry-After on the exact lookup, the sitemap and the story resolution, and 200 for another address.
7c. **Language-aware SEO check and status**: TestCheckArticleSEOIsLanguageAware (fakes) and TestArticleSEOIndex* "The site description counts for the default language only and follows a default change" (SQLite and real databases).
8. **MCP**: `list_articles` forwards `lang` (assert the request URL with a stub HTTP client in `internal/infrastructure/mcpserver`'s existing test style), tool descriptions contain no "cannot be changed".
9. **Catalogue drift**: `test/api_catalogue_drift_test.go` green with the two new routes; `web/src/data/apiCatalogue.ts` counts updated.
10. **Layering**: existing layering tests stay green; the public handler imports no persistence, models or gorm.
@@ -116,7 +116,7 @@ Feature: Translation flows, revision conflicts, delete scopes, article moves and
And a refusal that reaches the use case records one failed event with a plain reason that names no hidden data
Scenario: Persistence needs no new query
Then the use case calls the existing site GetByKey, LockStartingLanguage(site id), localization Rows(site id), SlugTaken(site id, lang, slug) and Articles.Create
Then the use case calls the existing site GetByKey, Sites.LockSite(site id), localization Rows(site id), SlugTaken(site id, lang, slug) and Articles.Create
And no repository, model, mapper or migration changes, so the real-database runs (task test:mysql, test:postgres) are not required for this change
Rule: Add translation opens from the language menu, the URL and the article list
@@ -17,7 +17,7 @@ Fixture: sites journal (id 1, default; en default, fr, de, ar; es disabled), ate
7. **Visibility and activity**: the new offline article is 404 for an anonymous GET, absent from the public lists of both sites, listed by `GET /v1/article/?site=atelier` for a writer; the event params hold the site key.
8. **Dependency rule**: the handler test imports no persistence, models or gorm; a grep-style check is not added here (architect covers it). The handler change is the `Site *string` field with `validate:"omitempty,sitekey"` and the pass-through in `CreateArticleCommand`; no domain, repository, mapper or migration change.
Real-database runs: `task test:mysql` and `task test:postgres` are not required, because no query, model or repository changes (existing `GetByKey`, `LockStartingLanguage`, `Rows`, `SlugTaken` and `Create` are already covered per site by `test/article_localization_storage_persistence_test.go` and `test/site_persistence_test.go`). If the implementation adds any query, add a persistence case there and run both.
Real-database runs: `task test:mysql` and `task test:postgres` are not required, because no query, model or repository changes (existing `GetByKey`, `LockSite`, `Rows`, `SlugTaken` and `Create` are already covered per site by `test/article_localization_storage_persistence_test.go` and `test/site_persistence_test.go`). If the implementation adds any query, add a persistence case there and run both.
Existing Go specs to amend (and nothing else): `test/article_create_language_test.go` (command carries the site key), `test/article_translation_create_http_test.go` (rows of the "unchanged when it is not a translation" table, the `site` plus `story_id` refusal), `test/article_title_rules_test.go` and `test/use_case_baseline_test.go` only where they build `CreateArticleCommand` or `PostArticle` (compile and default-site behaviour must be unchanged), `test/api_catalogue_drift_test.go` stays green because the route list does not change (check it still passes; if the catalogue documents request fields, update it with the docs). `test/seo_data_cleaner_test.go` fails `GetDefault` on purpose and must keep passing for the omitted-site path.