Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#1791 - Enable Assessments tab view and NOA view for Public Institution Users - Part 5 #1984

Merged
merged 10 commits into from
Jun 5, 2023
Merged

#1791 - Enable Assessments tab view and NOA view for Public Institution Users - Part 5 #1984

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
19debe6
application check
ann-aot Jun 1, 2023
b61b9f5
tests
ann-aot Jun 1, 2023
1201037
feedback
ann-aot Jun 2, 2023
f8eecf4
feedback
ann-aot Jun 2, 2023
722f783
reafctor
ann-aot Jun 2, 2023
18cf4b7
reafctor
ann-aot Jun 2, 2023
aed29f2
sonar
ann-aot Jun 2, 2023
4898575
test
ann-aot Jun 2, 2023
18f19c7
reafctor
ann-aot Jun 2, 2023
e951153
feedback
ann-aot Jun 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions ...end/apps/api/src/auth/decorators/institution/institution-student-data-access.decorator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,19 @@ export const INSTITUTION_HAS_STUDENT_DATA_ACCESS_KEY =

/**
* Provide context that it's consumer must be
* validated to have access to data of given student.
* validated to have access to the student
* and their application if present.
*/
export const HasStudentDataAccess = (studentIdParamName: string) =>
SetMetadata(INSTITUTION_HAS_STUDENT_DATA_ACCESS_KEY, studentIdParamName);
export const HasStudentDataAccess = (
studentIdParamName: string,
applicationIdParamName?: string,
) =>
SetMetadata(INSTITUTION_HAS_STUDENT_DATA_ACCESS_KEY, {
studentIdParamName,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yesterday post stand-up, I believe we decided to go with the approach of using institutionId in the query.
when did that change?

applicationIdParamName,
});

export interface HasStudentDataAccessParam {
studentIdParamName: string;
applicationIdParamName: string;
}
29 changes: 24 additions & 5 deletions ...ges/backend/apps/api/src/auth/guards/institution/institution-student-data-access.guard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@ import {
CanActivate,
ExecutionContext,
ForbiddenException,
BadRequestException,
} from "@nestjs/common";
import { Reflector } from "@nestjs/core";
import { IInstitutionUserToken } from "../..";
import { INSTITUTION_HAS_STUDENT_DATA_ACCESS_KEY } from "../../decorators";
import {
HasStudentDataAccessParam,
INSTITUTION_HAS_STUDENT_DATA_ACCESS_KEY,
} from "../../decorators";
import { InstitutionService } from "../../../services";

