Skip to content

Commit b2aca6e

Browse files
skwowetskwowet
committed
fix: resolve pr review comments
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
1 parent a57cc89 commit b2aca6e

16 files changed

Lines changed: 137 additions & 107 deletions

File tree

‎docs/adr/0007-test-factory-primitives-and-defaults.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
Shared test factories are easy to turn into hidden scenario builders: they invent data the test never declared, blur what is under assertion, and drift from the real insert shapes used in production code (see ADR-0006).
1010

11-
We need a simple, durable rule for how test data is created — what the factory does, what defaults may fill, and what the test must own — so the pattern does not divert as more factories are added.
11+
We need a simple, durable rule for how test data is created — what the factory does, what defaults may fill, and what the test must own — so the pattern does not diverge as more factories are added.
1212

1313
## Decision
1414

‎services/libs/data-access-layer/src/member-organization-affiliation/index.ts‎

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,17 @@ export async function changeMemberOrganizationAffiliationOverrides(
539539
return returnRows ? [] : undefined
540540
}
541541

542-
const rows: IMemberOrganizationAffiliationOverride[] = []
542+
type OverrideWriteRow = {
543+
id: string
544+
memberId: string
545+
memberOrganizationId: string
546+
allowAffiliation: boolean | null
547+
isPrimaryWorkExperience: boolean | null
548+
setAllowAffiliation: boolean
549+
setIsPrimaryWorkExperience: boolean
550+
}
551+
552+
const rows: OverrideWriteRow[] = []
543553

