Skip to content

Commit 352734f

Browse files
fix(api): continue package ban batches during in-flight bans
1 parent edb266b commit 352734f

3 files changed

Lines changed: 86 additions & 2 deletions

File tree

convex/packages.public.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,7 @@ const applyBanToOwnedPackagesBatchInternalHandler = (
536536
deletedBy: string;
537537
deletedByRole: "admin" | "moderator" | "user";
538538
cursor?: string;
539+
allowActiveOwnerBeforeCommit?: boolean;
539540
},
540541
{ deletedCount: number; revokedTokenCount: number; scheduled: boolean }
541542
>
@@ -6952,6 +6953,7 @@ describe("owned package sanction batches", () => {
69526953
bannedAt: 1_000,
69536954
deletedBy: "users:moderator",
69546955
deletedByRole: "moderator",
6956+
allowActiveOwnerBeforeCommit: true,
69556957
});
69566958

69576959
expect(result).toMatchObject({ deletedCount: 1, revokedTokenCount: 1, scheduled: false });
@@ -6988,6 +6990,82 @@ describe("owned package sanction batches", () => {
69886990
expect(patch).not.toHaveBeenCalledWith("packagePublishTokens:demo", expect.anything());
69896991
});
69906992

6993+
it("continues in-flight package ban pages before the user ban commit is visible", async () => {
6994+
const firstPage = makeOwnedPackageBatchCtx({
6995+
owner: { _id: "users:owner", deletedAt: undefined, deactivatedAt: undefined },
6996+
pkg: makePackageDoc({ _id: "packages:first", ownerUserId: "users:owner" }),
6997+
packageTokens: [
6998+
{
6999+
_id: "packagePublishTokens:first",
7000+
packageId: "packages:first",
7001+
version: "1.0.1",
7002+
revokedAt: undefined,
7003+
},
7004+
],
7005+
isDone: false,
7006+
continueCursor: "next-page",
7007+
});
7008+
7009+
const firstResult = await applyBanToOwnedPackagesBatchInternalHandler(firstPage.ctx as never, {
7010+
ownerUserId: "users:owner",
7011+
bannedAt: 1_000,
7012+
deletedBy: "users:moderator",
7013+
deletedByRole: "moderator",
7014+
allowActiveOwnerBeforeCommit: true,
7015+
});
7016+
7017+
expect(firstResult).toMatchObject({ deletedCount: 1, revokedTokenCount: 1, scheduled: true });
7018+
expect(firstPage.patch).toHaveBeenCalledWith(
7019+
"packages:first",
7020+
expect.objectContaining({ softDeletedAt: 1_000, softDeletedReason: "user.banned" }),
7021+
);
7022+
expect(firstPage.patch).toHaveBeenCalledWith("packagePublishTokens:first", {
7023+
revokedAt: 1_000,
7024+
});
7025+
expect(firstPage.runAfter).toHaveBeenCalledWith(
7026+
0,
7027+
expect.anything(),
7028+
expect.objectContaining({
7029+
cursor: "next-page",
7030+
allowActiveOwnerBeforeCommit: true,
7031+
}),
7032+
);
7033+
7034+
const secondPage = makeOwnedPackageBatchCtx({
7035+
owner: { _id: "users:owner", deletedAt: undefined, deactivatedAt: undefined },
7036+
pkg: makePackageDoc({ _id: "packages:second", ownerUserId: "users:owner" }),
7037+
packageTokens: [
7038+
{
7039+
_id: "packagePublishTokens:second",
7040+
packageId: "packages:second",
7041+
version: "1.0.1",
7042+
revokedAt: undefined,
7043+
},
7044+
],
7045+
});
7046+
7047+
const secondResult = await applyBanToOwnedPackagesBatchInternalHandler(
7048+
secondPage.ctx as never,
7049+
{
7050+
ownerUserId: "users:owner",
7051+
bannedAt: 1_000,
7052+
deletedBy: "users:moderator",
7053+
deletedByRole: "moderator",
7054+
cursor: "next-page",
7055+
allowActiveOwnerBeforeCommit: true,
7056+
},
7057+
);
7058+
7059+
expect(secondResult).toMatchObject({ deletedCount: 1, revokedTokenCount: 1, scheduled: false });
7060+
expect(secondPage.patch).toHaveBeenCalledWith(
7061+
"packages:second",
7062+
expect.objectContaining({ softDeletedAt: 1_000, softDeletedReason: "user.banned" }),
7063+
);
7064+
expect(secondPage.patch).toHaveBeenCalledWith("packagePublishTokens:second", {
7065+
revokedAt: 1_000,
7066+
});
7067+
});
7068+
69917069
it("retimestamps earlier ban-hidden packages during a later ban", async () => {
69927070
const { ctx, patch } = makeOwnedPackageBatchCtx({
69937071
owner: { _id: "users:owner", deletedAt: 2_000, deactivatedAt: undefined },

convex/packages.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2941,13 +2941,17 @@ export const applyBanToOwnedPackagesBatchInternal = internalMutation({
29412941
deletedBy: v.id("users"),
29422942
deletedByRole: v.union(v.literal("admin"), v.literal("moderator"), v.literal("user")),
29432943
cursor: v.optional(v.string()),
2944+
allowActiveOwnerBeforeCommit: v.optional(v.boolean()),
29442945
},
29452946
handler: async (ctx, args) => {
29462947
const owner = await ctx.db.get(args.ownerUserId);
2947-
const isInitialPage = args.cursor === undefined;
29482948
const ownerMatchesCurrentBan = owner?.deletedAt === args.bannedAt;
2949+
// Ban callers run this before users.deletedAt is visible; keep the allow flag
2950+
// through scheduled pages so multi-page batches do not stale-stop mid-ban.
29492951
const ownerIsActiveBeforeBanCommit =
2950-
isInitialPage && owner?.deletedAt === undefined && !owner?.deactivatedAt;
2952+
args.allowActiveOwnerBeforeCommit === true &&
2953+
owner?.deletedAt === undefined &&
2954+
!owner?.deactivatedAt;
29512955
if (
29522956
!owner ||
29532957
owner.deactivatedAt ||

convex/users.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1646,6 +1646,7 @@ async function banUserWithActor(
16461646
deletedBy: actor._id,
16471647
deletedByRole: actor.role === "admin" ? "admin" : "moderator",
16481648
cursor: undefined,
1649+
allowActiveOwnerBeforeCommit: true,
16491650
},
16501651
)) ?? {}) as { deletedCount?: number; revokedTokenCount?: number; scheduled?: boolean };
16511652
const deletedPackageCount = banPackagesResult.deletedCount ?? 0;
@@ -2106,6 +2107,7 @@ export const autobanMalwareAuthorInternal = internalMutation({
21062107
deletedBy: args.ownerUserId,
21072108
deletedByRole: "user",
21082109
cursor: undefined,
2110+
allowActiveOwnerBeforeCommit: true,
21092111
},
21102112
)) ?? {}) as { deletedCount?: number; revokedTokenCount?: number; scheduled?: boolean };
21112113
const deletedPackageCount = banPackagesResult.deletedCount ?? 0;

0 commit comments

Comments
 (0)