Skip to content

Commit 9fa9221

Browse files
committed
fix: review
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
1 parent ffa6ea4 commit 9fa9221

3 files changed

Lines changed: 40 additions & 27 deletions

File tree

backend/src/middlewares/segmentMiddleware.ts

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,46 +3,58 @@ import {
33
populateSegmentRelations,
44
} from '@crowd/data-access-layer/src/segments'
55
import { getServiceChildLogger } from '@crowd/logging'
6+
import { NextFunction, Request, Response } from 'express'
67

78
import SegmentRepository from '../database/repositories/segmentRepository'
9+
import { IRepositoryOptions } from '../database/repositories/IRepositoryOptions'
810

911
const log = getServiceChildLogger('segmentMiddleware')
1012

11-
export async function segmentMiddleware(req, res, next) {
13+
export async function segmentMiddleware(req: Request, _res: Response, next: NextFunction) {
1214
try {
1315
let segments: any = null
14-
const segmentRepository = new SegmentRepository(req)
16+
const segmentRepository = new SegmentRepository(req as unknown as IRepositoryOptions)
1517

1618
// Note: req.params is NOT available here. This middleware is registered via app.use(),
1719
// which runs before Express matches a specific route and populates req.params.
1820
// Any check on req.params (e.g. req.params.segmentId) would always be undefined.
1921
// Route handlers that need a specific segment by ID (e.g. GET /segment/:segmentId)
2022
// read req.params directly and ignore req.currentSegments entirely — so the
2123
// resolution below is harmless for those endpoints.
22-
if (req.query.segments) {
23-
// GET requests — req.query values can be string or string[], normalize to array
24-
const segmentIds = ([] as string[]).concat(req.query.segments)
24+
const querySegments = toStringArray(req.query.segments)
25+
const bodySegments = toStringArray((req.body as Record<string, unknown>)?.segments)
26+
27+
if (querySegments.length > 0) {
2528
segments = {
26-
rows: await resolveToLeafSegments(segmentRepository, segmentIds, req),
29+
rows: await resolveToLeafSegments(segmentRepository, querySegments, req),
2730
}
28-
} else if (req.body.segments) {
29-
// POST/PUT requests — body.segments should always be an array, but normalize defensively
30-
const segmentIds = ([] as string[]).concat(req.body.segments)
31+
} else if (bodySegments.length > 0) {
3132
segments = {
32-
rows: await resolveToLeafSegments(segmentRepository, segmentIds, req),
33+
rows: await resolveToLeafSegments(segmentRepository, bodySegments, req),
3334
}
3435
} else {
3536
segments = await segmentRepository.querySubprojects({ limit: 1, offset: 0 })
3637
}
3738

38-
req.currentSegments = segments.rows
39+
const options = req as unknown as IRepositoryOptions
40+
options.currentSegments = segments.rows
3941

4042
next()
4143
} catch (error) {
4244
next(error)
4345
}
4446
}
4547

48+
/**
49+
* Safely extracts a string[] from an unknown query/body value.
50+
* Rejects ParsedQs objects (e.g. ?segments[key]=val) that would cause type confusion.
51+
*/
52+
function toStringArray(value: unknown): string[] {
53+
if (value === undefined || value === null) return []
54+
const items = Array.isArray(value) ? value : [value]
55+
return items.filter((v): v is string => typeof v === 'string')
56+
}
57+
4658
/**
4759
* Resolves segment IDs to their leaf sub-projects.
4860
*
@@ -56,7 +68,7 @@ export async function segmentMiddleware(req, res, next) {
5668
async function resolveToLeafSegments(
5769
segmentRepository: SegmentRepository,
5870
segmentIds: string[],
59-
req: any,
71+
req: Request,
6072
) {
6173
const fetched = await segmentRepository.findInIds(segmentIds)
6274

@@ -69,7 +81,6 @@ async function resolveToLeafSegments(
6981
}
7082

7183
if (nonLeaf.length === 0) {
72-
// All IDs are already leaf segments — current behavior, no change.
7384
log.debug(
7485
{
7586
api: `${req.method} ${req.path}`,
@@ -82,19 +93,14 @@ async function resolveToLeafSegments(
8293

8394
const leafRecords = await segmentRepository.getSegmentSubprojects(segmentIds)
8495

85-
log.warn(
96+
log.debug(
8697
{
8798
api: `${req.method} ${req.path}`,
88-
'⚠️ WITHOUT_RESOLUTION_would_have_used': {
89-
segments: nonLeaf.map((s) => ({ id: s.id, name: s.name, level: segmentLevel(s) })),
90-
count: segmentIds.length,
91-
},
92-
'✅ WITH_RESOLUTION_will_use': {
93-
segments: leafRecords.map((s: any) => ({ id: s.id, name: (s as any).name })),
94-
count: leafRecords.length,
95-
},
99+
input_segments: nonLeaf.map((s) => ({ id: s.id, name: s.name, level: segmentLevel(s) })),
100+
resolved_leaf_segments: leafRecords.map((s: any) => ({ id: s.id, name: (s as any).name })),
101+
resolved_count: leafRecords.length,
96102
},
97-
`⚠️ NON-LEAF SEGMENT DETECTED — DB queries will use resolved leaf segments instead of the received ones`,
103+
'Non-leaf segments resolved to leaf sub-projects',
98104
)
99105

100106
return leafRecords.map(populateSegmentRelations)

frontend/src/modules/activity/components/activity-timeline.vue

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -373,14 +373,21 @@ const fetchActivities = async ({ reset } = { reset: false }) => {
373373
374374
loading.value = true;
375375
376+
let querySegments: string[];
377+
if (selectedSegment.value) {
378+
querySegments = [selectedSegment.value];
379+
} else if (selectedProjectGroup.value?.id) {
380+
querySegments = [selectedProjectGroup.value.id];
381+
} else {
382+
querySegments = [];
383+
}
384+
376385
const data = await ActivityService.query({
377386
filter: filterToApply,
378387
orderBy: 'timestamp_DESC',
379388
limit: limit.value,
380389
offset: offset.value,
381-
segments: selectedSegment.value
382-
? [selectedSegment.value]
383-
: [selectedProjectGroup.value.id],
390+
segments: querySegments,
384391
});
385392
386393
loading.value = false;

frontend/src/modules/member/components/member-selection-dropdown.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ const fetchFn = async ({ query, limit }) => {
8686
const options = await MemberService.listMembersAutocomplete({
8787
query,
8888
limit,
89-
segments: [selectedProjectGroup.value.id],
89+
segments: selectedProjectGroup.value?.id ? [selectedProjectGroup.value.id] : [],
9090
});
9191
9292
// Remove primary member from members that can be merged with

0 commit comments

Comments
 (0)