diff --git a/cli/src/__tests__/company-delete.test.ts b/cli/src/__tests__/company-delete.test.ts index 65e5a021..18a98cea 100644 --- a/cli/src/__tests__/company-delete.test.ts +++ b/cli/src/__tests__/company-delete.test.ts @@ -14,6 +14,7 @@ function makeCompany(overrides: Partial): Company { spentMonthlyCents: 0, requireBoardApprovalForNewAgents: false, brandColor: null, + logoAssetId: null, logoUrl: null, createdAt: new Date(), updatedAt: new Date(), diff --git a/docs/api/companies.md b/docs/api/companies.md index efcac8c2..e48d0176 100644 --- a/docs/api/companies.md +++ b/docs/api/companies.md @@ -38,7 +38,8 @@ PATCH /api/companies/{companyId} { "name": "Updated Name", "description": "Updated description", - "budgetMonthlyCents": 100000 + "budgetMonthlyCents": 100000, + "logoAssetId": "b9f5e911-6de5-4cd0-8dc6-a55a13bc02f6" } ``` @@ -60,6 +61,8 @@ Valid image content types: - `image/gif` - `image/svg+xml` +Then set the company logo by PATCHing the returned `assetId` into `logoAssetId`. + ## Archive Company ``` @@ -76,7 +79,8 @@ Archives a company. Archived companies are hidden from default listings. | `name` | string | Company name | | `description` | string | Company description | | `status` | string | `active`, `paused`, `archived` | -| `logoUrl` | string | Optional path or URL for the logo image | +| `logoAssetId` | string | Optional asset id for the stored logo image | +| `logoUrl` | string | Optional Paperclip asset content path for the stored logo image | | `budgetMonthlyCents` | number | Monthly budget limit | | `createdAt` | string | ISO timestamp | | `updatedAt` | string | ISO timestamp | diff --git a/packages/db/src/migrations/0030_hot_slipstream.sql b/packages/db/src/migrations/0030_hot_slipstream.sql deleted file mode 100644 index c00d29a3..00000000 --- a/packages/db/src/migrations/0030_hot_slipstream.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE "companies" ADD COLUMN "logo_url" text; \ No newline at end of file diff --git a/packages/db/src/migrations/0030_rich_magneto.sql b/packages/db/src/migrations/0030_rich_magneto.sql new file mode 100644 index 00000000..76d44de7 --- /dev/null +++ b/packages/db/src/migrations/0030_rich_magneto.sql @@ -0,0 +1,12 @@ +CREATE TABLE "company_logos" ( + "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, + "company_id" uuid NOT NULL, + "asset_id" uuid NOT NULL, + "created_at" timestamp with time zone DEFAULT now() NOT NULL, + "updated_at" timestamp with time zone DEFAULT now() NOT NULL +); +--> statement-breakpoint +ALTER TABLE "company_logos" ADD CONSTRAINT "company_logos_company_id_companies_id_fk" FOREIGN KEY ("company_id") REFERENCES "public"."companies"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint +ALTER TABLE "company_logos" ADD CONSTRAINT "company_logos_asset_id_assets_id_fk" FOREIGN KEY ("asset_id") REFERENCES "public"."assets"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint +CREATE UNIQUE INDEX "company_logos_company_uq" ON "company_logos" USING btree ("company_id");--> statement-breakpoint +CREATE UNIQUE INDEX "company_logos_asset_uq" ON "company_logos" USING btree ("asset_id"); \ No newline at end of file diff --git a/packages/db/src/migrations/meta/0030_snapshot.json b/packages/db/src/migrations/meta/0030_snapshot.json index 539d4be0..4f21ce46 100644 --- a/packages/db/src/migrations/meta/0030_snapshot.json +++ b/packages/db/src/migrations/meta/0030_snapshot.json @@ -1,5 +1,5 @@ { - "id": "eb9b85ec-2048-4168-bff1-0e987773342a", + "id": "ff007d90-e1a0-4df3-beab-a5be4a47273c", "prevId": "fdb36f4e-6463-497d-b704-22d33be9b450", "version": "7", "dialect": "postgresql", @@ -2140,12 +2140,6 @@ "primaryKey": false, "notNull": false }, - "logo_url": { - "name": "logo_url", - "type": "text", - "primaryKey": false, - "notNull": false - }, "created_at": { "name": "created_at", "type": "timestamp with time zone", @@ -2185,6 +2179,110 @@ "checkConstraints": {}, "isRLSEnabled": false }, + "public.company_logos": { + "name": "company_logos", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "asset_id": { + "name": "asset_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "company_logos_company_uq": { + "name": "company_logos_company_uq", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "company_logos_asset_uq": { + "name": "company_logos_asset_uq", + "columns": [ + { + "expression": "asset_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "company_logos_company_id_companies_id_fk": { + "name": "company_logos_company_id_companies_id_fk", + "tableFrom": "company_logos", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "company_logos_asset_id_assets_id_fk": { + "name": "company_logos_asset_id_assets_id_fk", + "tableFrom": "company_logos", + "tableTo": "assets", + "columnsFrom": [ + "asset_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.company_memberships": { "name": "company_memberships", "schema": "", diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index 852fa998..696c437a 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -215,8 +215,8 @@ { "idx": 30, "version": "7", - "when": 1773668505562, - "tag": "0030_hot_slipstream", + "when": 1773670925214, + "tag": "0030_rich_magneto", "breakpoints": true } ] diff --git a/packages/db/src/schema/companies.ts b/packages/db/src/schema/companies.ts index 3e75d4e6..29c82b71 100644 --- a/packages/db/src/schema/companies.ts +++ b/packages/db/src/schema/companies.ts @@ -15,7 +15,6 @@ export const companies = pgTable( .notNull() .default(true), brandColor: text("brand_color"), - logoUrl: text("logo_url"), createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), updatedAt: timestamp("updated_at", { withTimezone: true }).notNull().defaultNow(), }, diff --git a/packages/db/src/schema/company_logos.ts b/packages/db/src/schema/company_logos.ts new file mode 100644 index 00000000..13e0abe0 --- /dev/null +++ b/packages/db/src/schema/company_logos.ts @@ -0,0 +1,18 @@ +import { pgTable, uuid, timestamp, uniqueIndex } from "drizzle-orm/pg-core"; +import { companies } from "./companies.js"; +import { assets } from "./assets.js"; + +export const companyLogos = pgTable( + "company_logos", + { + id: uuid("id").primaryKey().defaultRandom(), + companyId: uuid("company_id").notNull().references(() => companies.id, { onDelete: "cascade" }), + assetId: uuid("asset_id").notNull().references(() => assets.id, { onDelete: "cascade" }), + createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), + updatedAt: timestamp("updated_at", { withTimezone: true }).notNull().defaultNow(), + }, + (table) => ({ + companyUq: uniqueIndex("company_logos_company_uq").on(table.companyId), + assetUq: uniqueIndex("company_logos_asset_uq").on(table.assetId), + }), +); diff --git a/packages/db/src/schema/index.ts b/packages/db/src/schema/index.ts index f173db45..422d7cdd 100644 --- a/packages/db/src/schema/index.ts +++ b/packages/db/src/schema/index.ts @@ -1,4 +1,5 @@ export { companies } from "./companies.js"; +export { companyLogos } from "./company_logos.js"; export { authUsers, authSessions, authAccounts, authVerifications } from "./auth.js"; export { instanceUserRoles } from "./instance_user_roles.js"; export { agents } from "./agents.js"; diff --git a/packages/shared/src/types/company.ts b/packages/shared/src/types/company.ts index 3002c7d3..e9022b93 100644 --- a/packages/shared/src/types/company.ts +++ b/packages/shared/src/types/company.ts @@ -11,6 +11,7 @@ export interface Company { spentMonthlyCents: number; requireBoardApprovalForNewAgents: boolean; brandColor: string | null; + logoAssetId: string | null; logoUrl: string | null; createdAt: Date; updatedAt: Date; diff --git a/packages/shared/src/validators/company.ts b/packages/shared/src/validators/company.ts index 65083c91..bb4851f4 100644 --- a/packages/shared/src/validators/company.ts +++ b/packages/shared/src/validators/company.ts @@ -1,19 +1,12 @@ import { z } from "zod"; import { COMPANY_STATUSES } from "../constants.js"; -const logoUrlSchema = z - .string() - .trim() - .max(2048) - .regex(/^\/api\/assets\/[^\s]+$|^https?:\/\/[^\s]+$/) - .nullable() - .optional(); +const logoAssetIdSchema = z.string().uuid().nullable().optional(); export const createCompanySchema = z.object({ name: z.string().min(1), description: z.string().optional().nullable(), budgetMonthlyCents: z.number().int().nonnegative().optional().default(0), - logoUrl: logoUrlSchema, }); export type CreateCompany = z.infer; @@ -25,7 +18,7 @@ export const updateCompanySchema = createCompanySchema spentMonthlyCents: z.number().int().nonnegative().optional(), requireBoardApprovalForNewAgents: z.boolean().optional(), brandColor: z.string().regex(/^#[0-9a-fA-F]{6}$/).nullable().optional(), - logoUrl: logoUrlSchema, + logoAssetId: logoAssetIdSchema, }); export type UpdateCompany = z.infer; diff --git a/server/src/__tests__/assets.test.ts b/server/src/__tests__/assets.test.ts index 85ddf56a..745b9a76 100644 --- a/server/src/__tests__/assets.test.ts +++ b/server/src/__tests__/assets.test.ts @@ -189,6 +189,31 @@ describe("POST /api/companies/:companyId/assets/images", () => { expect(createAssetMock).not.toHaveBeenCalled(); }); + it("allows supported non-image attachments outside the company logo namespace", async () => { + const text = createStorageService("text/plain"); + const app = createApp(text); + + createAssetMock.mockResolvedValue({ + ...createAsset(), + contentType: "text/plain", + originalFilename: "note.txt", + }); + + const res = await request(app) + .post("/api/companies/company-1/assets/images") + .field("namespace", "issues/drafts") + .attach("file", Buffer.from("hello"), "note.txt"); + + expect(res.status).toBe(201); + expect(text.putFile).toHaveBeenCalledWith({ + companyId: "company-1", + namespace: "assets/issues/drafts", + originalFilename: "note.txt", + contentType: "text/plain", + body: expect.any(Buffer), + }); + }); + it("rejects SVG image uploads that cannot be sanitized", async () => { const app = createApp(createStorageService("image/svg+xml")); createAssetMock.mockResolvedValue(createAsset()); diff --git a/server/src/routes/assets.ts b/server/src/routes/assets.ts index 67c9751f..0ec6ffb1 100644 --- a/server/src/routes/assets.ts +++ b/server/src/routes/assets.ts @@ -116,7 +116,19 @@ export function assetRoutes(db: Db, storage: StorageService) { return; } + const parsedMeta = createAssetImageMetadataSchema.safeParse(req.body ?? {}); + if (!parsedMeta.success) { + res.status(400).json({ error: "Invalid image metadata", details: parsedMeta.error.issues }); + return; + } + + const namespaceSuffix = parsedMeta.data.namespace ?? "general"; + const isCompanyLogoNamespace = namespaceSuffix === "companies" || namespaceSuffix.startsWith("companies/"); const contentType = (file.mimetype || "").toLowerCase(); + if (isCompanyLogoNamespace && !contentType.startsWith("image/")) { + res.status(422).json({ error: `Unsupported image type: ${contentType || "unknown"}` }); + return; + } if (contentType !== SVG_CONTENT_TYPE && !isAllowedContentType(contentType)) { res.status(422).json({ error: `Unsupported file type: ${contentType || "unknown"}` }); return; @@ -134,15 +146,6 @@ export function assetRoutes(db: Db, storage: StorageService) { res.status(422).json({ error: "Image is empty" }); return; } - - const parsedMeta = createAssetImageMetadataSchema.safeParse(req.body ?? {}); - if (!parsedMeta.success) { - res.status(400).json({ error: "Invalid image metadata", details: parsedMeta.error.issues }); - return; - } - - const namespaceSuffix = parsedMeta.data.namespace ?? "general"; - const isCompanyLogoNamespace = namespaceSuffix === "companies" || namespaceSuffix.startsWith("companies/"); if (isCompanyLogoNamespace && fileBody.length > MAX_COMPANY_LOGO_BYTES) { res.status(422).json({ error: `Image exceeds ${MAX_COMPANY_LOGO_BYTES} bytes` }); return; diff --git a/server/src/services/companies.ts b/server/src/services/companies.ts index 38a1f12f..42c4e972 100644 --- a/server/src/services/companies.ts +++ b/server/src/services/companies.ts @@ -2,6 +2,8 @@ import { eq, count } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { companies, + companyLogos, + assets, agents, agentApiKeys, agentRuntimeState, @@ -23,10 +25,41 @@ import { principalPermissionGrants, companyMemberships, } from "@paperclipai/db"; +import { notFound, unprocessable } from "../errors.js"; export function companyService(db: Db) { const ISSUE_PREFIX_FALLBACK = "CMP"; + const companySelection = { + id: companies.id, + name: companies.name, + description: companies.description, + status: companies.status, + issuePrefix: companies.issuePrefix, + issueCounter: companies.issueCounter, + budgetMonthlyCents: companies.budgetMonthlyCents, + spentMonthlyCents: companies.spentMonthlyCents, + requireBoardApprovalForNewAgents: companies.requireBoardApprovalForNewAgents, + brandColor: companies.brandColor, + logoAssetId: companyLogos.assetId, + createdAt: companies.createdAt, + updatedAt: companies.updatedAt, + }; + + function enrichCompany(company: T) { + return { + ...company, + logoUrl: company.logoAssetId ? `/api/assets/${company.logoAssetId}/content` : null, + }; + } + + function getCompanyQuery(database: Pick) { + return database + .select(companySelection) + .from(companies) + .leftJoin(companyLogos, eq(companyLogos.companyId, companies.id)); + } + function deriveIssuePrefixBase(name: string) { const normalized = name.toUpperCase().replace(/[^A-Z]/g, ""); return normalized.slice(0, 3) || ISSUE_PREFIX_FALLBACK; @@ -70,32 +103,97 @@ export function companyService(db: Db) { } return { - list: () => db.select().from(companies), + list: () => + getCompanyQuery(db).then((rows) => rows.map((row) => enrichCompany(row))), getById: (id: string) => - db - .select() - .from(companies) + getCompanyQuery(db) .where(eq(companies.id, id)) - .then((rows) => rows[0] ?? null), + .then((rows) => (rows[0] ? enrichCompany(rows[0]) : null)), - create: async (data: typeof companies.$inferInsert) => createCompanyWithUniquePrefix(data), + create: async (data: typeof companies.$inferInsert) => { + const created = await createCompanyWithUniquePrefix(data); + const row = await getCompanyQuery(db) + .where(eq(companies.id, created.id)) + .then((rows) => rows[0] ?? null); + if (!row) throw notFound("Company not found after creation"); + return enrichCompany(row); + }, - update: (id: string, data: Partial) => - db - .update(companies) - .set({ ...data, updatedAt: new Date() }) - .where(eq(companies.id, id)) - .returning() - .then((rows) => rows[0] ?? null), + update: ( + id: string, + data: Partial & { logoAssetId?: string | null }, + ) => + db.transaction(async (tx) => { + const existing = await getCompanyQuery(tx) + .where(eq(companies.id, id)) + .then((rows) => rows[0] ?? null); + if (!existing) return null; + + const { logoAssetId, ...companyPatch } = data; + + if (logoAssetId !== undefined && logoAssetId !== null) { + const nextLogoAsset = await tx + .select({ id: assets.id, companyId: assets.companyId }) + .from(assets) + .where(eq(assets.id, logoAssetId)) + .then((rows) => rows[0] ?? null); + if (!nextLogoAsset) throw notFound("Logo asset not found"); + if (nextLogoAsset.companyId !== existing.id) { + throw unprocessable("Logo asset must belong to the same company"); + } + } + + const updated = await tx + .update(companies) + .set({ ...companyPatch, updatedAt: new Date() }) + .where(eq(companies.id, id)) + .returning() + .then((rows) => rows[0] ?? null); + if (!updated) return null; + + if (logoAssetId === null) { + await tx.delete(companyLogos).where(eq(companyLogos.companyId, id)); + } else if (logoAssetId !== undefined) { + await tx + .insert(companyLogos) + .values({ + companyId: id, + assetId: logoAssetId, + }) + .onConflictDoUpdate({ + target: companyLogos.companyId, + set: { + assetId: logoAssetId, + updatedAt: new Date(), + }, + }); + } + + if (logoAssetId !== undefined && existing.logoAssetId && existing.logoAssetId !== logoAssetId) { + await tx.delete(assets).where(eq(assets.id, existing.logoAssetId)); + } + + return enrichCompany({ + ...updated, + logoAssetId: logoAssetId === undefined ? existing.logoAssetId : logoAssetId, + }); + }), archive: (id: string) => - db - .update(companies) - .set({ status: "archived", updatedAt: new Date() }) - .where(eq(companies.id, id)) - .returning() - .then((rows) => rows[0] ?? null), + db.transaction(async (tx) => { + const updated = await tx + .update(companies) + .set({ status: "archived", updatedAt: new Date() }) + .where(eq(companies.id, id)) + .returning() + .then((rows) => rows[0] ?? null); + if (!updated) return null; + const row = await getCompanyQuery(tx) + .where(eq(companies.id, id)) + .then((rows) => rows[0] ?? null); + return row ? enrichCompany(row) : null; + }), remove: (id: string) => db.transaction(async (tx) => { @@ -116,6 +214,8 @@ export function companyService(db: Db) { await tx.delete(principalPermissionGrants).where(eq(principalPermissionGrants.companyId, id)); await tx.delete(companyMemberships).where(eq(companyMemberships.companyId, id)); await tx.delete(issues).where(eq(issues.companyId, id)); + await tx.delete(companyLogos).where(eq(companyLogos.companyId, id)); + await tx.delete(assets).where(eq(assets.companyId, id)); await tx.delete(goals).where(eq(goals.companyId, id)); await tx.delete(projects).where(eq(projects.companyId, id)); await tx.delete(agents).where(eq(agents.companyId, id)); diff --git a/ui/src/api/companies.ts b/ui/src/api/companies.ts index eb53524b..bc21414e 100644 --- a/ui/src/api/companies.ts +++ b/ui/src/api/companies.ts @@ -18,7 +18,6 @@ export const companiesApi = { name: string; description?: string | null; budgetMonthlyCents?: number; - logoUrl?: string | null; }) => api.post("/companies", data), update: ( @@ -26,7 +25,7 @@ export const companiesApi = { data: Partial< Pick< Company, - "name" | "description" | "status" | "budgetMonthlyCents" | "requireBoardApprovalForNewAgents" | "brandColor" | "logoUrl" + "name" | "description" | "status" | "budgetMonthlyCents" | "requireBoardApprovalForNewAgents" | "brandColor" | "logoAssetId" > >, ) => api.patch(`/companies/${companyId}`, data), diff --git a/ui/src/context/CompanyContext.tsx b/ui/src/context/CompanyContext.tsx index 6785f8ff..86afa175 100644 --- a/ui/src/context/CompanyContext.tsx +++ b/ui/src/context/CompanyContext.tsx @@ -28,7 +28,6 @@ interface CompanyContextValue { name: string; description?: string | null; budgetMonthlyCents?: number; - logoUrl?: string | null; }) => Promise; } @@ -90,7 +89,6 @@ export function CompanyProvider({ children }: { children: ReactNode }) { name: string; description?: string | null; budgetMonthlyCents?: number; - logoUrl?: string | null; }) => companiesApi.create(data), onSuccess: (company) => { @@ -104,7 +102,6 @@ export function CompanyProvider({ children }: { children: ReactNode }) { name: string; description?: string | null; budgetMonthlyCents?: number; - logoUrl?: string | null; }) => { return createMutation.mutateAsync(data); }, diff --git a/ui/src/pages/CompanySettings.tsx b/ui/src/pages/CompanySettings.tsx index c8a5d5aa..3cd310ca 100644 --- a/ui/src/pages/CompanySettings.tsx +++ b/ui/src/pages/CompanySettings.tsx @@ -141,7 +141,7 @@ export function CompanySettings() { mutationFn: (file: File) => assetsApi .uploadImage(selectedCompanyId!, file, "companies") - .then((asset) => companiesApi.update(selectedCompanyId!, { logoUrl: asset.contentPath })), + .then((asset) => companiesApi.update(selectedCompanyId!, { logoAssetId: asset.assetId })), onSuccess: (company) => { syncLogoState(company.logoUrl); setLogoUploadError(null); @@ -149,7 +149,7 @@ export function CompanySettings() { }); const clearLogoMutation = useMutation({ - mutationFn: () => companiesApi.update(selectedCompanyId!, { logoUrl: null }), + mutationFn: () => companiesApi.update(selectedCompanyId!, { logoAssetId: null }), onSuccess: (company) => { setLogoUploadError(null); syncLogoState(company.logoUrl);