Add moderator delisting for published extensions

src/services/extensions/v1/database.ts

@@ -1,4 +1,4 @@
-import { and, eq, isNotNull, sql } from "drizzle-orm";
+import { and, eq, isNotNull, isNull, sql } from "drizzle-orm";
 import { ExtensionsDb } from "../../../lib/db";
 import { extensions, developers } from "../v2/db/schema";
 import { DatabaseResult } from "../../../lib/interfaces";
@@ -39,7 +39,10 @@ const EXTENSION_COLUMNS = {
 // non-null here because both queries below filter on published_at IS NOT NULL
 // and extensions_published_content_check makes that filter sufficient - a
 // published row cannot be missing any of them. That filter is also what keeps
-// unreviewed extensions out of the v1 catalogue.
+// unreviewed extensions out of the v1 catalogue. delisted_at IS NULL is the
+// other half: v1 shares this table with v2's public catalogue and must stay
+// in sync with what v2 hides, or a moderator delisting an extension would
+// pull it from the v2 catalogue while it stayed visible here.
 interface ExtensionRow {
   id: string;
   type: string;
@@ -65,7 +68,10 @@ export class ExtensionsDatabase {
   async getAllExtensions(type?: string): Promise<DatabaseResult<Extension[]>> {
     let rows: ExtensionRow[];
     try {
-      const published = isNotNull(extensions.publishedAt);
+      const published = and(
+        isNotNull(extensions.publishedAt),
+        isNull(extensions.delistedAt)
+      );
       rows = (await this.db
         .select(EXTENSION_COLUMNS)
         .from(extensions)
@@ -96,7 +102,8 @@ export class ExtensionsDatabase {
         .where(
           and(
             sql`LOWER(${extensions.id}) = LOWER(${id})`,
-            isNotNull(extensions.publishedAt)
+            isNotNull(extensions.publishedAt),
+            isNull(extensions.delistedAt)
           )
         )) as ExtensionRow[];
     } catch (error) {

src/services/extensions/v2/README.md

@@ -30,6 +30,17 @@ against it.
 - `POST /extensions/{id}/revisions/{revisionId}/approve` publishes the
   revision's content. `reject` leaves the published content untouched and the
   extension available to edit and resubmit.
+- `POST /extensions/{id}/delist` pulls an already-published extension out of
+  the public catalogue for cause (its upstream source disappearing, for
+  example). Moderator-only, and the inverse of neither `approve` nor
+  `reject`: content and history are kept, so the owner can still see and edit
+  the extension, and a moderator can re-list it by hand later. There is no
+  `relist` endpoint yet - see `ExtensionsDatabase.delist()`.
+- `GET /moderation/extensions/{id}` is a moderator's only full-record read of
+  an extension they don't own - the same `OwnedExtension` shape as
+  `GET /extensions/mine/{id}`, including `delisted`. Without it, a moderator
+  could delist an extension but never see why (their own or another
+  moderator's) again short of digging through `GET /extensions/{id}/revisions`.
 
 The id and the developer are properties of the extension, not of a revision: an
 edit cannot rename an extension or move it to another developer, and approving
@@ -38,9 +49,12 @@ most one developer profile, so no request body names one.
 
 ### Reading Owner State
 
-`GET /extensions/mine` and `GET /extensions/mine/{id}` return three independent
+`GET /extensions/mine` and `GET /extensions/mine/{id}` return four independent
 fields rather than a single derived status, because together they are the
-state and a derived enum could only disagree with them:
+state and a derived enum could only disagree with them. The table below
+covers three of them - `published`, `pending_revision` and `last_review` - the
+fourth, `delisted`, is documented separately just below since it is orthogonal
+to all three:
 
 | `published` | `pending_revision` | `last_review` | Meaning                                 |
 | ----------- | ------------------ | ------------- | --------------------------------------- |
@@ -54,7 +68,10 @@ state and a derived enum could only disagree with them:
 The adopted row is the one worth reading twice: migration 0021 published every
 extension that already existed, and those have no revisions at all, so a live
 extension with no review history is normal rather than a gap. `published`
-being set is the only thing that means "in the catalogue".
+being set is the only thing that means "in the catalogue" - except a fourth,
+independent field, `delisted`: set once a moderator removes a published
+extension for cause, it hides the row from both public catalogue reads
+without touching `published`, `pending_revision` or `last_review`.
 
 These are separate routes from the public `GET /extensions` and
 `GET /extensions/{id}`, which only ever return published content. A single path

src/services/extensions/v2/db/extensions.ts

@@ -1,4 +1,4 @@
-import { and, asc, eq, isNotNull, or, sql } from "drizzle-orm";
+import { and, asc, eq, isNotNull, isNull, or, sql } from "drizzle-orm";
 import { alias } from "drizzle-orm/sqlite-core";
 import { DatabaseError, DatabaseResult } from "../../../../lib/interfaces";
 import { ExtensionsDb } from "../../../../lib/db";
@@ -113,6 +113,8 @@ const {
 const OWNED_LIST_COLUMNS = {
   id: extensions.id,
   publishedAt: extensions.publishedAt,
+  delistedAt: extensions.delistedAt,
+  delistReason: extensions.delistReason,
   createdAt: extensions.createdAt,
   updatedAt: extensions.updatedAt,
   ...CARD_CONTENT_COLUMNS,
@@ -221,7 +223,10 @@ export class ExtensionsDatabase {
     filters: ExtensionListFilters = {}
   ): Promise<DatabaseResult<ExtensionListPage>> {
     const limit = filters.limit ?? 50;
-    const conditions = [isNotNull(extensions.publishedAt)];
+    const conditions = [
+      isNotNull(extensions.publishedAt),
+      isNull(extensions.delistedAt)
+    ];
     if (filters.type) conditions.push(eq(extensions.type, filters.type));
     if (filters.developerId)
       conditions.push(eq(extensions.developerId, filters.developerId));
@@ -268,7 +273,8 @@ export class ExtensionsDatabase {
         .where(
           and(
             sql`LOWER(${extensions.id}) = LOWER(${id})`,
-            isNotNull(extensions.publishedAt)
+            isNotNull(extensions.publishedAt),
+            isNull(extensions.delistedAt)
           )
         )) as PublishedRow[];
     } catch (error) {
@@ -526,6 +532,97 @@ export class ExtensionsDatabase {
       }
     };
   }
+
+  // A moderator's decision to pull an already-published extension from the
+  // catalogue for cause - its upstream source disappearing, for example.
+  // Content and history are kept (unlike withdraw(), which deletes a
+  // never-published row outright), so an owner can still see why and a
+  // moderator can re-list it later without the developer resubmitting from
+  // scratch. `AND delisted_at IS NULL` makes this a single atomic
+  // check-and-set, the same way ExtensionRevisionsDatabase.reject() guards
+  // on `status = 'pending'`.
+  async delist(
+    id: string,
+    moderatorId: string,
+    reason: string
+  ): Promise<DatabaseResult<{ id: string }>> {
+    let result;
+    try {
+      result = await this.db
+        .update(extensions)
+        .set({
+          delistedAt: sql`CURRENT_TIMESTAMP`,
+          delistReason: reason,
+          updatedAt: sql`CURRENT_TIMESTAMP`
+        })
+        .where(
+          and(
+            sql`LOWER(${extensions.id}) = LOWER(${id})`,
+            isNotNull(extensions.publishedAt),
+            isNull(extensions.delistedAt),
+            sql`EXISTS (
+              SELECT 1 FROM ${users}
+              WHERE ${users.id} = ${moderatorId} AND ${users.deletedAt} IS NULL
+            )`
+          )
+        );
+    } catch (error) {
+      return databaseError("delist", error);
+    }
+
+    if (!result.meta?.changes) {
+      return this.delistBlockedError(id, moderatorId);
+    }
+
+    return { data: { id }, error: null };
+  }
+
+  // Separates the three ways delist()'s guard can affect no rows, so the
+  // route can answer 404/409 rather than one opaque failure.
+  private async delistBlockedError(
+    id: string,
+    moderatorId: string
+  ): Promise<DatabaseResult<never>> {
+    const inactive = await inactiveActorError(this.db, moderatorId);
+    if (inactive) return { data: null, error: inactive };
+
+    let existing:
+      { publishedAt: string | null; delistedAt: string | null } | undefined;
+    try {
+      [existing] = await this.db
+        .select({
+          publishedAt: extensions.publishedAt,
+          delistedAt: extensions.delistedAt
+        })
+        .from(extensions)
+        .where(sql`LOWER(${extensions.id}) = LOWER(${id})`);
+    } catch (error) {
+      return databaseError("delist", error);
+    }
+    if (!existing) return notFound(id);
+    if (!existing.publishedAt) {
+      return {
+        data: null,
+        error: {
+          message: "Only a published extension can be delisted",
+          code: "CONFLICT"
+        }
+      };
+    }
+    if (existing.delistedAt) {
+      return {
+        data: null,
+        error: {
+          message: "This extension is already delisted",
+          code: "CONFLICT"
+        }
+      };
+    }
+    return {
+      data: null,
+      error: { message: "Extension could not be delisted", code: "CONFLICT" }
+    };
+  }
 }
 
 function invalidCursor(): DatabaseResult<never> {
@@ -655,6 +752,11 @@ function parseOwnedListRow(row: OwnedListRow): OwnedExtensionListItem {
           reviewed_at: row.reviewedAt
         }
       : null,
+    // delistReason is only ever null alongside delistedAt - delist() sets
+    // both in the same statement - so the cast below just states that.
+    delisted: row.delistedAt
+      ? { reason: row.delistReason as string, at: row.delistedAt }
+      : null,
     created_at: row.createdAt,
     updated_at: row.updatedAt
   };

src/services/extensions/v2/db/migrations/0022_add_extensions_delisted_at.sql

@@ -0,0 +1,6 @@
+DROP INDEX `idx_extensions_catalogue_order`;--> statement-breakpoint
+DROP INDEX `idx_extensions_type_catalogue_order`;--> statement-breakpoint
+ALTER TABLE `extensions` ADD `delisted_at` text;--> statement-breakpoint
+ALTER TABLE `extensions` ADD `delist_reason` text;--> statement-breakpoint
+CREATE INDEX `idx_extensions_catalogue_order` ON `extensions` (lower("id"),`id`) WHERE "extensions"."published_at" IS NOT NULL AND "extensions"."delisted_at" IS NULL;--> statement-breakpoint
+CREATE INDEX `idx_extensions_type_catalogue_order` ON `extensions` (`type`,lower("id"),`id`) WHERE "extensions"."published_at" IS NOT NULL AND "extensions"."delisted_at" IS NULL;