Skip to content

Commit ef09d90

Browse files
y4ndery4nder
authored
[STAGING] FAC-134 feat: qualitative analytics view for faculty analysis page (#347) (#348)
Backend for the new theme-first qualitative analytics surface on FacultyReportScreen (dean / chairperson / campus-head views). ## New - `GET /analytics/faculty/:facultyId/qualitative-summary` — aggregates sentiment distribution + themes (with sample quotes) for the latest completed pipeline in scope. Themes ordered by descending mention count. Sample quotes are PII-scrubbed (280-char cap, two-word capitalized-name regex redaction). ## Extended - `GET /analytics/faculty/:facultyId/report/comments` now accepts optional `sentiment` and `themeId` filters plus returns per-row `sentiment` + `themeIds[]` annotations when a pipeline is resolved. Filter semantics: `is_dominant = true` only (dominance invariant). ## Pipeline scope tightening (prerequisite that landed alongside) Context: faculty-page pipeline discovery was leaking/missing pipelines across roles because listPipelines used loose matching. Addressed here so the FAC-134 UI could be verified; formalized separately as FAC-136. - ListPipelines filter now binds unspecified scope keys to null for aggregate queries (strict scope isolation), and leaves dept/program/campus unbound for faculty-level queries so the same faculty pipeline is discoverable across DEAN/CHAIRPERSON/CAMPUS_HEAD with a new assertFacultyInScope authorization check. - CreatePipelineDto + ListPipelinesQueryDto now accept `questionnaireTypeCode`; server resolves to the latest ACTIVE version at create time. ## Tests - 50 analytics.service.spec tests (24 new): GetQualitativeSummary, findLatestCompletedPipelineByScope, extended comments with sentiment /themeId filters - 64 pipeline-orchestrator.service.spec tests (7 new): scope isolation, faculty-level query semantics, cross-role visibility All passing. Lint clean.
1 parent 1fe12de commit ef09d90

10 files changed

Lines changed: 1200 additions & 39 deletions

‎src/modules/analysis/dto/create-pipeline.dto.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { z } from 'zod';
22
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
3-
import { IsNotEmpty, IsOptional, IsUUID } from 'class-validator';
3+
import { IsNotEmpty, IsOptional, IsString, IsUUID } from 'class-validator';
44

55
export const createPipelineSchema = z.object({
66
semesterId: z.string().uuid(),
77
facultyId: z.string().uuid().optional(),
88
questionnaireVersionId: z.string().uuid().optional(),
9+
questionnaireTypeCode: z.string().min(1).optional(),
910
departmentId: z.string().uuid().optional(),
1011
programId: z.string().uuid().optional(),
1112
campusId: z.string().uuid().optional(),
@@ -30,6 +31,14 @@ export class CreatePipelineDto {
3031
@IsOptional()
3132
questionnaireVersionId?: string;
3233

34+
@ApiPropertyOptional({
35+
description:
36+
'Questionnaire type code; server resolves to the latest ACTIVE version for that type when versionId is omitted',
37+
})
38+
@IsString()
39+
@IsOptional()
40+
questionnaireTypeCode?: string;
41+
3342
@ApiPropertyOptional({ description: 'Department ID' })
3443
@IsUUID()
3544
@IsOptional()

‎src/modules/analysis/dto/list-pipelines.dto.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { z } from 'zod';
22
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
3-
import { IsNotEmpty, IsOptional, IsUUID } from 'class-validator';
3+
import { IsNotEmpty, IsOptional, IsString, IsUUID } from 'class-validator';
44

55
export const listPipelinesQuerySchema = z.object({
66
semesterId: z.string().uuid(),
@@ -10,6 +10,7 @@ export const listPipelinesQuerySchema = z.object({
1010
campusId: z.string().uuid().optional(),
1111
courseId: z.string().uuid().optional(),
1212
questionnaireVersionId: z.string().uuid().optional(),
13+
questionnaireTypeCode: z.string().min(1).optional(),
1314
});
1415

1516
export type ListPipelinesQueryInput = z.infer<typeof listPipelinesQuerySchema>;
@@ -49,4 +50,12 @@ export class ListPipelinesQueryDto {
4950
@IsUUID()
5051
@IsOptional()
5152
questionnaireVersionId?: string;
53+
54+
@ApiPropertyOptional({
55+
description:
56+
'Filter pipelines whose questionnaire version belongs to this type code',
57+
})
58+
@IsString()
59+
@IsOptional()
60+
questionnaireTypeCode?: string;
5261
}

‎src/modules/analysis/services/pipeline-orchestrator.service.spec.ts‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe('PipelineOrchestratorService', () => {
4747
};
4848

4949
beforeEach(async () => {
50+
const mockExecute = jest.fn().mockResolvedValue([]);
5051
mockFork = {
5152
findOne: jest.fn(),
5253
findOneOrFail: jest.fn(),
@@ -68,6 +69,8 @@ describe('PipelineOrchestratorService', () => {
6869
populate: jest.fn().mockResolvedValue(undefined),
6970
flush: jest.fn(),
7071
nativeUpdate: jest.fn(),
72+
getConnection: jest.fn().mockReturnValue({ execute: mockExecute }),
73+
execute: mockExecute,
7174
};
7275

7376
const mockEm = {
@@ -1087,6 +1090,149 @@ describe('PipelineOrchestratorService', () => {
10871090
ForbiddenException,
10881091
);
10891092
});
1093+
1094+
it('aggregate query (no facultyId): binds every unspecified scope key to null', async () => {
1095+
setCurrentUser('admin-1', [UserRole.SUPER_ADMIN]);
1096+
mockFork.find.mockResolvedValueOnce([]);
1097+
1098+
await service.ListPipelines({ semesterId });
1099+
1100+
const filter = mockFork.find.mock.calls[0][1] as Record<
1101+
string,
1102+
unknown
1103+
>;
1104+
expect(filter).toEqual({
1105+
semester: semesterId,
1106+
faculty: null,
1107+
course: null,
1108+
questionnaireVersion: null,
1109+
department: null,
1110+
program: null,
1111+
campus: null,
1112+
});
1113+
});
1114+
1115+
it('facultyId query: binds faculty/course/qV but leaves dept/program/campus unbound', async () => {
1116+
setCurrentUser('admin-1', [UserRole.SUPER_ADMIN]);
1117+
mockFork.find.mockResolvedValueOnce([]);
1118+
1119+
await service.ListPipelines({
1120+
semesterId,
1121+
facultyId: facultyOneId,
1122+
});
1123+
1124+
const filter = mockFork.find.mock.calls[0][1] as Record<
1125+
string,
1126+
unknown
1127+
>;
1128+
expect(filter.faculty).toBe(facultyOneId);
1129+
expect(filter.course).toBeNull();
1130+
expect(filter.questionnaireVersion).toBeNull();
1131+
// dept/program/campus unbound — a faculty pipeline stays
1132+
// discoverable regardless of which scoped role triggered it.
1133+
expect(filter).not.toHaveProperty('department');
1134+
expect(filter).not.toHaveProperty('program');
1135+
expect(filter).not.toHaveProperty('campus');
1136+
});
1137+
1138+
it('resolves questionnaireTypeCode to deep filter when no versionId', async () => {
1139+
setCurrentUser('admin-1', [UserRole.SUPER_ADMIN]);
1140+
mockFork.find.mockResolvedValueOnce([]);
1141+
1142+
await service.ListPipelines({
1143+
semesterId,
1144+
facultyId: facultyOneId,
1145+
questionnaireTypeCode: 'STUDENT_EVAL',
1146+
});
1147+
1148+
const filter = mockFork.find.mock.calls[0][1] as Record<
1149+
string,
1150+
unknown
1151+
>;
1152+
expect(filter.questionnaireVersion).toEqual({
1153+
questionnaire: { type: { code: 'STUDENT_EVAL' } },
1154+
});
1155+
});
1156+
1157+
it('questionnaireVersionId wins over questionnaireTypeCode if both passed', async () => {
1158+
setCurrentUser('admin-1', [UserRole.SUPER_ADMIN]);
1159+
mockFork.find.mockResolvedValueOnce([]);
1160+
1161+
const versionId = '550e8400-e29b-41d4-a716-446655440030';
1162+
await service.ListPipelines({
1163+
semesterId,
1164+
questionnaireVersionId: versionId,
1165+
questionnaireTypeCode: 'STUDENT_EVAL',
1166+
});
1167+
1168+
const filter = mockFork.find.mock.calls[0][1] as Record<
1169+
string,
1170+
unknown
1171+
>;
1172+
expect(filter.questionnaireVersion).toBe(versionId);
1173+
});
1174+
1175+
it('DEAN on faculty page: drops dept/program/campus filters (facultyId is more-specific scope)', async () => {
1176+
setCurrentUser('dean-1', [UserRole.DEAN]);
1177+
mockScopeResolver.ResolveDepartmentIds.mockResolvedValue([deptA]);
1178+
// assertFacultyInScope looks up faculty's dept via raw SQL execute
1179+
mockFork.execute.mockResolvedValueOnce([{ department_id: deptA }]);
1180+
mockFork.find.mockResolvedValueOnce([]);
1181+
1182+
await service.ListPipelines({
1183+
semesterId,
1184+
facultyId: facultyOneId,
1185+
questionnaireTypeCode: 'STUDENT_EVAL',
1186+
});
1187+
1188+
const filter = mockFork.find.mock.calls[0][1] as Record<
1189+
string,
1190+
unknown
1191+
>;
1192+
expect(filter.faculty).toBe(facultyOneId);
1193+
// dept/program/campus unbound so the same faculty pipeline is
1194+
// discoverable across DEAN/CHAIRPERSON/CAMPUS_HEAD as long as each
1195+
// has faculty access.
1196+
expect(filter).not.toHaveProperty('department');
1197+
expect(filter).not.toHaveProperty('program');
1198+
expect(filter).not.toHaveProperty('campus');
1199+
});
1200+
1201+
it('DEAN on faculty page: 403 when faculty belongs to foreign dept', async () => {
1202+
setCurrentUser('dean-1', [UserRole.DEAN]);
1203+
mockScopeResolver.ResolveDepartmentIds.mockResolvedValue([deptA]);
1204+
mockFork.execute.mockResolvedValueOnce([{ department_id: deptB }]);
1205+
1206+
await expect(
1207+
service.ListPipelines({
1208+
semesterId,
1209+
facultyId: facultyOneId,
1210+
questionnaireTypeCode: 'STUDENT_EVAL',
1211+
}),
1212+
).rejects.toThrow(ForbiddenException);
1213+
});
1214+
1215+
it('CAMPUS_HEAD on faculty page: finds a DEAN-triggered pipeline (dept/campus unbound)', async () => {
1216+
setCurrentUser('ch-1', [UserRole.CAMPUS_HEAD]);
1217+
mockScopeResolver.ResolveDepartmentIds.mockResolvedValue([deptA]);
1218+
mockFork.execute.mockResolvedValueOnce([{ department_id: deptA }]);
1219+
mockFork.find.mockResolvedValueOnce([]);
1220+
1221+
await service.ListPipelines({
1222+
semesterId,
1223+
facultyId: facultyOneId,
1224+
questionnaireTypeCode: 'STUDENT_EVAL',
1225+
});
1226+
1227+
const filter = mockFork.find.mock.calls[0][1] as Record<
1228+
string,
1229+
unknown
1230+
>;
1231+
expect(filter.faculty).toBe(facultyOneId);
1232+
expect(filter).not.toHaveProperty('campus');
1233+
expect(filter).not.toHaveProperty('department');
1234+
expect(filter).not.toHaveProperty('program');
1235+
});
10901236
});
10911237

10921238
describe('assertCanAccessPipeline (via GetPipelineStatus / GetRecommendations)', () => {

‎src/modules/analysis/services/pipeline-orchestrator.service.ts‎

Lines changed: 124 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,21 @@ export class PipelineOrchestratorService {
123123
// exactly one assigned scope and didn't specify one explicitly.
124124
const input = await this.assertCanCreatePipeline(parsed);
125125

126+
// Resolve questionnaireTypeCode → latest ACTIVE version. Pipelines
127+
// store a concrete `questionnaireVersionId` (submissions are bound to
128+
// a version), so we commit the type code to a version at trigger time.
129+
if (!input.questionnaireVersionId && input.questionnaireTypeCode) {
130+
const resolvedVersionId = await this.resolveLatestActiveVersionId(
131+
input.questionnaireTypeCode,
132+
);
133+
if (!resolvedVersionId) {
134+
throw new BadRequestException(
135+
`No ACTIVE questionnaire version exists for type "${input.questionnaireTypeCode}"`,
136+
);
137+
}
138+
input.questionnaireVersionId = resolvedVersionId;
139+
}
140+
126141
const fork = this.em.fork();
127142

128143
// Check for active duplicate
@@ -235,28 +250,60 @@ export class PipelineOrchestratorService {
235250
const filled = await this.fillAndAssertListScope(parsed);
236251

237252
const fork = this.em.fork();
253+
// Two filter modes:
254+
// 1. Faculty-level query (facultyId present): match on {semester,
255+
// faculty, qV, course} only. Dept/program/campus are metadata
256+
// about the triggering caller, NOT match criteria — so a
257+
// DEAN-triggered pipeline stays visible to a CAMPUS_HEAD viewing
258+
// the same faculty.
259+
// 2. Aggregate query (no facultyId): bind every scope key explicitly
260+
// (null by default) so narrower-scope pipelines don't leak.
261+
// Matches CreatePipeline's `existingFilter` shape.
238262
const filter: Record<string, unknown> = {
239263
semester: filled.semesterId,
264+
course: filled.courseId ?? null,
240265
};
241-
if (filled.facultyId) filter['faculty'] = filled.facultyId;
242-
if (filled.questionnaireVersionId)
266+
267+
if (filled.questionnaireVersionId) {
243268
filter['questionnaireVersion'] = filled.questionnaireVersionId;
244-
if (filled.courseId) filter['course'] = filled.courseId;
245-
if (filled.programId) filter['program'] = filled.programId;
246-
if (filled.campusId) filter['campus'] = filled.campusId;
247-
248-
// Default-filled departmentIds come through as string[] (via $in), a
249-
// client-provided scalar departmentId still works.
250-
if (filled.departmentIdSet) {
251-
filter['department'] = { $in: filled.departmentIdSet };
252-
} else if (filled.departmentId) {
253-
filter['department'] = filled.departmentId;
254-
}
255-
if (filled.programIdSet) {
256-
filter['program'] = { $in: filled.programIdSet };
269+
} else if (filled.questionnaireTypeCode) {
270+
filter['questionnaireVersion'] = {
271+
questionnaire: { type: { code: filled.questionnaireTypeCode } },
272+
};
273+
} else {
274+
filter['questionnaireVersion'] = null;
257275
}
258-
if (filled.campusIdSet) {
259-
filter['campus'] = { $in: filled.campusIdSet };
276+
277+
if (filled.facultyId) {
278+
filter['faculty'] = filled.facultyId;
279+
// Faculty-level query: intentionally leave dept/program/campus
280+
// unbound. See comment above.
281+
} else {
282+
filter['faculty'] = null;
283+
284+
if (filled.departmentId) {
285+
filter['department'] = filled.departmentId;
286+
} else if (filled.departmentIdSet) {
287+
filter['department'] = { $in: filled.departmentIdSet };
288+
} else {
289+
filter['department'] = null;
290+
}
291+
292+
if (filled.programId) {
293+
filter['program'] = filled.programId;
294+
} else if (filled.programIdSet) {
295+
filter['program'] = { $in: filled.programIdSet };
296+
} else {
297+
filter['program'] = null;
298+
}
299+
300+
if (filled.campusId) {
301+
filter['campus'] = filled.campusId;
302+
} else if (filled.campusIdSet) {
303+
filter['campus'] = { $in: filled.campusIdSet };
304+
} else {
305+
filter['campus'] = null;
306+
}
260307
}
261308

262309
return fork.find(AnalysisPipeline, filter, {
@@ -899,6 +946,21 @@ export class PipelineOrchestratorService {
899946
// is typically also FACULTY, and the more-permissive DEAN behavior
900947
// must win.
901948

949+
// Faculty-level queries (query.facultyId set) short-circuit the set
950+
// filters for scoped roles. Rationale: a faculty-level pipeline is
951+
// "the pipeline for faculty F" regardless of which scoped role
952+
// triggered it — its department/program/campus fields are metadata
953+
// about origin, not match criteria. We still enforce authorization
954+
// via the faculty's home department ∈ caller's resolved depts.
955+
const isScopedRole =
956+
user.roles.includes(UserRole.DEAN) ||
957+
user.roles.includes(UserRole.CHAIRPERSON) ||
958+
user.roles.includes(UserRole.CAMPUS_HEAD);
959+
if (query.facultyId && isScopedRole) {
960+
await this.assertFacultyInScope(query.facultyId, query.semesterId);
961+
return { ...query };
962+
}
963+
902964
if (user.roles.includes(UserRole.DEAN)) {
903965
const allowed = await this.scopeResolver.ResolveDepartmentIds(
904966
query.semesterId,
@@ -1035,6 +1097,51 @@ export class PipelineOrchestratorService {
10351097

10361098
// --- Private Helpers ---
10371099

1100+
private async assertFacultyInScope(
1101+
facultyId: string,
1102+
semesterId: string,
1103+
): Promise<void> {
1104+
const allowedDepts =
1105+
await this.scopeResolver.ResolveDepartmentIds(semesterId);
1106+
if (allowedDepts === null) return; // unrestricted (SUPER_ADMIN-equivalent)
1107+
1108+
const fork = this.em.fork();
1109+
const rows: { department_id: string | null }[] = await fork
1110+
.getConnection()
1111+
.execute(
1112+
'SELECT u.department_id FROM "user" u WHERE u.id = ? AND u.deleted_at IS NULL',
1113+
[facultyId],
1114+
);
1115+
if (rows.length === 0) {
1116+
throw new NotFoundException('Faculty not found');
1117+
}
1118+
const facultyDeptId = rows[0].department_id;
1119+
if (!facultyDeptId || !allowedDepts.includes(facultyDeptId)) {
1120+
throw new ForbiddenException('scope not in your assigned access');
1121+
}
1122+
}
1123+
1124+
private async resolveLatestActiveVersionId(
1125+
typeCode: string,
1126+
): Promise<string | null> {
1127+
const fork = this.em.fork();
1128+
const rows: { id: string }[] = await fork.getConnection().execute(
1129+
`SELECT qv.id
1130+
FROM questionnaire_version qv
1131+
JOIN questionnaire q ON q.id = qv.questionnaire_id
1132+
JOIN questionnaire_type qt ON qt.id = q.type_id
1133+
WHERE qt.code = ?
1134+
AND qv.status = 'ACTIVE'
1135+
AND qv.deleted_at IS NULL
1136+
AND q.deleted_at IS NULL
1137+
AND qt.deleted_at IS NULL
1138+
ORDER BY qv.version_number DESC
1139+
LIMIT 1`,
1140+
[typeCode],
1141+
);
1142+
return rows[0]?.id ?? null;
1143+
}
1144+
10381145
private BuildScopeFromPipeline(pipeline: AnalysisPipeline): ScopeFilter {
10391146
const scope: ScopeFilter = { semester: pipeline.semester.id };
10401147
if (pipeline.faculty) scope.faculty = pipeline.faculty.id;

0 commit comments

Comments
 (0)