From 25552d89fdec60aca1b4ba0767a583e16287cdaf Mon Sep 17 00:00:00 2001 From: Tale Agent Date: Wed, 24 Jun 2026 04:34:16 +0000 Subject: [PATCH 1/2] fix(platform): require organization name (#1951) --- .../settings/components/settings-row.tsx | 14 +++- .../components/organization-settings.test.tsx | 65 +++++++++++++++++++ .../components/organization-settings.tsx | 24 ++++++- services/platform/convex/auth.ts | 14 ++++ services/platform/messages/de.json | 3 +- services/platform/messages/en.json | 1 + services/platform/messages/fr.json | 3 +- 7 files changed, 119 insertions(+), 5 deletions(-) diff --git a/services/platform/app/features/settings/components/settings-row.tsx b/services/platform/app/features/settings/components/settings-row.tsx index 084b6ac1a..2aa3c0c3a 100644 --- a/services/platform/app/features/settings/components/settings-row.tsx +++ b/services/platform/app/features/settings/components/settings-row.tsx @@ -4,6 +4,7 @@ import { Description } from '@tale/ui/description'; import { Stack } from '@tale/ui/layout'; import { forwardRef, useId, type HTMLAttributes, type ReactNode } from 'react'; +import { useT } from '@/lib/i18n/client'; import { cn } from '@/lib/utils/cn'; interface SettingsRowProps extends Omit< @@ -12,6 +13,8 @@ interface SettingsRowProps extends Omit< > { label: ReactNode; description?: ReactNode; + /** Append a red required asterisk to the label (mirrors `Label`'s required). */ + required?: boolean; /** Right-side control (switch, button, copy field, link). */ children: ReactNode; } @@ -22,7 +25,8 @@ interface SettingsRowProps extends Omit< * narrow viewports so the right-side control wraps cleanly. */ export const SettingsRow = forwardRef( - ({ label, description, children, className, ...props }, ref) => { + ({ label, description, required, children, className, ...props }, ref) => { + const { t } = useT('common'); const id = useId(); const labelId = `${id}-label`; const descId = description ? `${id}-desc` : undefined; @@ -44,6 +48,14 @@ export const SettingsRow = forwardRef( className="text-foreground text-sm leading-none font-medium" > {label} + {required && ( + + * + + )} {description && {description}} diff --git a/services/platform/app/features/settings/organization/components/organization-settings.test.tsx b/services/platform/app/features/settings/organization/components/organization-settings.test.tsx index 346c75f5d..331dee02a 100644 --- a/services/platform/app/features/settings/organization/components/organization-settings.test.tsx +++ b/services/platform/app/features/settings/organization/components/organization-settings.test.tsx @@ -1,4 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; +import { z } from 'zod'; import { useFormEditor } from '@/app/components/ui/editor'; import { checkAccessibility } from '@/tests/utils/a11y'; @@ -96,6 +97,70 @@ describe('OrganizationSettingsView page load', () => { }); }); +// Mirror the container's schema so the harness exercises the same validation +// the real form uses (name required, locale free-form). +const organizationSchema = z.object({ + name: z.string().trim().min(1, 'Organization name is required'), + defaultLocale: z.string(), +}); + +function ValidationHarness({ orgName }: { orgName: string }) { + const editor = useFormEditor
({ + data: { name: orgName, defaultLocale: 'en' }, + schema: organizationSchema, + save, + }); + holder.current = editor; + return ( + + ); +} + +describe('OrganizationSettingsView name validation', () => { + it('blocks an empty org name with an inline error and an invalid form', async () => { + render(); + await waitFor(() => expect(holder.current?.isLoading).toBe(false)); + expect(holder.current?.isValid).toBe(true); + + const orgNameField = screen.getByRole('textbox', { + name: 'Organization name', + }); + fireEvent.change(orgNameField, { target: { value: '' } }); + + await waitFor(() => + expect( + screen.getByText('Organization name is required'), + ).toBeInTheDocument(), + ); + expect(holder.current?.isValid).toBe(false); + }); + + it('clears the error once a non-empty name is entered', async () => { + render(); + await waitFor(() => expect(holder.current?.isLoading).toBe(false)); + + const orgNameField = screen.getByRole('textbox', { + name: 'Organization name', + }); + fireEvent.change(orgNameField, { target: { value: '' } }); + await waitFor(() => expect(holder.current?.isValid).toBe(false)); + + fireEvent.change(orgNameField, { target: { value: 'New name' } }); + await waitFor(() => expect(holder.current?.isValid).toBe(true)); + expect( + screen.queryByText('Organization name is required'), + ).not.toBeInTheDocument(); + }); +}); + describe('OrganizationSettingsView locale select', () => { it('does not mark the form dirty on a spurious empty change', async () => { render(); diff --git a/services/platform/app/features/settings/organization/components/organization-settings.tsx b/services/platform/app/features/settings/organization/components/organization-settings.tsx index 7bd9e78ac..72d4bd3b0 100644 --- a/services/platform/app/features/settings/organization/components/organization-settings.tsx +++ b/services/platform/app/features/settings/organization/components/organization-settings.tsx @@ -5,6 +5,7 @@ import { Skeletonize } from '@tale/ui/skeleton-context'; import { useQueryClient } from '@tanstack/react-query'; import { useCallback, useMemo, useState } from 'react'; import { Controller } from 'react-hook-form'; +import { z } from 'zod'; import { AccessDenied } from '@/app/components/layout/access-denied'; import { CopyableField } from '@/app/components/ui/data-display/copyable-field'; @@ -106,7 +107,12 @@ export function OrganizationSettingsView({ const { t: tGlobal } = useT('global'); const { form, isLoading, isSaving } = controller; - const { handleSubmit, register, control } = form; + const { + handleSubmit, + register, + control, + formState: { errors }, + } = form; const localeOptions = useMemo( () => @@ -138,6 +144,7 @@ export function OrganizationSettingsView({ className="py-5" label={tSettings('organization.title')} description={tSettings('organization.nameDescription')} + required > {/* Fixed-width control column, full-width on mobile where the row stacks. `wrapperClassName="w-full"` lets the bare Input fill it @@ -146,6 +153,8 @@ export function OrganizationSettingsView({ @@ -310,9 +319,19 @@ export function OrganizationSettings({ }) { const { t: tAccessDenied } = useT('accessDenied'); const { t: tToast } = useT('toast'); + const { t: tSettings } = useT('settings'); const queryClient = useQueryClient(); const { toast } = useToast(); + const organizationSchema = useMemo( + () => + z.object({ + name: z.string().trim().min(1, tSettings('organization.nameRequired')), + defaultLocale: z.string(), + }), + [tSettings], + ); + const ability = useAbility(); const abilityLoading = useAbilityLoading(); const { data: organization, isLoading: isOrgLoading } = @@ -351,7 +370,7 @@ export function OrganizationSettings({ await authClient.organization.update({ organizationId: organization._id, data: { - name: data.name.trim() || undefined, + name: data.name.trim(), metadata: updatedMetadata, }, }); @@ -374,6 +393,7 @@ export function OrganizationSettings({ const editor = useFormEditor({ data: initialData, + schema: organizationSchema, save, }); diff --git a/services/platform/convex/auth.ts b/services/platform/convex/auth.ts index 87d5cdce4..89529f025 100644 --- a/services/platform/convex/auth.ts +++ b/services/platform/convex/auth.ts @@ -708,6 +708,20 @@ export const getAuthOptions = (ctx: GenericCtx) => { // through a `Record` view so the field // shape matches Better Auth's loose update payload type. const orgPatch = data.organization as Record; + // Org name is required: reject a rename that clears it (empty or + // whitespace-only). Mirrors the client-side `.min(1)` validation so + // an empty name can't slip past the auth boundary via a direct API + // call. Only enforced when `name` is part of the patch — a name-less + // update (e.g. locale-only) leaves the stored name untouched. + const rawName = orgPatch.name; + if (rawName !== undefined) { + if (typeof rawName !== 'string' || rawName.trim().length === 0) { + throw new APIError('BAD_REQUEST', { + message: 'Organization name is required.', + }); + } + orgPatch.name = rawName.trim(); + } const rawSlug = orgPatch.slug; if (typeof rawSlug !== 'string') return; const normalizedSlug = rawSlug.toLowerCase(); diff --git a/services/platform/messages/de.json b/services/platform/messages/de.json index 0260aa833..e329fc51b 100644 --- a/services/platform/messages/de.json +++ b/services/platform/messages/de.json @@ -5006,10 +5006,11 @@ } }, "organization": { - "title": "Organisation (optional)", + "title": "Organisationsname", "detailsTitle": "Organisationsdetails", "detailsDescription": "Grundlegende Informationen zu deiner Organisation.", "nameDescription": "Der Anzeigename deiner Organisation.", + "nameRequired": "Organisationsname ist erforderlich", "localeDescription": "Die Hauptsprache deiner Organisation.", "organizationId": "Organisations-ID", "organizationIdDescription": "Verwende diese ID in API-Anfragen und Integrationen.", diff --git a/services/platform/messages/en.json b/services/platform/messages/en.json index 6281225e6..d7b5a6098 100644 --- a/services/platform/messages/en.json +++ b/services/platform/messages/en.json @@ -5272,6 +5272,7 @@ "detailsTitle": "Organization details", "detailsDescription": "Basic information about your organization.", "nameDescription": "The display name for your organization.", + "nameRequired": "Organization name is required", "localeDescription": "Primary language for your organization.", "organizationId": "Organization ID", "organizationIdDescription": "Reference this ID in API requests and integrations.", diff --git a/services/platform/messages/fr.json b/services/platform/messages/fr.json index 60dda921e..f35d5c07c 100644 --- a/services/platform/messages/fr.json +++ b/services/platform/messages/fr.json @@ -5007,10 +5007,11 @@ } }, "organization": { - "title": "Organisation (facultatif)", + "title": "Nom de l'organisation", "detailsTitle": "Détails de l'organisation", "detailsDescription": "Informations de base sur ton organisation.", "nameDescription": "Le nom d'affichage de ton organisation.", + "nameRequired": "Le nom de l'organisation est requis", "localeDescription": "La langue principale de ton organisation.", "organizationId": "ID de l'organisation", "organizationIdDescription": "Utilise cet ID dans les requêtes API et les intégrations.", From 65a2476bc97e47124819938b8c3e20b7251e2e9a Mon Sep 17 00:00:00 2001 From: tale-agent Date: Wed, 24 Jun 2026 06:07:33 +0000 Subject: [PATCH 2/2] fix: address review feedback for required organization name (#1951) --- .../components/organization-settings.test.tsx | 49 +++++++++++---- .../components/organization-settings.tsx | 5 +- services/platform/convex/auth.test.ts | 61 ++++++++++++++++++- services/platform/convex/auth.ts | 11 ++-- .../lib/shared/schemas/organizations.ts | 10 +++ 5 files changed, 116 insertions(+), 20 deletions(-) diff --git a/services/platform/app/features/settings/organization/components/organization-settings.test.tsx b/services/platform/app/features/settings/organization/components/organization-settings.test.tsx index 331dee02a..2e4fcf98c 100644 --- a/services/platform/app/features/settings/organization/components/organization-settings.test.tsx +++ b/services/platform/app/features/settings/organization/components/organization-settings.test.tsx @@ -1,12 +1,19 @@ import { describe, it, expect, vi } from 'vitest'; -import { z } from 'zod'; +import { z } from 'zod/v4'; import { useFormEditor } from '@/app/components/ui/editor'; +import { organizationNameSchema } from '@/lib/shared/schemas/organizations'; +import enMessages from '@/messages/en.json'; import { checkAccessibility } from '@/tests/utils/a11y'; import { fireEvent, render, screen, waitFor } from '@/tests/utils/render'; import { OrganizationSettingsView } from './organization-settings'; +// Resolve user-facing labels through the translation layer rather than +// hardcoding English, matching the platform i18n rule for tests. +const orgNameLabel = enMessages.settings.organization.title; +const nameRequiredMessage = enMessages.settings.organization.nameRequired; + // The view now embeds the Members section, which subscribes to Convex via // `useMembers`. This suite renders the view in isolation (no Convex provider) // to exercise the locale-select dirty guard, so stub the members table out — @@ -87,7 +94,7 @@ describe('OrganizationSettingsView page load', () => { // The org-name field, labeled by `settings.organization.title`, resolves to // the current (non-empty) org name once the form applies its baseline. const orgNameField = screen.getByRole('textbox', { - name: 'Organization name', + name: orgNameLabel, }); await waitFor(() => expect(orgNameField).toHaveValue('Acme Industries')); // Mirror the E2E's "non-empty value" intent. @@ -97,10 +104,12 @@ describe('OrganizationSettingsView page load', () => { }); }); -// Mirror the container's schema so the harness exercises the same validation -// the real form uses (name required, locale free-form). +// Build the harness schema from the same shared `organizationNameSchema` the +// real container wires in, with the i18n message resolved through the +// translation layer — so the test guards the actual validation rule and never +// drifts on a hardcoded English literal. const organizationSchema = z.object({ - name: z.string().trim().min(1, 'Organization name is required'), + name: organizationNameSchema(nameRequiredMessage), defaultLocale: z.string(), }); @@ -131,14 +140,30 @@ describe('OrganizationSettingsView name validation', () => { expect(holder.current?.isValid).toBe(true); const orgNameField = screen.getByRole('textbox', { - name: 'Organization name', + name: orgNameLabel, }); fireEvent.change(orgNameField, { target: { value: '' } }); await waitFor(() => - expect( - screen.getByText('Organization name is required'), - ).toBeInTheDocument(), + expect(screen.getByText(nameRequiredMessage)).toBeInTheDocument(), + ); + expect(holder.current?.isValid).toBe(false); + }); + + it('treats a whitespace-only org name as invalid', async () => { + render(); + await waitFor(() => expect(holder.current?.isLoading).toBe(false)); + expect(holder.current?.isValid).toBe(true); + + const orgNameField = screen.getByRole('textbox', { + name: orgNameLabel, + }); + // The schema trims before checking `.min(1)`, so a name of only spaces must + // fail exactly as an empty string does — the trim is the rule this PR adds. + fireEvent.change(orgNameField, { target: { value: ' ' } }); + + await waitFor(() => + expect(screen.getByText(nameRequiredMessage)).toBeInTheDocument(), ); expect(holder.current?.isValid).toBe(false); }); @@ -148,16 +173,14 @@ describe('OrganizationSettingsView name validation', () => { await waitFor(() => expect(holder.current?.isLoading).toBe(false)); const orgNameField = screen.getByRole('textbox', { - name: 'Organization name', + name: orgNameLabel, }); fireEvent.change(orgNameField, { target: { value: '' } }); await waitFor(() => expect(holder.current?.isValid).toBe(false)); fireEvent.change(orgNameField, { target: { value: 'New name' } }); await waitFor(() => expect(holder.current?.isValid).toBe(true)); - expect( - screen.queryByText('Organization name is required'), - ).not.toBeInTheDocument(); + expect(screen.queryByText(nameRequiredMessage)).not.toBeInTheDocument(); }); }); diff --git a/services/platform/app/features/settings/organization/components/organization-settings.tsx b/services/platform/app/features/settings/organization/components/organization-settings.tsx index 72d4bd3b0..82ef14d12 100644 --- a/services/platform/app/features/settings/organization/components/organization-settings.tsx +++ b/services/platform/app/features/settings/organization/components/organization-settings.tsx @@ -5,7 +5,7 @@ import { Skeletonize } from '@tale/ui/skeleton-context'; import { useQueryClient } from '@tanstack/react-query'; import { useCallback, useMemo, useState } from 'react'; import { Controller } from 'react-hook-form'; -import { z } from 'zod'; +import { z } from 'zod/v4'; import { AccessDenied } from '@/app/components/layout/access-denied'; import { CopyableField } from '@/app/components/ui/data-display/copyable-field'; @@ -30,6 +30,7 @@ import { useToast } from '@/app/hooks/use-toast'; import { authClient } from '@/lib/auth-client'; import { useT } from '@/lib/i18n/client'; import { SUPPORTED_AGENT_LOCALES } from '@/lib/shared/constants/agents'; +import { organizationNameSchema } from '@/lib/shared/schemas/organizations'; import { getOrganizationDefaultLocale } from '@/lib/shared/utils/get-organization-default-locale'; type Organization = { @@ -326,7 +327,7 @@ export function OrganizationSettings({ const organizationSchema = useMemo( () => z.object({ - name: z.string().trim().min(1, tSettings('organization.nameRequired')), + name: organizationNameSchema(tSettings('organization.nameRequired')), defaultLocale: z.string(), }), [tSettings], diff --git a/services/platform/convex/auth.test.ts b/services/platform/convex/auth.test.ts index 785bc623f..ee690585d 100644 --- a/services/platform/convex/auth.test.ts +++ b/services/platform/convex/auth.test.ts @@ -12,7 +12,9 @@ vi.mock('better-auth', () => ({ })); vi.mock('better-auth/plugins', () => ({ - organization: vi.fn(() => ({})), + // Return the config so tests can reach `organizationHooks` (the real plugin + // would consume them internally and expose nothing). + organization: vi.fn((config: unknown) => config), twoFactor: vi.fn(() => ({})), })); @@ -97,3 +99,60 @@ describe('auth trustedOrigins', () => { vi.unstubAllEnvs(); }); }); + +describe('beforeUpdateOrganization name guard', () => { + beforeEach(() => { + vi.resetModules(); + }); + + // Pull the `beforeUpdateOrganization` hook out of the org plugin config the + // mock above passes through, so we can exercise the server-side gate directly + // — the acceptance criterion requires an empty name to be blocked on the + // server too, and neither the client unit tests nor the Playwright E2E reach + // this branch. + async function getBeforeUpdateOrganization() { + vi.stubEnv('SITE_URL', 'https://app.example.com'); + const { getAuthOptions } = await import('./auth'); + const options = getAuthOptions({} as never); + // oxlint-disable-next-line typescript/no-explicit-any -- test reaches into the loose better-auth plugin config + const orgPlugin = (options.plugins as any[]).find( + (plugin) => plugin?.organizationHooks?.beforeUpdateOrganization, + ); + expect(orgPlugin).toBeDefined(); + return orgPlugin.organizationHooks.beforeUpdateOrganization as ( + data: unknown, + ) => Promise; + } + + it('rejects a whitespace-only name', async () => { + const beforeUpdate = await getBeforeUpdateOrganization(); + await expect( + beforeUpdate({ organization: { name: ' ' } }), + ).rejects.toThrow(); + vi.unstubAllEnvs(); + }); + + it('rejects a non-string name', async () => { + const beforeUpdate = await getBeforeUpdateOrganization(); + await expect( + beforeUpdate({ organization: { name: 123 } }), + ).rejects.toThrow(); + vi.unstubAllEnvs(); + }); + + it('trims and persists a valid name', async () => { + const beforeUpdate = await getBeforeUpdateOrganization(); + const data = { organization: { name: ' Acme ' } }; + await beforeUpdate(data); + expect(data.organization.name).toBe('Acme'); + vi.unstubAllEnvs(); + }); + + it('leaves a name-absent (locale-only) update untouched', async () => { + const beforeUpdate = await getBeforeUpdateOrganization(); + const data = { organization: { metadata: { defaultLocale: 'de' } } }; + await expect(beforeUpdate(data)).resolves.toBeUndefined(); + expect(data.organization).toEqual({ metadata: { defaultLocale: 'de' } }); + vi.unstubAllEnvs(); + }); +}); diff --git a/services/platform/convex/auth.ts b/services/platform/convex/auth.ts index 89529f025..4aca2af83 100644 --- a/services/platform/convex/auth.ts +++ b/services/platform/convex/auth.ts @@ -15,6 +15,7 @@ import { import { assertValidOrgSlug } from '../lib/shared/constants/org-slug'; import { isReservedOrgSlug } from '../lib/shared/constants/reserved-org-slugs'; +import { organizationNameSchema } from '../lib/shared/schemas/organizations'; import { sessionIdleWindowSeconds } from '../lib/shared/session-idle'; import { getOrganizationDefaultLocale } from '../lib/shared/utils/get-organization-default-locale'; import { isRecord, getString } from '../lib/utils/type-utils'; @@ -713,14 +714,16 @@ export const getAuthOptions = (ctx: GenericCtx) => { // an empty name can't slip past the auth boundary via a direct API // call. Only enforced when `name` is part of the patch — a name-less // update (e.g. locale-only) leaves the stored name untouched. - const rawName = orgPatch.name; - if (rawName !== undefined) { - if (typeof rawName !== 'string' || rawName.trim().length === 0) { + if (orgPatch.name !== undefined) { + const parsedName = organizationNameSchema().safeParse( + orgPatch.name, + ); + if (!parsedName.success) { throw new APIError('BAD_REQUEST', { message: 'Organization name is required.', }); } - orgPatch.name = rawName.trim(); + orgPatch.name = parsedName.data; } const rawSlug = orgPatch.slug; if (typeof rawSlug !== 'string') return; diff --git a/services/platform/lib/shared/schemas/organizations.ts b/services/platform/lib/shared/schemas/organizations.ts index ccdbaff41..9602f9288 100644 --- a/services/platform/lib/shared/schemas/organizations.ts +++ b/services/platform/lib/shared/schemas/organizations.ts @@ -10,3 +10,13 @@ const memberRoleLiterals = [ ] as const; export const memberRoleSchema = z.enum(memberRoleLiterals); export type MemberRole = z.infer; + +/** + * Organization name is required: non-empty once trimmed. Shared so the client + * settings form and the Convex `beforeUpdateOrganization` hook enforce the same + * rule from a single source — neither layer can drift from the other. The + * optional `message` lets the client inject a localized error; the server + * relies on `safeParse` success/failure and supplies its own message. + */ +export const organizationNameSchema = (message?: string) => + z.string().trim().min(1, message);