From 7523c9c0288aad4ad4d8ca8a38254b399c55d089 Mon Sep 17 00:00:00 2001
From: Aditya Garud <153842990+yashranaway@users.noreply.github.com>
Date: Thu, 29 Oct 2026 20:37:19 +0000
Subject: [PATCH] fix(web): clean up space icons in their own organization

---
 apps/web/__tests__/unit/delete-space.test.ts  | 68 +++++++++++++++++--
 apps/web/actions/organization/delete-space.ts |  2 +-
 2 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/apps/web/__tests__/unit/delete-space.test.ts b/apps/web/__tests__/unit/delete-space.test.ts
index 73bf215c7b0..61c7935da1f 100644
--- a/apps/web/__tests__/unit/delete-space.test.ts
+++ b/apps/web/__tests__/unit/delete-space.test.ts
@@ -1,6 +1,7 @@
 import { Space } from "@cap/web-domain";
 import { getTableName } from "drizzle-orm";
 import type { MySqlTable } from "drizzle-orm/mysql-core";
+import { Effect } from "effect";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 
 const mocks = vi.hoisted(() => ({
@@ -8,6 +9,9 @@ const mocks = vi.hoisted(() => ({
 	manager: vi.fn(),
 	runPromise: vi.fn(),
 	transaction: vi.fn(),
+	bucketAccess: vi.fn(),
+	listObjects: vi.fn(),
+	deleteObjects: vi.fn(),
 	events: [] as string[],
 }));
 
@@ -47,7 +51,7 @@ function deleteQuery(prefix: string) {
 
 vi.mock("@cap/database", () => ({
 	db: () => ({
-		select: selectQuery("", () => [{ id: "space-1" }]),
+		select: selectQuery("", () => [{ id: "space-1", organizationId: "org" }]),
 		delete: deleteQuery(""),
 		transaction: mocks.transaction,
 	}),
@@ -56,7 +60,9 @@ vi.mock("@cap/database/auth/session", () => ({ getCurrentUser: mocks.user }));
 vi.mock("@/actions/organization/space-authorization", () => ({
 	requireSpaceManager: mocks.manager,
 }));
-vi.mock("@cap/web-backend", () => ({ S3Buckets: {} }));
+vi.mock("@cap/web-backend", () => ({
+	S3Buckets: { getBucketAccess: mocks.bucketAccess },
+}));
 vi.mock("@/lib/server", () => ({ runPromise: mocks.runPromise }));
 vi.mock("next/cache", () => ({ revalidatePath: vi.fn() }));
 
@@ -68,9 +74,22 @@ beforeEach(() => {
 	mocks.events.length = 0;
 	mocks.user.mockResolvedValue({ id: "user-1", activeOrganizationId: "org" });
 	mocks.manager.mockResolvedValue({ canManage: true });
-	mocks.runPromise.mockImplementation(async () => {
-		mocks.events.push("s3");
-	});
+	mocks.listObjects.mockReturnValue(Effect.succeed({}));
+	mocks.deleteObjects.mockReturnValue(Effect.void);
+	mocks.bucketAccess.mockReturnValue(
+		Effect.succeed([
+			{
+				listObjects: mocks.listObjects,
+				deleteObjects: mocks.deleteObjects,
+			},
+		]),
+	);
+	mocks.runPromise.mockImplementation(
+		(effect: Effect.Effect<unknown, unknown>) => {
+			mocks.events.push("s3");
+			return Effect.runPromise(effect);
+		},
+	);
 	mocks.transaction.mockImplementation(
 		async (callback: (tx: unknown) => Promise<unknown>) => {
 			mocks.events.push("begin");
@@ -84,6 +103,27 @@ beforeEach(() => {
 });
 
 describe("deleting a space", () => {
+	it("cleans up icons in the space's organization after the caller switches organizations", async () => {
+		mocks.user.mockResolvedValue({
+			id: "user-1",
+			activeOrganizationId: "other-org",
+		});
+		const key = "organizations/org/spaces/space-1/icon.png";
+		mocks.listObjects.mockImplementation(({ prefix }: { prefix: string }) =>
+			Effect.succeed({
+				Contents: key.startsWith(prefix) ? [{ Key: key }] : [],
+			}),
+		);
+
+		await expect(deleteSpace(spaceId)).resolves.toEqual({ success: true });
+
+		expect(mocks.manager).toHaveBeenCalledWith("user-1", spaceId);
+		expect(mocks.listObjects).toHaveBeenCalledWith({
+			prefix: "organizations/org/spaces/space-1/",
+		});
+		expect(mocks.deleteObjects).toHaveBeenCalledWith([{ Key: key }]);
+	});
+
 	it("locks the space and removes its records in one transaction before storage cleanup", async () => {
 		await expect(deleteSpace(spaceId)).resolves.toEqual({ success: true });
 		expect(mocks.events).toEqual([
@@ -98,6 +138,9 @@ describe("deleting a space", () => {
 			"commit",
 			"s3",
 		]);
+		expect(mocks.listObjects).toHaveBeenCalledWith({
+			prefix: "organizations/org/spaces/space-1/",
+		});
 	});
 
 	it("does not touch records when the caller cannot manage the space", async () => {
@@ -106,6 +149,21 @@ describe("deleting a space", () => {
 			success: false,
 		});
 		expect(mocks.transaction).not.toHaveBeenCalled();
+		expect(mocks.bucketAccess).not.toHaveBeenCalled();
 		expect(mocks.events).toEqual(["select:spaces"]);
 	});
+
+	it("keeps the space deleted when storage cleanup fails", async () => {
+		vi.spyOn(console, "error").mockImplementation(() => {});
+		mocks.listObjects.mockReturnValue(
+			Effect.fail(new Error("Storage unavailable")),
+		);
+
+		await expect(deleteSpace(spaceId)).resolves.toEqual({ success: true });
+
+		expect(mocks.events.indexOf("commit")).toBeLessThan(
+			mocks.events.indexOf("s3"),
+		);
+		expect(mocks.deleteObjects).not.toHaveBeenCalled();
+	});
 });
diff --git a/apps/web/actions/organization/delete-space.ts b/apps/web/actions/organization/delete-space.ts
index 2f67ea53f45..09cc62fda02 100644
--- a/apps/web/actions/organization/delete-space.ts
+++ b/apps/web/actions/organization/delete-space.ts
@@ -76,7 +76,7 @@ export async function deleteSpace(
 				const [bucket] = yield* S3Buckets.getBucketAccess(Option.none());
 
 				const listedObjects = yield* bucket.listObjects({
-					prefix: `organizations/${user.activeOrganizationId}/spaces/${spaceId}/`,
+					prefix: `organizations/${spaceData.organizationId}/spaces/${spaceId}/`,
 				});
 
 				if (listedObjects.Contents) {