diff --git a/app/api/providers/methods.ts b/app/api/providers/methods.ts index d7f86b2..da6b8b6 100644 --- a/app/api/providers/methods.ts +++ b/app/api/providers/methods.ts @@ -2,7 +2,7 @@ import type { SupabaseClient } from "@supabase/supabase-js"; import type { Provider, ProviderInput } from "~/types/Provider"; const TABLE_NAME = "providers"; -const JOIN_TABLE_NAME = "resource_providers"; +const COUNTS_VIEW_NAME = "resource_provider_counts"; export const getProviderAPI = (client: SupabaseClient) => { const table = client.from(TABLE_NAME); @@ -56,21 +56,21 @@ export const getProviderAPI = (client: SupabaseClient) => { return data; }, + delete: async (id: number) => { - const { error } = await table.delete().eq("id", id); + const { data, error } = await table.delete().eq("id", id).select("id"); if (error) { throw error; } + + return (data ?? []).length > 0; }, - /** - * How many resources each provider is currently attributed to, keyed by - * provider id. Used to warn before removing a provider that is still in use. - */ + getResourceCounts: async () => { const { data, error } = await client - .from(JOIN_TABLE_NAME) - .select("provider_id"); + .from(COUNTS_VIEW_NAME) + .select("provider_id, resource_count"); if (error) { throw error; @@ -78,7 +78,7 @@ export const getProviderAPI = (client: SupabaseClient) => { const counts: Record = {}; for (const row of data ?? []) { - counts[row.provider_id] = (counts[row.provider_id] ?? 0) + 1; + counts[row.provider_id] = Number(row.resource_count); } return counts; diff --git a/app/routes/authenticated/providers/index.tsx b/app/routes/authenticated/providers/index.tsx index a6f75ba..456cfc2 100644 --- a/app/routes/authenticated/providers/index.tsx +++ b/app/routes/authenticated/providers/index.tsx @@ -41,6 +41,31 @@ import type { Provider } from "~/types/Provider"; export const middleware = [authMiddleware]; +type FieldErrors = Partial>; + +const parseProviderId = (value: FormDataEntryValue | null) => { + const trimmed = String(value ?? "").trim(); + if (!/^\d+$/.test(trimmed)) { + return null; + } + + const id = Number(trimmed); + return Number.isSafeInteger(id) && id > 0 ? id : null; +}; + +const INVALID_URL_MESSAGE = + "Enter a valid web address, for example example.org/logo.svg"; + +/** Keep the underlying reason visible — an RLS denial reads the same as a typo otherwise. */ +const describeFailure = (error: unknown, fallback: string) => { + const detail = + error && typeof error === "object" && "message" in error + ? String((error as { message: unknown }).message) + : ""; + + return detail ? `${fallback} ${detail}` : fallback; +}; + export const loader: LoaderFunction = async ({ request }) => { const { client } = getDatabaseClient(request); const providerAPI = getProviderAPI(client); @@ -53,10 +78,27 @@ export const loader: LoaderFunction = async ({ request }) => { return { providers, resourceCounts }; }; -/** Treat blank form fields as "not set" rather than empty strings. */ -const optionalField = (value: FormDataEntryValue | null) => { +type UrlField = { ok: true; value: string | null } | { ok: false }; + +const optionalUrl = (value: FormDataEntryValue | null): UrlField => { const trimmed = String(value ?? "").trim(); - return trimmed.length > 0 ? trimmed : null; + if (!trimmed) { + return { ok: true, value: null }; + } + + const candidate = /^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed) + ? trimmed + : `https://${trimmed}`; + + try { + const url = new URL(candidate); + if (url.protocol !== "http:" && url.protocol !== "https:") { + return { ok: false }; + } + return { ok: true, value: url.toString() }; + } catch { + return { ok: false }; + } }; export const action: ActionFunction = async ({ request }) => { @@ -66,16 +108,27 @@ export const action: ActionFunction = async ({ request }) => { const intent = formData.get("intent"); if (intent === "delete") { - const id = Number(formData.get("id")); - if (!Number.isInteger(id)) { + const id = parseProviderId(formData.get("id")); + if (id === null) { return data({ message: "Invalid provider." }, { status: 400 }); } try { - await providerAPI.delete(id); + const removed = await providerAPI.delete(id); + if (!removed) { + return data( + { message: "That provider no longer exists." }, + { status: 404 }, + ); + } + return data({ message: "Provider removed.", ok: true }); - } catch { - return data({ message: "Failed to remove provider." }, { status: 400 }); + } catch (error) { + console.error("Failed to delete provider", error); + return data( + { message: describeFailure(error, "Failed to remove provider.") }, + { status: 400 }, + ); } } @@ -84,14 +137,31 @@ export const action: ActionFunction = async ({ request }) => { } const name = String(formData.get("name") ?? "").trim(); + const logoUrl = optionalUrl(formData.get("logo_url")); + const websiteUrl = optionalUrl(formData.get("website_url")); + + const fieldErrors: FieldErrors = {}; if (!name) { - return data({ message: "Name is required." }, { status: 400 }); + fieldErrors.name = "Name is required."; + } + if (!websiteUrl.ok) { + fieldErrors.website_url = INVALID_URL_MESSAGE; + } + if (!logoUrl.ok) { + fieldErrors.logo_url = INVALID_URL_MESSAGE; + } + + if (Object.keys(fieldErrors).length > 0) { + return data( + { message: "Please fix the highlighted fields.", fieldErrors }, + { status: 400 }, + ); } const values = { name, - logo_url: optionalField(formData.get("logo_url")), - website_url: optionalField(formData.get("website_url")), + logo_url: logoUrl.ok ? logoUrl.value : null, + website_url: websiteUrl.ok ? websiteUrl.value : null, }; try { @@ -100,33 +170,43 @@ export const action: ActionFunction = async ({ request }) => { return data({ message: `Added ${name}.`, ok: true }); } - const id = Number(formData.get("id")); - if (!Number.isInteger(id)) { + const id = parseProviderId(formData.get("id")); + if (id === null) { return data({ message: "Invalid provider." }, { status: 400 }); } await providerAPI.updateById(id, values); return data({ message: `Updated ${name}.`, ok: true }); - } catch { + } catch (error) { + console.error(`Failed to ${intent} provider`, error); return data( { - message: + message: describeFailure( + error, intent === "create" ? "Failed to add provider." : "Failed to update provider.", + ), }, { status: 400 }, ); } }; -type ActionResponse = { message: string; ok?: boolean }; +type ActionResponse = { + message: string; + ok?: boolean; + fieldErrors?: FieldErrors; +}; + +const hostnameOf = (url: string) => { + try { + return new URL(url).hostname; + } catch { + return url; + } +}; -/** - * Provider logos are usually wide wordmarks, so they are scaled to fit rather - * than cropped into a square. Falls back to the initial if the logo is missing - * or fails to load, matching the map's "Provided By" behaviour. - */ const ProviderLogo = ({ provider }: { provider: Provider }) => { const [failed, setFailed] = useState(false); @@ -164,6 +244,7 @@ const ProvidersPage = () => { resourceCounts: Record; }>(); const actionData = useActionData(); + const fieldErrors = actionData?.fieldErrors; const navigation = useNavigation(); const isSubmitting = navigation.state === "submitting"; @@ -171,7 +252,6 @@ const ProvidersPage = () => { const [isCreating, setIsCreating] = useState(false); const [deleting, setDeleting] = useState(null); - // Close the dialogs once a submission has succeeded. useEffect(() => { if (actionData?.ok) { setEditing(null); @@ -263,7 +343,7 @@ const ProvidersPage = () => { gap: 0.5, }} > - {new URL(provider.website_url).hostname} + {hostnameOf(provider.website_url)} ) : ( @@ -335,23 +415,33 @@ const ProvidersPage = () => { autoFocus defaultValue={editing?.name ?? ""} placeholder="Share Food Program" + error={Boolean(fieldErrors?.name)} + helperText={fieldErrors?.name} /> diff --git a/supabase/migrations/20260908000000_resource_provider_counts.sql b/supabase/migrations/20260908000000_resource_provider_counts.sql new file mode 100644 index 0000000..ca3bbe9 --- /dev/null +++ b/supabase/migrations/20260908000000_resource_provider_counts.sql @@ -0,0 +1,27 @@ +-- ============================================================================ +-- resource_provider_counts: how many resources each provider is attributed to. +-- +-- The dashboard previously selected every `resource_providers` row and tallied +-- them in JS. PostgREST caps a response at max-rows (1000 by default) and +-- returns the first page WITHOUT an error, so that tally silently under-reports +-- once the join table outgrows the cap. +-- +-- That number is what the "remove provider" confirmation uses to warn that +-- resources are about to lose their "Provided by" credit, so an undercount +-- reads as "safe to delete" — the failure is both silent and destructive. +-- +-- Mirrors the existing `resource_edit_counts` view. +-- ============================================================================ + +begin; + +create or replace view public.resource_provider_counts + with (security_invoker = true) + as + select provider_id, count(*) as resource_count + from public.resource_providers + group by provider_id; + +grant select on public.resource_provider_counts to anon, authenticated; + +commit; diff --git a/supabase/rollback/20260908000000_resource_provider_counts.down.sql b/supabase/rollback/20260908000000_resource_provider_counts.down.sql new file mode 100644 index 0000000..c0baee1 --- /dev/null +++ b/supabase/rollback/20260908000000_resource_provider_counts.down.sql @@ -0,0 +1,5 @@ +begin; + +drop view if exists public.resource_provider_counts; + +commit;