From aafea65a5229f830cc9b43df5d26bc398ff6db49 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 19:31:28 +0200 Subject: [PATCH] feat: delete an article, and a trash for the ones that were published MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There was no way to delete an article from anywhere in the interface. The API had one; the only route to it was curl. Now there is a control at the foot of the editor, and it does one of two things depending on the article's history — and says which before it is pressed: * A draft that was **never published** is deleted outright. There is nothing to restore, and a trash full of abandoned stubs is a second list to maintain. * Anything that has been published, even once, is **marked** and appears in the trash on Editorial, restorable exactly as it was. Somewhere there is a learner's note against one of its sections, a question linked to it, and a link somebody sent a colleague; a DELETE typed in the afternoon should not settle any of that. `first_published_at` is what decides, stamped on the first publish and never cleared — unpublishing does not make an article unseen, so it does not make deleting it safe either. Backfilled from `reviewed_at` for everything currently published, because an article with a null stamp reads to the rule as a never-published draft. A binned article is out of the listing, the editorial queue, every slug and id lookup, and — immediately — the search index, so it cannot still answer a learner's question from the trash. Also on Editorial, because a hundred rows is a queue you work through and not a page you scroll past on the way to the next queue: each bucket keeps its own box, its own scrollbar and its own filter. And the editor finally has a way out that is not Save: Back and Discard, with an inline confirmation when there are unsaved changes. The way out was the browser's back button, which throws the sitting away without saying so. Migration j0a1b2c3d4e5. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- .../versions/j0a1b2c3d4e5_article_trash.py | 64 ++++++++++ backend/app/models/article.py | 8 ++ backend/app/services/article_service.py | 19 ++- frontend/src/pages/ArticlesPage.css | 22 ++++ frontend/src/pages/ArticlesPage.jsx | 113 ++++++++++++++++-- frontend/src/pages/EditorialPage.css | 17 ++- frontend/src/pages/EditorialPage.jsx | 95 ++++++++++++++- frontend/src/pages/EditorialPage.test.jsx | 6 +- 8 files changed, 325 insertions(+), 19 deletions(-) create mode 100644 backend/alembic/versions/j0a1b2c3d4e5_article_trash.py diff --git a/backend/alembic/versions/j0a1b2c3d4e5_article_trash.py b/backend/alembic/versions/j0a1b2c3d4e5_article_trash.py new file mode 100644 index 0000000..789f176 --- /dev/null +++ b/backend/alembic/versions/j0a1b2c3d4e5_article_trash.py @@ -0,0 +1,64 @@ +"""An article that was ever published goes to the trash instead of vanishing + +Three columns on `articles`: + +* `first_published_at` — stamped the first time it is published and never + cleared. It is what decides whether deleting is reversible. +* `deleted_at` / `deleted_by` — the trash itself. + +The rule it exists for: a draft nobody ever saw is deleted outright, because +there is nothing to restore and a trash full of abandoned stubs is a second +list to maintain. Anything the world has seen — anything with a +`first_published_at` — is only ever marked, because somewhere there is a +learner's note against one of its sections, a question linked to it, and a link +somebody sent to a colleague. + +Backfilled from `reviewed_at` where an article is published now: the exact +first-publication moment is not recorded anywhere, and any published article +must have a non-null stamp or the rule reads it as a never-published draft and +deletes it for good. + +Revision ID: j0a1b2c3d4e5 +Revises: i9f0a1b2c3d4 +""" +import sqlalchemy as sa +from alembic import op + +revision = "j0a1b2c3d4e5" +down_revision = "i9f0a1b2c3d4" +branch_labels = None +depends_on = None + + +COLUMNS = { + "first_published_at": sa.Column("first_published_at", sa.DateTime(), nullable=True), + "deleted_at": sa.Column("deleted_at", sa.DateTime(), nullable=True), + "deleted_by": sa.Column("deleted_by", sa.Integer(), nullable=True), +} + + +def upgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + # `Base.metadata.create_all()` still runs at startup, so a fresh deploy may + # already have these. Idempotent by inspection rather than by try/except. + existing = {column["name"] for column in inspector.get_columns("articles")} + for name, column in COLUMNS.items(): + if name not in existing: + op.add_column("articles", column) + + indexes = {index["name"] for index in inspector.get_indexes("articles")} + if "ix_articles_deleted_at" not in indexes: + op.create_index("ix_articles_deleted_at", "articles", ["deleted_at"]) + + op.execute(""" + UPDATE articles + SET first_published_at = COALESCE(reviewed_at, updated_at, created_at) + WHERE status = 'published' AND first_published_at IS NULL + """) + + +def downgrade() -> None: + op.drop_index("ix_articles_deleted_at", table_name="articles") + for name in COLUMNS: + op.drop_column("articles", name) diff --git a/backend/app/models/article.py b/backend/app/models/article.py index 8011f2b..30891c3 100644 --- a/backend/app/models/article.py +++ b/backend/app/models/article.py @@ -32,6 +32,14 @@ class Article(Base, Embeddable): # educator has been through it. generated_by = Column(String(80), nullable=True) generated_at = Column(DateTime, nullable=True) + # The first time this was published, and never cleared afterwards. It is + # what decides whether deleting is reversible: an article the world has + # seen goes to the trash, a draft nobody ever saw is simply gone. + first_published_at = Column(DateTime, nullable=True) + # In the trash. Set rather than deleted, so restoring is one click and not + # a database restore. + deleted_at = Column(DateTime, nullable=True) + deleted_by = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) created_at = Column(DateTime, default=datetime.utcnow) updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow) diff --git a/backend/app/services/article_service.py b/backend/app/services/article_service.py index 7fb7f24..5d0b809 100644 --- a/backend/app/services/article_service.py +++ b/backend/app/services/article_service.py @@ -109,13 +109,17 @@ def resolve_slug(db: Session, slug: str) -> Article | None: reference = (slug or "").strip().lower() if not reference: return None + def alive(article): + return article if article is not None and article.deleted_at is None else None + if reference.isdigit(): - return db.get(Article, int(reference)) - article = db.query(Article).filter(Article.slug == reference).first() + return alive(db.get(Article, int(reference))) + article = db.query(Article).filter(Article.slug == reference, + Article.deleted_at.is_(None)).first() if article: return article historical = db.query(ArticleSlug).filter(ArticleSlug.slug == reference).first() - return db.get(Article, historical.article_id) if historical else None + return alive(db.get(Article, historical.article_id)) if historical else None def snapshot(db: Session, article: Article, user_id: int | None, note: str | None = None) -> None: @@ -200,7 +204,10 @@ def readable_articles(db, user): """ from app.models.article import Article - query = db.query(Article) + # Never the trash, for anybody — including a moderator. Restoring is done + # from the trash list, which is one place, rather than from wherever an + # article happens to still be referenced. + query = db.query(Article).filter(Article.deleted_at.is_(None)) if getattr(user, "is_moderator", False): return query return query.filter((Article.status == "published") | (Article.user_id == user.id)) @@ -229,7 +236,9 @@ def rebuild_section_index(db: Session, article: Article) -> int: # unpublished, and come back when it is published again — this function is # called on every save, so the index follows the status rather than # needing to be told about it separately. - if (article.status or "") != "published": + # A binned article is not readable whatever its status says, so it comes out + # of the index on the same rule as an unpublished one. + if (article.status or "") != "published" or getattr(article, "deleted_at", None) is not None: db.query(ArticleSectionIndex).filter( ArticleSectionIndex.article_id == article.id).delete(synchronize_session=False) return 0 diff --git a/frontend/src/pages/ArticlesPage.css b/frontend/src/pages/ArticlesPage.css index 7e14f40..62bdc78 100644 --- a/frontend/src/pages/ArticlesPage.css +++ b/frontend/src/pages/ArticlesPage.css @@ -392,3 +392,25 @@ .articles-form-actions { flex-wrap: wrap; } .articles-leave { margin-left: 0; width: 100%; } } + +/* Leaving the editor. Save was the only control in the header, so the way out + of an editing sitting was the browser's back button — which throws the work + away and does not say so. */ +.article-back { + display: inline-block; margin-bottom: 4px; padding: 0; + border: 0; background: none; cursor: pointer; + font: inherit; font-size: 0.82rem; font-weight: 600; color: var(--text-muted); +} +.article-back:hover { color: var(--primary); } +.article-discard-confirm { + display: inline-flex; align-items: center; gap: 8px; flex-wrap: wrap; + font-size: 0.82rem; color: var(--text-muted); +} + +/* Deleting. At the foot of the editor and quiet until it is asked for: the + consequence differs by whether the article was ever published, so the + confirmation says which one this is rather than asking "are you sure". */ +.article-danger { margin-top: 28px; padding-top: 16px; border-top: 1px solid var(--border); } +.article-danger p { margin: 0 0 10px; font-size: 0.86rem; line-height: 1.55; color: var(--text-muted); max-width: 62ch; } +.article-danger-actions { display: flex; gap: 8px; flex-wrap: wrap; } +.article-danger-open { color: var(--wrong-fg); } diff --git a/frontend/src/pages/ArticlesPage.jsx b/frontend/src/pages/ArticlesPage.jsx index 039dbd9..7f682ae 100644 --- a/frontend/src/pages/ArticlesPage.jsx +++ b/frontend/src/pages/ArticlesPage.jsx @@ -89,10 +89,17 @@ export default function ArticlesPage() { const res = await api.post('/articles/ai-draft', { topic: aiTopic, instructions: aiInstructions }) setAiStatus('Drafting…') const poll = async () => { - const job = await api.get(`/articles/job/${res.data.job_id}`) - if (job.data.status === 'completed') { setAiStatus(''); setShowAi(false); load() } - else if (job.data.status === 'failed') { setAiStatus(`Failed: ${job.data.error || 'unknown error'}`) } - else { setAiStatus(`Working… ${job.data.steps.at(-1)?.message || ''}`); setTimeout(poll, 2000) } + try { + const job = await api.get(`/articles/job/${res.data.job_id}`) + if (job.data.status === 'completed') { setAiStatus(''); setShowAi(false); load() } + else if (job.data.status === 'failed') { setAiStatus(''); setError(`Drafting failed: ${job.data.error || 'unknown error'}`) } + else { setAiStatus(`Working… ${job.data.steps.at(-1)?.message || ''}`); setTimeout(poll, 2000) } + } catch { + // A poll that throws used to reject into nothing and leave the panel + // saying "Drafting…" for ever. The job may well still be running. + setAiStatus('') + setError('Lost track of that job. It may still be running — reload in a moment.') + } } setTimeout(poll, 1000) } catch (err) { setError(typeof err.response?.data?.detail === 'string' ? err.response.data.detail : 'Could not start drafting') } @@ -110,7 +117,7 @@ export default function ArticlesPage() { {user?.is_moderator && (
- +
)} @@ -124,8 +131,14 @@ export default function ArticlesPage() {