Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,18 @@ Use tRPC as the bridge between client components and server logic:

All database access belongs in `src/server/repositories/`. Build queries with Kysely's query builder; avoid raw SQL (`sql` template tag) unless there is no alternative.

### CamelCasePlugin

A `CamelCasePlugin` translates between camelCase (used in the Kysely query builder) and snake_case (used in the actual PostgreSQL columns). Use **camelCase** in the query builder API and **snake_case** in raw SQL (`sql` template tag / migrations with raw SQL).

```ts
// ✅ Query builder — camelCase
db.selectFrom("invitation").where("invitation.senderOrganisationId", "=", id);

// ✅ Raw SQL — snake_case
sql`UPDATE invitation SET sender_organisation_id = ${id}`;
```

### JSONPlugin

The database is configured with a custom `JSONPlugin` that automatically serialises JavaScript objects and arrays into JSONB when writing to the database. **Do not call `JSON.stringify()` on values passed to Kysely queries** — it's handled for you and double-encoding will corrupt the data.
Expand Down
4 changes: 4 additions & 0 deletions bin/cmd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,15 @@ program
.option("--email <email>")
.option("--name <name>")
.option("--organisationId <organisationId>")
.option("--senderOrganisationId <senderOrganisationId>")
.description("Create an invitation for a user")
.action(async (options) => {
const invitation = await createInvitation({
email: options.email,
name: options.name,
organisationId: options.organisationId,
senderOrganisationId:
options.senderOrganisationId || options.organisationId,
});

logger.info(`Created invitation ${invitation.id}`);
Expand Down Expand Up @@ -151,6 +154,7 @@ program
email: user.email,
name: user.name,
organisationId: orgs[0].id,
senderOrganisationId: orgs[0].id,
});

logger.info(`Created invitation ${invitation.id}`);
Expand Down
50 changes: 50 additions & 0 deletions migrations/1774658921005_invitation_sender_organisation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { sql } from "kysely";
import type { Kysely } from "kysely";

export async function up(db: Kysely<any>): Promise<void> {
// Add nullable column first
await db.schema
.alterTable("invitation")
.addColumn("senderOrganisationId", "uuid")
.execute();

// Backfill: assign all existing invitations to the admin organisation
await sql`
UPDATE invitation
SET "sender_organisation_id" = (
SELECT id FROM organisation WHERE name = 'Common Knowledge' LIMIT 1
)
Comment on lines +12 to +17
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The backfill relies on an organisation named 'Common Knowledge' existing. In environments where it doesn't, the subquery returns NULL and the subsequent NOT NULL alteration will fail. Consider backfilling from existing data instead (e.g., set sender_organisation_id = organisation_id), or create/select a guaranteed default sender org.

Suggested change
// Backfill: assign all existing invitations to the admin organisation
await sql`
UPDATE invitation
SET "sender_organisation_id" = (
SELECT id FROM organisation WHERE name = 'Common Knowledge' LIMIT 1
)
// Backfill from existing invitation data to avoid relying on a specific organisation seed
await sql`
UPDATE invitation
SET "sender_organisation_id" = "organisation_id"

Copilot uses AI. Check for mistakes.
WHERE "sender_organisation_id" IS NULL
`.execute(db);

Comment on lines +12 to +20
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migration backfill hard-codes an organisation named 'Common Knowledge'. If that row doesn’t exist in an environment, the UPDATE will keep sender_organisation_id NULL and the subsequent setNotNull() will fail, breaking the migration. Consider backfilling from existing invitation.organisation_id (or another deterministic value) and/or explicitly creating/validating the fallback organisation before enforcing NOT NULL.

Suggested change
// Backfill: assign all existing invitations to the admin organisation
await sql`
UPDATE invitation
SET "sender_organisation_id" = (
SELECT id FROM organisation WHERE name = 'Common Knowledge' LIMIT 1
)
WHERE "sender_organisation_id" IS NULL
`.execute(db);
// Backfill deterministically from the invitation's existing organisation
await sql`
UPDATE invitation
SET "sender_organisation_id" = "organisation_id"
WHERE "sender_organisation_id" IS NULL
`.execute(db);
const remainingNullSenderOrganisationIds = await sql<{ count: string }>`
SELECT COUNT(*)::text AS count
FROM invitation
WHERE "sender_organisation_id" IS NULL
`.execute(db);
if (Number(remainingNullSenderOrganisationIds.rows[0]?.count ?? 0) > 0) {
throw new Error(
"Migration 1774658921005_invitation_sender_organisation failed: some invitation rows do not have an organisation_id to backfill sender_organisation_id",
);
}

Copilot uses AI. Check for mistakes.
// Add foreign key constraint
await db.schema
.alterTable("invitation")
.addForeignKeyConstraint(
"invitationSenderOrganisationIdFKey",
["senderOrganisationId"],
"organisation",
["id"],
(cb) => cb.onDelete("cascade").onUpdate("cascade"),
)
.execute();

// Make column not null after backfill
await db.schema
.alterTable("invitation")
.alterColumn("senderOrganisationId", (col) => col.setNotNull())
.execute();
Comment on lines +21 to +37
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This FK is configured with onDelete('set null') but the column is made NOT NULL immediately afterwards. If a sender organisation is deleted, Postgres will try to set sender_organisation_id to NULL and the delete will fail. Either keep the column nullable (and handle NULLs in queries) or change the FK action to RESTRICT/CASCADE to match the NOT NULL constraint.

Copilot uses AI. Check for mistakes.
}

