fix(10-04): prettier format + remove unnecessary type assertions
- Run prettier on all new/modified PWA files (CredentialSheet, SetupBanner, AdminPage, admin.spec.ts) - Remove unnecessary 'as React.RefObject<HTMLElement | null>' casts flagged by @typescript-eslint/no-unnecessary-type-assertion - Format pre-existing API files from Plans 02/03 (me.ts, user.test.ts, requireAdmin.test.ts, me.test.ts) - All 270 API tests + 191 PWA vitest tests pass; lint/typecheck/build clean
This commit is contained in:
+31
-30
@@ -167,35 +167,36 @@ const meNoEchoHook = (result: { success: boolean }, c: Context) => {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
meRouter.post(
|
meRouter.post('/credential', zValidator('json', meCredentialSchema, meNoEchoHook), async (c) => {
|
||||||
'/credential',
|
// Pitfall 6: ALWAYS resolve currentUserId from the session — never from the body.
|
||||||
zValidator('json', meCredentialSchema, meNoEchoHook),
|
const currentUserId = await resolveUserId(c);
|
||||||
async (c) => {
|
if (!currentUserId) {
|
||||||
// Pitfall 6: ALWAYS resolve currentUserId from the session — never from the body.
|
return c.json({ error: 'Unauthorized' }, 401);
|
||||||
const currentUserId = await resolveUserId(c);
|
}
|
||||||
if (!currentUserId) {
|
|
||||||
return c.json({ error: 'Unauthorized' }, 401);
|
const { fastmailEmail, appPassword, providerType } = c.req.valid('json');
|
||||||
|
// T-10-10: NEVER log appPassword or c.req.valid('json') here
|
||||||
|
|
||||||
|
try {
|
||||||
|
// D-07: identical validate→encrypt→store→sync path as admin, but always with
|
||||||
|
// currentUserId (not a body userId). Admin passes the target member's userId;
|
||||||
|
// self-service passes the authenticated session userId. Same helper, same argument order.
|
||||||
|
await validateEncryptAndStoreCredential(
|
||||||
|
currentUserId,
|
||||||
|
fastmailEmail,
|
||||||
|
appPassword,
|
||||||
|
providerType,
|
||||||
|
);
|
||||||
|
} catch (err) {
|
||||||
|
if (err instanceof CredentialValidationError) {
|
||||||
|
return c.json({ error: 'Invalid request' }, 400);
|
||||||
}
|
}
|
||||||
|
console.error(
|
||||||
|
'[me/POST /credential] Unexpected error:',
|
||||||
|
err instanceof Error ? err.message : String(err),
|
||||||
|
);
|
||||||
|
return c.json({ error: 'Service unavailable' }, 503);
|
||||||
|
}
|
||||||
|
|
||||||
const { fastmailEmail, appPassword, providerType } = c.req.valid('json');
|
return c.json({ ok: true }, 200);
|
||||||
// T-10-10: NEVER log appPassword or c.req.valid('json') here
|
});
|
||||||
|
|
||||||
try {
|
|
||||||
// D-07: identical validate→encrypt→store→sync path as admin, but always with
|
|
||||||
// currentUserId (not a body userId). Admin passes the target member's userId;
|
|
||||||
// self-service passes the authenticated session userId. Same helper, same argument order.
|
|
||||||
await validateEncryptAndStoreCredential(currentUserId, fastmailEmail, appPassword, providerType);
|
|
||||||
} catch (err) {
|
|
||||||
if (err instanceof CredentialValidationError) {
|
|
||||||
return c.json({ error: 'Invalid request' }, 400);
|
|
||||||
}
|
|
||||||
console.error(
|
|
||||||
'[me/POST /credential] Unexpected error:',
|
|
||||||
err instanceof Error ? err.message : String(err),
|
|
||||||
);
|
|
||||||
return c.json({ error: 'Service unavailable' }, 503);
|
|
||||||
}
|
|
||||||
|
|
||||||
return c.json({ ok: true }, 200);
|
|
||||||
},
|
|
||||||
);
|
|
||||||
|
|||||||
@@ -314,7 +314,15 @@ describe('upsertUser', () => {
|
|||||||
}
|
}
|
||||||
// Re-fetch after insert
|
// Re-fetch after insert
|
||||||
return makeSelectChain([
|
return makeSelectChain([
|
||||||
{ id: 10, oidcIss: iss, oidcSub: sub, displayName: null, color: COLOR_PALETTE[0], isAdmin: true, createdAt: new Date() },
|
{
|
||||||
|
id: 10,
|
||||||
|
oidcIss: iss,
|
||||||
|
oidcSub: sub,
|
||||||
|
displayName: null,
|
||||||
|
color: COLOR_PALETTE[0],
|
||||||
|
isAdmin: true,
|
||||||
|
createdAt: new Date(),
|
||||||
|
},
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
mockDb.insert.mockReturnValue(makeInsertChain([{ id: 10 }]));
|
mockDb.insert.mockReturnValue(makeInsertChain([{ id: 10 }]));
|
||||||
@@ -344,7 +352,15 @@ describe('upsertUser', () => {
|
|||||||
return makeSelectChain([{ count: 1 }]);
|
return makeSelectChain([{ count: 1 }]);
|
||||||
}
|
}
|
||||||
return makeSelectChain([
|
return makeSelectChain([
|
||||||
{ id: 11, oidcIss: iss, oidcSub: sub, displayName: null, color: COLOR_PALETTE[1], isAdmin: false, createdAt: new Date() },
|
{
|
||||||
|
id: 11,
|
||||||
|
oidcIss: iss,
|
||||||
|
oidcSub: sub,
|
||||||
|
displayName: null,
|
||||||
|
color: COLOR_PALETTE[1],
|
||||||
|
isAdmin: false,
|
||||||
|
createdAt: new Date(),
|
||||||
|
},
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
mockDb.insert.mockReturnValue(makeInsertChain([{ id: 11 }]));
|
mockDb.insert.mockReturnValue(makeInsertChain([{ id: 11 }]));
|
||||||
|
|||||||
@@ -24,7 +24,13 @@ vi.mock('../../src/db/client.js', () => ({
|
|||||||
|
|
||||||
// Bring in the ContextVariableMap augmentation (sets up c.get('user') typing)
|
// Bring in the ContextVariableMap augmentation (sets up c.get('user') typing)
|
||||||
vi.mock('../../src/auth/devBypass.js', () => ({
|
vi.mock('../../src/auth/devBypass.js', () => ({
|
||||||
DEV_USER: { id: 1, oidcIss: 'dev', oidcSub: 'dev-user', displayName: 'Dev User', color: '#4A90D9' },
|
DEV_USER: {
|
||||||
|
id: 1,
|
||||||
|
oidcIss: 'dev',
|
||||||
|
oidcSub: 'dev-user',
|
||||||
|
displayName: 'Dev User',
|
||||||
|
color: '#4A90D9',
|
||||||
|
},
|
||||||
devAuthBypass: () => async (_c: unknown, next: () => Promise<void>) => next(),
|
devAuthBypass: () => async (_c: unknown, next: () => Promise<void>) => next(),
|
||||||
COLOR_PALETTE: ['#4A90D9'],
|
COLOR_PALETTE: ['#4A90D9'],
|
||||||
}));
|
}));
|
||||||
|
|||||||
@@ -167,13 +167,16 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () =
|
|||||||
let callCount = 0;
|
let callCount = 0;
|
||||||
vi.mocked(db.select).mockImplementation(() => {
|
vi.mocked(db.select).mockImplementation(() => {
|
||||||
callCount++;
|
callCount++;
|
||||||
const limitFn = callCount === 1
|
const limitFn =
|
||||||
? vi.fn().mockResolvedValue([{ isAdmin: true }]) // users.isAdmin lookup
|
callCount === 1
|
||||||
: vi.fn().mockResolvedValue([]); // memberCredentials lookup (none)
|
? vi.fn().mockResolvedValue([{ isAdmin: true }]) // users.isAdmin lookup
|
||||||
|
: vi.fn().mockResolvedValue([]); // memberCredentials lookup (none)
|
||||||
return {
|
return {
|
||||||
from: vi.fn().mockReturnValue({
|
from: vi.fn().mockReturnValue({
|
||||||
where: vi.fn().mockReturnValue({ limit: limitFn }),
|
where: vi.fn().mockReturnValue({ limit: limitFn }),
|
||||||
innerJoin: vi.fn().mockReturnValue({ innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }) }),
|
innerJoin: vi.fn().mockReturnValue({
|
||||||
|
innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }),
|
||||||
|
}),
|
||||||
}),
|
}),
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
} as any;
|
} as any;
|
||||||
@@ -183,7 +186,9 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () =
|
|||||||
const res = await app.request('/api/me');
|
const res = await app.request('/api/me');
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
|
|
||||||
const body = (await res.json()) as { user: { id: number; isAdmin: boolean; needsProviderSetup: boolean } };
|
const body = (await res.json()) as {
|
||||||
|
user: { id: number; isAdmin: boolean; needsProviderSetup: boolean };
|
||||||
|
};
|
||||||
expect(body.user).toHaveProperty('isAdmin');
|
expect(body.user).toHaveProperty('isAdmin');
|
||||||
expect(body.user.isAdmin).toBe(true); // DB returns true, not hardcoded
|
expect(body.user.isAdmin).toBe(true); // DB returns true, not hardcoded
|
||||||
});
|
});
|
||||||
@@ -194,13 +199,16 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () =
|
|||||||
let callCount = 0;
|
let callCount = 0;
|
||||||
vi.mocked(db.select).mockImplementation(() => {
|
vi.mocked(db.select).mockImplementation(() => {
|
||||||
callCount++;
|
callCount++;
|
||||||
const limitFn = callCount === 1
|
const limitFn =
|
||||||
? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup
|
callCount === 1
|
||||||
: vi.fn().mockResolvedValue([]); // no member_credentials row
|
? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup
|
||||||
|
: vi.fn().mockResolvedValue([]); // no member_credentials row
|
||||||
return {
|
return {
|
||||||
from: vi.fn().mockReturnValue({
|
from: vi.fn().mockReturnValue({
|
||||||
where: vi.fn().mockReturnValue({ limit: limitFn }),
|
where: vi.fn().mockReturnValue({ limit: limitFn }),
|
||||||
innerJoin: vi.fn().mockReturnValue({ innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }) }),
|
innerJoin: vi.fn().mockReturnValue({
|
||||||
|
innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }),
|
||||||
|
}),
|
||||||
}),
|
}),
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
} as any;
|
} as any;
|
||||||
@@ -221,13 +229,16 @@ describe('GET /api/me — isAdmin + needsProviderSetup (Plan 10-02, D-03)', () =
|
|||||||
let callCount = 0;
|
let callCount = 0;
|
||||||
vi.mocked(db.select).mockImplementation(() => {
|
vi.mocked(db.select).mockImplementation(() => {
|
||||||
callCount++;
|
callCount++;
|
||||||
const limitFn = callCount === 1
|
const limitFn =
|
||||||
? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup
|
callCount === 1
|
||||||
: vi.fn().mockResolvedValue([{ id: 7 }]); // has member_credentials row
|
? vi.fn().mockResolvedValue([{ isAdmin: false }]) // users.isAdmin lookup
|
||||||
|
: vi.fn().mockResolvedValue([{ id: 7 }]); // has member_credentials row
|
||||||
return {
|
return {
|
||||||
from: vi.fn().mockReturnValue({
|
from: vi.fn().mockReturnValue({
|
||||||
where: vi.fn().mockReturnValue({ limit: limitFn }),
|
where: vi.fn().mockReturnValue({ limit: limitFn }),
|
||||||
innerJoin: vi.fn().mockReturnValue({ innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }) }),
|
innerJoin: vi.fn().mockReturnValue({
|
||||||
|
innerJoin: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue([]) }),
|
||||||
|
}),
|
||||||
}),
|
}),
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
} as any;
|
} as any;
|
||||||
|
|||||||
@@ -100,9 +100,7 @@ test.describe('Non-admin user — admin nav entry hidden + /admin redirect', ()
|
|||||||
await page.waitForURL(/\/calendar/, { timeout: 10_000 });
|
await page.waitForURL(/\/calendar/, { timeout: 10_000 });
|
||||||
// Should have landed on /calendar
|
// Should have landed on /calendar
|
||||||
const url = new URL(page.url());
|
const url = new URL(page.url());
|
||||||
expect(url.pathname, `Expected /calendar but got ${url.pathname}`).toMatch(
|
expect(url.pathname, `Expected /calendar but got ${url.pathname}`).toMatch(/^\/(calendar)?$/);
|
||||||
/^\/(calendar)?$/,
|
|
||||||
);
|
|
||||||
// "Admin Settings" heading must NOT be present
|
// "Admin Settings" heading must NOT be present
|
||||||
await expect(page.getByRole('heading', { name: 'Admin Settings' })).toHaveCount(0);
|
await expect(page.getByRole('heading', { name: 'Admin Settings' })).toHaveCount(0);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -296,7 +296,9 @@ export function CredentialSheet({
|
|||||||
id="credential-helper"
|
id="credential-helper"
|
||||||
style={{
|
style={{
|
||||||
fontSize: 'var(--text-label-size, 13px)',
|
fontSize: 'var(--text-label-size, 13px)',
|
||||||
color: validationError ? 'var(--color-destructive, #DC2626)' : 'var(--color-text-secondary)',
|
color: validationError
|
||||||
|
? 'var(--color-destructive, #DC2626)'
|
||||||
|
: 'var(--color-text-secondary)',
|
||||||
lineHeight: 1.4,
|
lineHeight: 1.4,
|
||||||
marginBottom: 'var(--space-6, 24px)',
|
marginBottom: 'var(--space-6, 24px)',
|
||||||
display: 'flex',
|
display: 'flex',
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ import { CredentialSheet } from './CredentialSheet.js';
|
|||||||
|
|
||||||
export function SetupBanner() {
|
export function SetupBanner() {
|
||||||
const [sheetOpen, setSheetOpen] = useState(false);
|
const [sheetOpen, setSheetOpen] = useState(false);
|
||||||
|
// Use HTMLButtonElement for the ref (assignable to the CredentialSheet's HTMLElement trigger)
|
||||||
const ctaRef = useRef<HTMLButtonElement>(null);
|
const ctaRef = useRef<HTMLButtonElement>(null);
|
||||||
|
|
||||||
const meQuery = useQuery({
|
const meQuery = useQuery({
|
||||||
@@ -124,7 +125,7 @@ export function SetupBanner() {
|
|||||||
onClose={() => setSheetOpen(false)}
|
onClose={() => setSheetOpen(false)}
|
||||||
mode="self-service"
|
mode="self-service"
|
||||||
memberName={memberName}
|
memberName={memberName}
|
||||||
triggerRef={ctaRef as React.RefObject<HTMLElement | null>}
|
triggerRef={ctaRef}
|
||||||
/>
|
/>
|
||||||
</>
|
</>
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -77,8 +77,7 @@ export function AdminPage() {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// Derive current saved shared calendar id from the data
|
// Derive current saved shared calendar id from the data
|
||||||
const currentSharedId =
|
const currentSharedId = calendarsQuery.data?.calendars.find((c) => c.isShared)?.id ?? null;
|
||||||
calendarsQuery.data?.calendars.find((c) => c.isShared)?.id ?? null;
|
|
||||||
|
|
||||||
// Effective selected = user pick OR fallback to current saved
|
// Effective selected = user pick OR fallback to current saved
|
||||||
const effectiveSelected = selectedCalendarId ?? currentSharedId;
|
const effectiveSelected = selectedCalendarId ?? currentSharedId;
|
||||||
@@ -97,8 +96,7 @@ export function AdminPage() {
|
|||||||
// Open credential sheet for a member
|
// Open credential sheet for a member
|
||||||
function openSheet(member: AdminMember, buttonRef: React.RefObject<HTMLButtonElement | null>) {
|
function openSheet(member: AdminMember, buttonRef: React.RefObject<HTMLButtonElement | null>) {
|
||||||
// Capture the button so focus can return on close
|
// Capture the button so focus can return on close
|
||||||
(triggerRef as React.MutableRefObject<HTMLElement | null>).current =
|
(triggerRef as React.MutableRefObject<HTMLElement | null>).current = buttonRef.current;
|
||||||
buttonRef.current;
|
|
||||||
setSheetMember(member);
|
setSheetMember(member);
|
||||||
setSheetMode(member.hasCredential ? 'admin-rotate' : 'admin-add');
|
setSheetMode(member.hasCredential ? 'admin-rotate' : 'admin-add');
|
||||||
setSheetOpen(true);
|
setSheetOpen(true);
|
||||||
@@ -298,7 +296,7 @@ export function AdminPage() {
|
|||||||
mode={sheetMode}
|
mode={sheetMode}
|
||||||
memberName={sheetMember.displayName}
|
memberName={sheetMember.displayName}
|
||||||
memberId={sheetMember.id}
|
memberId={sheetMember.id}
|
||||||
triggerRef={triggerRef as React.RefObject<HTMLElement | null>}
|
triggerRef={triggerRef}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
Reference in New Issue
Block a user