#1791 - Enable Assessments tab view and NOA view for Public Institution Users - Part 5 #1984
Conversation
ann-aot
added
Test Case
ann-aot
requested review from
andrewsignori-aot,
dheepak-aot,
sh16011993,
guru-aot,
andrepestana-aot and
JasonCTang
...kend/apps/api/src/route-controllers/student-appeal/student-appeal.institutions.controller.ts
Outdated
Show resolved
Hide resolved
...ckages/backend/apps/api/src/auth/guards/institution/institution-student-data-access.guard.ts
Outdated
Show resolved
Hide resolved
sources/packages/backend/apps/api/src/services/institution/institution.service.ts
Outdated
Show resolved
Hide resolved
sources/packages/web/src/components/common/NoticeOfAssessmentFormView.vue
Outdated
Show resolved
Hide resolved
| applicationIdParamName?: string, | ||
| ) => | ||
| SetMetadata(INSTITUTION_HAS_STUDENT_DATA_ACCESS_KEY, [ | ||
| studentIdParamName, |
There was a problem hiding this comment.
Yesterday post stand-up, I believe we decided to go with the approach of using institutionId in the query.
when did that change?
...src/route-controllers/application-exception/application-exception.institutions.controller.ts
Show resolved
Hide resolved
| * to disbursement dynamic data.\ | ||
| * @param options options, | ||
| * - `studentId` studentId student to whom the award details belong to. | ||
| * - `applicationId` applications id. |
There was a problem hiding this comment.
Maybe we can add more information to the comments, for instance, that the applicationId is used for authorization purposes in this case.
* - `applicationId` application is used for authorization purposes to ensure that a user has access to the specific application data.
.../packages/backend/apps/api/src/services/disbursement-receipt/disbursement-receipt.service.ts
Outdated
Show resolved
Hide resolved
...ackend/apps/api/src/auth/decorators/institution/institution-student-data-access.decorator.ts
Show resolved
Hide resolved
...ackages/backend/apps/api/src/services/application-exception/application-exception.service.ts
Outdated
Show resolved
Hide resolved
...ackages/backend/apps/api/src/services/application-exception/application-exception.service.ts
Outdated
Show resolved
Hide resolved
| .innerJoin("application.student", "student") | ||
| .andWhere("student.id = :studentId", { studentId }); | ||
| } | ||
|
|
||
| if (applicationId) { | ||
| exception.andWhere("application.id = :applicationId", { applicationId }); |
There was a problem hiding this comment.
I'm confused here. I was in an assumption that, with decorators in place we will not be again doing the same applicationId condition in query in the services.
There was a problem hiding this comment.
not sure, if I got your question, the decorator will check if the student has access to the student id and application. each service will check if the entity is related to the application and student here, exception is related to the application and student.
There was a problem hiding this comment.
I understood the point later. Thanks for explaining.
...ackend/apps/api/src/auth/decorators/institution/institution-student-data-access.decorator.ts
Outdated
Show resolved
Hide resolved
| maskMSFAA?: boolean; | ||
| }, | ||
| ): Promise<AssessmentNOAAPIOutDTO> { | ||
| const assessment = await this.assessmentService.getAssessmentForNOA( | ||
| assessmentId, | ||
| options?.studentId, | ||
| { studentId: options?.studentId, applicationId: options?.applicationId }, |
There was a problem hiding this comment.
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.
|
Kudos, SonarCloud Quality Gate passed!
|
Backend Unit Tests Coverage Report
|
E2E Workflow Workers Coverage Report
|
E2E Queue Consumers Coverage Report
|
E2E SIMS API Coverage Report
|
@HasStudentDataAccessto acceptapplicationIdas an optional parameter and it applied to requests, history, award, noa, exception, and appeals.