export async function down(db: Kysely<any>): Promise<void> {
await db.schema
.alterTable("invitation")
.dropConstraint("invitationSenderOrganisationIdFKey")
.execute();

await db.schema
.alterTable("invitation")
.dropColumn("senderOrganisationId")
.execute();
}
9 changes: 9 additions & 0 deletions resources/areaSets/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,13 @@ export const areaSetsMetadata: AreaSetMetadata[] = [
codeKey: "DioNumber",
nameKey: "Diocese",
},
{
code: AreaSetCode.ICB22,
name: "NHS Integrated Care Boards 2022",
filename: "icb_2022.geojson",
link: "https://open-geography-portalx-ons.hub.arcgis.com/api/download/v1/items/92362df594aa408aaa7a581ac83fb348/geojson?layers=0",
isNationalGridSRID: true,
codeKey: "ICB22CD",
nameKey: "ICB22NM",
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { PlusIcon } from "lucide-react";
import { type FormEvent, useMemo, useState } from "react";
import { toast } from "sonner";
import FormFieldWrapper from "@/components/forms/FormFieldWrapper";
import { useOrganisations } from "@/hooks/useOrganisations";
import { useTRPC } from "@/services/trpc/react";
import { Button } from "@/shadcn/ui/button";
import { Checkbox } from "@/shadcn/ui/checkbox";
Expand Down Expand Up @@ -43,6 +44,7 @@ export default function CreateInvitationModal() {
Set<string>
>(new Set());

const { organisationId: senderOrganisationId } = useOrganisations();
const trpc = useTRPC();
const client = useQueryClient();

Expand All @@ -62,9 +64,7 @@ export default function CreateInvitationModal() {
client.invalidateQueries({
queryKey: trpc.organisation.listAll.queryKey(),
});
client.invalidateQueries({
queryKey: trpc.invitation.list.queryKey(),
});
client.invalidateQueries(trpc.invitation.list.queryFilter());
},
onError: (error) => {
toast.error("Failed to create invitation.", {
Expand Down Expand Up @@ -169,6 +169,7 @@ export default function CreateInvitationModal() {
}

createInvitationMutate({
senderOrganisationId: senderOrganisationId ?? "",
organisationId,
Comment on lines 171 to 173
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

senderOrganisationId is sent as an empty string when no organisation is selected/loaded. That will deterministically fail server-side (FORBIDDEN) and shows as a generic toast error. Prefer blocking submission until organisationId is available (e.g., disable the submit button / early-return with a user-facing message) and avoid sending placeholder "" values.

Copilot uses AI. Check for mistakes.
organisationName,
email,
Comment on lines 171 to 175
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This passes senderOrganisationId as an empty string when useOrganisations() hasn’t resolved yet. That can cause the server to error (UUID comparison) and surface as an INTERNAL_SERVER_ERROR. Consider blocking submit until senderOrganisationId is available (or showing an explicit error) instead of sending "".

Copilot uses AI. Check for mistakes.
Expand Down
7 changes: 6 additions & 1 deletion src/app/(private)/(dashboards)/invite-organisation/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { useQuery } from "@tanstack/react-query";
import { redirect } from "next/navigation";
import { useCurrentUser } from "@/hooks";
import { useOrganisations } from "@/hooks/useOrganisations";
import { UserRole } from "@/models/User";
import { useTRPC } from "@/services/trpc/react";
import {
Expand All @@ -17,13 +18,17 @@ import CreateInvitationModal from "./CreateInvitationModal";

export default function InviteOrganisationPage() {
const { currentUser } = useCurrentUser();
const { organisationId } = useOrganisations();
const trpc = useTRPC();
const isAllowed =
currentUser?.role === UserRole.Advocate ||
currentUser?.role === UserRole.Superadmin;

const { data: invitations, isPending: invitationsLoading } = useQuery(
trpc.invitation.list.queryOptions(undefined, { enabled: isAllowed }),
trpc.invitation.list.queryOptions(
{ senderOrganisationId: organisationId ?? "" },
{ enabled: isAllowed && Boolean(organisationId) },
),
);

if (!isAllowed) {
Expand Down
13 changes: 13 additions & 0 deletions src/app/(private)/map/[id]/components/Choropleth/configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,19 @@ export const CHOROPLETH_LAYER_CONFIGS: Record<
},
},
],
ICB22: [
{
areaSetCode: AreaSetCode.ICB22,
minZoom: 0,
requiresBoundingBox: false,
mapbox: {
featureCodeProperty: "ICB22CD",
featureNameProperty: "ICB22NM",
layerId: "icb_2022-7ehb90",
sourceId: "commonknowledge.4x90o7q9",
},
},
],
};

export const HEX_CHOROPLETH_LAYER_CONFIG: ChoroplethLayerConfig = {
Expand Down
4 changes: 4 additions & 0 deletions src/labels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const AreaSetCodeLabels: Record<AreaSetCode, string> = {
SDZ22: "Scottish Data Zone",
SIZ22: "Scottish Intermediate Zone",
COED26: "Church of England Diocese",
ICB22: "NHS Integrated Care Board",
};

export const AreaSetCodeYears: Record<AreaSetCode, string> = {
Expand All @@ -57,6 +58,7 @@ export const AreaSetCodeYears: Record<AreaSetCode, string> = {
SDZ22: "2022 &mdash; present",
SIZ22: "2022 &mdash; present",
COED26: "2026 &mdash; present",
ICB22: "2022 &mdash; present",
};

export const AreaSetGroupCodeLabels: Record<AreaSetGroupCode, string> = {
Expand All @@ -73,6 +75,7 @@ export const AreaSetGroupCodeLabels: Record<AreaSetGroupCode, string> = {
SENC22: "Senedd Constituency",
SOA22: "Scottish Inter. Zone ➔ Data Zone ➔ Output Area",
COED26: "Church of England Diocese",
ICB22: "NHS Integrated Care Board",
};

export const AreaSetGroupCodeYears: Record<AreaSetGroupCode, string> = {
Expand All @@ -89,6 +92,7 @@ export const AreaSetGroupCodeYears: Record<AreaSetGroupCode, string> = {
SENC22: "2022 &mdash; present",
SOA22: "2022 &mdash; present",
COED26: "2026 &mdash; present",
ICB22: "2022 &mdash; present",
};

export const FilterTypeLabels: Record<
Expand Down
3 changes: 3 additions & 0 deletions src/models/AreaSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export enum AreaSetCode {
SPC22 = "SPC22",
SENC22 = "SENC22",
COED26 = "COED26",
ICB22 = "ICB22",
}
export const areaSetCodes = Object.values(AreaSetCode);

Expand All @@ -37,6 +38,7 @@ export enum AreaSetGroupCode {
SENC22 = "SENC22",
SOA22 = "SOA22",
COED26 = "COED26",
ICB22 = "ICB22",
}
export const areaSetGroupCodes = Object.values(AreaSetGroupCode);

Expand All @@ -58,6 +60,7 @@ export const AreaSetSizes: Record<AreaSetCode, number> = {
[AreaSetCode.CTYUA24]: 6,
[AreaSetCode.CAUTH25]: 6,
[AreaSetCode.COED26]: 6,
[AreaSetCode.ICB22]: 6,
[AreaSetCode.UKR18]: 8,
[AreaSetCode.UKC24]: 12,
};
Expand Down
1 change: 1 addition & 0 deletions src/models/Invitation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export const invitationSchema = z.object({
email: z.string().email().trim().toLowerCase(),
name: z.string().trim(),
organisationId: z.string(),
senderOrganisationId: z.string(),
userId: z.string().nullish(),
createdAt: z.date(),
updatedAt: z.date(),
Expand Down
4 changes: 3 additions & 1 deletion src/server/repositories/Invitation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ export function createInvitation(invitation: NewInvitation) {
.executeTakeFirstOrThrow();
}

export function listPendingInvitations() {
export function listPendingInvitations(senderOrganisationId: string) {
return db
.selectFrom("invitation")
.leftJoin("organisation", "invitation.organisationId", "organisation.id")
.where("invitation.userId", "is", null)
Comment thread
joaquimds marked this conversation as resolved.
.where("invitation.used", "=", false)
.where("invitation.senderOrganisationId", "=", senderOrganisationId)
.select([
"invitation.id",
"invitation.email",
Expand Down
32 changes: 30 additions & 2 deletions src/server/trpc/routers/invitation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from "@/server/repositories/Invitation";
import {
findOrganisationById,
findOrganisationForUser,
upsertOrganisation,
} from "@/server/repositories/Organisation";
import logger from "@/server/services/logger";
Expand All @@ -24,6 +25,7 @@ export const invitationRouter = router({
.object({
name: z.string(),
email: z.string().email(),
senderOrganisationId: z.string(),
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The create input schema allows an empty senderOrganisationId (and the client currently sends "" when organisationId is not set). This will always fail the membership check and surfaces as a FORBIDDEN error, which is confusing and not a validation error. Consider validating senderOrganisationId as non-empty (and/or UUID) at the zod layer so this becomes a BAD_REQUEST with field errors.

Suggested change
senderOrganisationId: z.string(),
senderOrganisationId: z
.string()
.trim()
.min(1, "Sender organisation is required"),

Copilot uses AI. Check for mistakes.
organisationId: z.string().nullish(),
organisationName: z.string().nullish(),
mapSelections: z
Expand All @@ -40,8 +42,19 @@ export const invitationRouter = router({
path: ["organisationId", "organisationName"],
}),
)
.mutation(async ({ input }) => {
.mutation(async ({ input, ctx }) => {
try {
const senderOrg = await findOrganisationForUser(
input.senderOrganisationId,
ctx.user.id,
);
if (!senderOrg) {
throw new TRPCError({
code: "FORBIDDEN",
message: "You do not belong to the sender organisation",
});
}

let org;
if (input.organisationId) {
org = await findOrganisationById(input.organisationId);
Expand All @@ -66,6 +79,7 @@ export const invitationRouter = router({
email: input.email.toLowerCase().trim(),
name: input.name,
organisationId: org.id,
senderOrganisationId: senderOrg.id,
});

const secret = new TextEncoder().encode(process.env.JWT_SECRET || "");
Expand All @@ -87,7 +101,21 @@ export const invitationRouter = router({
});
}
}),
list: advocateProcedure.query(() => listPendingInvitations()),
list: advocateProcedure
.input(z.object({ senderOrganisationId: z.string().uuid() }))
.query(async ({ input, ctx }) => {
const org = await findOrganisationForUser(
input.senderOrganisationId,
ctx.user.id,
);
if (!org) {
throw new TRPCError({
code: "FORBIDDEN",
message: "You do not belong to this organisation",
});
}
return listPendingInvitations(org.id);
}),
listForUser: protectedProcedure.query(async ({ ctx }) => {
return findPendingInvitationsByEmail(ctx.user.email);
}),
Expand Down
1 change: 1 addition & 0 deletions src/server/trpc/routers/organisation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export const organisationRouter = router({
email,
name: input.name,
organisationId: ctx.organisation.id,
senderOrganisationId: ctx.organisation.id,
});

if (existingUser) {
Expand Down
Loading
Loading