@Injectable()
Expand All @@ -18,11 +22,10 @@ export class InstitutionStudentDataAccessGuard implements CanActivate {

async canActivate(context: ExecutionContext): Promise<boolean> {
const institutionStudentDataAccessParam =
this.reflector.getAllAndOverride<string>(
this.reflector.getAllAndOverride<HasStudentDataAccessParam>(
INSTITUTION_HAS_STUDENT_DATA_ACCESS_KEY,
[context.getHandler(), context.getClass()],
);

if (!institutionStudentDataAccessParam) {
return true;
}
Expand All @@ -34,14 +37,30 @@ export class InstitutionStudentDataAccessGuard implements CanActivate {
user: IInstitutionUserToken;
params: Record<string, string>;
} = context.switchToHttp().getRequest();

if (user?.isActive) {
const studentId = +params[institutionStudentDataAccessParam];
let applicationId = undefined;
const studentId =
+params[institutionStudentDataAccessParam.studentIdParamName];
if (!studentId) {
// Student id not found in the url.
throw new BadRequestException("Student id not found in the url.");
}
if (institutionStudentDataAccessParam.applicationIdParamName) {
applicationId =
+params[institutionStudentDataAccessParam.applicationIdParamName];
if (!applicationId) {
// Application id not found in the url.
throw new BadRequestException("Application id not found in the url.");
}
}

const hasStudentDataAccess =
await this.institutionService.hasStudentDataAccess(
user.authorizations.institutionId,
studentId,
{ applicationId },
);

if (hasStudentDataAccess) {
return true;
}
Expand Down
8 changes: 4 additions & 4 deletions ...ception/_tests_/e2e/application-exception.institution.controller.getException.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe("ApplicationExceptionInstitutionsController(e2e)-getException", () => {
);
const applicationException = application.applicationException;

const endpoint = `/institutions/application-exception/student/${application.student.id}/exception/${applicationException.id}`;
const endpoint = `/institutions/application-exception/student/${application.student.id}/application/${application.id}/exception/${applicationException.id}`;
const institutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeFUser,
);
Expand Down Expand Up @@ -96,7 +96,7 @@ describe("ApplicationExceptionInstitutionsController(e2e)-getException", () => {
{ applicationExceptionStatus: ApplicationExceptionStatus.Approved },
);

const endpoint = `/institutions/application-exception/student/${application.student.id}/exception/${collegeCApplication.applicationException.id}`;
const endpoint = `/institutions/application-exception/student/${application.student.id}/application/${application.id}/exception/${collegeCApplication.applicationException.id}`;
const institutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeFUser,
);
Expand Down Expand Up @@ -129,7 +129,7 @@ describe("ApplicationExceptionInstitutionsController(e2e)-getException", () => {
const collegeCInstitutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeCUser,
);
const endpoint = `/institutions/application-exception/student/${student.id}/exception/9999999`;
const endpoint = `/institutions/application-exception/student/${student.id}/application/${collegeCApplication.id}/exception/9999999`;

// Act/Assert
await request(app.getHttpServer())
Expand All @@ -152,7 +152,7 @@ describe("ApplicationExceptionInstitutionsController(e2e)-getException", () => {
InstitutionTokenTypes.CollegeFUser,
);

const endpoint = `/institutions/application-exception/student/${student.id}/exception/9999999`;
const endpoint = `/institutions/application-exception/student/${student.id}/application/9999999/exception/9999999`;

// Act/Assert
await request(app.getHttpServer())
Expand Down
10 changes: 6 additions & 4 deletions ...i/src/route-controllers/application-exception/application-exception.controller.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export class ApplicationExceptionControllerService {
* @param exceptionId exception to be retrieved.
* @param options options
* - `studentId` student id.
* - `applicationId` application id.
* - `assessDetails`, if true, will return access details.
* @returns student application exception information.
*/
Expand All @@ -29,14 +30,15 @@ export class ApplicationExceptionControllerService {
exceptionId: number,
options?: {
studentId?: number;
applicationId?: number;
assessDetails?: boolean;
},
): Promise<T> {
const applicationException =
await this.applicationExceptionService.getExceptionDetails(
exceptionId,
options?.studentId,
);
await this.applicationExceptionService.getExceptionDetails(exceptionId, {
studentId: options?.studentId,
applicationId: options?.applicationId,
});
if (!applicationException) {
throw new NotFoundException("Student application exception not found.");
}
Expand Down
8 changes: 5 additions & 3 deletions .../route-controllers/application-exception/application-exception.institutions.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { ApplicationExceptionControllerService } from "./application-exception.c
@AllowAuthorizedParty(AuthorizedParties.institution)
@Controller("application-exception")
@IsBCPublicInstitution()
@HasStudentDataAccess("studentId")
@HasStudentDataAccess("studentId", "applicationId")
@ApiTags(`${ClientTypeBaseRoute.Institution}-application-exception`)
export class ApplicationExceptionInstitutionsController extends BaseController {
constructor(
Expand All @@ -29,20 +29,22 @@ export class ApplicationExceptionInstitutionsController extends BaseController {
/**
* Get a student application exception of a student.
* @param studentId student id.
* @param applicationId application id.
* @param exceptionId exception to be retrieved.
* @returns student application exception information.
*/
@Get("student/:studentId/exception/:exceptionId")
@Get("student/:studentId/application/:applicationId/exception/:exceptionId")
@ApiNotFoundResponse({
description: "Student application exception not found.",
})
async getException(
@Param("studentId", ParseIntPipe) studentId: number,
@Param("applicationId", ParseIntPipe) applicationId: number,
@Param("exceptionId", ParseIntPipe) exceptionId: number,
): Promise<ApplicationExceptionAPIOutDTO> {
return this.applicationExceptionControllerService.getExceptionDetails<ApplicationExceptionAPIOutDTO>(
exceptionId,
{ studentId },
{ studentId, applicationId },
);
}
}
10 changes: 5 additions & 5 deletions ...ment/_tests_/e2e/assessment.institutions.controller.getAssessmentAwardDetails.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentAwardDetails", () =
] = disbursementReceiptsValue.grantAmount;
});

const endpoint = `/institutions/assessment/student/${student.id}/assessment/${assessment.id}/award`;
const endpoint = `/institutions/assessment/student/${student.id}/application/${application.id}/assessment/${assessment.id}/award`;
const institutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeFUser,
);
Expand Down Expand Up @@ -172,7 +172,7 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentAwardDetails", () =
},
);
await db.msfaaNumber.save(currentMSFAA);
await saveFakeApplicationDisbursements(db.dataSource, {
const application = await saveFakeApplicationDisbursements(db.dataSource, {
institution: collegeF,
institutionLocation: collegeFLocation,
student,
Expand All @@ -183,7 +183,7 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentAwardDetails", () =
InstitutionTokenTypes.CollegeFUser,
);

const endpoint = `/institutions/assessment/student/${student.id}/assessment/9999999/award`;
const endpoint = `/institutions/assessment/student/${student.id}/application/${application.id}/assessment/9999999/award`;

// Act/Assert
await request(app.getHttpServer())
Expand Down Expand Up @@ -213,7 +213,7 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentAwardDetails", () =
const collegeCInstitutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeCUser,
);
const endpoint = `/institutions/assessment/student/${student.id}/assessment/9999999/award`;
const endpoint = `/institutions/assessment/student/${student.id}/application/${collegeCApplication.id}/assessment/9999999/award`;

// Act/Assert
await request(app.getHttpServer())
Expand All @@ -236,7 +236,7 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentAwardDetails", () =
InstitutionTokenTypes.CollegeFUser,
);

const endpoint = `/institutions/assessment/student/${student.id}/assessment/9999999/award`;
const endpoint = `/institutions/assessment/student/${student.id}/application/9999999/assessment/9999999/award`;

// Act/Assert
await request(app.getHttpServer())
Expand Down
12 changes: 6 additions & 6 deletions ...rs/assessment/_tests_/e2e/assessment.institutions.controller.getAssessmentNOA.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentNOA", () => {
disbursement.valueAmount;
});

const endpoint = `/institutions/assessment/student/${student.id}/assessment/${assessment.id}/noa`;
const endpoint = `/institutions/assessment/student/${student.id}/application/${application.id}/assessment/${assessment.id}/noa`;
const institutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeFUser,
);
Expand Down Expand Up @@ -149,7 +149,7 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentNOA", () => {
application.currentAssessment.noaApprovalStatus = AssessmentStatus.required;
application.currentAssessment.assessmentData = null;
await db.application.save(application);
const endpoint = `/institutions/assessment/student/${student.id}/assessment/${application.currentAssessment.id}/noa`;
const endpoint = `/institutions/assessment/student/${student.id}/application/${application.id}/assessment/${application.currentAssessment.id}/noa`;
const institutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeFUser,
);
Expand Down Expand Up @@ -177,14 +177,14 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentNOA", () => {
},
);
await db.msfaaNumber.save(currentMSFAA);
await saveFakeApplicationDisbursements(db.dataSource, {
const application = await saveFakeApplicationDisbursements(db.dataSource, {
institution: collegeF,
institutionLocation: collegeFLocation,
student,
msfaaNumber: currentMSFAA,
});

const endpoint = `/institutions/assessment/student/${student.id}/assessment/9999999/noa`;
const endpoint = `/institutions/assessment/student/${student.id}/application/${application.id}/assessment/9999999/noa`;
const institutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeFUser,
);
Expand Down Expand Up @@ -217,7 +217,7 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentNOA", () => {
const collegeCInstitutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeCUser,
);
const endpoint = `/institutions/assessment/student/${student.id}/assessment/9999999/noa`;
const endpoint = `/institutions/assessment/student/${student.id}/application/${collegeCApplication.id}/assessment/9999999/noa`;

