Skip to content

Commit 430d262

Browse files
ntietje1ntietje1
committed
fixes
1 parent 69fc0ec commit 430d262

11 files changed

Lines changed: 93 additions & 26 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ jobs:
5151
with:
5252
annotations: true
5353
fail-on-issues: false
54-
format: json
54+
format: sarif
5555

5656
react-compiler:
5757
name: React Compiler Health

‎convex/_test/factories.helper.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export async function createUserProfile(
8585
const data = {
8686
...defaults,
8787
...rest,
88-
...(username ? { username: assertUsername(username) } : {}),
88+
...(username !== undefined ? { username: assertUsername(username) } : {}),
8989
}
9090
const id = await t.run(async (ctx) => {
9191
return await ctx.db.insert('userProfiles', data)
@@ -118,7 +118,7 @@ export async function createCampaignWithDm(
118118
...defaults,
119119
...rest,
120120
dmUserId: dmProfile._id,
121-
...(slug ? { slug: assertCampaignSlug(slug) } : {}),
121+
...(slug !== undefined ? { slug: assertCampaignSlug(slug) } : {}),
122122
}
123123
const campaignId = await t.run(async (ctx) => {
124124
return await ctx.db.insert('campaigns', campaignData)
@@ -229,7 +229,8 @@ async function insertSidebarItem(
229229
name: _name,
230230
...sidebarOverrides
231231
} = overrides ?? {}
232-
const validatedSlug = slug ? assertSidebarItemSlug(slug) : assertSidebarItemSlug(slugify(name))
232+
const validatedSlug =
233+
slug !== undefined ? assertSidebarItemSlug(slug) : assertSidebarItemSlug(slugify(name))
233234

234235
const sharedData = {
235236
...sidebarItemBase(campaignId, creatorProfileId, name),

‎convex/sidebarItems/__tests__/sidebarItemValidation.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,15 @@ import {
2121
} from '../validation/name'
2222
import {
2323
getAncestorIds,
24+
validateCreateParentTarget,
2425
validateNoCircularParent,
2526
validateNoCircularParentAsync,
2627
} from '../validation/parent'
2728
import { validateLocalSidebarMove } from '../validation/move'
2829
import { validateItemSlug } from '../validation/slug'
2930
import type { Id } from '../../_generated/dataModel'
31+
import { SIDEBAR_ITEM_TYPES } from '../types/baseTypes'
32+
import type { AnySidebarItem } from '../types/types'
3033

3134
describe('validateItemName', () => {
3235
it('accepts a valid name', () => {
@@ -376,6 +379,25 @@ describe('validateLocalSidebarMove', () => {
376379
expect(result).toEqual({ valid: true })
377380
})
378381

382+
it('rejects missing target parents before other move validation', () => {
383+
const result = validateLocalSidebarMove(
384+
{
385+
itemId: testId<'sidebarItems'>('item'),
386+
name: 'Alpha',
387+
parentId: testId<'sidebarItems'>('missing-parent'),
388+
},
389+
{
390+
getParent: () => undefined,
391+
getSiblings: () => [],
392+
},
393+
)
394+
395+
expect(result).toEqual({
396+
valid: false,
397+
error: 'Cannot move item: target parent does not exist',
398+
})
399+
})
400+
379401
it('checks sibling conflicts for regular moves', () => {
380402
const result = validateLocalSidebarMove(
381403
{
@@ -393,6 +415,33 @@ describe('validateLocalSidebarMove', () => {
393415
})
394416
})
395417

418+
describe('validateCreateParentTarget', () => {
419+
it('rejects path targets whose base parent is not a folder', () => {
420+
const noteId = testId<'sidebarItems'>('note-parent')
421+
const noteParent = {
422+
_id: noteId,
423+
type: SIDEBAR_ITEM_TYPES.notes,
424+
parentId: null,
425+
name: 'Leaf Note',
426+
} as unknown as AnySidebarItem
427+
428+
const result = validateCreateParentTarget(
429+
{
430+
kind: 'path',
431+
baseParentId: noteId,
432+
pathSegments: [],
433+
},
434+
new Map([[noteId, noteParent]]),
435+
new Map([[null, [noteParent]]]),
436+
)
437+
438+
expect(result).toEqual({
439+
valid: false,
440+
error: 'Parent is not a folder',
441+
})
442+
})
443+
})
444+
396445
describe('cross-table slug uniqueness', () => {
397446
const t = createTestContext()
398447

‎convex/sidebarItems/functions/moveSidebarItem.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ import { deduplicateName } from './defaultItemName'
1616
import { trashTree, restoreTreeDescendants } from './treeOperations'
1717
import { getSidebarItem } from './getSidebarItem'
1818
import { collectDescendants } from './collectDescendants'
19-
import { assertSidebarItemName } from '../validation/name'
2019
import type { SidebarItemLocation } from '../types/baseTypes'
2120
import type { AnySidebarItemRow } from '../types/types'
2221
import type { CampaignMutationCtx } from '../../functions'
2322
import type { Id } from '../../_generated/dataModel'
2423
import type { SidebarItemName } from '../validation/name'
24+
import type { SidebarItemSlug } from '../validation/slug'
2525

2626
const clearDeletion = { deletionTime: null, deletedBy: null }
2727

@@ -63,19 +63,19 @@ async function resyncRelativeLinksForMovedItems(
6363
async function resolveRestoreConflicts(
6464
ctx: CampaignMutationCtx,
6565
item: AnySidebarItemRow,
66-
): Promise<{ name?: SidebarItemName; slug?: string }> {
66+
): Promise<{ name?: SidebarItemName; slug?: SidebarItemSlug }> {
6767
const siblings = await getSidebarItemsByParent(ctx, {
6868
parentId: item.parentId,
6969
})
7070
const otherNames = siblings.filter((s) => s._id !== item._id).map((s) => s.name)
7171

72-
const uniqueName = assertSidebarItemName(deduplicateName(item.name, otherNames))
72+
const uniqueName = deduplicateName(item.name, otherNames) as SidebarItemName
7373
const uniqueSlug = await findUniqueSidebarItemSlug(ctx, {
7474
itemId: item._id,
7575
name: uniqueName,
7676
})
7777

78-
const patch: { name?: SidebarItemName; slug?: string } = {}
78+
const patch: { name?: SidebarItemName; slug?: SidebarItemSlug } = {}
7979
if (uniqueName !== item.name) patch.name = uniqueName
8080
if (uniqueSlug !== item.slug) patch.slug = uniqueSlug
8181
return patch

‎convex/sidebarItems/functions/treeOperations.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { SIDEBAR_ITEM_LOCATION, SIDEBAR_ITEM_TYPES } from '../types/baseTypes'
22
import { findUniqueSidebarItemSlug } from '../validation/orchestration'
3-
import { assertSidebarItemName } from '../validation/name'
43
import { collectDescendants } from './collectDescendants'
54
import { hardDeleteItem } from './hardDeleteItem'
65
import type { SidebarItemLocation } from '../types/baseTypes'
76
import type { CampaignMutationCtx } from '../../functions'
87
import type { Id } from '../../_generated/dataModel'
98
import type { MutationCtx } from '../../_generated/server'
109
import type { AnySidebarItemRow } from '../types/types'
10+
import type { SidebarItemName } from '../validation/name'
1111

1212
type ItemOperation = (ctx: MutationCtx, item: AnySidebarItemRow) => Promise<void>
1313

@@ -59,7 +59,7 @@ export async function restoreTreeDescendants(
5959
if (i._id === item._id) return
6060
if (i.location !== SIDEBAR_ITEM_LOCATION.trash) return
6161

62-
const name = assertSidebarItemName(i.name)
62+
const name = i.name as SidebarItemName
6363
const slug = await findUniqueSidebarItemSlug(ctx, {
6464
itemId: i._id,
6565
name,

‎convex/sidebarItems/validation/move.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ export function validateLocalSidebarMove(
3737
return { valid: true }
3838
}
3939

40+
if (parentId !== null && !getParent(parentId)) {
41+
return {
42+
valid: false,
43+
error: 'Cannot move item: target parent does not exist',
44+
}
45+
}
46+
4047
const parentResult = validateNoCircularParent(itemId, parentId, getParent)
4148
if (!parentResult.valid) {
4249
return {

‎convex/sidebarItems/validation/orchestration.ts‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
import { findUniqueSlug } from '../../common/slug'
22
import { CAMPAIGN_MEMBER_ROLE } from '../../campaigns/types'
33
import { ERROR_CODE, throwClientError } from '../../errors'
4-
import { hasAtLeastPermissionLevel } from '../../permissions/hasAtLeastPermissionLevel'
54
import { PERMISSION_LEVEL } from '../../permissions/types'
6-
import { getSidebarItemPermissionLevel } from '../../sidebarShares/functions/sidebarItemPermissions'
75
import type { CampaignQueryCtx } from '../../functions'
86
import type { Id } from '../../_generated/dataModel'
97
import { getSidebarItem } from '../functions/getSidebarItem'
10-
import { getSidebarItemWithContent } from '../functions/getSidebarItemWithContent'
118
import { getSidebarItemsByParent } from '../functions/getSidebarItemsByParent'
129
import { SIDEBAR_ITEM_TYPES } from '../types/baseTypes'
1310
import type { AnySidebarItem } from '../types/types'
@@ -75,10 +72,10 @@ export async function validateSidebarParentChange(
7572
if (!parentFromDb) {
7673
throwClientError(ERROR_CODE.NOT_FOUND, 'Parent not found')
7774
}
78-
if (parentFromDb && parentFromDb.type !== SIDEBAR_ITEM_TYPES.folders) {
75+
if (parentFromDb.type !== SIDEBAR_ITEM_TYPES.folders) {
7976
throwClientError(ERROR_CODE.VALIDATION_FAILED, 'Parent must be a folder')
8077
}
81-
if (parentFromDb && parentFromDb.location !== item.location) {
78+
if (parentFromDb.location !== item.location) {
8279
throwClientError(
8380
ERROR_CODE.VALIDATION_FAILED,
8481
'Cannot move items into a folder in a different location',
@@ -97,20 +94,17 @@ export async function validateSidebarCreateParent(
9794
): Promise<void> {
9895
const { membership } = ctx
9996
if (parentId) {
100-
const parentItem = await getSidebarItemWithContent(ctx, parentId)
97+
const parentItem = await getSidebarItem(ctx, parentId)
10198
if (!parentItem) {
10299
throwClientError(ERROR_CODE.NOT_FOUND, 'Parent not found')
103100
}
104101
if (parentItem.type !== SIDEBAR_ITEM_TYPES.folders) {
105102
throwClientError(ERROR_CODE.VALIDATION_FAILED, 'Parent must be a folder')
106103
}
107-
const level = await getSidebarItemPermissionLevel(ctx, { item: parentItem })
108-
if (!hasAtLeastPermissionLevel(level, PERMISSION_LEVEL.FULL_ACCESS)) {
109-
throwClientError(
110-
ERROR_CODE.PERMISSION_DENIED,
111-
'You do not have sufficient permission for this item',
112-
)
113-
}
104+
await requireItemAccess(ctx, {
105+
rawItem: parentItem,
106+
requiredLevel: PERMISSION_LEVEL.FULL_ACCESS,
107+
})
114108
} else if (membership.role !== CAMPAIGN_MEMBER_ROLE.DM) {
115109
throwClientError(ERROR_CODE.PERMISSION_DENIED, 'Only the DM can create items at the root level')
116110
}

‎convex/sidebarItems/validation/parent.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,15 @@ export function validateCreateParentTarget(
239239
if (!parentStack) {
240240
return { valid: false, error: 'Parent not found' }
241241
}
242+
if (parentTarget.baseParentId !== null) {
243+
const baseParent = itemsMap.get(parentTarget.baseParentId)
244+
if (!baseParent) {
245+
return { valid: false, error: 'Parent not found' }
246+
}
247+
if (baseParent.type !== SIDEBAR_ITEM_TYPES.folders) {
248+
return { valid: false, error: 'Parent is not a folder' }
249+
}
250+
}
242251

243252
const traversalStack: Array<ParentRef> = [...parentStack]
244253

‎src/features/campaigns/hooks/useCampaign.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type { CampaignSlug } from 'convex/campaigns/validation'
1111
import type { Username } from 'convex/users/validation'
1212
import { useAuthQuery } from '~/shared/hooks/useAuthQuery'
1313

14-
export type CampaignRouteIdentity = {
14+
type CampaignRouteIdentity = {
1515
dmUsername: Username
1616
campaignSlug: CampaignSlug
1717
}

‎src/features/settings/components/tabs/campaign-people/__tests__/people-tab.test.tsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { createCampaign, createCampaignMember } from '~/test/factories/campaign-
88
import { mockAuthQuery } from '~/test/mocks/convex-mocks'
99
import { TestWrapper } from '~/test/test-wrapper'
1010
import { useAuthQuery } from '~/shared/hooks/useAuthQuery'
11+
import { getOrigin } from '~/shared/utils/origin'
1112

1213
vi.mock('~/features/campaigns/hooks/useCampaign', () => ({
1314
useOptionalCampaign: vi.fn(),
@@ -44,10 +45,15 @@ vi.mock('~/shared/hooks/useAuthQuery', () => ({
4445
useAuthQuery: vi.fn(),
4546
}))
4647

48+
vi.mock('~/shared/utils/origin', () => ({
49+
getOrigin: vi.fn(),
50+
}))
51+
4752
describe('PeopleTab', () => {
4853
beforeEach(() => {
4954
vi.mocked(useOptionalCampaign).mockReset()
5055
vi.mocked(useAuthQuery).mockReset()
56+
vi.mocked(getOrigin).mockReset()
5157
})
5258

5359
it('renders on campaign routes without requiring CampaignProvider', () => {
@@ -106,6 +112,7 @@ describe('PeopleTab', () => {
106112
isCampaignLoaded: true,
107113
campaignId: campaign._id,
108114
})
115+
vi.mocked(getOrigin).mockReturnValue('https://example.test')
109116
const readyMembers = [
110117
createCampaignMember({
111118
campaignId: campaign._id,
@@ -127,7 +134,7 @@ describe('PeopleTab', () => {
127134
)
128135

129136
expect(screen.getByTestId('invite-link-section')).toHaveTextContent(
130-
`${import.meta.env.VITE_SITE_URL}/join/testdm/${campaign.slug}`,
137+
`https://example.test/join/testdm/${campaign.slug}`,
131138
)
132139
expect(screen.getByTestId('members-section')).toBeInTheDocument()
133140
expect(screen.getByTestId('pending-requests-section')).toBeInTheDocument()

0 commit comments

Comments
 (0)
⚡