Skip to content

Commit 1fe12de

Browse files
authored
[STAGING] FAC-132 feat: role-aware analysis pipeline triggering and output surfacing (#336) (#337)
Backend half of the FAC-132 integration slice — wires role + scope guards onto the analysis pipeline endpoints and exposes the list/discovery surface the frontend needs. - AnalysisController: @UseJwtGuard(DEAN, CHAIRPERSON, CAMPUS_HEAD, SUPER_ADMIN) at the class level with method-level widening to include FACULTY for GET reads; new GET /analysis/pipelines list endpoint; ParseUUIDPipe on :id params. - PipelineOrchestratorService: scope-authorization helpers (assertCanCreatePipeline, fillAndAssertListScope, assertCanAccessPipeline) gate Create/Confirm/Cancel/GetPipelineStatus/ GetRecommendations; scoped roles (DEAN/CHAIRPERSON/CAMPUS_HEAD) tried before FACULTY/STUDENT so multi-role Moodle users (DEAN+FACULTY) aren't falsely rejected; auto-fills the scope axis when the caller has exactly one assigned scope, else 400. - TD-8: partial unique index uq_analysis_pipeline_active_scope enforces one active pipeline per (semester, scope) tuple at the DB with a text-literal 'NONE' sentinel for nullable FKs. CreatePipeline wraps flush in a try/catch for UniqueConstraintViolationException and re- fetches the winner (idempotent race recovery). existingFilter now binds every non-provided scope field to null, matching the index. - TD-9: pipeline-status response 'scope' replaced with paired IDs + display values so the frontend can use IDs for cache keys and display values for UI. Create/Confirm/Cancel now also return PipelineSummary shape. - ScopeResolverService: add public ResolveCampusIds(semesterId) helper scoped to campuses hosting the given semester. - DI: AnalysisModule registers User (for RolesGuard.UserRepository) and imports DataLoaderModule (for CurrentUserInterceptor.UserLoader). - Tests: 62/62 passing — scope authorization matrix across all roles, 404-precedes-403 (AC-17), unique-index race handling, DEAN+FACULTY multi-role precedence, list-endpoint delegation. - Docs: Access Control section in analysis-pipeline.md and pipeline- scope addendum in scope-resolution.md.
1 parent a193f0f commit 1fe12de

14 files changed

Lines changed: 2214 additions & 68 deletions

_bmad-output/implementation-artifacts/tech-spec-fac-132-analysis-pipeline-interaction.md

Lines changed: 824 additions & 0 deletions
Large diffs are not rendered by default.

docs/architecture/scope-resolution.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,10 @@ When a user holds multiple roles, the highest-priority role wins:
152152
2. `DEAN` — department-level scope
153153
3. `CHAIRPERSON` — program-level scope (narrower)
154154

155+
### Pipeline-scope authorization
156+
157+
Analysis-pipeline authorization (create/confirm/cancel/read) reuses `ResolveDepartmentIds`, `ResolveProgramIds`, and `ResolveCampusIds`. Pipeline-specific rules — required axis per role, list default-fill, FACULTY auto-override, 404-before-403 ordering, and the active-scope unique index — live in [analysis-pipeline.md §Access Control](../workflows/analysis-pipeline.md#access-control). `PipelineOrchestratorService` calls these resolvers before any DB write or flush so a foreign caller cannot cause side effects.
158+
155159
## Source Files
156160

157161
| File | Purpose |

docs/workflows/analysis-pipeline.md

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,3 +154,93 @@ Returns a structured response with:
154154
- Sentiment gate statistics
155155
- Warnings and error messages
156156
- Timestamps (created, confirmed, completed)
157+
158+
The `scope` object returns **both IDs and display values** side-by-side (e.g., `{ semesterId, semesterCode, departmentId, departmentCode, facultyId, facultyName, ... }`). IDs are used by the frontend for cache keys and lookups; display values are used for rendering. See FAC-132 TD-9 for the contract.
159+
160+
## Discovery
161+
162+
**Endpoint:** `GET /analysis/pipelines`
163+
164+
Returns the 10 most recent pipelines matching the query, ordered by `createdAt DESC`. Query parameters:
165+
166+
| Parameter | Required | Description |
167+
| ------------------------ | -------- | ----------------------------------- |
168+
| `semesterId` | Yes | Target semester |
169+
| `facultyId` | No | Filter to a specific faculty member |
170+
| `departmentId` | No | Filter to a department |
171+
| `programId` | No | Filter to a program |
172+
| `campusId` | No | Filter to a campus |
173+
| `courseId` | No | Filter to a course |
174+
| `questionnaireVersionId` | No | Filter to a specific version |
175+
176+
Scope-filling behavior per role is documented in [Access Control](#access-control).
177+
178+
## Access Control
179+
180+
Authorization for every `/analysis/*` endpoint is enforced at two layers:
181+
182+
1. **Role guard (`@UseJwtGuard` + `RolesGuard`)** — class-level allowlist plus per-method widening. Roles outside the allowlist get `403 Forbidden` before the service runs.
183+
2. **Service-layer scope check**`PipelineOrchestratorService` validates the caller's institutional scope against the pipeline's scope fields via `ScopeResolverService`. Belt-and-braces against guard misconfiguration.
184+
185+
### Role allowlist per endpoint
186+
187+
| Endpoint | Allowed roles |
188+
| ---------------------------------------------- | ---------------------------------------------------- |
189+
| `POST /analysis/pipelines` | SUPER_ADMIN, DEAN, CHAIRPERSON, CAMPUS_HEAD |
190+
| `POST /analysis/pipelines/:id/confirm` | SUPER_ADMIN, DEAN, CHAIRPERSON, CAMPUS_HEAD |
191+
| `POST /analysis/pipelines/:id/cancel` | SUPER_ADMIN, DEAN, CHAIRPERSON, CAMPUS_HEAD |
192+
| `GET /analysis/pipelines` | SUPER_ADMIN, DEAN, CHAIRPERSON, CAMPUS_HEAD, FACULTY |
193+
| `GET /analysis/pipelines/:id/status` | SUPER_ADMIN, DEAN, CHAIRPERSON, CAMPUS_HEAD, FACULTY |
194+
| `GET /analysis/pipelines/:id/recommendations` | SUPER_ADMIN, DEAN, CHAIRPERSON, CAMPUS_HEAD, FACULTY |
195+
196+
STUDENT and ADMIN are never in the allowlist. STUDENT is end-user; ADMIN is reserved for admin-console operations (not analytics).
197+
198+
### Create / Confirm / Cancel / Read-by-id scope matrix
199+
200+
These operations require an **explicit scope filter** on the axis appropriate to the caller's role. Absence of the filter returns `400 Bad Request` with `"scope filter required for your role"`. A filter outside the caller's resolved set returns `403 Forbidden` with `"scope not in your assigned access"`.
201+
202+
| Role | Required on create | Validation against |
203+
| ----------- | ---------------------------- | ------------------------------------------- |
204+
| SUPER_ADMIN | None (`semesterId` only OK) | — (unrestricted) |
205+
| DEAN | `departmentId` | `ResolveDepartmentIds(semesterId)` |
206+
| CHAIRPERSON | `programId` | `ResolveProgramIds(semesterId)` |
207+
| CAMPUS_HEAD | `campusId` OR `departmentId` | `ResolveCampusIds` / `ResolveDepartmentIds` |
208+
| FACULTY | _(blocked at guard — 403)_ ||
209+
| STUDENT | _(blocked at guard — 403)_ ||
210+
211+
**Read-by-id additional rules:**
212+
213+
- FACULTY may read `GET /:id/status` or `GET /:id/recommendations` only when `pipeline.faculty.id === user.id`. Department-scoped pipelines (null `faculty` FK) are 403 for FACULTY.
214+
- A pipeline with a **null** scope field on the caller's axis (e.g., `pipeline.department === null` for a DEAN) is 403 for all scoped roles — null means "no filter", i.e., broader-than-role access, reserved for SUPER_ADMIN.
215+
- Pipelines not found return `404 Not Found` **before** the scope check, so 404 takes precedence over 403. This exposes a minor existence-oracle for scoped roles who know a foreign UUID. Bounded by UUID opacity and the fact that FACULTY never learns foreign UUIDs (see list auto-override below).
216+
217+
### List scope-filling
218+
219+
`GET /analysis/pipelines` fills in the caller's resolved scope when the corresponding query parameter is omitted — this differs from create, which 400s on absence. Rationale: the dean dashboard can call `listPipelines({ semesterId })` without needing to enumerate department UUIDs on the client.
220+
221+
| Role | Behavior |
222+
| ----------- | ------------------------------------------------------------------------------------------------------------------------------------------- |
223+
| SUPER_ADMIN | Query unchanged. |
224+
| DEAN | If `departmentId` omitted, service fills an IN-filter with `ResolveDepartmentIds(semesterId)`. If provided, verify ∈ resolved set else 403. |
225+
| CHAIRPERSON | Same with `programId` and `ResolveProgramIds(semesterId)`. |
226+
| CAMPUS_HEAD | Same with `campusId` and `ResolveCampusIds(semesterId)`. |
227+
| FACULTY | `facultyId` is silently overridden to the caller's own user id. Any foreign `facultyId` in the query is dropped. |
228+
| STUDENT | 403. |
229+
230+
The FACULTY auto-override prevents enumeration of other faculty's pipelines and ensures FACULTY never learns foreign pipeline UUIDs.
231+
232+
### Population requirement for ownership checks
233+
234+
`ConfirmPipeline`, `CancelPipeline`, `GetPipelineStatus`, and `GetRecommendations` populate `['faculty', 'department', 'program', 'campus']` on the pipeline before running `assertCanAccessPipeline`. Reading `pipeline.faculty?.id` through an unpopulated reference proxy is fragile — explicit populate is load-bearing.
235+
236+
### Scope check ordering for side-effecting endpoints
237+
238+
For `ConfirmPipeline` and `CancelPipeline`, the scope check runs **before** any `fork.flush()`, status mutation, or queue enqueue. A foreign caller must never cause side effects — even when an unrelated check (e.g., worker URL misconfiguration) would otherwise fail the pipeline.
239+
240+
### Unique-scope invariant
241+
242+
A partial unique index (`uq_analysis_pipeline_active_scope`) enforces one active pipeline per `(semester, scope tuple)` at the DB level. Concurrent creates with identical scope produce `UniqueConstraintViolationException`; the orchestrator catches it and re-fetches the winner so both callers see the same pipeline id (idempotent).
243+
244+
### Scope drift mid-pipeline
245+
246+
A DEAN who triggers a pipeline and is then reassigned to a different department mid-execution loses read access to their own pipeline. This mirrors `/analytics/*` behavior and is intentional — the scope check evaluates against current assignments, not historical ones.

src/migrations/.snapshot-faculytics_db.json

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@
7777
"autoincrement": false,
7878
"primary": false,
7979
"nullable": false,
80-
"unique": false,
80+
"unique": true,
8181
"length": 255,
8282
"precision": null,
8383
"scale": null,
@@ -408,6 +408,23 @@
408408
"keyName": "analysis_pipeline_semester_id_status_index",
409409
"unique": false,
410410
"primary": false
411+
},
412+
{
413+
"columnNames": [
414+
"semester_id",
415+
"COALESCE(faculty_id, NONE::character varying)",
416+
"COALESCE(department_id, NONE::character varying)",
417+
"COALESCE(program_id, NONE::character varying)",
418+
"COALESCE(campus_id, NONE::character varying)",
419+
"COALESCE(course_id, NONE::character varying)",
420+
"COALESCE(questionnaire_version_id, NONE::character varying)"
421+
],
422+
"composite": true,
423+
"constraint": false,
424+
"keyName": "uq_analysis_pipeline_active_scope",
425+
"unique": true,
426+
"primary": false,
427+
"expression": "CREATE UNIQUE INDEX uq_analysis_pipeline_active_scope ON public.analysis_pipeline USING btree (semester_id, COALESCE(faculty_id, 'NONE'::character varying), COALESCE(department_id, 'NONE'::character varying), COALESCE(program_id, 'NONE'::character varying), COALESCE(campus_id, 'NONE'::character varying), COALESCE(course_id, 'NONE'::character varying), COALESCE(questionnaire_version_id, 'NONE'::character varying)) WHERE ((status <> ALL (ARRAY['COMPLETED'::text, 'FAILED'::text, 'CANCELLED'::text])) AND (deleted_at IS NULL))"
411428
}
412429
],
413430
"checks": [],
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { Migration } from '@mikro-orm/migrations';
2+
3+
export class Migration20260414155236 extends Migration {
4+
// FAC-132: partial unique index enforcing one canonical pipeline per
5+
// (semester, scope) tuple while any non-terminal pipeline exists.
6+
//
7+
// Why a partial index (not @Unique decorator or plain unique constraint):
8+
// - `analysis_pipeline` scope FKs (faculty_id, department_id, ...) are
9+
// nullable. Postgres treats NULL as distinct in unique constraints, so a
10+
// plain constraint would permit unlimited duplicate (semester, NULL,
11+
// NULL, ...) active pipelines. COALESCE-to-sentinel fixes this.
12+
// - Soft-deleted rows and terminal-state pipelines (COMPLETED/FAILED/
13+
// CANCELLED) must NOT participate — a new active pipeline for a scope
14+
// should always be insertable once the previous one settles.
15+
//
16+
// FK columns are varchar(255) (see Migration20260313170918) — the text
17+
// literal 'NONE' is the sentinel. Do NOT cast to uuid; the columns are
18+
// text-typed.
19+
20+
private readonly INDEX_NAME = 'uq_analysis_pipeline_active_scope';
21+
22+
override async up(): Promise<void> {
23+
this.addSql(`
24+
CREATE UNIQUE INDEX "${this.INDEX_NAME}"
25+
ON "analysis_pipeline" (
26+
"semester_id",
27+
COALESCE("faculty_id", 'NONE'),
28+
COALESCE("department_id", 'NONE'),
29+
COALESCE("program_id", 'NONE'),
30+
COALESCE("campus_id", 'NONE'),
31+
COALESCE("course_id", 'NONE'),
32+
COALESCE("questionnaire_version_id", 'NONE')
33+
)
34+
WHERE "status" NOT IN ('COMPLETED', 'FAILED', 'CANCELLED')
35+
AND "deleted_at" IS NULL;
36+
`);
37+
}
38+
39+
override async down(): Promise<void> {
40+
this.addSql(`DROP INDEX IF EXISTS "${this.INDEX_NAME}";`);
41+
}
42+
}

src/modules/analysis/analysis.controller.spec.ts

Lines changed: 78 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
import { Test, TestingModule } from '@nestjs/testing';
2+
import { AuthGuard } from '@nestjs/passport';
23
import { AnalysisController } from './analysis.controller';
34
import { PipelineOrchestratorService } from './services/pipeline-orchestrator.service';
45
import { PipelineStatus } from './enums';
56
import {
67
auditTestProviders,
78
overrideAuditInterceptors,
89
} from '../audit/testing/audit-test.helpers';
10+
import { RolesGuard } from 'src/security/guards/roles.guard';
11+
import { CurrentUserInterceptor } from '../common/interceptors/current-user.interceptor';
912

1013
const makeMockPipeline = (
1114
overrides: Partial<Record<string, unknown>> = {},
1215
) => ({
1316
id: 'p1',
1417
status: PipelineStatus.AWAITING_CONFIRMATION,
15-
semester: { id: 's1' },
18+
semester: { id: 's1', code: 'S2026' },
1619
faculty: undefined,
1720
questionnaireVersion: undefined,
1821
department: undefined,
@@ -27,6 +30,7 @@ const makeMockPipeline = (
2730
warnings: [],
2831
errorMessage: undefined,
2932
createdAt: new Date('2026-01-01T00:00:00Z'),
33+
updatedAt: new Date('2026-01-01T00:00:00Z'),
3034
confirmedAt: undefined,
3135
completedAt: undefined,
3236
...overrides,
@@ -36,6 +40,7 @@ describe('AnalysisController', () => {
3640
let controller: AnalysisController;
3741
let mockOrchestrator: {
3842
CreatePipeline: jest.Mock;
43+
ListPipelines: jest.Mock;
3944
ConfirmPipeline: jest.Mock;
4045
CancelPipeline: jest.Mock;
4146
GetPipelineStatus: jest.Mock;
@@ -45,6 +50,7 @@ describe('AnalysisController', () => {
4550
beforeEach(async () => {
4651
mockOrchestrator = {
4752
CreatePipeline: jest.fn(),
53+
ListPipelines: jest.fn(),
4854
ConfirmPipeline: jest.fn(),
4955
CancelPipeline: jest.fn(),
5056
GetPipelineStatus: jest.fn(),
@@ -60,7 +66,16 @@ describe('AnalysisController', () => {
6066
},
6167
...auditTestProviders(),
6268
],
63-
});
69+
})
70+
.overrideGuard(AuthGuard('jwt'))
71+
.useValue({ canActivate: () => true })
72+
.overrideGuard(RolesGuard)
73+
.useValue({ canActivate: () => true })
74+
.overrideInterceptor(CurrentUserInterceptor)
75+
.useValue({
76+
intercept: (_ctx: unknown, next: { handle: () => unknown }) =>
77+
next.handle(),
78+
});
6479
const module: TestingModule =
6580
await overrideAuditInterceptors(builder).compile();
6681

@@ -81,17 +96,20 @@ describe('AnalysisController', () => {
8196
user: { userId: 'u1', moodleUserId: 1 },
8297
} as unknown as Parameters<typeof controller.CreatePipeline>[1];
8398

84-
const result = await controller.CreatePipeline(body, req);
99+
const result = (await controller.CreatePipeline(
100+
body,
101+
req,
102+
)) as unknown as {
103+
id: string;
104+
status: string;
105+
scope: { semesterId: string };
106+
coverage: { responseRate: number };
107+
};
85108

86-
expect(result).toEqual(
87-
expect.objectContaining({
88-
id: 'p1',
89-
status: PipelineStatus.AWAITING_CONFIRMATION,
90-
semesterId: 's1',
91-
triggeredById: 'u1',
92-
responseRate: 0.5,
93-
}),
94-
);
109+
expect(result.id).toBe('p1');
110+
expect(result.status).toBe(PipelineStatus.AWAITING_CONFIRMATION);
111+
expect(result.scope.semesterId).toBe('s1');
112+
expect(result.coverage.responseRate).toBe(0.5);
95113
expect(mockOrchestrator.CreatePipeline).toHaveBeenCalledWith(body, 'u1');
96114
});
97115
});
@@ -103,15 +121,15 @@ describe('AnalysisController', () => {
103121
});
104122
mockOrchestrator.ConfirmPipeline.mockResolvedValue(mockPipeline);
105123

106-
const result = await controller.ConfirmPipeline('p1');
124+
const result = (await controller.ConfirmPipeline('p1')) as unknown as {
125+
id: string;
126+
status: string;
127+
scope: { semesterId: string };
128+
};
107129

108-
expect(result).toEqual(
109-
expect.objectContaining({
110-
id: 'p1',
111-
status: PipelineStatus.EMBEDDING_CHECK,
112-
semesterId: 's1',
113-
}),
114-
);
130+
expect(result.id).toBe('p1');
131+
expect(result.status).toBe(PipelineStatus.EMBEDDING_CHECK);
132+
expect(result.scope.semesterId).toBe('s1');
115133
expect(mockOrchestrator.ConfirmPipeline).toHaveBeenCalledWith('p1');
116134
});
117135
});
@@ -123,15 +141,15 @@ describe('AnalysisController', () => {
123141
});
124142
mockOrchestrator.CancelPipeline.mockResolvedValue(mockPipeline);
125143

126-
const result = await controller.CancelPipeline('p1');
144+
const result = (await controller.CancelPipeline('p1')) as unknown as {
145+
id: string;
146+
status: string;
147+
scope: { semesterId: string };
148+
};
127149

128-
expect(result).toEqual(
129-
expect.objectContaining({
130-
id: 'p1',
131-
status: PipelineStatus.CANCELLED,
132-
semesterId: 's1',
133-
}),
134-
);
150+
expect(result.id).toBe('p1');
151+
expect(result.status).toBe(PipelineStatus.CANCELLED);
152+
expect(result.scope.semesterId).toBe('s1');
135153
expect(mockOrchestrator.CancelPipeline).toHaveBeenCalledWith('p1');
136154
});
137155
});
@@ -141,7 +159,13 @@ describe('AnalysisController', () => {
141159
const mockStatus = {
142160
id: 'p1',
143161
status: PipelineStatus.SENTIMENT_ANALYSIS,
144-
scope: { semester: 'S2026' },
162+
// TD-9 shape — paired IDs + display values
163+
scope: {
164+
semesterId: 's1',
165+
semesterCode: 'S2026',
166+
departmentId: null,
167+
departmentCode: null,
168+
},
145169
coverage: { totalEnrolled: 100, submissionCount: 50 },
146170
stages: {
147171
embeddings: {
@@ -189,6 +213,31 @@ describe('AnalysisController', () => {
189213
});
190214
});
191215

216+
describe('ListPipelines', () => {
217+
it('should delegate to orchestrator.ListPipelines and map to summary DTOs', async () => {
218+
const mockPipeline = makeMockPipeline({
219+
semester: { id: 's1', code: 'S2026' },
220+
});
221+
mockOrchestrator.ListPipelines.mockResolvedValue([mockPipeline]);
222+
223+
const query = { semesterId: 's1' };
224+
const result = await controller.ListPipelines(query);
225+
226+
expect(mockOrchestrator.ListPipelines).toHaveBeenCalledWith(query);
227+
expect(Array.isArray(result)).toBe(true);
228+
expect(result).toHaveLength(1);
229+
const first = result[0] as unknown as {
230+
id: string;
231+
status: string;
232+
scope: { semesterId: string; semesterCode: string };
233+
};
234+
expect(first.id).toBe('p1');
235+
expect(first.status).toBe(PipelineStatus.AWAITING_CONFIRMATION);
236+
expect(first.scope.semesterId).toBe('s1');
237+
expect(first.scope.semesterCode).toBe('S2026');
238+
});
239+
});
240+
192241
describe('GetRecommendations', () => {
193242
it('should delegate to orchestrator.GetRecommendations with correct id', async () => {
194243
const mockResponse = {

0 commit comments

Comments
 (0)