From 256fe4522cf30f3dbbdb90fc7b6195d1775bc3d9 Mon Sep 17 00:00:00 2001 From: Saxon Fletcher Date: Fri, 6 Mar 2026 11:33:58 +1000 Subject: [PATCH] fixes --- .../protected/[partnerslug]/items/page.tsx | 54 +++++++++++------- .../marketplace/app/protected/actions.test.ts | 55 +++++++++++++++++++ apps/marketplace/app/protected/actions.ts | 21 ++++++- apps/marketplace/components/item-form.tsx | 18 +++--- .../lib/marketplace/item-draft.test.ts | 27 ++++++++- .../marketplace/lib/marketplace/item-draft.ts | 9 ++- .../lib/marketplace/review-state.test.ts | 17 ++++++ .../lib/marketplace/review-state.ts | 18 ++++++ .../lib/marketplace/server.test.ts | 14 ++++- apps/marketplace/lib/marketplace/server.ts | 22 +++++++- apps/marketplace/supabase/schemas/schemas.sql | 12 +++- .../tests/components/item-form.test.tsx | 16 ++++-- 12 files changed, 241 insertions(+), 42 deletions(-) diff --git a/apps/marketplace/app/protected/[partnerslug]/items/page.tsx b/apps/marketplace/app/protected/[partnerslug]/items/page.tsx index 5752c42c91a..0bb79418466 100644 --- a/apps/marketplace/app/protected/[partnerslug]/items/page.tsx +++ b/apps/marketplace/app/protected/[partnerslug]/items/page.tsx @@ -2,6 +2,7 @@ import { Plus, Search } from 'lucide-react' import Link from 'next/link' import { notFound } from 'next/navigation' import { + Badge, Button, Card, Input, @@ -22,6 +23,7 @@ import { } from 'ui-patterns/PageHeader' import { PageSection, PageSectionContent } from 'ui-patterns/PageSection' +import { deriveLatestReviewStatusDisplay } from '@/lib/marketplace/review-state' import { getMarketplaceSidebarData } from '@/lib/marketplace/server' type PartnerItemsPageProps = { @@ -108,29 +110,41 @@ export default async function PartnerItemsPage({ params, searchParams }: Partner Item Slug + Status - {filteredItems.map((item) => ( - - - - {item.title} - - - - - /{item.slug} - - - - ))} + {filteredItems.map((item) => { + const statusDisplay = deriveLatestReviewStatusDisplay(item.latestReviewStatus) + + return ( + + + + {item.title} + + + + + /{item.slug} + + + + {statusDisplay ? ( + {statusDisplay.label} + ) : ( + No review + )} + + + ) + })} diff --git a/apps/marketplace/app/protected/actions.test.ts b/apps/marketplace/app/protected/actions.test.ts index 6d7a56bbb5e..2cb7aeec5f1 100644 --- a/apps/marketplace/app/protected/actions.test.ts +++ b/apps/marketplace/app/protected/actions.test.ts @@ -343,6 +343,29 @@ describe('protected actions', () => { expect(result).toEqual({ itemId: 20, itemSlug: 'template-item', partnerSlug: 'acme' }) }) + it('creates template drafts without a template package', async () => { + createClientMock.mockResolvedValue( + createSupabaseMock({ + user: { id: 'user-1' }, + fromHandler: (table, state) => { + if (table === 'items' && state.op === 'insert') { + return success({ id: 21, slug: 'draft-template' }) + } + return success(null) + }, + }) + ) + + const formData = new FormData() + formData.set('partnerId', '1') + formData.set('partnerSlug', 'acme') + formData.set('title', 'Draft Template') + formData.set('type', 'template') + + const result = await createItemDraftAction(formData) + expect(result).toEqual({ itemId: 21, itemSlug: 'draft-template', partnerSlug: 'acme' }) + }) + it('throws when template item review upsert fails', async () => { createClientMock.mockResolvedValue( createSupabaseMock({ @@ -400,6 +423,9 @@ describe('protected actions', () => { createSupabaseMock({ user: { id: 'reviewer' }, fromHandler: (table, state) => { + if (table === 'items' && state.op === 'select') { + return success({ type: 'oauth', registry_item_url: null, url: 'https://example.com' }) + } if (table === 'item_reviews' && state.op === 'select') { return success({ status: 'draft' }) } @@ -570,6 +596,9 @@ describe('protected actions', () => { createSupabaseMock({ user: { id: 'reviewer' }, fromHandler: (table, state) => { + if (table === 'items' && state.op === 'select') { + return success({ type: 'oauth', registry_item_url: null, url: 'https://example.com' }) + } if (table === 'item_reviews' && state.op === 'select') { return failure('existing review query failed') } @@ -586,11 +615,37 @@ describe('protected actions', () => { await expect(requestItemReviewAction(formData)).rejects.toThrow('existing review query failed') }) + it('rejects template review requests when no template package has been uploaded', async () => { + createClientMock.mockResolvedValue( + createSupabaseMock({ + user: { id: 'reviewer' }, + fromHandler: (table, state) => { + if (table === 'items' && state.op === 'select') { + return success({ type: 'template', registry_item_url: null, url: null }) + } + return success(null) + }, + }) + ) + + const formData = new FormData() + formData.set('itemId', '7') + formData.set('itemSlug', 'my-item') + formData.set('partnerSlug', 'acme') + + await expect(requestItemReviewAction(formData)).rejects.toThrow( + 'Template items require a template ZIP package before publishing or requesting review' + ) + }) + it('throws when request-review upsert fails', async () => { createClientMock.mockResolvedValue( createSupabaseMock({ user: { id: 'reviewer' }, fromHandler: (table, state) => { + if (table === 'items' && state.op === 'select') { + return success({ type: 'oauth', registry_item_url: null, url: 'https://example.com' }) + } if (table === 'item_reviews' && state.op === 'select') { return success({ status: 'rejected' }) } diff --git a/apps/marketplace/app/protected/actions.ts b/apps/marketplace/app/protected/actions.ts index f51312fcbf6..d8cd6614d99 100644 --- a/apps/marketplace/app/protected/actions.ts +++ b/apps/marketplace/app/protected/actions.ts @@ -359,7 +359,7 @@ export async function createItemDraftAction(formData: FormData) { const slugSource = typeof slugInput === 'string' && slugInput.trim() ? slugInput : title const slug = slugify(slugSource) - ensureItemDraftConstraints({ type, slug, url, templateZip }) + ensureItemDraftConstraints({ type, slug, url, templateZip, published, intent }) const { data: item, error } = await supabase .from('items') @@ -470,6 +470,7 @@ export async function updateItemDraftAction(formData: FormData) { url, templateZip, existingRegistryItemUrl, + published, }) const templateRegistryUrl = @@ -559,6 +560,24 @@ export async function requestItemReviewAction(formData: FormData) { const itemId = Number(parseRequiredString(formData, 'itemId')) const itemSlug = parseRequiredString(formData, 'itemSlug') const partnerSlug = parseRequiredString(formData, 'partnerSlug') + const { data: item, error: itemError } = await supabase + .from('items') + .select('type, registry_item_url, url') + .eq('id', itemId) + .single() + + if (itemError || !item) { + throw new Error(itemError?.message ?? 'Unable to load item') + } + + ensureItemDraftConstraints({ + type: parseItemType(item.type), + slug: itemSlug, + url: item.url, + templateZip: null, + existingRegistryItemUrl: item.registry_item_url, + intent: 'request_review', + }) const { data: existingReview, error: existingReviewError } = await supabase .from('item_reviews') diff --git a/apps/marketplace/components/item-form.tsx b/apps/marketplace/components/item-form.tsx index b20fb830f57..ecdb142f4fa 100644 --- a/apps/marketplace/components/item-form.tsx +++ b/apps/marketplace/components/item-form.tsx @@ -380,14 +380,14 @@ export function ItemForm(props: ItemFormProps) { setError(null) setSuccess(null) + const intent = submitIntentRef.current if (parsed.data.type === 'template') { const hasExistingRegistryFile = Boolean(item?.registry_item_url) - if (isCreateMode && !templateZipFile) { - setError('Upload a template ZIP package that includes template.json.') - return - } - if (!isCreateMode && !templateZipFile && !hasExistingRegistryFile) { - setError('Upload a template ZIP package that includes template.json.') + const requiresTemplatePackage = parsed.data.published || intent === 'request_review' + if (requiresTemplatePackage && !templateZipFile && !hasExistingRegistryFile) { + setError( + 'Upload a template ZIP package that includes template.json before publishing or requesting review.' + ) return } } @@ -397,7 +397,6 @@ export function ItemForm(props: ItemFormProps) { } const formData = new FormData() - const intent = submitIntentRef.current const trimmedSlug = parsed.data.slug?.trim() formData.set('partnerId', String(props.partner.id)) @@ -764,6 +763,11 @@ export function ItemForm(props: ItemFormProps) { Selected package: {templateZipFile.name}

) : null} + {!hasTemplateFilesForTree ? ( +

+ Optional while drafting. Required before publishing or requesting review. +

+ ) : null} )} diff --git a/apps/marketplace/lib/marketplace/item-draft.test.ts b/apps/marketplace/lib/marketplace/item-draft.test.ts index b7df978324e..936f8ccb444 100644 --- a/apps/marketplace/lib/marketplace/item-draft.test.ts +++ b/apps/marketplace/lib/marketplace/item-draft.test.ts @@ -60,8 +60,9 @@ describe('item-draft utils', () => { slug: 'template-item', url: null, templateZip: null, + published: true, }) - ).toThrow('Template items require a template ZIP package') + ).toThrow('Template items require a template ZIP package before publishing or requesting review') expect(() => ensureItemDraftConstraints({ @@ -69,10 +70,34 @@ describe('item-draft utils', () => { slug: 'template-item', url: null, templateZip: templateFile, + published: true, }) ).not.toThrow() }) + it('allows template drafts without a template package', () => { + expect(() => + ensureItemDraftConstraints({ + type: 'template', + slug: 'template-item', + url: null, + templateZip: null, + }) + ).not.toThrow() + }) + + it('requires a template package when requesting review', () => { + expect(() => + ensureItemDraftConstraints({ + type: 'template', + slug: 'template-item', + url: null, + templateZip: null, + intent: 'request_review', + }) + ).toThrow('Template items require a template ZIP package before publishing or requesting review') + }) + it('parses template zip file only when provided', () => { const formData = new FormData() const zip = new File(['zip'], 'template.zip', { type: 'application/zip' }) diff --git a/apps/marketplace/lib/marketplace/item-draft.ts b/apps/marketplace/lib/marketplace/item-draft.ts index 8036f348349..47f81a6ae2a 100644 --- a/apps/marketplace/lib/marketplace/item-draft.ts +++ b/apps/marketplace/lib/marketplace/item-draft.ts @@ -56,12 +56,16 @@ export function ensureItemDraftConstraints({ url, templateZip, existingRegistryItemUrl, + published = false, + intent = 'save', }: { type: 'oauth' | 'template' | null slug: string url: string | null templateZip: File | null existingRegistryItemUrl?: string | null + published?: boolean + intent?: 'save' | 'request_review' }): asserts type is 'oauth' | 'template' { if (!slug) { throw new Error('Item slug cannot be empty') @@ -72,7 +76,8 @@ export function ensureItemDraftConstraints({ if (type === 'oauth' && !url) { throw new Error('OAuth items require a listing URL') } - if (type === 'template' && !templateZip && !existingRegistryItemUrl) { - throw new Error('Template items require a template ZIP package') + const requiresTemplatePackage = type === 'template' && (published || intent === 'request_review') + if (requiresTemplatePackage && !templateZip && !existingRegistryItemUrl) { + throw new Error('Template items require a template ZIP package before publishing or requesting review') } } diff --git a/apps/marketplace/lib/marketplace/review-state.test.ts b/apps/marketplace/lib/marketplace/review-state.test.ts index 44723239339..ec2cef73fc1 100644 --- a/apps/marketplace/lib/marketplace/review-state.test.ts +++ b/apps/marketplace/lib/marketplace/review-state.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { + deriveLatestReviewStatusDisplay, deriveOpenReviewState, deriveReviewDecisionDefaults, isReviewStatus, @@ -34,6 +35,22 @@ describe('review-state utils', () => { }) }) + it('derives review status labels and badge variants', () => { + expect(deriveLatestReviewStatusDisplay('approved')).toEqual({ + label: 'Approved', + variant: 'success', + }) + expect(deriveLatestReviewStatusDisplay('rejected')).toEqual({ + label: 'Rejected', + variant: 'destructive', + }) + expect(deriveLatestReviewStatusDisplay('draft')).toEqual({ + label: 'Draft', + variant: 'warning', + }) + expect(deriveLatestReviewStatusDisplay(null)).toBeNull() + }) + it('derives review decision defaults with safe fallback', () => { expect(deriveReviewDecisionDefaults(null)).toEqual({ status: 'pending_review', diff --git a/apps/marketplace/lib/marketplace/review-state.ts b/apps/marketplace/lib/marketplace/review-state.ts index c71eb5036b8..88e9c927f6c 100644 --- a/apps/marketplace/lib/marketplace/review-state.ts +++ b/apps/marketplace/lib/marketplace/review-state.ts @@ -19,6 +19,24 @@ export function deriveOpenReviewState(latestStatus: string | null | undefined) { return { hasOpenReview, isApproved, openReviewStatusLabel } } +export function deriveLatestReviewStatusDisplay(latestStatus: string | null | undefined) { + const { isApproved, openReviewStatusLabel } = deriveOpenReviewState(latestStatus) + + if (isApproved) { + return { label: 'Approved', variant: 'success' as const } + } + + if (latestStatus === 'rejected') { + return { label: 'Rejected', variant: 'destructive' as const } + } + + if (openReviewStatusLabel) { + return { label: openReviewStatusLabel, variant: 'warning' as const } + } + + return null +} + export function deriveReviewDecisionDefaults( latestReview: | { diff --git a/apps/marketplace/lib/marketplace/server.test.ts b/apps/marketplace/lib/marketplace/server.test.ts index d5a593609d5..7cad0d8304c 100644 --- a/apps/marketplace/lib/marketplace/server.test.ts +++ b/apps/marketplace/lib/marketplace/server.test.ts @@ -58,8 +58,14 @@ describe('getMarketplaceSidebarData', () => { in: () => ({ order: async () => ({ data: [ - { id: 11, partner_id: 1, slug: 'a-item', title: 'A Item' }, - { id: 10, partner_id: 1, slug: 'b-item', title: 'B Item' }, + { + id: 11, + partner_id: 1, + slug: 'a-item', + title: 'A Item', + item_reviews: { status: 'approved' }, + }, + { id: 10, partner_id: 1, slug: 'b-item', title: 'B Item', item_reviews: null }, ], error: null, }), @@ -75,6 +81,10 @@ describe('getMarketplaceSidebarData', () => { const result = await getMarketplaceSidebarData() expect(result.partners.map((partner) => partner.slug)).toEqual(['acme', 'reviewers']) expect(result.partners[0]?.items.map((item) => item.slug)).toEqual(['a-item', 'b-item']) + expect(result.partners[0]?.items.map((item) => item.latestReviewStatus)).toEqual([ + 'approved', + null, + ]) expect(result.isReviewerMember).toBe(true) }) }) diff --git a/apps/marketplace/lib/marketplace/server.ts b/apps/marketplace/lib/marketplace/server.ts index 086b9b18e49..4aac52210ed 100644 --- a/apps/marketplace/lib/marketplace/server.ts +++ b/apps/marketplace/lib/marketplace/server.ts @@ -1,4 +1,5 @@ import { createClient } from '@/lib/supabase/server' +import type { ReviewStatus } from '@/lib/marketplace/review-state' export type PartnerSidebarData = { id: number @@ -10,6 +11,7 @@ export type PartnerSidebarData = { id: number slug: string title: string + latestReviewStatus: ReviewStatus | null }> } @@ -58,7 +60,7 @@ export async function getMarketplaceSidebarData() { if (partnerIds.length > 0) { const { data: items, error: itemsError } = await supabase .from('items') - .select('id, partner_id, slug, title') + .select('id, partner_id, slug, title, item_reviews(status)') .in('partner_id', partnerIds) .order('title', { ascending: true }) @@ -66,13 +68,29 @@ export async function getMarketplaceSidebarData() { throw new Error(itemsError.message) } - for (const item of items ?? []) { + for (const item of (items ?? []) as Array<{ + id: number + partner_id: number + slug: string + title: string + item_reviews?: + | { + status: ReviewStatus | null + } + | Array<{ + status: ReviewStatus | null + }> + | null + }>) { const partner = partnerMap.get(item.partner_id) if (!partner) continue + const latestReview = Array.isArray(item.item_reviews) ? item.item_reviews[0] : item.item_reviews + partner.items.push({ id: item.id, slug: item.slug, title: item.title, + latestReviewStatus: latestReview?.status ?? null, }) } } diff --git a/apps/marketplace/supabase/schemas/schemas.sql b/apps/marketplace/supabase/schemas/schemas.sql index b01d3374ca0..ba700214152 100644 --- a/apps/marketplace/supabase/schemas/schemas.sql +++ b/apps/marketplace/supabase/schemas/schemas.sql @@ -231,8 +231,16 @@ create table public.items ( updated_at timestamptz not null default now(), constraint items_slug_format check (slug ~ '^[a-z0-9]+(?:-[a-z0-9]+)*$'), constraint items_type_destination_check check ( - (type = 'oauth' and url is not null and registry_item_url is null) - or (type = 'template' and url is null) + ( + type = 'oauth' + and registry_item_url is null + and (published = false or url is not null) + ) + or ( + type = 'template' + and url is null + and (published = false or registry_item_url is not null) + ) ) ); diff --git a/apps/marketplace/tests/components/item-form.test.tsx b/apps/marketplace/tests/components/item-form.test.tsx index 4095c18d827..ac30e7216ef 100644 --- a/apps/marketplace/tests/components/item-form.test.tsx +++ b/apps/marketplace/tests/components/item-form.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react' +import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -32,16 +32,22 @@ describe('ItemForm', () => { vi.clearAllMocks() }) - it('shows template zip validation error in create mode', async () => { + it('allows creating a template draft without a zip package', async () => { const user = userEvent.setup() + createItemDraftActionMock.mockResolvedValue({ + itemId: 1, + itemSlug: 'auth-template', + partnerSlug: 'acme', + }) + render() await user.type(screen.getByPlaceholderText('Authentication starter'), 'Auth Template') await user.click(screen.getByRole('button', { name: 'Create item' })) + await waitFor(() => expect(createItemDraftActionMock).toHaveBeenCalledTimes(1)) expect( - await screen.findByText('Upload a template ZIP package that includes template.json.') - ).toBeInTheDocument() - expect(createItemDraftActionMock).not.toHaveBeenCalled() + screen.queryByText('Upload a template ZIP package that includes template.json.') + ).not.toBeInTheDocument() }) })