From 25066aa341dc803030b7eabc67a97c5b98f4c5de Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Thu, 28 May 2026 14:47:22 -0500 Subject: [PATCH 01/21] add partner-config.read/update privileges, map to PartnerAdmin PR 2 introduces admin endpoints for the cross-year matching toggle. Privileges and role mapping land first so the controller wire-up has something to authorize against. --- app/models/src/dtos/privileges.ts | 4 +++- app/models/src/dtos/role-privileges.ts | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/src/dtos/privileges.ts b/app/models/src/dtos/privileges.ts index 1246ab99..71420e94 100644 --- a/app/models/src/dtos/privileges.ts +++ b/app/models/src/dtos/privileges.ts @@ -3,4 +3,6 @@ export type PrivilegeKey = | 'partner-earthmover-bundle.create' | 'partner-earthmover-bundle.delete' | 'school-year-config.read' - | 'school-year-config.update'; + | 'school-year-config.update' + | 'partner-config.read' + | 'partner-config.update'; diff --git a/app/models/src/dtos/role-privileges.ts b/app/models/src/dtos/role-privileges.ts index c28d1397..7b0803cd 100644 --- a/app/models/src/dtos/role-privileges.ts +++ b/app/models/src/dtos/role-privileges.ts @@ -10,6 +10,8 @@ export const rolePrivileges: Record> = Object.freeze 'partner-earthmover-bundle.delete', 'school-year-config.read', 'school-year-config.update', + 'partner-config.read', + 'partner-config.update', ]) ), User: Object.freeze(new Set(['school-year-config.read'])), From 9022825233eb4092cac279c6153c1a75ce854620 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Thu, 28 May 2026 14:56:42 -0500 Subject: [PATCH 02/21] add failing tests for GET /partners/config Red step: tests describe the read endpoint's auth gating, partner-scoped column reflection, and eduCredsExist coming from canConnect. --- .../integration/tests/partners-config.spec.ts | 71 +++++++++++++++++++ app/models/src/dtos/index.ts | 1 + app/models/src/dtos/partner-config.dto.ts | 18 +++++ 3 files changed, 90 insertions(+) create mode 100644 app/api/integration/tests/partners-config.spec.ts create mode 100644 app/models/src/dtos/partner-config.dto.ts diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts new file mode 100644 index 00000000..95472bf8 --- /dev/null +++ b/app/api/integration/tests/partners-config.spec.ts @@ -0,0 +1,71 @@ +import request from 'supertest'; +import { userA } from '../fixtures/user-fixtures'; +import { tenantA } from '../fixtures/context-fixtures/tenant-fixtures'; +import { partnerA } from '../fixtures/context-fixtures/partner-fixtures'; +import { idpA } from '../fixtures/context-fixtures/idp-fixtures'; +import { authHelper } from '../helpers/oidc/auth-flow'; +import { EduSnowflakePoolService } from 'api/src/earthbeam/api/edu-snowflake-pool.service'; + +describe('GET /partners/config', () => { + const endpoint = '/partners/config'; + const userRoleA = 'runway.test.user'; + const partnerAdminRoleA = 'runway.test.partneradmin'; + + it('should reject unauthenticated requests', async () => { + const res = await request(app.getHttpServer()).get(endpoint); + expect(res.status).toBe(401); + }); + + it('should reject non-PartnerAdmin users', async () => { + const cookieA = (await authHelper.login(idpA, userA, tenantA, userRoleA)).cookies; + const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [cookieA]); + expect(res.status).toBe(403); + }); + + describe('as PartnerAdmin', () => { + let adminCookieA: string; + let canConnectSpy: jest.SpyInstance; + + beforeEach(async () => { + adminCookieA = (await authHelper.login(idpA, userA, tenantA, partnerAdminRoleA)).cookies; + canConnectSpy = jest + .spyOn(app.get(EduSnowflakePoolService), 'canConnect') + .mockResolvedValue(false); + }); + + afterEach(() => { + canConnectSpy.mockRestore(); + }); + + it('reflects the cross_year_matching_enabled column', async () => { + await global.prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: false }, + }); + let res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); + expect(res.status).toBe(200); + expect(res.body.crossYearMatchingEnabled).toBe(false); + + await global.prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: true }, + }); + res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); + expect(res.body.crossYearMatchingEnabled).toBe(true); + }); + + it('reflects canConnect for eduCredsExist', async () => { + canConnectSpy.mockResolvedValue(true); + const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); + expect(res.status).toBe(200); + expect(res.body.eduCredsExist).toBe(true); + expect(canConnectSpy).toHaveBeenCalledWith(partnerA.id); + }); + + it('returns eduCredsExist=false when canConnect is false', async () => { + canConnectSpy.mockResolvedValue(false); + const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); + expect(res.body.eduCredsExist).toBe(false); + }); + }); +}); diff --git a/app/models/src/dtos/index.ts b/app/models/src/dtos/index.ts index 1485034f..18a1175c 100644 --- a/app/models/src/dtos/index.ts +++ b/app/models/src/dtos/index.ts @@ -8,6 +8,7 @@ export * from './user.dto'; export * from './ods-config.dto'; export * from './school-year.dto'; export * from './school-year-config.dto'; +export * from './partner-config.dto'; export * from './job.dto'; export * from './job-template.dto'; export * from './file.dto'; diff --git a/app/models/src/dtos/partner-config.dto.ts b/app/models/src/dtos/partner-config.dto.ts new file mode 100644 index 00000000..115a39dc --- /dev/null +++ b/app/models/src/dtos/partner-config.dto.ts @@ -0,0 +1,18 @@ +import { Expose } from 'class-transformer'; +import { IsBoolean } from 'class-validator'; +import { makeSerializer } from '../utils/make-serializer'; + +export class GetPartnerConfigDto { + @Expose() + crossYearMatchingEnabled: boolean; + + @Expose() + eduCredsExist: boolean; +} + +export const toGetPartnerConfigDto = makeSerializer(GetPartnerConfigDto); + +export class PutPartnerConfigDto { + @IsBoolean() + crossYearMatchingEnabled: boolean; +} From cdddc1ec8aa97c4c912396f6138e2b9970d51550 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Thu, 28 May 2026 15:38:24 -0500 Subject: [PATCH 03/21] add GET /partners/config Returns the partner's crossYearMatchingEnabled and an eduCredsExist indicator derived from EduSnowflakePoolService.canConnect, so the FE can render the toggle and gate the enable action when creds are missing without ever seeing the creds themselves. Wires PartnersModule against EarthbeamApiModule for the pool service; the module now exports EduSnowflakePoolService. --- .../src/earthbeam/api/earthbeam-api.module.ts | 2 +- app/api/src/partners/partners.controller.ts | 42 ++++++++++++++++++- app/api/src/partners/partners.module.ts | 3 +- 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/app/api/src/earthbeam/api/earthbeam-api.module.ts b/app/api/src/earthbeam/api/earthbeam-api.module.ts index 7db89898..324defe2 100644 --- a/app/api/src/earthbeam/api/earthbeam-api.module.ts +++ b/app/api/src/earthbeam/api/earthbeam-api.module.ts @@ -29,6 +29,6 @@ import { EventEmitterModule } from 'api/src/event-emitter/event-emitter.module'; ], providers: [EarthbeamApiService, EduSnowflakePoolService], controllers: [EarthbeamApiController], - exports: [], + exports: [EduSnowflakePoolService], }) export class EarthbeamApiModule {} diff --git a/app/api/src/partners/partners.controller.ts b/app/api/src/partners/partners.controller.ts index 29b3a49f..85d34f2a 100644 --- a/app/api/src/partners/partners.controller.ts +++ b/app/api/src/partners/partners.controller.ts @@ -1,13 +1,51 @@ -import { Controller, Post } from '@nestjs/common'; +import { Body, Controller, Get, Inject, Post, Put } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; +import { PrismaClient, Tenant } from '@prisma/client'; +import { + PutPartnerConfigDto, + toGetPartnerConfigDto, +} from '@edanalytics/models'; +import { PRISMA_APP_USER } from '../database'; import { Authorize } from '../auth/helpers/authorize.decorator'; +import { Tenant as TenantDecorator } from '../auth/helpers/tenant.decorator'; +import { EduSnowflakePoolService } from '../earthbeam/api/edu-snowflake-pool.service'; @Controller() @ApiTags('Partners') export class PartnersController { - constructor() {} + constructor( + @Inject(PRISMA_APP_USER) private prisma: PrismaClient, + private eduPool: EduSnowflakePoolService, + ) {} @Authorize('partner-earthmover-bundle.create') @Post(':type/:bundleKey') async enableBundle() {} + + @Authorize('partner-config.read') + @Get('config') + async getConfig(@TenantDecorator() tenant: Tenant) { + const partner = await this.prisma.partner.findUniqueOrThrow({ + where: { id: tenant.partnerId }, + select: { crossYearMatchingEnabled: true }, + }); + const eduCredsExist = await this.eduPool.canConnect(tenant.partnerId); + return toGetPartnerConfigDto({ + crossYearMatchingEnabled: partner.crossYearMatchingEnabled, + eduCredsExist, + }); + } + + @Authorize('partner-config.update') + @Put('config') + async updateConfig( + @TenantDecorator() tenant: Tenant, + @Body() body: PutPartnerConfigDto, + ) { + await this.prisma.partner.update({ + where: { id: tenant.partnerId }, + data: { crossYearMatchingEnabled: body.crossYearMatchingEnabled }, + }); + return { status: 'ok' }; + } } diff --git a/app/api/src/partners/partners.module.ts b/app/api/src/partners/partners.module.ts index 253c6c86..74076353 100644 --- a/app/api/src/partners/partners.module.ts +++ b/app/api/src/partners/partners.module.ts @@ -1,8 +1,9 @@ import { Module } from '@nestjs/common'; import { PartnersController } from './partners.controller'; +import { EarthbeamApiModule } from '../earthbeam/api/earthbeam-api.module'; @Module({ - imports: [], + imports: [EarthbeamApiModule], providers: [], controllers: [PartnersController], }) From 2b328bb5863a0dd4a37fc03b9dd6583e35705789 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Thu, 28 May 2026 15:40:56 -0500 Subject: [PATCH 04/21] add PUT /partners/config tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers auth gating, the partner-scoped column write, the no-gate-on-creds policy, partner isolation, and invalid-body rejection. The PUT handler itself shipped with the prior commit alongside the GET handler — keeping the test file as a single round-trip surface kept the controller wire-up coherent, so the red/green pair here lands as one commit rather than two. --- .../integration/tests/partners-config.spec.ts | 110 +++++++++++++++++- 1 file changed, 107 insertions(+), 3 deletions(-) diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts index 95472bf8..374f6029 100644 --- a/app/api/integration/tests/partners-config.spec.ts +++ b/app/api/integration/tests/partners-config.spec.ts @@ -1,8 +1,8 @@ import request from 'supertest'; -import { userA } from '../fixtures/user-fixtures'; -import { tenantA } from '../fixtures/context-fixtures/tenant-fixtures'; +import { userA, userX } from '../fixtures/user-fixtures'; +import { tenantA, tenantX } from '../fixtures/context-fixtures/tenant-fixtures'; import { partnerA } from '../fixtures/context-fixtures/partner-fixtures'; -import { idpA } from '../fixtures/context-fixtures/idp-fixtures'; +import { idpA, idpX } from '../fixtures/context-fixtures/idp-fixtures'; import { authHelper } from '../helpers/oidc/auth-flow'; import { EduSnowflakePoolService } from 'api/src/earthbeam/api/edu-snowflake-pool.service'; @@ -69,3 +69,107 @@ describe('GET /partners/config', () => { }); }); }); + +describe('PUT /partners/config', () => { + const endpoint = '/partners/config'; + const userRoleA = 'runway.test.user'; + const partnerAdminRoleA = 'runway.test.partneradmin'; + const partnerAdminRoleX = 'Runway.PartnerAdmin'; + + it('should reject unauthenticated requests', async () => { + const res = await request(app.getHttpServer()) + .put(endpoint) + .send({ crossYearMatchingEnabled: true }); + expect(res.status).toBe(401); + }); + + it('should reject non-PartnerAdmin users', async () => { + const cookieA = (await authHelper.login(idpA, userA, tenantA, userRoleA)).cookies; + const res = await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [cookieA]) + .send({ crossYearMatchingEnabled: true }); + expect(res.status).toBe(403); + }); + + describe('as PartnerAdmin', () => { + let adminCookieA: string; + let canConnectSpy: jest.SpyInstance; + + beforeEach(async () => { + adminCookieA = (await authHelper.login(idpA, userA, tenantA, partnerAdminRoleA)).cookies; + canConnectSpy = jest + .spyOn(app.get(EduSnowflakePoolService), 'canConnect') + .mockResolvedValue(false); + await global.prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: false }, + }); + }); + + afterEach(() => { + canConnectSpy.mockRestore(); + }); + + it('updates cross_year_matching_enabled on the partner row', async () => { + const res = await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [adminCookieA]) + .send({ crossYearMatchingEnabled: true }); + expect(res.status).toBe(200); + expect(res.body).toEqual({ status: 'ok' }); + + const row = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id } }); + expect(row.crossYearMatchingEnabled).toBe(true); + }); + + it('subsequent GET reflects the new value', async () => { + await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [adminCookieA]) + .send({ crossYearMatchingEnabled: true }); + + const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); + expect(res.body.crossYearMatchingEnabled).toBe(true); + }); + + it('succeeds when enabling without creds (backend does not gate)', async () => { + canConnectSpy.mockResolvedValue(false); + const res = await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [adminCookieA]) + .send({ crossYearMatchingEnabled: true }); + expect(res.status).toBe(200); + + const row = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id } }); + expect(row.crossYearMatchingEnabled).toBe(true); + }); + + it('only modifies the session partner', async () => { + const adminCookieX = (await authHelper.login(idpX, userX, tenantX, partnerAdminRoleX)).cookies; + await global.prisma.partner.update({ + where: { id: 'partner-x' }, + data: { crossYearMatchingEnabled: false }, + }); + + const res = await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [adminCookieX]) + .send({ crossYearMatchingEnabled: true }); + expect(res.status).toBe(200); + + const partnerARow = await global.prisma.partner.findUniqueOrThrow({ + where: { id: partnerA.id }, + }); + expect(partnerARow.crossYearMatchingEnabled).toBe(false); + }); + + it('rejects invalid body', async () => { + const res = await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [adminCookieA]) + .send({ crossYearMatchingEnabled: 'nope' }); + expect(res.status).toBe(400); + }); + }); +}); From 24fa08b66a6fcd9dbe49a772b562ac9d4ebb8382 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Thu, 28 May 2026 15:45:55 -0500 Subject: [PATCH 05/21] add CrossYearMatchingSection to Admin page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New FE section + partner-config queries. Toggle is disabled when EDU creds are missing AND it's currently off — preventing partner admins from enabling a feature that can't function while still allowing them to turn it back off if creds disappear after the fact. EDU connection state is shown as a badge alongside a note about cred rotation requiring an app restart for now (the optional refresh-pool endpoint is deferred). --- app/fe/src/app/Pages/Admin/AdminPage.tsx | 2 + .../Pages/Admin/CrossYearMatchingSection.tsx | 76 +++++++++++++++++++ .../app/api/queries/partner-config.queries.ts | 22 ++++++ 3 files changed, 100 insertions(+) create mode 100644 app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx create mode 100644 app/fe/src/app/api/queries/partner-config.queries.ts diff --git a/app/fe/src/app/Pages/Admin/AdminPage.tsx b/app/fe/src/app/Pages/Admin/AdminPage.tsx index 994714c5..7831ebfc 100644 --- a/app/fe/src/app/Pages/Admin/AdminPage.tsx +++ b/app/fe/src/app/Pages/Admin/AdminPage.tsx @@ -1,6 +1,7 @@ import { Box, VStack } from '@chakra-ui/react'; import { useMe } from '../../api/queries/me.queries'; import { SchoolYearConfigSection } from './SchoolYearConfigSection'; +import { CrossYearMatchingSection } from './CrossYearMatchingSection'; export const AdminPage = () => { const { data: me } = useMe(); @@ -12,6 +13,7 @@ export const AdminPage = () => { admin settings ({partnerId}) + ); }; diff --git a/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx b/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx new file mode 100644 index 00000000..aba4d534 --- /dev/null +++ b/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx @@ -0,0 +1,76 @@ +import { Badge, Box, HStack, Switch, Text, VStack } from '@chakra-ui/react'; +import { useQuery } from '@tanstack/react-query'; +import { + partnerConfigQuery, + useUpdatePartnerConfig, +} from '../../api/queries/partner-config.queries'; + +const switchSx = { + '.chakra-switch__track': { + bg: 'blue.800', + _checked: { bg: 'green.300' }, + }, + '.chakra-switch__thumb': { bg: 'blue.50' }, +} as const; + +export const CrossYearMatchingSection = () => { + const { data: config, isLoading } = useQuery(partnerConfigQuery); + const update = useUpdatePartnerConfig(); + + if (isLoading || !config) { + return loading...; + } + + // FE-side gate: spec disables the enable action when EDU creds are missing. + // The backend tolerates the enabled-without-creds state; this is UX guidance. + const toggleDisabled = !config.eduCredsExist && !config.crossYearMatchingEnabled; + + return ( + + + cross-year ID matching + + + + + + enable cross-year ID matching + + when on, jobs match student IDs against prior-year rosters via EDU. + + + + update.mutate({ crossYearMatchingEnabled: e.target.checked }) + } + /> + + + + EDU connection + + {config.eduCredsExist ? 'connected' : 'not connected'} + + + + {!config.eduCredsExist && ( + + EDU credentials are provisioned in AWS Secrets Manager. Cred rotation + currently requires an app restart to take effect. + + )} + + + + ); +}; diff --git a/app/fe/src/app/api/queries/partner-config.queries.ts b/app/fe/src/app/api/queries/partner-config.queries.ts new file mode 100644 index 00000000..3fa8d234 --- /dev/null +++ b/app/fe/src/app/api/queries/partner-config.queries.ts @@ -0,0 +1,22 @@ +import { GetPartnerConfigDto, PutPartnerConfigDto } from '@edanalytics/models'; +import { useMutation, useQueryClient } from '@tanstack/react-query'; +import { apiClient, methods } from '../methods'; + +const QUERY_KEY = ['partner-config']; + +export const partnerConfigQuery = { + queryKey: QUERY_KEY, + queryFn: () => methods.getOne('/partners/config', GetPartnerConfigDto), +}; + +export const useUpdatePartnerConfig = () => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async (body: PutPartnerConfigDto) => { + return apiClient.put('/partners/config', body); + }, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: QUERY_KEY }); + }, + }); +}; From c3419767898410aeabc37491e06934f979c879e7 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Thu, 28 May 2026 15:52:19 -0500 Subject: [PATCH 06/21] gate PUT /partners/config on EDU creds when enabling Reverses the original "FE-only gate" decision: backend now rejects crossYearMatchingEnabled=true with 400 when canConnect is false. Disabling is always allowed, so an admin can still turn the feature off if creds disappear after the fact. Reason for the change: FE-only gating is bypassable via direct API call, and canConnect short-circuits on the pool cache so the extra check is effectively free on the hot path. --- .../integration/tests/partners-config.spec.ts | 23 ++++++++++++++++--- app/api/src/partners/partners.controller.ts | 13 ++++++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts index 374f6029..b956baec 100644 --- a/app/api/integration/tests/partners-config.spec.ts +++ b/app/api/integration/tests/partners-config.spec.ts @@ -100,7 +100,7 @@ describe('PUT /partners/config', () => { adminCookieA = (await authHelper.login(idpA, userA, tenantA, partnerAdminRoleA)).cookies; canConnectSpy = jest .spyOn(app.get(EduSnowflakePoolService), 'canConnect') - .mockResolvedValue(false); + .mockResolvedValue(true); await global.prisma.partner.update({ where: { id: partnerA.id }, data: { crossYearMatchingEnabled: false }, @@ -133,16 +133,33 @@ describe('PUT /partners/config', () => { expect(res.body.crossYearMatchingEnabled).toBe(true); }); - it('succeeds when enabling without creds (backend does not gate)', async () => { + it('rejects enabling when EDU creds are missing', async () => { canConnectSpy.mockResolvedValue(false); const res = await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieA]) .send({ crossYearMatchingEnabled: true }); + expect(res.status).toBe(400); + + const row = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id } }); + expect(row.crossYearMatchingEnabled).toBe(false); + }); + + it('allows disabling even when EDU creds are missing', async () => { + await global.prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: true }, + }); + canConnectSpy.mockResolvedValue(false); + + const res = await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [adminCookieA]) + .send({ crossYearMatchingEnabled: false }); expect(res.status).toBe(200); const row = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id } }); - expect(row.crossYearMatchingEnabled).toBe(true); + expect(row.crossYearMatchingEnabled).toBe(false); }); it('only modifies the session partner', async () => { diff --git a/app/api/src/partners/partners.controller.ts b/app/api/src/partners/partners.controller.ts index 85d34f2a..9bcc4023 100644 --- a/app/api/src/partners/partners.controller.ts +++ b/app/api/src/partners/partners.controller.ts @@ -1,4 +1,12 @@ -import { Body, Controller, Get, Inject, Post, Put } from '@nestjs/common'; +import { + BadRequestException, + Body, + Controller, + Get, + Inject, + Post, + Put, +} from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { PrismaClient, Tenant } from '@prisma/client'; import { @@ -42,6 +50,9 @@ export class PartnersController { @TenantDecorator() tenant: Tenant, @Body() body: PutPartnerConfigDto, ) { + if (body.crossYearMatchingEnabled && !(await this.eduPool.canConnect(tenant.partnerId))) { + throw new BadRequestException('EDU credentials are not configured for this partner.'); + } await this.prisma.partner.update({ where: { id: tenant.partnerId }, data: { crossYearMatchingEnabled: body.crossYearMatchingEnabled }, From 93100c2b1774ecd66d76402b1b132befeb5f5d19 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Fri, 29 May 2026 05:32:46 -0500 Subject: [PATCH 07/21] rework cross-year matching admin UX Mirrors the school-year config pane's interaction model: view-only by default with an "edit" affordance, save/cancel footer while editing, unsaved-changes blocker, and a confirm-changes modal. View mode shows an enabled/disabled badge in place of the switch. Reframes the section as "partner-wide configuration" with cross-year matching ("source roster from EDU") as its first setting, in anticipation of more partner-level toggles landing here later. EDU connection status is rendered inline with the icon-chip + label treatment from the ODS Configs page so admins encounter the same visual vocabulary across surfaces. Helptext picks up the missing-creds requirement when applicable, so admins see why the toggle is unavailable without needing to read the backend error. --- .../Pages/Admin/CrossYearMatchingSection.tsx | 258 +++++++++++++++--- 1 file changed, 221 insertions(+), 37 deletions(-) diff --git a/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx b/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx index aba4d534..15cbc45c 100644 --- a/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx +++ b/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx @@ -1,9 +1,23 @@ -import { Badge, Box, HStack, Switch, Text, VStack } from '@chakra-ui/react'; +import { useEffect, useState } from 'react'; +import { + Badge, + Box, + Button, + HStack, + Switch, + Text, + VStack, + useDisclosure, +} from '@chakra-ui/react'; import { useQuery } from '@tanstack/react-query'; +import { useBlocker } from '@tanstack/react-router'; import { partnerConfigQuery, useUpdatePartnerConfig, } from '../../api/queries/partner-config.queries'; +import { ConfirmChangesModal } from './ConfirmChangesModal'; +import { RunwayErrorBox } from '../../components/Form/RunwayFormErrorBox'; +import { IconCheckmark, IconExclamation } from '../../../assets/icons'; const switchSx = { '.chakra-switch__track': { @@ -17,60 +31,230 @@ export const CrossYearMatchingSection = () => { const { data: config, isLoading } = useQuery(partnerConfigQuery); const update = useUpdatePartnerConfig(); + const [isEditing, setIsEditing] = useState(false); + const [draftEnabled, setDraftEnabled] = useState(false); + const [generalError, setGeneralError] = useState(null); + const { isOpen, onOpen, onClose } = useDisclosure(); + const [modalMode, setModalMode] = useState<'save' | 'leave'>('save'); + const [pendingLeaveAction, setPendingLeaveAction] = useState(null); + + useEffect(() => { + if (config && !isEditing) setDraftEnabled(config.crossYearMatchingEnabled); + }, [config?.crossYearMatchingEnabled, isEditing]); + + const hasChanges = !!config && draftEnabled !== config.crossYearMatchingEnabled; + const shouldWarnAboutUnsavedChanges = isEditing && hasChanges && !update.isPending; + const blocker = useBlocker({ condition: shouldWarnAboutUnsavedChanges }); + + useEffect(() => { + if (!shouldWarnAboutUnsavedChanges) return; + const handleBeforeUnload = (event: BeforeUnloadEvent) => { + event.preventDefault(); + event.returnValue = ''; + }; + window.addEventListener('beforeunload', handleBeforeUnload); + return () => window.removeEventListener('beforeunload', handleBeforeUnload); + }, [shouldWarnAboutUnsavedChanges]); + + useEffect(() => { + if (blocker.status === 'blocked') { + setModalMode('leave'); + onOpen(); + } + }, [blocker.status, onOpen]); + if (isLoading || !config) { return loading...; } - // FE-side gate: spec disables the enable action when EDU creds are missing. - // The backend tolerates the enabled-without-creds state; this is UX guidance. - const toggleDisabled = !config.eduCredsExist && !config.crossYearMatchingEnabled; + // Backend rejects enable-when-no-creds; mirror that on the FE so the + // affordance disappears before the user can hit a 400. + const cannotEnable = !config.eduCredsExist; + const switchDisabled = !isEditing || (cannotEnable && !draftEnabled); + + const startEdit = () => { + setDraftEnabled(config.crossYearMatchingEnabled); + setIsEditing(true); + }; + + const handleCancel = () => { + if (shouldWarnAboutUnsavedChanges) { + setPendingLeaveAction('cancel'); + setModalMode('leave'); + onOpen(); + return; + } + setDraftEnabled(config.crossYearMatchingEnabled); + setIsEditing(false); + }; + + const handleSave = () => { + if (!hasChanges) return; + setModalMode('save'); + onOpen(); + }; + + const handleSaveConfirm = () => { + update.mutate( + { crossYearMatchingEnabled: draftEnabled }, + { + onSuccess: () => { + onClose(); + setIsEditing(false); + }, + onError: () => { + onClose(); + setGeneralError('Something went wrong saving your changes. Please try again.'); + }, + }, + ); + }; + + const handleLeaveConfirm = () => { + onClose(); + if (blocker.status === 'blocked') { + blocker.proceed(); + return; + } + if (pendingLeaveAction === 'cancel') { + setPendingLeaveAction(null); + setDraftEnabled(config.crossYearMatchingEnabled); + setIsEditing(false); + } + }; + + const handleModalClose = () => { + onClose(); + if (blocker.status === 'blocked') blocker.reset(); + setPendingLeaveAction(null); + }; + + const changes = hasChanges + ? [ + `source roster from EDU: ${config.crossYearMatchingEnabled ? 'yes' : 'no'} → ${ + draftEnabled ? 'yes' : 'no' + }`, + ] + : []; return ( - - - cross-year ID matching - - - - - - enable cross-year ID matching - - when on, jobs match student IDs against prior-year rosters via EDU. - - + + + + partner-wide configuration + + {!isEditing && ( + + )} + + + {generalError && } + + + + + source roster from EDU + + {isEditing ? ( - update.mutate({ crossYearMatchingEnabled: e.target.checked }) - } + isChecked={draftEnabled} + isDisabled={switchDisabled || update.isPending} + onChange={(e) => setDraftEnabled(e.target.checked)} /> - - - - EDU connection + ) : ( - {config.eduCredsExist ? 'connected' : 'not connected'} + {config.crossYearMatchingEnabled ? 'enabled' : 'disabled'} - - - {!config.eduCredsExist && ( - - EDU credentials are provisioned in AWS Secrets Manager. Cred rotation - currently requires an app restart to take effect. - )} - + + + {config.eduCredsExist ? : } + + + {config.eduCredsExist ? 'EDU connected' : 'EDU not connected'} + + + + when on, IDs that fail to match the ODS roster can match against an + EDU cross-year roster and be made available for side-loading. + {!config.eduCredsExist && ( + <> An EDU connection must be configured to enable this setting. + )} + + + + {isEditing && ( + + + + + )} + + ); }; From 29ea7f359d6096f9a442b31d9f7da08a64992def Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Mon, 1 Jun 2026 11:17:16 -0500 Subject: [PATCH 08/21] clear save error banner on retry and edit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit generalError was set on mutation failure but never cleared, so a transient "something went wrong" banner could linger after a successful retry — the section stays mounted across saves, unlike SchoolYearConfigEditForm. Clear it when entering edit mode and at the start of each save attempt. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx b/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx index 15cbc45c..9b610073 100644 --- a/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx +++ b/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx @@ -73,6 +73,7 @@ export const CrossYearMatchingSection = () => { const switchDisabled = !isEditing || (cannotEnable && !draftEnabled); const startEdit = () => { + setGeneralError(null); setDraftEnabled(config.crossYearMatchingEnabled); setIsEditing(true); }; @@ -95,6 +96,7 @@ export const CrossYearMatchingSection = () => { }; const handleSaveConfirm = () => { + setGeneralError(null); update.mutate( { crossYearMatchingEnabled: draftEnabled }, { From 10e27954e45954ac1b1881a6156f12b426bdea07 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Mon, 1 Jun 2026 11:17:16 -0500 Subject: [PATCH 09/21] tighten partners-config tests lock GET to the exact { crossYearMatchingEnabled, eduCredsExist } shape so an accidental partner-field or secret leak is caught, and assert the session partner's row actually flipped in the scoping test (previously it only checked the other partner stayed put, so a no-op would pass). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration/tests/partners-config.spec.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts index b956baec..960a0fe6 100644 --- a/app/api/integration/tests/partners-config.spec.ts +++ b/app/api/integration/tests/partners-config.spec.ts @@ -67,6 +67,17 @@ describe('GET /partners/config', () => { const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); expect(res.body.eduCredsExist).toBe(false); }); + + it('returns exactly { crossYearMatchingEnabled, eduCredsExist } and nothing else', async () => { + canConnectSpy.mockResolvedValue(true); + await global.prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: true }, + }); + const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); + expect(res.status).toBe(200); + expect(res.body).toEqual({ crossYearMatchingEnabled: true, eduCredsExist: true }); + }); }); }); @@ -175,6 +186,11 @@ describe('PUT /partners/config', () => { .send({ crossYearMatchingEnabled: true }); expect(res.status).toBe(200); + const partnerXRow = await global.prisma.partner.findUniqueOrThrow({ + where: { id: 'partner-x' }, + }); + expect(partnerXRow.crossYearMatchingEnabled).toBe(true); + const partnerARow = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id }, }); From b9dd73c1f910e2e5c1f6330a90571d013ef308c2 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Mon, 1 Jun 2026 12:25:31 -0500 Subject: [PATCH 10/21] collapse cross-year help text and wire toggle a11y via form components the help text grew long enough to warrant hiding the detail behind a show more/less disclosure, keeping only the one-line summary visible. the disclosure button carries aria-expanded/aria-controls and the collapsed region is display:none when closed, so screen readers only announce it when expanded. convert the toggle row to FormControl/FormLabel/FormHelperText so the switch gets its accessible name and description wired by chakra instead of by hand. the EDU status block keeps an explicit id so the switch's aria-describedby still references it (chakra merges it with the helper text id). move the "EDU connection required" warning out of the help text to sit directly under the EDU status badge, where it's both more discoverable and still part of the switch description. add a plain FormControl theme variant that strips the container chrome (bg/padding/radius/focus glow) for controls that supply their own layout, like this toggle row nested in a contentBox. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Pages/Admin/CrossYearMatchingSection.tsx | 153 ++++++++++++------ app/fe/src/app/theme/formThemes.ts | 11 ++ 2 files changed, 118 insertions(+), 46 deletions(-) diff --git a/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx b/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx index 9b610073..f21a2ee0 100644 --- a/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx +++ b/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx @@ -3,6 +3,10 @@ import { Badge, Box, Button, + Collapse, + FormControl, + FormHelperText, + FormLabel, HStack, Switch, Text, @@ -35,6 +39,7 @@ export const CrossYearMatchingSection = () => { const [draftEnabled, setDraftEnabled] = useState(false); const [generalError, setGeneralError] = useState(null); const { isOpen, onOpen, onClose } = useDisclosure(); + const { isOpen: isHelpOpen, onToggle: onToggleHelp } = useDisclosure(); const [modalMode, setModalMode] = useState<'save' | 'leave'>('save'); const [pendingLeaveAction, setPendingLeaveAction] = useState(null); @@ -108,7 +113,7 @@ export const CrossYearMatchingSection = () => { onClose(); setGeneralError('Something went wrong saving your changes. Please try again.'); }, - }, + } ); }; @@ -163,54 +168,110 @@ export const CrossYearMatchingSection = () => { {generalError && } - - - source roster from EDU - - {isEditing ? ( - setDraftEnabled(e.target.checked)} - /> - ) : ( - - {config.crossYearMatchingEnabled ? 'enabled' : 'disabled'} - - )} - - - {config.eduCredsExist ? : } - - + + - {config.eduCredsExist ? 'EDU connected' : 'EDU not connected'} - - - - when on, IDs that fail to match the ODS roster can match against an - EDU cross-year roster and be made available for side-loading. - {!config.eduCredsExist && ( - <> An EDU connection must be configured to enable this setting. + Cross-year roster for ID matching + + {isEditing ? ( + setDraftEnabled(e.target.checked)} + /> + ) : ( + + {config.crossYearMatchingEnabled ? 'enabled' : 'disabled'} + )} - - + + + + {config.eduCredsExist ? : } + + + {config.eduCredsExist ? 'EDU connected' : 'EDU not connected'} + + + {!config.eduCredsExist && ( + + An EDU connection must be configured to enable this setting. + + )} + + + + + + Allow Runway to process records for students who were rostered in any year + available in EDU, even if the student is not rostered in the ODS year. + + + + + For jobs sent to an ODS, Runway will continue to match IDs against the ODS + roster first. If an ID does not match against the ODS roster, Runway will + attempt to match against the cross-year roster from EDU. If the ID matches + the cross-year roster, the student will be made available for side-loading + to EDU. IDs that do not match against either roster will follow the normal + unmatched ID review flow. + + + For jobs NOT sent to an ODS, Runway will use the cross-year roster from EDU, + if this setting is enabled. Otherwise, non-ODS jobs will expect a roster + file in S3. + + + + + + + + + {isEditing && ( diff --git a/app/fe/src/app/theme/formThemes.ts b/app/fe/src/app/theme/formThemes.ts index 2020a0e5..18ae360f 100644 --- a/app/fe/src/app/theme/formThemes.ts +++ b/app/fe/src/app/theme/formThemes.ts @@ -35,6 +35,17 @@ const FormControl = formControlHelper.defineMultiStyleConfig({ _hover: { ...noOutline, ...pointer }, }, }), + // No container chrome — for controls that supply their own layout + // (e.g. a toggle row already nested in a contentBox). + plain: formControlHelper.definePartsStyle({ + container: { + bg: 'transparent', + borderRadius: '0', + padding: '0', + _focusWithin: { boxShadow: 'none' }, + _hover: { boxShadow: 'none' }, + }, + }), }, }); From 47328dbd72b4236d5bc5a17fa9dff24730705648 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Mon, 1 Jun 2026 13:50:23 -0500 Subject: [PATCH 11/21] rename CrossYearMatchingSection to PartnerConfig the admin section will host additional partner-wide settings beyond the cross-year matching toggle, so name it for the broader role. the cross-year-specific aria ids stay as-is since they describe that specific control. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/fe/src/app/Pages/Admin/AdminPage.tsx | 4 ++-- .../Admin/{CrossYearMatchingSection.tsx => PartnerConfig.tsx} | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename app/fe/src/app/Pages/Admin/{CrossYearMatchingSection.tsx => PartnerConfig.tsx} (99%) diff --git a/app/fe/src/app/Pages/Admin/AdminPage.tsx b/app/fe/src/app/Pages/Admin/AdminPage.tsx index 7831ebfc..43dc5818 100644 --- a/app/fe/src/app/Pages/Admin/AdminPage.tsx +++ b/app/fe/src/app/Pages/Admin/AdminPage.tsx @@ -1,7 +1,7 @@ import { Box, VStack } from '@chakra-ui/react'; import { useMe } from '../../api/queries/me.queries'; import { SchoolYearConfigSection } from './SchoolYearConfigSection'; -import { CrossYearMatchingSection } from './CrossYearMatchingSection'; +import { PartnerConfig } from './PartnerConfig'; export const AdminPage = () => { const { data: me } = useMe(); @@ -13,7 +13,7 @@ export const AdminPage = () => { admin settings ({partnerId}) - + ); }; diff --git a/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx similarity index 99% rename from app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx rename to app/fe/src/app/Pages/Admin/PartnerConfig.tsx index f21a2ee0..3f0fa200 100644 --- a/app/fe/src/app/Pages/Admin/CrossYearMatchingSection.tsx +++ b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx @@ -31,7 +31,7 @@ const switchSx = { '.chakra-switch__thumb': { bg: 'blue.50' }, } as const; -export const CrossYearMatchingSection = () => { +export const PartnerConfig = () => { const { data: config, isLoading } = useQuery(partnerConfigQuery); const update = useUpdatePartnerConfig(); From b6a7b3ec93a8c2691c778ae0baae2f690ebc6bc9 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Mon, 1 Jun 2026 14:06:14 -0500 Subject: [PATCH 12/21] add optimistic concurrency to partner config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mirror the school-year-config stale-write pattern so a partner admin can't silently clobber a concurrent change as more settings land in this section. GET /partners/config returns the partner row's modifiedOn in x-config-modified-at; PUT requires x-if-config-modified-at and 409s on mismatch (or a missing header), returning lastModifiedOn/ lastModifiedBy. no migration needed — partner already carries modified_on/modified_by_id, maintained by the update_meta trigger. the FE captures the header on read and sends it back on save, surfacing a distinct "reload and try again" message on conflict. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration/tests/partners-config.spec.ts | 50 +++++++++++++++++++ app/api/src/partners/partners.controller.ts | 38 +++++++++++++- app/fe/src/app/Pages/Admin/PartnerConfig.tsx | 20 ++++++-- .../app/api/queries/partner-config.queries.ts | 24 +++++++-- 4 files changed, 122 insertions(+), 10 deletions(-) diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts index 960a0fe6..de54c338 100644 --- a/app/api/integration/tests/partners-config.spec.ts +++ b/app/api/integration/tests/partners-config.spec.ts @@ -122,10 +122,19 @@ describe('PUT /partners/config', () => { canConnectSpy.mockRestore(); }); + // The PUT requires an x-if-config-modified-at header matching the row's + // current modifiedOn (optimistic concurrency). GET surfaces it. + const getModifiedAt = async (cookie: string) => { + const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [cookie]); + return res.headers['x-config-modified-at'] as string; + }; + it('updates cross_year_matching_enabled on the partner row', async () => { + const ifModifiedAt = await getModifiedAt(adminCookieA); const res = await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieA]) + .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: true }); expect(res.status).toBe(200); expect(res.body).toEqual({ status: 'ok' }); @@ -135,9 +144,11 @@ describe('PUT /partners/config', () => { }); it('subsequent GET reflects the new value', async () => { + const ifModifiedAt = await getModifiedAt(adminCookieA); await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieA]) + .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: true }); const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); @@ -145,10 +156,12 @@ describe('PUT /partners/config', () => { }); it('rejects enabling when EDU creds are missing', async () => { + const ifModifiedAt = await getModifiedAt(adminCookieA); canConnectSpy.mockResolvedValue(false); const res = await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieA]) + .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: true }); expect(res.status).toBe(400); @@ -161,11 +174,13 @@ describe('PUT /partners/config', () => { where: { id: partnerA.id }, data: { crossYearMatchingEnabled: true }, }); + const ifModifiedAt = await getModifiedAt(adminCookieA); canConnectSpy.mockResolvedValue(false); const res = await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieA]) + .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: false }); expect(res.status).toBe(200); @@ -173,6 +188,39 @@ describe('PUT /partners/config', () => { expect(row.crossYearMatchingEnabled).toBe(false); }); + it('rejects a stale write with 409', async () => { + const staleModifiedAt = await getModifiedAt(adminCookieA); + + // another writer changes the config after this client loaded it + await global.prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: true }, + }); + + const res = await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [adminCookieA]) + .set('x-if-config-modified-at', staleModifiedAt) + .send({ crossYearMatchingEnabled: false }); + expect(res.status).toBe(409); + expect(res.body.lastModifiedOn).toBeTruthy(); + + // the rejected write left the other writer's value in place + const row = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id } }); + expect(row.crossYearMatchingEnabled).toBe(true); + }); + + it('rejects a write missing the if-config-modified-at header with 409', async () => { + const res = await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [adminCookieA]) + .send({ crossYearMatchingEnabled: true }); + expect(res.status).toBe(409); + + const row = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id } }); + expect(row.crossYearMatchingEnabled).toBe(false); + }); + it('only modifies the session partner', async () => { const adminCookieX = (await authHelper.login(idpX, userX, tenantX, partnerAdminRoleX)).cookies; await global.prisma.partner.update({ @@ -180,9 +228,11 @@ describe('PUT /partners/config', () => { data: { crossYearMatchingEnabled: false }, }); + const ifModifiedAt = await getModifiedAt(adminCookieX); const res = await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieX]) + .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: true }); expect(res.status).toBe(200); diff --git a/app/api/src/partners/partners.controller.ts b/app/api/src/partners/partners.controller.ts index 9bcc4023..17324506 100644 --- a/app/api/src/partners/partners.controller.ts +++ b/app/api/src/partners/partners.controller.ts @@ -1,11 +1,14 @@ import { BadRequestException, Body, + ConflictException, Controller, Get, + Headers, Inject, Post, Put, + Res, } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { PrismaClient, Tenant } from '@prisma/client'; @@ -13,6 +16,7 @@ import { PutPartnerConfigDto, toGetPartnerConfigDto, } from '@edanalytics/models'; +import { Response } from 'express'; import { PRISMA_APP_USER } from '../database'; import { Authorize } from '../auth/helpers/authorize.decorator'; import { Tenant as TenantDecorator } from '../auth/helpers/tenant.decorator'; @@ -32,12 +36,16 @@ export class PartnersController { @Authorize('partner-config.read') @Get('config') - async getConfig(@TenantDecorator() tenant: Tenant) { + async getConfig( + @TenantDecorator() tenant: Tenant, + @Res({ passthrough: true }) res: Response, + ) { const partner = await this.prisma.partner.findUniqueOrThrow({ where: { id: tenant.partnerId }, - select: { crossYearMatchingEnabled: true }, + select: { crossYearMatchingEnabled: true, modifiedOn: true }, }); const eduCredsExist = await this.eduPool.canConnect(tenant.partnerId); + res.setHeader('x-config-modified-at', partner.modifiedOn.toISOString()); return toGetPartnerConfigDto({ crossYearMatchingEnabled: partner.crossYearMatchingEnabled, eduCredsExist, @@ -48,11 +56,37 @@ export class PartnersController { @Put('config') async updateConfig( @TenantDecorator() tenant: Tenant, + @Headers('x-if-config-modified-at') ifModifiedAtHeader: string | undefined, @Body() body: PutPartnerConfigDto, ) { if (body.crossYearMatchingEnabled && !(await this.eduPool.canConnect(tenant.partnerId))) { throw new BadRequestException('EDU credentials are not configured for this partner.'); } + + const partner = await this.prisma.partner.findUniqueOrThrow({ + where: { id: tenant.partnerId }, + select: { + modifiedOn: true, + userPartnerModifiedByIdTouser: { select: { givenName: true, familyName: true } }, + }, + }); + + // Check-then-write: concurrent requests can both pass before either writes. + // Acceptable for a low-frequency admin config surface. + const ifModifiedAt = ifModifiedAtHeader ?? null; + const currentModifiedAt = partner.modifiedOn.toISOString(); + if (ifModifiedAt !== currentModifiedAt) { + const lastModifiedBy = partner.userPartnerModifiedByIdTouser; + throw new ConflictException({ + statusCode: 409, + message: 'Config has been modified since you loaded it.', + lastModifiedOn: currentModifiedAt, + lastModifiedBy: lastModifiedBy + ? `${lastModifiedBy.givenName} ${lastModifiedBy.familyName}` + : null, + }); + } + await this.prisma.partner.update({ where: { id: tenant.partnerId }, data: { crossYearMatchingEnabled: body.crossYearMatchingEnabled }, diff --git a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx index 3f0fa200..fab4381b 100644 --- a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx +++ b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx @@ -32,7 +32,9 @@ const switchSx = { } as const; export const PartnerConfig = () => { - const { data: config, isLoading } = useQuery(partnerConfigQuery); + const { data, isLoading } = useQuery(partnerConfigQuery); + const config = data?.config; + const modifiedAt = data?.modifiedAt ?? null; const update = useUpdatePartnerConfig(); const [isEditing, setIsEditing] = useState(false); @@ -103,15 +105,25 @@ export const PartnerConfig = () => { const handleSaveConfirm = () => { setGeneralError(null); update.mutate( - { crossYearMatchingEnabled: draftEnabled }, + { body: { crossYearMatchingEnabled: draftEnabled }, modifiedAt }, { onSuccess: () => { onClose(); setIsEditing(false); }, - onError: () => { + onError: (error: any) => { onClose(); - setGeneralError('Something went wrong saving your changes. Please try again.'); + if (error?.status === 409 || error?.statusCode === 409) { + const by = error.lastModifiedBy ?? error.data?.lastModifiedBy; + const on = error.lastModifiedOn ?? error.data?.lastModifiedOn; + setGeneralError( + `This setting was changed${by ? ` by ${by}` : ''}${ + on ? ` at ${new Date(on).toLocaleString()}` : '' + }. Please reload the page and try again.` + ); + } else { + setGeneralError('Something went wrong saving your changes. Please try again.'); + } }, } ); diff --git a/app/fe/src/app/api/queries/partner-config.queries.ts b/app/fe/src/app/api/queries/partner-config.queries.ts index 3fa8d234..25dd5144 100644 --- a/app/fe/src/app/api/queries/partner-config.queries.ts +++ b/app/fe/src/app/api/queries/partner-config.queries.ts @@ -1,19 +1,35 @@ import { GetPartnerConfigDto, PutPartnerConfigDto } from '@edanalytics/models'; +import { plainToInstance } from 'class-transformer'; import { useMutation, useQueryClient } from '@tanstack/react-query'; -import { apiClient, methods } from '../methods'; +import { apiClient, apiClientRaw } from '../methods'; const QUERY_KEY = ['partner-config']; export const partnerConfigQuery = { queryKey: QUERY_KEY, - queryFn: () => methods.getOne('/partners/config', GetPartnerConfigDto), + queryFn: async () => { + const res = await apiClientRaw.get('/partners/config'); + const headerValue = res.headers['x-config-modified-at']; + return { + config: plainToInstance(GetPartnerConfigDto, res.data), + modifiedAt: Array.isArray(headerValue) ? headerValue[0] : (headerValue ?? null), + } satisfies { config: GetPartnerConfigDto; modifiedAt: string | null }; + }, }; export const useUpdatePartnerConfig = () => { const queryClient = useQueryClient(); return useMutation({ - mutationFn: async (body: PutPartnerConfigDto) => { - return apiClient.put('/partners/config', body); + mutationFn: async ({ + body, + modifiedAt, + }: { + body: PutPartnerConfigDto; + modifiedAt: string | null; + }) => { + return apiClient.put('/partners/config', body, { + headers: modifiedAt ? { 'x-if-config-modified-at': modifiedAt } : undefined, + }); }, onSuccess: () => { queryClient.invalidateQueries({ queryKey: QUERY_KEY }); From 850fd26d6af1bd8f08e665002ed2058f7092b38b Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Mon, 1 Jun 2026 14:48:55 -0500 Subject: [PATCH 13/21] back out partner-config stale-write check, leave a note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the optimistic-concurrency check was more machinery than a single boolean toggle warrants. revert it for now and leave a TODO on the PUT handler so we revisit concurrency before this section gains more settings, when last-write-wins would actually risk silent clobbers. deferring the choice of approach too — the school-year-config header/409 pattern doesn't obviously map onto a one-row partner update. --- .../integration/tests/partners-config.spec.ts | 50 --------------- app/api/src/partners/partners.controller.ts | 64 +++---------------- app/fe/src/app/Pages/Admin/PartnerConfig.tsx | 20 ++---- .../app/api/queries/partner-config.queries.ts | 24 ++----- 4 files changed, 18 insertions(+), 140 deletions(-) diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts index de54c338..960a0fe6 100644 --- a/app/api/integration/tests/partners-config.spec.ts +++ b/app/api/integration/tests/partners-config.spec.ts @@ -122,19 +122,10 @@ describe('PUT /partners/config', () => { canConnectSpy.mockRestore(); }); - // The PUT requires an x-if-config-modified-at header matching the row's - // current modifiedOn (optimistic concurrency). GET surfaces it. - const getModifiedAt = async (cookie: string) => { - const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [cookie]); - return res.headers['x-config-modified-at'] as string; - }; - it('updates cross_year_matching_enabled on the partner row', async () => { - const ifModifiedAt = await getModifiedAt(adminCookieA); const res = await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieA]) - .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: true }); expect(res.status).toBe(200); expect(res.body).toEqual({ status: 'ok' }); @@ -144,11 +135,9 @@ describe('PUT /partners/config', () => { }); it('subsequent GET reflects the new value', async () => { - const ifModifiedAt = await getModifiedAt(adminCookieA); await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieA]) - .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: true }); const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); @@ -156,12 +145,10 @@ describe('PUT /partners/config', () => { }); it('rejects enabling when EDU creds are missing', async () => { - const ifModifiedAt = await getModifiedAt(adminCookieA); canConnectSpy.mockResolvedValue(false); const res = await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieA]) - .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: true }); expect(res.status).toBe(400); @@ -174,13 +161,11 @@ describe('PUT /partners/config', () => { where: { id: partnerA.id }, data: { crossYearMatchingEnabled: true }, }); - const ifModifiedAt = await getModifiedAt(adminCookieA); canConnectSpy.mockResolvedValue(false); const res = await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieA]) - .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: false }); expect(res.status).toBe(200); @@ -188,39 +173,6 @@ describe('PUT /partners/config', () => { expect(row.crossYearMatchingEnabled).toBe(false); }); - it('rejects a stale write with 409', async () => { - const staleModifiedAt = await getModifiedAt(adminCookieA); - - // another writer changes the config after this client loaded it - await global.prisma.partner.update({ - where: { id: partnerA.id }, - data: { crossYearMatchingEnabled: true }, - }); - - const res = await request(app.getHttpServer()) - .put(endpoint) - .set('Cookie', [adminCookieA]) - .set('x-if-config-modified-at', staleModifiedAt) - .send({ crossYearMatchingEnabled: false }); - expect(res.status).toBe(409); - expect(res.body.lastModifiedOn).toBeTruthy(); - - // the rejected write left the other writer's value in place - const row = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id } }); - expect(row.crossYearMatchingEnabled).toBe(true); - }); - - it('rejects a write missing the if-config-modified-at header with 409', async () => { - const res = await request(app.getHttpServer()) - .put(endpoint) - .set('Cookie', [adminCookieA]) - .send({ crossYearMatchingEnabled: true }); - expect(res.status).toBe(409); - - const row = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id } }); - expect(row.crossYearMatchingEnabled).toBe(false); - }); - it('only modifies the session partner', async () => { const adminCookieX = (await authHelper.login(idpX, userX, tenantX, partnerAdminRoleX)).cookies; await global.prisma.partner.update({ @@ -228,11 +180,9 @@ describe('PUT /partners/config', () => { data: { crossYearMatchingEnabled: false }, }); - const ifModifiedAt = await getModifiedAt(adminCookieX); const res = await request(app.getHttpServer()) .put(endpoint) .set('Cookie', [adminCookieX]) - .set('x-if-config-modified-at', ifModifiedAt) .send({ crossYearMatchingEnabled: true }); expect(res.status).toBe(200); diff --git a/app/api/src/partners/partners.controller.ts b/app/api/src/partners/partners.controller.ts index 17324506..ca518f71 100644 --- a/app/api/src/partners/partners.controller.ts +++ b/app/api/src/partners/partners.controller.ts @@ -1,22 +1,7 @@ -import { - BadRequestException, - Body, - ConflictException, - Controller, - Get, - Headers, - Inject, - Post, - Put, - Res, -} from '@nestjs/common'; +import { BadRequestException, Body, Controller, Get, Inject, Post, Put } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { PrismaClient, Tenant } from '@prisma/client'; -import { - PutPartnerConfigDto, - toGetPartnerConfigDto, -} from '@edanalytics/models'; -import { Response } from 'express'; +import { PutPartnerConfigDto, toGetPartnerConfigDto } from '@edanalytics/models'; import { PRISMA_APP_USER } from '../database'; import { Authorize } from '../auth/helpers/authorize.decorator'; import { Tenant as TenantDecorator } from '../auth/helpers/tenant.decorator'; @@ -27,7 +12,7 @@ import { EduSnowflakePoolService } from '../earthbeam/api/edu-snowflake-pool.ser export class PartnersController { constructor( @Inject(PRISMA_APP_USER) private prisma: PrismaClient, - private eduPool: EduSnowflakePoolService, + private eduPool: EduSnowflakePoolService ) {} @Authorize('partner-earthmover-bundle.create') @@ -36,57 +21,28 @@ export class PartnersController { @Authorize('partner-config.read') @Get('config') - async getConfig( - @TenantDecorator() tenant: Tenant, - @Res({ passthrough: true }) res: Response, - ) { + async getConfig(@TenantDecorator() tenant: Tenant) { const partner = await this.prisma.partner.findUniqueOrThrow({ where: { id: tenant.partnerId }, - select: { crossYearMatchingEnabled: true, modifiedOn: true }, + select: { crossYearMatchingEnabled: true }, }); const eduCredsExist = await this.eduPool.canConnect(tenant.partnerId); - res.setHeader('x-config-modified-at', partner.modifiedOn.toISOString()); return toGetPartnerConfigDto({ crossYearMatchingEnabled: partner.crossYearMatchingEnabled, eduCredsExist, }); } + // TODO: add an optimistic-concurrency / stale-write check before this + // section grows beyond the single cross-year toggle. With one boolean a + // last-write-wins clobber is harmless, but once multiple settings share this + // endpoint, concurrent admin edits could silently overwrite each other. @Authorize('partner-config.update') @Put('config') - async updateConfig( - @TenantDecorator() tenant: Tenant, - @Headers('x-if-config-modified-at') ifModifiedAtHeader: string | undefined, - @Body() body: PutPartnerConfigDto, - ) { + async updateConfig(@TenantDecorator() tenant: Tenant, @Body() body: PutPartnerConfigDto) { if (body.crossYearMatchingEnabled && !(await this.eduPool.canConnect(tenant.partnerId))) { throw new BadRequestException('EDU credentials are not configured for this partner.'); } - - const partner = await this.prisma.partner.findUniqueOrThrow({ - where: { id: tenant.partnerId }, - select: { - modifiedOn: true, - userPartnerModifiedByIdTouser: { select: { givenName: true, familyName: true } }, - }, - }); - - // Check-then-write: concurrent requests can both pass before either writes. - // Acceptable for a low-frequency admin config surface. - const ifModifiedAt = ifModifiedAtHeader ?? null; - const currentModifiedAt = partner.modifiedOn.toISOString(); - if (ifModifiedAt !== currentModifiedAt) { - const lastModifiedBy = partner.userPartnerModifiedByIdTouser; - throw new ConflictException({ - statusCode: 409, - message: 'Config has been modified since you loaded it.', - lastModifiedOn: currentModifiedAt, - lastModifiedBy: lastModifiedBy - ? `${lastModifiedBy.givenName} ${lastModifiedBy.familyName}` - : null, - }); - } - await this.prisma.partner.update({ where: { id: tenant.partnerId }, data: { crossYearMatchingEnabled: body.crossYearMatchingEnabled }, diff --git a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx index fab4381b..3f0fa200 100644 --- a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx +++ b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx @@ -32,9 +32,7 @@ const switchSx = { } as const; export const PartnerConfig = () => { - const { data, isLoading } = useQuery(partnerConfigQuery); - const config = data?.config; - const modifiedAt = data?.modifiedAt ?? null; + const { data: config, isLoading } = useQuery(partnerConfigQuery); const update = useUpdatePartnerConfig(); const [isEditing, setIsEditing] = useState(false); @@ -105,25 +103,15 @@ export const PartnerConfig = () => { const handleSaveConfirm = () => { setGeneralError(null); update.mutate( - { body: { crossYearMatchingEnabled: draftEnabled }, modifiedAt }, + { crossYearMatchingEnabled: draftEnabled }, { onSuccess: () => { onClose(); setIsEditing(false); }, - onError: (error: any) => { + onError: () => { onClose(); - if (error?.status === 409 || error?.statusCode === 409) { - const by = error.lastModifiedBy ?? error.data?.lastModifiedBy; - const on = error.lastModifiedOn ?? error.data?.lastModifiedOn; - setGeneralError( - `This setting was changed${by ? ` by ${by}` : ''}${ - on ? ` at ${new Date(on).toLocaleString()}` : '' - }. Please reload the page and try again.` - ); - } else { - setGeneralError('Something went wrong saving your changes. Please try again.'); - } + setGeneralError('Something went wrong saving your changes. Please try again.'); }, } ); diff --git a/app/fe/src/app/api/queries/partner-config.queries.ts b/app/fe/src/app/api/queries/partner-config.queries.ts index 25dd5144..3fa8d234 100644 --- a/app/fe/src/app/api/queries/partner-config.queries.ts +++ b/app/fe/src/app/api/queries/partner-config.queries.ts @@ -1,35 +1,19 @@ import { GetPartnerConfigDto, PutPartnerConfigDto } from '@edanalytics/models'; -import { plainToInstance } from 'class-transformer'; import { useMutation, useQueryClient } from '@tanstack/react-query'; -import { apiClient, apiClientRaw } from '../methods'; +import { apiClient, methods } from '../methods'; const QUERY_KEY = ['partner-config']; export const partnerConfigQuery = { queryKey: QUERY_KEY, - queryFn: async () => { - const res = await apiClientRaw.get('/partners/config'); - const headerValue = res.headers['x-config-modified-at']; - return { - config: plainToInstance(GetPartnerConfigDto, res.data), - modifiedAt: Array.isArray(headerValue) ? headerValue[0] : (headerValue ?? null), - } satisfies { config: GetPartnerConfigDto; modifiedAt: string | null }; - }, + queryFn: () => methods.getOne('/partners/config', GetPartnerConfigDto), }; export const useUpdatePartnerConfig = () => { const queryClient = useQueryClient(); return useMutation({ - mutationFn: async ({ - body, - modifiedAt, - }: { - body: PutPartnerConfigDto; - modifiedAt: string | null; - }) => { - return apiClient.put('/partners/config', body, { - headers: modifiedAt ? { 'x-if-config-modified-at': modifiedAt } : undefined, - }); + mutationFn: async (body: PutPartnerConfigDto) => { + return apiClient.put('/partners/config', body); }, onSuccess: () => { queryClient.invalidateQueries({ queryKey: QUERY_KEY }); From f277a3782fdae98c99c8734938a235b0b3fe3ec7 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Mon, 1 Jun 2026 15:20:12 -0500 Subject: [PATCH 14/21] consolidate GET /partners/config tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the four as-PartnerAdmin GET cases overlapped — the exact-shape check already covered the true cases of the column and creds assertions. collapse to two cases (both-true, both-false), each asserting the full { crossYearMatchingEnabled, eduCredsExist } body so the no-field-leak guard stays, plus the canConnect-called-with-partnerId check. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration/tests/partners-config.spec.ts | 34 +++++-------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts index 960a0fe6..d0dc05cd 100644 --- a/app/api/integration/tests/partners-config.spec.ts +++ b/app/api/integration/tests/partners-config.spec.ts @@ -37,46 +37,30 @@ describe('GET /partners/config', () => { canConnectSpy.mockRestore(); }); - it('reflects the cross_year_matching_enabled column', async () => { - await global.prisma.partner.update({ - where: { id: partnerA.id }, - data: { crossYearMatchingEnabled: false }, - }); - let res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); - expect(res.status).toBe(200); - expect(res.body.crossYearMatchingEnabled).toBe(false); - + // The body mirrors the partner's toggle column and canConnect, and is + // exactly { crossYearMatchingEnabled, eduCredsExist } — asserting the full + // shape guards against leaking extra partner fields. One case per boolean. + it('returns the toggle column and EDU creds state (both true)', async () => { + canConnectSpy.mockResolvedValue(true); await global.prisma.partner.update({ where: { id: partnerA.id }, data: { crossYearMatchingEnabled: true }, }); - res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); - expect(res.body.crossYearMatchingEnabled).toBe(true); - }); - - it('reflects canConnect for eduCredsExist', async () => { - canConnectSpy.mockResolvedValue(true); const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); expect(res.status).toBe(200); - expect(res.body.eduCredsExist).toBe(true); + expect(res.body).toEqual({ crossYearMatchingEnabled: true, eduCredsExist: true }); expect(canConnectSpy).toHaveBeenCalledWith(partnerA.id); }); - it('returns eduCredsExist=false when canConnect is false', async () => { + it('returns the toggle column and EDU creds state (both false)', async () => { canConnectSpy.mockResolvedValue(false); - const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); - expect(res.body.eduCredsExist).toBe(false); - }); - - it('returns exactly { crossYearMatchingEnabled, eduCredsExist } and nothing else', async () => { - canConnectSpy.mockResolvedValue(true); await global.prisma.partner.update({ where: { id: partnerA.id }, - data: { crossYearMatchingEnabled: true }, + data: { crossYearMatchingEnabled: false }, }); const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); expect(res.status).toBe(200); - expect(res.body).toEqual({ crossYearMatchingEnabled: true, eduCredsExist: true }); + expect(res.body).toEqual({ crossYearMatchingEnabled: false, eduCredsExist: false }); }); }); }); From 3d0a1b41c961b032969d37295523e72a1ed91421 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Mon, 1 Jun 2026 15:35:51 -0500 Subject: [PATCH 15/21] consolidate PUT /partners/config tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drop the subsequent-GET test (PUT-writes-row + GET-reflects-column already cover the round trip transitively), and fold the basic update test into the partner-scoping test — acting as partner X proves the write both lands on the session's partner and leaves others untouched, which subsumes the plain A-update plus { status: 'ok' }. move the X test to the end so the partner-A cases stay next to the beforeEach that sets them up. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration/tests/partners-config.spec.ts | 44 ++++++------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts index d0dc05cd..4e6ea8fc 100644 --- a/app/api/integration/tests/partners-config.spec.ts +++ b/app/api/integration/tests/partners-config.spec.ts @@ -106,28 +106,6 @@ describe('PUT /partners/config', () => { canConnectSpy.mockRestore(); }); - it('updates cross_year_matching_enabled on the partner row', async () => { - const res = await request(app.getHttpServer()) - .put(endpoint) - .set('Cookie', [adminCookieA]) - .send({ crossYearMatchingEnabled: true }); - expect(res.status).toBe(200); - expect(res.body).toEqual({ status: 'ok' }); - - const row = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id } }); - expect(row.crossYearMatchingEnabled).toBe(true); - }); - - it('subsequent GET reflects the new value', async () => { - await request(app.getHttpServer()) - .put(endpoint) - .set('Cookie', [adminCookieA]) - .send({ crossYearMatchingEnabled: true }); - - const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); - expect(res.body.crossYearMatchingEnabled).toBe(true); - }); - it('rejects enabling when EDU creds are missing', async () => { canConnectSpy.mockResolvedValue(false); const res = await request(app.getHttpServer()) @@ -157,7 +135,18 @@ describe('PUT /partners/config', () => { expect(row.crossYearMatchingEnabled).toBe(false); }); - it('only modifies the session partner', async () => { + it('rejects invalid body', async () => { + const res = await request(app.getHttpServer()) + .put(endpoint) + .set('Cookie', [adminCookieA]) + .send({ crossYearMatchingEnabled: 'nope' }); + expect(res.status).toBe(400); + }); + + // Acts as partner X (not the beforeEach's partner A) so this proves the + // write lands on the session's partner — not a fixed one — and leaves the + // other partner untouched, on top of the basic update + { status: 'ok' }. + it("updates only the session partner's row", async () => { const adminCookieX = (await authHelper.login(idpX, userX, tenantX, partnerAdminRoleX)).cookies; await global.prisma.partner.update({ where: { id: 'partner-x' }, @@ -169,6 +158,7 @@ describe('PUT /partners/config', () => { .set('Cookie', [adminCookieX]) .send({ crossYearMatchingEnabled: true }); expect(res.status).toBe(200); + expect(res.body).toEqual({ status: 'ok' }); const partnerXRow = await global.prisma.partner.findUniqueOrThrow({ where: { id: 'partner-x' }, @@ -180,13 +170,5 @@ describe('PUT /partners/config', () => { }); expect(partnerARow.crossYearMatchingEnabled).toBe(false); }); - - it('rejects invalid body', async () => { - const res = await request(app.getHttpServer()) - .put(endpoint) - .set('Cookie', [adminCookieA]) - .send({ crossYearMatchingEnabled: 'nope' }); - expect(res.status).toBe(400); - }); }); }); From d97fd71283a4c75e9d5e93108e7f3905de61cd88 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Mon, 1 Jun 2026 16:23:06 -0500 Subject: [PATCH 16/21] tidy PartnerConfig state and naming - inline the one-off switchSx onto the Switch - rename the modal disclosure to isModalOpen/onModalOpen/onModalClose so it's distinguishable from the help disclosure - make the edit buffer a draftConfig: PutPartnerConfigDto | null instead of a fake { crossYearMatchingEnabled: false } default; derive a non-null draft (falling back to the saved config) for rendering, and null draftConfig whenever leaving edit mode so the type honestly signals when an edit is in flight - drop the now-redundant sync effect (the draft fallback covers it) - comment the beforeunload guard Co-Authored-By: Claude Opus 4.8 (1M context) --- app/fe/src/app/Pages/Admin/PartnerConfig.tsx | 92 +++++++++++--------- 1 file changed, 49 insertions(+), 43 deletions(-) diff --git a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx index 3f0fa200..e9058b7e 100644 --- a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx +++ b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx @@ -13,6 +13,7 @@ import { VStack, useDisclosure, } from '@chakra-ui/react'; +import { PutPartnerConfigDto } from '@edanalytics/models'; import { useQuery } from '@tanstack/react-query'; import { useBlocker } from '@tanstack/react-router'; import { @@ -23,34 +24,29 @@ import { ConfirmChangesModal } from './ConfirmChangesModal'; import { RunwayErrorBox } from '../../components/Form/RunwayFormErrorBox'; import { IconCheckmark, IconExclamation } from '../../../assets/icons'; -const switchSx = { - '.chakra-switch__track': { - bg: 'blue.800', - _checked: { bg: 'green.300' }, - }, - '.chakra-switch__thumb': { bg: 'blue.50' }, -} as const; - export const PartnerConfig = () => { const { data: config, isLoading } = useQuery(partnerConfigQuery); const update = useUpdatePartnerConfig(); const [isEditing, setIsEditing] = useState(false); - const [draftEnabled, setDraftEnabled] = useState(false); + // null until the user starts editing (or config first loads); the saved + // config is the source of truth otherwise. + const [draftConfig, setDraftConfig] = useState(null); const [generalError, setGeneralError] = useState(null); - const { isOpen, onOpen, onClose } = useDisclosure(); + const { isOpen: isModalOpen, onOpen: onModalOpen, onClose: onModalClose } = useDisclosure(); const { isOpen: isHelpOpen, onToggle: onToggleHelp } = useDisclosure(); const [modalMode, setModalMode] = useState<'save' | 'leave'>('save'); const [pendingLeaveAction, setPendingLeaveAction] = useState(null); - useEffect(() => { - if (config && !isEditing) setDraftEnabled(config.crossYearMatchingEnabled); - }, [config?.crossYearMatchingEnabled, isEditing]); - - const hasChanges = !!config && draftEnabled !== config.crossYearMatchingEnabled; + const hasChanges = + !!config && + !!draftConfig && + draftConfig.crossYearMatchingEnabled !== config.crossYearMatchingEnabled; const shouldWarnAboutUnsavedChanges = isEditing && hasChanges && !update.isPending; const blocker = useBlocker({ condition: shouldWarnAboutUnsavedChanges }); + // Native guard for unsaved edits on tab close / refresh / external nav (the + // cases the in-app router blocker below can't intercept). useEffect(() => { if (!shouldWarnAboutUnsavedChanges) return; const handleBeforeUnload = (event: BeforeUnloadEvent) => { @@ -64,22 +60,26 @@ export const PartnerConfig = () => { useEffect(() => { if (blocker.status === 'blocked') { setModalMode('leave'); - onOpen(); + onModalOpen(); } - }, [blocker.status, onOpen]); + }, [blocker.status, onModalOpen]); if (isLoading || !config) { return loading...; } + // draftConfig is null outside of editing; fall back to the saved config so + // the edit controls always have a concrete value to render and submit. + const draft = draftConfig ?? { crossYearMatchingEnabled: config.crossYearMatchingEnabled }; + // Backend rejects enable-when-no-creds; mirror that on the FE so the // affordance disappears before the user can hit a 400. const cannotEnable = !config.eduCredsExist; - const switchDisabled = !isEditing || (cannotEnable && !draftEnabled); + const switchDisabled = !isEditing || (cannotEnable && !draft.crossYearMatchingEnabled); const startEdit = () => { setGeneralError(null); - setDraftEnabled(config.crossYearMatchingEnabled); + setDraftConfig({ crossYearMatchingEnabled: config.crossYearMatchingEnabled }); setIsEditing(true); }; @@ -87,51 +87,49 @@ export const PartnerConfig = () => { if (shouldWarnAboutUnsavedChanges) { setPendingLeaveAction('cancel'); setModalMode('leave'); - onOpen(); + onModalOpen(); return; } - setDraftEnabled(config.crossYearMatchingEnabled); + setDraftConfig(null); setIsEditing(false); }; const handleSave = () => { if (!hasChanges) return; setModalMode('save'); - onOpen(); + onModalOpen(); }; const handleSaveConfirm = () => { setGeneralError(null); - update.mutate( - { crossYearMatchingEnabled: draftEnabled }, - { - onSuccess: () => { - onClose(); - setIsEditing(false); - }, - onError: () => { - onClose(); - setGeneralError('Something went wrong saving your changes. Please try again.'); - }, - } - ); + update.mutate(draft, { + onSuccess: () => { + onModalClose(); + setDraftConfig(null); + setIsEditing(false); + }, + onError: () => { + onModalClose(); + setGeneralError('Something went wrong saving your changes. Please try again.'); + }, + }); }; const handleLeaveConfirm = () => { - onClose(); + onModalClose(); if (blocker.status === 'blocked') { blocker.proceed(); return; } if (pendingLeaveAction === 'cancel') { setPendingLeaveAction(null); - setDraftEnabled(config.crossYearMatchingEnabled); + setDraftConfig(null); setIsEditing(false); } }; const handleModalClose = () => { - onClose(); + onModalClose(); if (blocker.status === 'blocked') blocker.reset(); setPendingLeaveAction(null); }; @@ -139,7 +137,7 @@ export const PartnerConfig = () => { const changes = hasChanges ? [ `source roster from EDU: ${config.crossYearMatchingEnabled ? 'yes' : 'no'} → ${ - draftEnabled ? 'yes' : 'no' + draft.crossYearMatchingEnabled ? 'yes' : 'no' }`, ] : []; @@ -181,11 +179,19 @@ export const PartnerConfig = () => { {isEditing ? ( setDraftEnabled(e.target.checked)} + onChange={(e) => + setDraftConfig({ ...draft, crossYearMatchingEnabled: e.target.checked }) + } /> ) : ( { )} Date: Tue, 2 Jun 2026 10:51:43 -0500 Subject: [PATCH 17/21] clarify partners-config test mocks and scoping assertion Address review nits on the integration tests: - Drop the dead canConnect mock default in the GET beforeEach; every test in that block already sets the value it exercises, so the baseline only read as a misleading default. - Remove the verbose header comment on "updates only the session partner's row" (the name and assertions speak for themselves) and leave a one-line note on the load-bearing partner-A untouched assertion instead. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/api/integration/tests/partners-config.spec.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts index 4e6ea8fc..dbbd1291 100644 --- a/app/api/integration/tests/partners-config.spec.ts +++ b/app/api/integration/tests/partners-config.spec.ts @@ -28,9 +28,8 @@ describe('GET /partners/config', () => { beforeEach(async () => { adminCookieA = (await authHelper.login(idpA, userA, tenantA, partnerAdminRoleA)).cookies; - canConnectSpy = jest - .spyOn(app.get(EduSnowflakePoolService), 'canConnect') - .mockResolvedValue(false); + // Each test sets the canConnect result it exercises. + canConnectSpy = jest.spyOn(app.get(EduSnowflakePoolService), 'canConnect'); }); afterEach(() => { @@ -143,9 +142,6 @@ describe('PUT /partners/config', () => { expect(res.status).toBe(400); }); - // Acts as partner X (not the beforeEach's partner A) so this proves the - // write lands on the session's partner — not a fixed one — and leaves the - // other partner untouched, on top of the basic update + { status: 'ok' }. it("updates only the session partner's row", async () => { const adminCookieX = (await authHelper.login(idpX, userX, tenantX, partnerAdminRoleX)).cookies; await global.prisma.partner.update({ @@ -165,6 +161,8 @@ describe('PUT /partners/config', () => { }); expect(partnerXRow.crossYearMatchingEnabled).toBe(true); + // Load-bearing: partner A must be left untouched, proving the update is + // scoped to the session's partnerId rather than affecting other partners. const partnerARow = await global.prisma.partner.findUniqueOrThrow({ where: { id: partnerA.id }, }); From b49d93b9927e119f0bead237556077f31f65d97a Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Tue, 2 Jun 2026 11:03:04 -0500 Subject: [PATCH 18/21] split confirm-changes modal into purpose-named dialog plus shell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The single ConfirmChangesModal was being puppeted into two unrelated dialogs via a modalMode flag — "confirm these changes" and "you have unsaved changes, leave?" — with every prop ternaried on the mode and the state machinery duplicated across both Admin forms. Extract a generic ConfirmModal shell. ConfirmChangesModal becomes a thin wrapper over it that owns the change-list rendering; the unsaved-changes dialog just uses ConfirmModal directly with literal copy (not worth its own wrapper — it only presets two strings). Both forms now drive two separate useDisclosure instances, dropping modalMode and all the JSX ternaries. Behavior is unchanged. Addresses Bjorn's review note on the wonky dual-purpose modal. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app/Pages/Admin/ConfirmChangesModal.tsx | 92 +++++-------------- app/fe/src/app/Pages/Admin/ConfirmModal.tsx | 72 +++++++++++++++ app/fe/src/app/Pages/Admin/PartnerConfig.tsx | 49 +++++----- .../Pages/Admin/SchoolYearConfigEditForm.tsx | 49 +++++----- 4 files changed, 144 insertions(+), 118 deletions(-) create mode 100644 app/fe/src/app/Pages/Admin/ConfirmModal.tsx diff --git a/app/fe/src/app/Pages/Admin/ConfirmChangesModal.tsx b/app/fe/src/app/Pages/Admin/ConfirmChangesModal.tsx index 26ee6ddc..da3146c2 100644 --- a/app/fe/src/app/Pages/Admin/ConfirmChangesModal.tsx +++ b/app/fe/src/app/Pages/Admin/ConfirmChangesModal.tsx @@ -1,75 +1,31 @@ -import { - Box, - Button, - HStack, - List, - ListItem, - Modal, - ModalBody, - ModalCloseButton, - ModalContent, - ModalFooter, - ModalHeader, - ModalOverlay, -} from '@chakra-ui/react'; +import { List, ListItem } from '@chakra-ui/react'; +import { ConfirmModal } from './ConfirmModal'; interface Props { isOpen: boolean; onClose: () => void; onConfirm: () => void; - title?: string; - description?: string; - confirmLabel?: string; - changes?: string[]; + changes: string[]; } -export const ConfirmChangesModal = ({ - isOpen, - onClose, - onConfirm, - title = 'confirm changes', - description = 'The following changes will be saved:', - confirmLabel = 'confirm', - changes = [], -}: Props) => { - return ( - - - - {title} - - - 0 ? '200' : '0'}> - {description} - - {changes.length > 0 && ( - - {changes.map((change, i) => ( - - {change} - - ))} - - )} - - - - - - - - - - ); -}; +/** Confirms a set of pending edits before saving them. */ +export const ConfirmChangesModal = ({ isOpen, onClose, onConfirm, changes }: Props) => ( + + {changes.length > 0 && ( + + {changes.map((change, i) => ( + + {change} + + ))} + + )} + +); diff --git a/app/fe/src/app/Pages/Admin/ConfirmModal.tsx b/app/fe/src/app/Pages/Admin/ConfirmModal.tsx new file mode 100644 index 00000000..7d30df50 --- /dev/null +++ b/app/fe/src/app/Pages/Admin/ConfirmModal.tsx @@ -0,0 +1,72 @@ +import { ReactNode } from 'react'; +import { + Box, + Button, + HStack, + Modal, + ModalBody, + ModalCloseButton, + ModalContent, + ModalFooter, + ModalHeader, + ModalOverlay, +} from '@chakra-ui/react'; + +interface Props { + isOpen: boolean; + onClose: () => void; + onConfirm: () => void; + title: string; + description: string; + confirmLabel: string; + // Optional extra body content rendered below the description (e.g. a list of + // pending changes). + children?: ReactNode; +} + +/** + * Generic confirm/cancel dialog shell. Not used directly — see the intent-named + * wrappers (ConfirmChangesModal, ConfirmLeaveModal) that own the copy. + */ +export const ConfirmModal = ({ + isOpen, + onClose, + onConfirm, + title, + description, + confirmLabel, + children, +}: Props) => { + return ( + + + + {title} + + + + {description} + + {children} + + + + + + + + + + ); +}; diff --git a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx index e9058b7e..1d22b08d 100644 --- a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx +++ b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx @@ -21,6 +21,7 @@ import { useUpdatePartnerConfig, } from '../../api/queries/partner-config.queries'; import { ConfirmChangesModal } from './ConfirmChangesModal'; +import { ConfirmModal } from './ConfirmModal'; import { RunwayErrorBox } from '../../components/Form/RunwayFormErrorBox'; import { IconCheckmark, IconExclamation } from '../../../assets/icons'; @@ -33,9 +34,9 @@ export const PartnerConfig = () => { // config is the source of truth otherwise. const [draftConfig, setDraftConfig] = useState(null); const [generalError, setGeneralError] = useState(null); - const { isOpen: isModalOpen, onOpen: onModalOpen, onClose: onModalClose } = useDisclosure(); + const { isOpen: isSaveOpen, onOpen: onSaveOpen, onClose: onSaveClose } = useDisclosure(); + const { isOpen: isLeaveOpen, onOpen: onLeaveOpen, onClose: onLeaveClose } = useDisclosure(); const { isOpen: isHelpOpen, onToggle: onToggleHelp } = useDisclosure(); - const [modalMode, setModalMode] = useState<'save' | 'leave'>('save'); const [pendingLeaveAction, setPendingLeaveAction] = useState(null); const hasChanges = @@ -59,10 +60,9 @@ export const PartnerConfig = () => { useEffect(() => { if (blocker.status === 'blocked') { - setModalMode('leave'); - onModalOpen(); + onLeaveOpen(); } - }, [blocker.status, onModalOpen]); + }, [blocker.status, onLeaveOpen]); if (isLoading || !config) { return loading...; @@ -86,8 +86,7 @@ export const PartnerConfig = () => { const handleCancel = () => { if (shouldWarnAboutUnsavedChanges) { setPendingLeaveAction('cancel'); - setModalMode('leave'); - onModalOpen(); + onLeaveOpen(); return; } setDraftConfig(null); @@ -96,27 +95,26 @@ export const PartnerConfig = () => { const handleSave = () => { if (!hasChanges) return; - setModalMode('save'); - onModalOpen(); + onSaveOpen(); }; const handleSaveConfirm = () => { setGeneralError(null); update.mutate(draft, { onSuccess: () => { - onModalClose(); + onSaveClose(); setDraftConfig(null); setIsEditing(false); }, onError: () => { - onModalClose(); + onSaveClose(); setGeneralError('Something went wrong saving your changes. Please try again.'); }, }); }; const handleLeaveConfirm = () => { - onModalClose(); + onLeaveClose(); if (blocker.status === 'blocked') { blocker.proceed(); return; @@ -128,8 +126,8 @@ export const PartnerConfig = () => { } }; - const handleModalClose = () => { - onModalClose(); + const handleLeaveCancel = () => { + onLeaveClose(); if (blocker.status === 'blocked') blocker.reset(); setPendingLeaveAction(null); }; @@ -312,17 +310,18 @@ export const PartnerConfig = () => { )} + ); diff --git a/app/fe/src/app/Pages/Admin/SchoolYearConfigEditForm.tsx b/app/fe/src/app/Pages/Admin/SchoolYearConfigEditForm.tsx index 238e51f4..94a6900f 100644 --- a/app/fe/src/app/Pages/Admin/SchoolYearConfigEditForm.tsx +++ b/app/fe/src/app/Pages/Admin/SchoolYearConfigEditForm.tsx @@ -19,6 +19,7 @@ import { useUpdateSchoolYearConfig } from '../../api/queries/school-year-config. import { useBlocker } from '@tanstack/react-router'; import { RunwayErrorBox } from '../../components/Form/RunwayFormErrorBox'; import { ConfirmChangesModal } from './ConfirmChangesModal'; +import { ConfirmModal } from './ConfirmModal'; const switchSx = { '.chakra-switch__track': { @@ -62,8 +63,8 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, ); const [staleError, setStaleError] = useState<{ lastModifiedOn: string; lastModifiedBy: string | null } | null>(null); const [generalError, setGeneralError] = useState(null); - const { isOpen, onOpen, onClose } = useDisclosure(); - const [modalMode, setModalMode] = useState<'save' | 'leave'>('save'); + const { isOpen: isSaveOpen, onOpen: onSaveOpen, onClose: onSaveClose } = useDisclosure(); + const { isOpen: isLeaveOpen, onOpen: onLeaveOpen, onClose: onLeaveClose } = useDisclosure(); const [pendingLeaveAction, setPendingLeaveAction] = useState(null); const mutation = useUpdateSchoolYearConfig(); @@ -97,10 +98,9 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, useEffect(() => { if (blocker.status === 'blocked') { - setModalMode('leave'); - onOpen(); + onLeaveOpen(); } - }, [blocker.status, onOpen]); + }, [blocker.status, onLeaveOpen]); const updateRow = (schoolYearId: string, field: 'isEnabled' | 'sendToOds', value: boolean) => { setRows((prev) => @@ -110,15 +110,13 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, const handleSave = () => { if (!hasChanges) return; - setModalMode('save'); - onOpen(); + onSaveOpen(); }; const handleCancel = () => { if (shouldWarnAboutUnsavedChanges) { setPendingLeaveAction('cancel'); - setModalMode('leave'); - onOpen(); + onLeaveOpen(); return; } onCancel(); @@ -136,11 +134,11 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, }, { onSuccess: () => { - onClose(); + onSaveClose(); onSaved(); }, onError: (error: any) => { - onClose(); + onSaveClose(); if (error?.status === 409 || error?.statusCode === 409) { setStaleError({ lastModifiedOn: error.lastModifiedOn ?? error.data?.lastModifiedOn, @@ -155,7 +153,7 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, }; const handleLeaveConfirm = () => { - onClose(); + onLeaveClose(); if (blocker.status === 'blocked') { blocker.proceed(); return; @@ -166,8 +164,8 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, } }; - const handleModalClose = () => { - onClose(); + const handleLeaveCancel = () => { + onLeaveClose(); if (blocker.status === 'blocked') { blocker.reset(); } @@ -252,17 +250,18 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, + ); From 372ac28d5ea136308012c430803a3c1fbd93ff53 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Tue, 2 Jun 2026 11:22:40 -0500 Subject: [PATCH 19/21] match confirm-modal change label to the toggle label The PartnerConfig confirm dialog described the setting as "source roster from EDU" while the form labels it "Cross-year roster for ID matching". Use the same wording so the confirmation reads consistently. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/fe/src/app/Pages/Admin/PartnerConfig.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx index 1d22b08d..a0a1016a 100644 --- a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx +++ b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx @@ -134,7 +134,7 @@ export const PartnerConfig = () => { const changes = hasChanges ? [ - `source roster from EDU: ${config.crossYearMatchingEnabled ? 'yes' : 'no'} → ${ + `Cross-year roster for ID matching: ${config.crossYearMatchingEnabled ? 'yes' : 'no'} → ${ draft.crossYearMatchingEnabled ? 'yes' : 'no' }`, ] From d530687955b595cce68ea81640b854ea007d079a Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Tue, 2 Jun 2026 11:32:24 -0500 Subject: [PATCH 20/21] distinguish discarding edits from leaving the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cancel button reused the "unsaved changes — leave without saving?" dialog, which was confusing: cancelling stays on the page and discards edits, it doesn't navigate away. Give it its own "discard changes" dialog and reserve the leave dialog for the navigation blocker. Each exit now has its own useDisclosure and confirm handler, so the pendingLeaveAction discriminator and the branching in handleLeaveConfirm are gone. Applied to both Admin edit forms. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/fe/src/app/Pages/Admin/PartnerConfig.tsx | 39 ++++++++++++------- .../Pages/Admin/SchoolYearConfigEditForm.tsx | 34 +++++++++------- 2 files changed, 43 insertions(+), 30 deletions(-) diff --git a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx index a0a1016a..6aaad84d 100644 --- a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx +++ b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx @@ -35,9 +35,11 @@ export const PartnerConfig = () => { const [draftConfig, setDraftConfig] = useState(null); const [generalError, setGeneralError] = useState(null); const { isOpen: isSaveOpen, onOpen: onSaveOpen, onClose: onSaveClose } = useDisclosure(); + // Two distinct exits: discarding edits (cancel button, stay on the page) vs. + // navigating away with unsaved changes (router blocker / tab close). + const { isOpen: isDiscardOpen, onOpen: onDiscardOpen, onClose: onDiscardClose } = useDisclosure(); const { isOpen: isLeaveOpen, onOpen: onLeaveOpen, onClose: onLeaveClose } = useDisclosure(); const { isOpen: isHelpOpen, onToggle: onToggleHelp } = useDisclosure(); - const [pendingLeaveAction, setPendingLeaveAction] = useState(null); const hasChanges = !!config && @@ -83,14 +85,22 @@ export const PartnerConfig = () => { setIsEditing(true); }; + const discardEdits = () => { + setDraftConfig(null); + setIsEditing(false); + }; + const handleCancel = () => { if (shouldWarnAboutUnsavedChanges) { - setPendingLeaveAction('cancel'); - onLeaveOpen(); + onDiscardOpen(); return; } - setDraftConfig(null); - setIsEditing(false); + discardEdits(); + }; + + const handleDiscardConfirm = () => { + onDiscardClose(); + discardEdits(); }; const handleSave = () => { @@ -115,21 +125,12 @@ export const PartnerConfig = () => { const handleLeaveConfirm = () => { onLeaveClose(); - if (blocker.status === 'blocked') { - blocker.proceed(); - return; - } - if (pendingLeaveAction === 'cancel') { - setPendingLeaveAction(null); - setDraftConfig(null); - setIsEditing(false); - } + if (blocker.status === 'blocked') blocker.proceed(); }; const handleLeaveCancel = () => { onLeaveClose(); if (blocker.status === 'blocked') blocker.reset(); - setPendingLeaveAction(null); }; const changes = hasChanges @@ -315,6 +316,14 @@ export const PartnerConfig = () => { onConfirm={handleSaveConfirm} changes={changes} /> + (null); const [generalError, setGeneralError] = useState(null); const { isOpen: isSaveOpen, onOpen: onSaveOpen, onClose: onSaveClose } = useDisclosure(); + // Two distinct exits: discarding edits (cancel button, stay on the page) vs. + // navigating away with unsaved changes (router blocker / tab close). + const { isOpen: isDiscardOpen, onOpen: onDiscardOpen, onClose: onDiscardClose } = useDisclosure(); const { isOpen: isLeaveOpen, onOpen: onLeaveOpen, onClose: onLeaveClose } = useDisclosure(); - const [pendingLeaveAction, setPendingLeaveAction] = useState(null); const mutation = useUpdateSchoolYearConfig(); const changes = describeChanges(data, rows); @@ -115,13 +117,17 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, const handleCancel = () => { if (shouldWarnAboutUnsavedChanges) { - setPendingLeaveAction('cancel'); - onLeaveOpen(); + onDiscardOpen(); return; } onCancel(); }; + const handleDiscardConfirm = () => { + onDiscardClose(); + onCancel(); + }; + const handleSaveConfirm = () => { mutation.mutate( { @@ -154,22 +160,12 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, const handleLeaveConfirm = () => { onLeaveClose(); - if (blocker.status === 'blocked') { - blocker.proceed(); - return; - } - if (pendingLeaveAction === 'cancel') { - setPendingLeaveAction(null); - onCancel(); - } + if (blocker.status === 'blocked') blocker.proceed(); }; const handleLeaveCancel = () => { onLeaveClose(); - if (blocker.status === 'blocked') { - blocker.reset(); - } - setPendingLeaveAction(null); + if (blocker.status === 'blocked') blocker.reset(); }; return ( @@ -255,6 +251,14 @@ export const SchoolYearConfigEditForm = ({ data, modifiedAt, tableSx, onCancel, onConfirm={handleSaveConfirm} changes={changes} /> + Date: Tue, 2 Jun 2026 11:42:04 -0500 Subject: [PATCH 21/21] rename eduCredsExist to canConnectToEdu The partner-config read field is sourced straight from EduSnowflakePoolService.canConnect, so name it to mirror that primitive rather than implying a separate "do creds exist" check. Renamed across the DTO, controller, FE consumer, and integration tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/api/integration/tests/partners-config.spec.ts | 6 +++--- app/api/src/partners/partners.controller.ts | 4 ++-- app/fe/src/app/Pages/Admin/PartnerConfig.tsx | 12 ++++++------ app/models/src/dtos/partner-config.dto.ts | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/api/integration/tests/partners-config.spec.ts b/app/api/integration/tests/partners-config.spec.ts index dbbd1291..ed222cf1 100644 --- a/app/api/integration/tests/partners-config.spec.ts +++ b/app/api/integration/tests/partners-config.spec.ts @@ -37,7 +37,7 @@ describe('GET /partners/config', () => { }); // The body mirrors the partner's toggle column and canConnect, and is - // exactly { crossYearMatchingEnabled, eduCredsExist } — asserting the full + // exactly { crossYearMatchingEnabled, canConnectToEdu } — asserting the full // shape guards against leaking extra partner fields. One case per boolean. it('returns the toggle column and EDU creds state (both true)', async () => { canConnectSpy.mockResolvedValue(true); @@ -47,7 +47,7 @@ describe('GET /partners/config', () => { }); const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); expect(res.status).toBe(200); - expect(res.body).toEqual({ crossYearMatchingEnabled: true, eduCredsExist: true }); + expect(res.body).toEqual({ crossYearMatchingEnabled: true, canConnectToEdu: true }); expect(canConnectSpy).toHaveBeenCalledWith(partnerA.id); }); @@ -59,7 +59,7 @@ describe('GET /partners/config', () => { }); const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [adminCookieA]); expect(res.status).toBe(200); - expect(res.body).toEqual({ crossYearMatchingEnabled: false, eduCredsExist: false }); + expect(res.body).toEqual({ crossYearMatchingEnabled: false, canConnectToEdu: false }); }); }); }); diff --git a/app/api/src/partners/partners.controller.ts b/app/api/src/partners/partners.controller.ts index ca518f71..d33b6fd9 100644 --- a/app/api/src/partners/partners.controller.ts +++ b/app/api/src/partners/partners.controller.ts @@ -26,10 +26,10 @@ export class PartnersController { where: { id: tenant.partnerId }, select: { crossYearMatchingEnabled: true }, }); - const eduCredsExist = await this.eduPool.canConnect(tenant.partnerId); + const canConnectToEdu = await this.eduPool.canConnect(tenant.partnerId); return toGetPartnerConfigDto({ crossYearMatchingEnabled: partner.crossYearMatchingEnabled, - eduCredsExist, + canConnectToEdu, }); } diff --git a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx index 6aaad84d..61069553 100644 --- a/app/fe/src/app/Pages/Admin/PartnerConfig.tsx +++ b/app/fe/src/app/Pages/Admin/PartnerConfig.tsx @@ -76,7 +76,7 @@ export const PartnerConfig = () => { // Backend rejects enable-when-no-creds; mirror that on the FE so the // affordance disappears before the user can hit a 400. - const cannotEnable = !config.eduCredsExist; + const cannotEnable = !config.canConnectToEdu; const switchDisabled = !isEditing || (cannotEnable && !draft.crossYearMatchingEnabled); const startEdit = () => { @@ -207,22 +207,22 @@ export const PartnerConfig = () => { - {config.eduCredsExist ? : } + {config.canConnectToEdu ? : } - {config.eduCredsExist ? 'EDU connected' : 'EDU not connected'} + {config.canConnectToEdu ? 'EDU connected' : 'EDU not connected'} - {!config.eduCredsExist && ( + {!config.canConnectToEdu && ( An EDU connection must be configured to enable this setting. diff --git a/app/models/src/dtos/partner-config.dto.ts b/app/models/src/dtos/partner-config.dto.ts index 115a39dc..2017f64d 100644 --- a/app/models/src/dtos/partner-config.dto.ts +++ b/app/models/src/dtos/partner-config.dto.ts @@ -7,7 +7,7 @@ export class GetPartnerConfigDto { crossYearMatchingEnabled: boolean; @Expose() - eduCredsExist: boolean; + canConnectToEdu: boolean; } export const toGetPartnerConfigDto = makeSerializer(GetPartnerConfigDto);