// Act/Assert
await request(app.getHttpServer())
Expand All @@ -240,7 +240,7 @@ describe("AssessmentInstitutionsController(e2e)-getAssessmentNOA", () => {
InstitutionTokenTypes.CollegeFUser,
);

const endpoint = `/institutions/assessment/student/${student.id}/assessment/9999999/noa`;
const endpoint = `/institutions/assessment/student/${student.id}/application/9999999/assessment/9999999/noa`;

// Act/Assert
await request(app.getHttpServer())
Expand Down
18 changes: 13 additions & 5 deletions ...ckages/backend/apps/api/src/route-controllers/assessment/assessment.controller.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,21 @@ export class AssessmentControllerService {
* @param assessmentId assessment id to be retrieved.
* @param options for NOA.
* - `studentId` optional student for authorization when needed.
* - `applicationId` application id,
* - `maskMSFAA` mask MSFAA or not.
* @returns notice of assessment data.
*/
async getAssessmentNOA(
assessmentId: number,
options?: {
studentId?: number;
applicationId?: number;
maskMSFAA?: boolean;
},
): Promise<AssessmentNOAAPIOutDTO> {
const assessment = await this.assessmentService.getAssessmentForNOA(
assessmentId,
options?.studentId,
{ studentId: options?.studentId, applicationId: options?.applicationId },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to raise a point that was a subject of a conversation with @dheepak-aot, due to the fact the applicationId was authorized by the decorator, we no longer need to include the studentId in the queries unless required for some other reason than authorization.

);

if (!assessment) {
Expand Down Expand Up @@ -152,17 +154,23 @@ export class AssessmentControllerService {
* @param assessmentId assessment to which awards details belong to.
* @param includeDocumentNumber when true document number is mapped
* to disbursement dynamic data.
* @param studentId studentId student to whom the award details belong to.
* @param options options,
* - `studentId` studentId student to whom the award details belong to.
* - `applicationId` application is used for authorization purposes to
* ensure that a user has access to the specific application data.
* @returns estimated and actual award details.
*/
async getAssessmentAwardDetails(
assessmentId: number,
includeDocumentNumber = false,
studentId?: number,
options?: {
studentId?: number;
applicationId?: number;
},
): Promise<AwardDetailsAPIOutDTO> {
const assessment = await this.assessmentService.getAssessmentForNOA(
assessmentId,
studentId,
options,
);

if (!assessment) {
Expand All @@ -180,7 +188,7 @@ export class AssessmentControllerService {
const disbursementReceipts =
await this.disbursementReceiptService.getDisbursementReceiptByAssessment(
assessmentId,
studentId,
options,
);
if (disbursementReceipts.length) {
finalAward = this.populateDisbursementReceiptAwardValues(
Expand Down
Loading