544554
for (const d of data) {
545555
if (
@@ -551,11 +561,14 @@ export async function changeMemberOrganizationAffiliationOverrides(
551561
}
552562

553563
rows.push({
554-
id: uuid(),
564+
id: d.id ?? uuid(),
555565
memberId: d.memberId,
556566
memberOrganizationId: d.memberOrganizationId,
557-
allowAffiliation: d.allowAffiliation,
558-
isPrimaryWorkExperience: d.isPrimaryWorkExperience,
567+
// undefined → null on insert; explicit null stays null
568+
allowAffiliation: d.allowAffiliation ?? null,
569+
isPrimaryWorkExperience: d.isPrimaryWorkExperience ?? null,
570+
setAllowAffiliation: d.allowAffiliation !== undefined,
571+
setIsPrimaryWorkExperience: d.isPrimaryWorkExperience !== undefined,
559572
})
560573
}
561574

@@ -567,11 +580,13 @@ export async function changeMemberOrganizationAffiliationOverrides(
567580
.map(
568581
(_, i) => `
569582
(
570-
$(id_${i}),
571-
$(memberId_${i}),
572-
$(memberOrganizationId_${i}),
573-
$(allowAffiliation_${i}),
574-
$(isPrimaryWorkExperience_${i})
583+
$(id_${i})::uuid,
584+
$(memberId_${i})::uuid,
585+
$(memberOrganizationId_${i})::uuid,
586+
$(allowAffiliation_${i})::boolean,
587+
$(isPrimaryWorkExperience_${i})::boolean,
588+
$(setAllowAffiliation_${i})::boolean,
589+
$(setIsPrimaryWorkExperience_${i})::boolean
575590
)
576591
`,
577592
)
@@ -584,24 +599,61 @@ export async function changeMemberOrganizationAffiliationOverrides(
584599
acc[`memberOrganizationId_${i}`] = row.memberOrganizationId
585600
acc[`allowAffiliation_${i}`] = row.allowAffiliation
586601
acc[`isPrimaryWorkExperience_${i}`] = row.isPrimaryWorkExperience
602+
acc[`setAllowAffiliation_${i}`] = row.setAllowAffiliation
603+
acc[`setIsPrimaryWorkExperience_${i}`] = row.setIsPrimaryWorkExperience
587604
return acc
588605
},
589606
{} as Record<string, unknown>,
590607
)
591608

609+
// CTE carries per-row "was this field provided?" flags so ON CONFLICT can
610+
// distinguish omit (keep existing) from explicit null (clear).
592611
const query = `
612+
WITH input (
613+
id,
614+
"memberId",
615+
"memberOrganizationId",
616+
"allowAffiliation",
617+
"isPrimaryWorkExperience",
618+
"setAllowAffiliation",
619+
"setIsPrimaryWorkExperience"
620+
) AS (
621+
VALUES ${valuesSql}
622+
)
593623
INSERT INTO "memberOrganizationAffiliationOverrides" (
594624
id,
595625
"memberId",
596626
"memberOrganizationId",
597627
"allowAffiliation",
598628
"isPrimaryWorkExperience"
599629
)
600-
VALUES ${valuesSql}
630+
SELECT
631+
id,
632+
"memberId",
633+
"memberOrganizationId",
634+
"allowAffiliation",
635+
"isPrimaryWorkExperience"
636+
FROM input
601637
ON CONFLICT ("memberId", "memberOrganizationId")
602638
DO UPDATE SET
603-
"allowAffiliation" = COALESCE(EXCLUDED."allowAffiliation", "memberOrganizationAffiliationOverrides"."allowAffiliation"),
604-
"isPrimaryWorkExperience" = COALESCE(EXCLUDED."isPrimaryWorkExperience", "memberOrganizationAffiliationOverrides"."isPrimaryWorkExperience")
639+
"allowAffiliation" = CASE
640+
WHEN (
641+
SELECT i."setAllowAffiliation"
642+
FROM input i
643+
WHERE i."memberId" = EXCLUDED."memberId"
644+
AND i."memberOrganizationId" = EXCLUDED."memberOrganizationId"
645+
) THEN EXCLUDED."allowAffiliation"
646+
ELSE "memberOrganizationAffiliationOverrides"."allowAffiliation"
647+
END,
648+
"isPrimaryWorkExperience" = CASE
649+
WHEN (
650+
SELECT i."setIsPrimaryWorkExperience"
651+
FROM input i
652+
WHERE i."memberId" = EXCLUDED."memberId"
653+
AND i."memberOrganizationId" = EXCLUDED."memberOrganizationId"
654+
) THEN EXCLUDED."isPrimaryWorkExperience"
655+
ELSE "memberOrganizationAffiliationOverrides"."isPrimaryWorkExperience"
656+
END
605657
${returnRows ? 'RETURNING *' : ''}
606658
`
607659

‎services/libs/data-access-layer/src/members/base.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,14 @@ export async function createMember(qx: QueryExecutor, data: MemberDbInsert): Pro
757757
const id = data.id ?? generateUUIDv1()
758758
const ts = new Date()
759759
const dbInstance = getDbInstance()
760-
const columnSet = new dbInstance.helpers.ColumnSet(MEMBER_INSERT_COLUMNS, {
760+
761+
// Omit `score` when unset so Postgres keeps DEFAULT -1 (undefined would insert NULL).
762+
let columns = MEMBER_INSERT_COLUMNS
763+
if (data.score === undefined) {
764+
columns = MEMBER_INSERT_COLUMNS.filter((column) => column !== 'score')
765+
}
766+
767+
const columnSet = new dbInstance.helpers.ColumnSet(columns, {
761768
table: {
762769
table: 'members',
763770
},

‎services/libs/data-access-layer/src/segments/index.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,10 @@ export async function insertSegments(
186186
failOnConflict = false,
187187
returnRows = false,
188188
): Promise<SegmentDbRow[] | number> {
189+
if (segments.length === 0) {
190+
return returnRows ? [] : 0
191+
}
192+
189193
const ts = new Date()
190194

191195
const query = prepareBulkInsert(

‎services/libs/test-kit/src/factories/activity-relation.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,10 @@ interface GenerateActivityRelationsInput {
2727
memberOrganizations?: MemberOrganizationDbRow[]
2828
}
2929

30-
export const withActivityRelationDefaults = (
31-
data: Partial<ActivityRelationDbInsert>[],
32-
): ActivityRelationDbInsert[] =>
33-
withDefaults<ActivityRelationDbInsert>({
34-
activityId: () => generateUUIDv1(),
35-
sourceId: () => generateUUIDv1(),
36-
})(data)
30+
export const withActivityRelationDefaults = withDefaults<ActivityRelationDbInsert>()({
31+
activityId: () => generateUUIDv1(),
32+
sourceId: () => generateUUIDv1(),
33+
})
3734

3835
/** Build N activityRelation rows from member context. */
3936
export function generateActivityRelations(

‎services/libs/test-kit/src/factories/defaults.ts‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,28 @@
1-
type DefaultValue<T> = T | (() => T)
1+
type DefaultValue<V> = V | (() => V)
22

3-
type Defaults<T> = {
4-
[K in keyof T]?: DefaultValue<T[K]>
3+
type DefaultsFor<T extends object, K extends keyof T> = {
4+
[P in K]: DefaultValue<T[P]>
55
}
66

7-
function resolveDefault<T>(value: DefaultValue<T>): T {
8-
return typeof value === 'function' ? (value as () => T)() : value
7+
/** Required keys of T stay required; keys covered by defaults become optional. */
8+
type WithDefaultsRow<T extends object, K extends keyof T> = Omit<T, K> & Partial<Pick<T, K>>
9+
10+
function resolveDefault<V>(value: DefaultValue<V>): V {
11+
if (typeof value === 'function') {
12+
return (value as () => V)()
13+
}
14+
return value as V
915
}
1016

11-
function applyDefaults<T extends object>(row: Partial<T>, defaults: Defaults<T>): T {
17+
function applyDefaults<T extends object, K extends keyof T>(
18+
row: WithDefaultsRow<T, K>,
19+
defaults: DefaultsFor<T, K>,
20+
): T {
1221
const result = { ...row } as T
1322

14-
for (const key of Object.keys(defaults) as (keyof T)[]) {
15-
const defaultValue = defaults[key]
16-
if (result[key] === undefined && defaultValue !== undefined) {
17-
result[key] = resolveDefault(defaultValue as DefaultValue<T[typeof key]>)
23+
for (const key of Object.keys(defaults) as K[]) {
24+
if (result[key] === undefined) {
25+
result[key] = resolveDefault(defaults[key]) as T[K]
1826
}
1927
}
2028

@@ -23,8 +31,14 @@ function applyDefaults<T extends object>(row: Partial<T>, defaults: Defaults<T>)
2331

2432
/**
2533
* Fills missing allowlisted fields on each row. Explicit values always win.
26-
* Vue-style: primitives as values, objects/arrays (and per-row values) as factories.
34+
* Keys provided in `defaults` become optional on input; all other required
35+
* keys of T remain required at compile time.
36+
*
37+
* Usage: `withDefaults<MemberDbInsert>()({ id: () => ..., displayName: () => ... })`
2738
*/
28-
export function withDefaults<T extends object>(defaults: Defaults<T>) {
29-
return (rows: Partial<T>[]): T[] => rows.map((row) => applyDefaults(row, defaults))
39+
export function withDefaults<T extends object>() {
40+
return <K extends keyof T>(defaults: DefaultsFor<T, K>) => {
41+
return (rows: Array<WithDefaultsRow<T, K>>): T[] =>
42+
rows.map((row) => applyDefaults(row, defaults))
43+
}
3044
}

‎services/libs/test-kit/src/factories/member-identity.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,9 @@ import type { MemberIdentityDbInsert, MemberIdentityDbRow } from '@crowd/types'
55

66
import { withDefaults } from './defaults'
77

8-
export const withMemberIdentityDefaults = (
9-
data: Partial<MemberIdentityDbInsert>[],
10-
): MemberIdentityDbInsert[] =>
11-
withDefaults<MemberIdentityDbInsert>({
12-
id: () => generateUUIDv1(),
13-
verified: true,
14-
})(data)
8+
export const withMemberIdentityDefaults = withDefaults<MemberIdentityDbInsert>()({
9+
id: () => generateUUIDv1(),
10+
})
1511

1612
export async function createMemberIdentities(
1713
qx: QueryExecutor,

‎services/libs/test-kit/src/factories/member-organization-affiliation-override.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export async function createMemberOrganizationAffiliationOverrides(
1616
return changeMemberOrganizationAffiliationOverrides(
1717
qx,
1818
data.map((row) => ({
19+
id: row.id,
1920
memberId: row.memberId,
2021
memberOrganizationId: row.memberOrganizationId,
2122
allowAffiliation: row.allowAffiliation,

‎services/libs/test-kit/src/factories/member-organization.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,7 @@
1-
import { generateUUIDv1 } from '@crowd/common'
21
import { insertMemberOrganizations } from '@crowd/data-access-layer'
32
import type { QueryExecutor } from '@crowd/database'
43
import type { MemberOrganizationDbInsert, MemberOrganizationDbRow } from '@crowd/types'
54

6-
import { withDefaults } from './defaults'
7-
8-
export const withMemberOrganizationDefaults = (
9-
data: Partial<MemberOrganizationDbInsert>[],
10-
): MemberOrganizationDbInsert[] =>
11-
withDefaults<MemberOrganizationDbInsert>({
12-
id: () => generateUUIDv1(),
13-
verified: false,
14-
})(data)
15-
165
export async function createMemberOrganizations(
176
qx: QueryExecutor,
187
data: MemberOrganizationDbInsert[],

‎services/libs/test-kit/src/factories/member-segment-affiliation.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,7 @@
1-
import { generateUUIDv1 } from '@crowd/common'
21
import { insertMemberSegmentAffiliationRows } from '@crowd/data-access-layer'
32
import type { QueryExecutor } from '@crowd/database'
43
import type { MemberSegmentAffiliationDbInsert, MemberSegmentAffiliationDbRow } from '@crowd/types'
54

6-
import { withDefaults } from './defaults'
7-
8-
export const withMemberSegmentAffiliationDefaults = (
9-
data: Partial<MemberSegmentAffiliationDbInsert>[],
10-
): MemberSegmentAffiliationDbInsert[] =>
11-
withDefaults<MemberSegmentAffiliationDbInsert>({
12-
id: () => generateUUIDv1(),
13-
verified: false,
14-
})(data)
15-
165
export async function createMemberSegmentAffiliations(
176
qx: QueryExecutor,
187
data: MemberSegmentAffiliationDbInsert[],

0 commit comments

Comments
 (0)