Skip to content

Commit 79a8ad3

Browse files
committed
refactor: nitpicks after self pr review
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
1 parent f1eb31b commit 79a8ad3

8 files changed

Lines changed: 39 additions & 98 deletions

File tree

backend/src/api/public/v1/members/project-affiliations/patchProjectAffiliation.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
fetchMemberSegmentAffiliationsForProject,
1111
findMaintainerRoles,
1212
findMemberById,
13-
insertMemberSegmentAffiliations,
13+
insertVerifiedMemberSegmentAffiliations,
1414
} from '@crowd/data-access-layer'
1515
import type { ISegmentAffiliationWithOrg } from '@crowd/data-access-layer'
1616
import { deleteMemberSegmentAffiliations } from '@crowd/data-access-layer/src/member_segment_affiliations'
@@ -83,7 +83,7 @@ export async function patchProjectAffiliation(req: Request, res: Response): Prom
8383
await deleteMemberSegmentAffiliations(tx, { memberId, segmentId: projectId })
8484

8585
if (affiliations.length > 0) {
86-
await insertMemberSegmentAffiliations(
86+
await insertVerifiedMemberSegmentAffiliations(
8787
tx,
8888
memberId,
8989
projectId,

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

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

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[] = []
542+
const rows: IMemberOrganizationAffiliationOverride[] = []
553543

554544
for (const d of data) {
555545
if (
@@ -564,11 +554,8 @@ export async function changeMemberOrganizationAffiliationOverrides(
564554
id: d.id ?? uuid(),
565555
memberId: d.memberId,
566556
memberOrganizationId: d.memberOrganizationId,
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,
557+
allowAffiliation: d.allowAffiliation,
558+
isPrimaryWorkExperience: d.isPrimaryWorkExperience,
572559
})
573560
}
574561

@@ -580,13 +567,11 @@ export async function changeMemberOrganizationAffiliationOverrides(
580567
.map(
581568
(_, i) => `
582569
(
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
570+
$(id_${i}),
571+
$(memberId_${i}),
572+
$(memberOrganizationId_${i}),
573+
$(allowAffiliation_${i}),
574+
$(isPrimaryWorkExperience_${i})
590575
)
591576
`,
592577
)
@@ -599,61 +584,24 @@ export async function changeMemberOrganizationAffiliationOverrides(
599584
acc[`memberOrganizationId_${i}`] = row.memberOrganizationId
600585
acc[`allowAffiliation_${i}`] = row.allowAffiliation
601586
acc[`isPrimaryWorkExperience_${i}`] = row.isPrimaryWorkExperience
602-
acc[`setAllowAffiliation_${i}`] = row.setAllowAffiliation
603-
acc[`setIsPrimaryWorkExperience_${i}`] = row.setIsPrimaryWorkExperience
604587
return acc
605588
},
606589
{} as Record<string, unknown>,
607590
)
608591

609-
// CTE carries per-row "was this field provided?" flags so ON CONFLICT can
610-
// distinguish omit (keep existing) from explicit null (clear).
611592
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-
)
623593
INSERT INTO "memberOrganizationAffiliationOverrides" (
624594
id,
625595
"memberId",
626596
"memberOrganizationId",
627597
"allowAffiliation",
628598
"isPrimaryWorkExperience"
629599
)
630-
SELECT
631-
id,
632-
"memberId",
633-
"memberOrganizationId",
634-
"allowAffiliation",
635-
"isPrimaryWorkExperience"
636-
FROM input
600+
VALUES ${valuesSql}
637601
ON CONFLICT ("memberId", "memberOrganizationId")
638602
DO UPDATE SET
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
603+
"allowAffiliation" = COALESCE(EXCLUDED."allowAffiliation", "memberOrganizationAffiliationOverrides"."allowAffiliation"),
604+
"isPrimaryWorkExperience" = COALESCE(EXCLUDED."isPrimaryWorkExperience", "memberOrganizationAffiliationOverrides"."isPrimaryWorkExperience")
657605
${returnRows ? 'RETURNING *' : ''}
658606
`
659607

services/libs/data-access-layer/src/member_segment_affiliations/index.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,19 +81,19 @@ export async function findMemberAffiliations(
8181
)
8282
}
8383

84-
export async function insertMemberSegmentAffiliationRows(
84+
export async function insertMemberSegmentAffiliations(
8585
qx: QueryExecutor,
8686
affiliations: MemberSegmentAffiliationDbInsert[],
8787
failOnConflict: boolean,
8888
returnRows: true,
8989
): Promise<MemberSegmentAffiliationDbRow[]>
90-
export async function insertMemberSegmentAffiliationRows(
90+
export async function insertMemberSegmentAffiliations(
9191
qx: QueryExecutor,
9292
affiliations: MemberSegmentAffiliationDbInsert[],
9393
failOnConflict?: boolean,
9494
returnRows?: false,
9595
): Promise<number>
96-
export async function insertMemberSegmentAffiliationRows(
96+
export async function insertMemberSegmentAffiliations(
9797
qx: QueryExecutor,
9898
affiliations: MemberSegmentAffiliationDbInsert[],
9999
failOnConflict = false,
@@ -116,14 +116,10 @@ export async function insertMemberSegmentAffiliationRows(
116116
'verifiedBy',
117117
],
118118
affiliations.map((a) => ({
119+
...a,
119120
id: a.id ?? generateUUIDv1(),
120-
memberId: a.memberId,
121-
segmentId: a.segmentId,
122-
organizationId: a.organizationId ?? null,
123-
dateStart: a.dateStart ?? null,
124-
dateEnd: a.dateEnd ?? null,
121+
// NOT NULL column — must set explicitly while listed in INSERT, else undefined → NULL
125122
verified: a.verified ?? false,
126-
verifiedBy: a.verifiedBy ?? null,
127123
})),
128124
failOnConflict ? undefined : 'DO NOTHING',
129125
returnRows,
@@ -136,10 +132,10 @@ export async function insertMemberSegmentAffiliationRows(
136132
return qx.result(query)
137133
}
138134

139-
/** @deprecated Prefer `insertMemberSegmentAffiliationRows` with full insert payloads. */
135+
/** @deprecated Prefer `insertMemberSegmentAffiliations` with full insert payloads. */
140136
// eslint-disable-next-line @typescript-eslint/no-explicit-any
141137
export async function insertMemberAffiliations(qx: QueryExecutor, memberId: string, data: any[]) {
142-
return insertMemberSegmentAffiliationRows(
138+
return insertMemberSegmentAffiliations(
143139
qx,
144140
data.map((item) => ({
145141
memberId,

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -758,18 +758,6 @@ export async function createMember(qx: QueryExecutor, data: MemberDbInsert): Pro
758758
const ts = new Date()
759759
const dbInstance = getDbInstance()
760760

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, {
768-
table: {
769-
table: 'members',
770-
},
771-
})
772-
773761
const dbData: Record<string, unknown> = {
774762
...data,
775763
id,
@@ -779,6 +767,16 @@ export async function createMember(qx: QueryExecutor, data: MemberDbInsert): Pro
779767
updatedAt: ts,
780768
}
781769

770+
// Omit unset columns so Postgres DEFAULT applies (listing a column with
771+
// undefined/null bypasses DEFAULT and inserts NULL instead).
772+
const columns = MEMBER_INSERT_COLUMNS.filter((column) => dbData[column] !== undefined)
773+
774+
const columnSet = new dbInstance.helpers.ColumnSet(columns, {
775+
table: {
776+
table: 'members',
777+
},
778+
})
779+
782780
if (Array.isArray(dbData.contributions)) {
783781
dbData.contributions = JSON.stringify(dbData.contributions)
784782
}

services/libs/data-access-layer/src/members/projectAffiliations.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,9 @@ export interface ISegmentAffiliationInsert {
129129
}
130130

131131
/**
132-
* Insert multiple segment affiliations for a member + project (segment) combination.
133-
* All inserted affiliations are marked as verified.
132+
* Insert multiple verified segment affiliations for a member + project (segment) combination.
134133
*/
135-
export async function insertMemberSegmentAffiliations(
134+
export async function insertVerifiedMemberSegmentAffiliations(
136135
qx: QueryExecutor,
137136
memberId: string,
138137
segmentId: string,

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

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

8-
export async function createMemberOrganizationAffiliationOverrides(
8+
export async function upsertMemberOrganizationAffiliationOverrides(
99
qx: QueryExecutor,
1010
data: MemberOrganizationAffiliationOverrideDbInsert[],
1111
): Promise<MemberOrganizationAffiliationOverrideDbRow[]> {
@@ -19,8 +19,8 @@ export async function createMemberOrganizationAffiliationOverrides(
1919
id: row.id,
2020
memberId: row.memberId,
2121
memberOrganizationId: row.memberOrganizationId,
22-
allowAffiliation: row.allowAffiliation,
23-
isPrimaryWorkExperience: row.isPrimaryWorkExperience,
22+
allowAffiliation: row.allowAffiliation ?? undefined,
23+
isPrimaryWorkExperience: row.isPrimaryWorkExperience ?? undefined,
2424
})),
2525
true,
2626
)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { insertMemberSegmentAffiliationRows } from '@crowd/data-access-layer'
1+
import { insertMemberSegmentAffiliations } from '@crowd/data-access-layer'
22
import type { QueryExecutor } from '@crowd/database'
33
import type { MemberSegmentAffiliationDbInsert, MemberSegmentAffiliationDbRow } from '@crowd/types'
44

@@ -10,5 +10,5 @@ export async function createMemberSegmentAffiliations(
1010
return []
1111
}
1212

13-
return insertMemberSegmentAffiliationRows(qx, data, true, true)
13+
return insertMemberSegmentAffiliations(qx, data, true, true)
1414
}

services/libs/types/src/members.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,8 @@ export interface IMemberOpensearch {
228228

229229
export interface IChangeAffiliationOverrideData {
230230
id?: string
231-
allowAffiliation?: boolean | null
232-
isPrimaryWorkExperience?: boolean | null
231+
allowAffiliation?: boolean
232+
isPrimaryWorkExperience?: boolean
233233
memberOrganizationId: string
234234
memberId: string
235235
}

0 commit comments

Comments
 (0)