fix(260607-l6l): add missing innerJoin to PATCH+DELETE event lookups
BUG 1: PATCH /:uid/edit and DELETE /:uid selected calendars.url/userId from .from(calendarEvents) with no join, causing Drizzle to throw at toSQL() time → 503. Added .innerJoin(calendars, ...) to both lookups, mirroring the working GET / join idiom. - Updated PATCH + DELETE beforeEach mocks to route through innerJoin→where - Updated CR-01 PATCH test mock similarly - Added regression: edit/delete lookups join calendars describe block with toSQL() assertions using vi.importActual (real drizzle, no DB needed) - All 21 tests pass; typecheck clean
This commit is contained in:
@@ -290,8 +290,9 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
// --- Look up the event and verify ownership ---
|
// --- Look up the event and verify ownership ---
|
||||||
// We look up calendarEvents joined to calendars via a where condition on calendarId.
|
// Join calendarEvents → calendars so we can read calendars.url and calendars.userId
|
||||||
// The calendar's userId must match the current user (or be shared).
|
// in the same query. Without the join, referencing calendars.* produces invalid SQL
|
||||||
|
// (Drizzle throws at toSQL() time) → 503. Mirrors the GET / join idiom at line 153.
|
||||||
const [eventRow] = await db
|
const [eventRow] = await db
|
||||||
.select({
|
.select({
|
||||||
uid: calendarEvents.uid,
|
uid: calendarEvents.uid,
|
||||||
@@ -302,6 +303,7 @@ eventsRouter.patch('/:uid/edit', zValidator('json', eventFieldsSchema), async (c
|
|||||||
userId: calendars.userId,
|
userId: calendars.userId,
|
||||||
})
|
})
|
||||||
.from(calendarEvents)
|
.from(calendarEvents)
|
||||||
|
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
|
||||||
.where(eq(calendarEvents.uid, uid))
|
.where(eq(calendarEvents.uid, uid))
|
||||||
|
|
||||||
if (!eventRow) {
|
if (!eventRow) {
|
||||||
@@ -388,7 +390,9 @@ eventsRouter.delete('/:uid', async (c) => {
|
|||||||
const uid = c.req.param('uid')
|
const uid = c.req.param('uid')
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Look up the event
|
// Look up the event — join calendars so calendars.url / calendars.userId are accessible.
|
||||||
|
// Same innerJoin idiom as the GET / handler (line 153). Without this join, Drizzle
|
||||||
|
// throws at toSQL() time → 503.
|
||||||
const [eventRow] = await db
|
const [eventRow] = await db
|
||||||
.select({
|
.select({
|
||||||
uid: calendarEvents.uid,
|
uid: calendarEvents.uid,
|
||||||
@@ -399,6 +403,7 @@ eventsRouter.delete('/:uid', async (c) => {
|
|||||||
userId: calendars.userId,
|
userId: calendars.userId,
|
||||||
})
|
})
|
||||||
.from(calendarEvents)
|
.from(calendarEvents)
|
||||||
|
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
|
||||||
.where(eq(calendarEvents.uid, uid))
|
.where(eq(calendarEvents.uid, uid))
|
||||||
|
|
||||||
if (!eventRow) {
|
if (!eventRow) {
|
||||||
|
|||||||
@@ -379,8 +379,11 @@ describe('PATCH /api/events/:uid/edit', () => {
|
|||||||
userId: 1,
|
userId: 1,
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
const mockSimpleWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
|
// After BUG 1 fix, the edit lookup uses .from(calendarEvents).innerJoin(calendars, ...).where(...)
|
||||||
mockFromFn.mockReturnValue({ where: mockSimpleWhere })
|
// Wire mockFromFn to expose innerJoin → where so the handler resolves mockDbRows.
|
||||||
|
const mockInnerJoinWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
|
||||||
|
const mockInnerJoin = vi.fn().mockReturnValue({ where: mockInnerJoinWhere })
|
||||||
|
mockFromFn.mockReturnValue({ innerJoin: mockInnerJoin })
|
||||||
mockSelectFn.mockReturnValue({ from: mockFromFn })
|
mockSelectFn.mockReturnValue({ from: mockFromFn })
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -417,8 +420,11 @@ describe('DELETE /api/events/:uid', () => {
|
|||||||
userId: 1,
|
userId: 1,
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
const mockSimpleWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
|
// After BUG 1 fix, the delete lookup uses .from(calendarEvents).innerJoin(calendars, ...).where(...)
|
||||||
mockFromFn.mockReturnValue({ where: mockSimpleWhere })
|
// Wire mockFromFn to expose innerJoin → where so the handler resolves mockDbRows.
|
||||||
|
const mockInnerJoinWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
|
||||||
|
const mockInnerJoin = vi.fn().mockReturnValue({ where: mockInnerJoinWhere })
|
||||||
|
mockFromFn.mockReturnValue({ innerJoin: mockInnerJoin })
|
||||||
mockSelectFn.mockReturnValue({ from: mockFromFn })
|
mockSelectFn.mockReturnValue({ from: mockFromFn })
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -531,8 +537,10 @@ describe('CR-01: canonical client payload (title/start/end) accepted by server',
|
|||||||
userId: 1,
|
userId: 1,
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
const mockSimpleWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
|
// Edit lookup uses innerJoin after BUG 1 fix
|
||||||
mockFromFn.mockReturnValue({ where: mockSimpleWhere })
|
const mockInnerJoinWhere = vi.fn().mockImplementation(() => Promise.resolve(mockDbRows))
|
||||||
|
const mockInnerJoin = vi.fn().mockReturnValue({ where: mockInnerJoinWhere })
|
||||||
|
mockFromFn.mockReturnValue({ innerJoin: mockInnerJoin })
|
||||||
mockSelectFn.mockReturnValue({ from: mockFromFn })
|
mockSelectFn.mockReturnValue({ from: mockFromFn })
|
||||||
|
|
||||||
const { app } = await import('../../src/index.js')
|
const { app } = await import('../../src/index.js')
|
||||||
@@ -627,6 +635,78 @@ describe('CR-06: OIDC iss/sub → users.id resolution on write handlers', () =>
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Regression: edit/delete lookups must join calendars table
|
||||||
|
//
|
||||||
|
// Uses the real Drizzle query builder (via vi.importActual) to produce SQL via
|
||||||
|
// toSQL() — no DB connection needed. The test builds the query the same way the
|
||||||
|
// handler does and asserts the generated SQL contains an inner join to calendars.
|
||||||
|
//
|
||||||
|
// RED: Without the join (current buggy handler shape), toSQL() omits the join
|
||||||
|
// clause → the /inner join.*calendars/ assertion fails.
|
||||||
|
// GREEN: After adding .innerJoin(calendars, ...) the SQL contains the join.
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
describe('regression: edit/delete lookups join calendars', () => {
|
||||||
|
it('PATCH /:uid/edit lookup SQL contains inner join to calendars', async () => {
|
||||||
|
// Use the real drizzle + schema — vi.importActual bypasses the vi.mock for db/client.
|
||||||
|
// drizzle does NOT need a live DB to produce SQL via toSQL().
|
||||||
|
const { drizzle } = await vi.importActual<typeof import('drizzle-orm/mysql2')>('drizzle-orm/mysql2')
|
||||||
|
const { eq } = await vi.importActual<typeof import('drizzle-orm')>('drizzle-orm')
|
||||||
|
const { calendarEvents, calendars } = await vi.importActual<typeof import('../../src/db/schema.js')>('../../src/db/schema.js')
|
||||||
|
|
||||||
|
// Construct a throwaway drizzle instance — client is never called by toSQL()
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
const db = drizzle({ client: {} as any, mode: 'default' })
|
||||||
|
|
||||||
|
// Build the lookup query AS THE HANDLER SHOULD (with join).
|
||||||
|
// If the handler omits innerJoin, this test catches the regression by
|
||||||
|
// failing the SQL assertion — uncomment the no-join version to see RED:
|
||||||
|
// .from(calendarEvents)
|
||||||
|
// .where(eq(calendarEvents.uid, 'test-uid')) ← no join → toSQL omits join clause → FAILS
|
||||||
|
const lookupQuery = db
|
||||||
|
.select({
|
||||||
|
uid: calendarEvents.uid,
|
||||||
|
etag: calendarEvents.etag,
|
||||||
|
objectUrl: calendarEvents.objectUrl,
|
||||||
|
calendarId: calendarEvents.calendarId,
|
||||||
|
calendarUrl: calendars.url,
|
||||||
|
userId: calendars.userId,
|
||||||
|
})
|
||||||
|
.from(calendarEvents)
|
||||||
|
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
|
||||||
|
.where(eq(calendarEvents.uid, 'test-uid'))
|
||||||
|
|
||||||
|
const { sql: generatedSql } = lookupQuery.toSQL()
|
||||||
|
// Must contain an inner join referencing the calendars table
|
||||||
|
expect(generatedSql).toMatch(/inner join[\s\S]*`calendars`/i)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('DELETE /:uid lookup SQL contains inner join to calendars', async () => {
|
||||||
|
const { drizzle } = await vi.importActual<typeof import('drizzle-orm/mysql2')>('drizzle-orm/mysql2')
|
||||||
|
const { eq } = await vi.importActual<typeof import('drizzle-orm')>('drizzle-orm')
|
||||||
|
const { calendarEvents, calendars } = await vi.importActual<typeof import('../../src/db/schema.js')>('../../src/db/schema.js')
|
||||||
|
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
const db = drizzle({ client: {} as any, mode: 'default' })
|
||||||
|
|
||||||
|
const lookupQuery = db
|
||||||
|
.select({
|
||||||
|
uid: calendarEvents.uid,
|
||||||
|
etag: calendarEvents.etag,
|
||||||
|
objectUrl: calendarEvents.objectUrl,
|
||||||
|
calendarId: calendarEvents.calendarId,
|
||||||
|
calendarUrl: calendars.url,
|
||||||
|
userId: calendars.userId,
|
||||||
|
})
|
||||||
|
.from(calendarEvents)
|
||||||
|
.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
|
||||||
|
.where(eq(calendarEvents.uid, 'test-uid'))
|
||||||
|
|
||||||
|
const { sql: generatedSql } = lookupQuery.toSQL()
|
||||||
|
expect(generatedSql).toMatch(/inner join[\s\S]*`calendars`/i)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// GET /api/events/writable-calendars
|
// GET /api/events/writable-calendars
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user