#1791 - Enable Assessments tab view and NOA view for Public Institution Users - Part 1 #1926
Conversation
ann-aot
requested review from
dheepak-aot,
guru-aot,
sh16011993,
hellolynn-tbtb,
andrewsignori-aot,
andrepestana-aot and
JasonCTang
ann-aot
added
Institution
ann-aot
changed the title
.../packages/backend/apps/api/src/route-controllers/assessment/assessment.controller.service.ts
Show resolved
Hide resolved
...ages/backend/apps/api/src/route-controllers/assessment/assessment.institutions.controller.ts
Outdated
Show resolved
Hide resolved
...ages/backend/apps/api/src/route-controllers/assessment/assessment.institutions.controller.ts
Outdated
Show resolved
Hide resolved
...ages/backend/apps/api/src/route-controllers/assessment/assessment.institutions.controller.ts
Outdated
Show resolved
Hide resolved
...s/backend/apps/api/src/route-controllers/student-appeal/student-appeal.controller.service.ts
Outdated
Show resolved
Hide resolved
...s/backend/apps/api/src/route-controllers/student-appeal/student-appeal.controller.service.ts
Show resolved
Hide resolved
...s/backend/apps/api/src/route-controllers/student-appeal/student-appeal.controller.service.ts
Show resolved
Hide resolved
| ): Promise<DetailedStudentAppealAPIOutDTO> { | ||
| return this.studentAppealControllerService.getStudentAppealWithRequest( | ||
| appealId, | ||
| undefined, |
There was a problem hiding this comment.
If we move the studentId into options, we can avoid this.
|
|
||
| /** | ||
| * Get the student appeal details. | ||
| * @param appealId appeal id to be retrieved. |
There was a problem hiding this comment.
The order of comments are reversed.
There was a problem hiding this comment.
are we following the order of params in the comments everywhere else 😄 .
updated
| @@ -0,0 +1,49 @@ | |||
| import { Controller, Param, Get, ParseIntPipe } from "@nestjs/common"; | |||
| import { AuthorizedParties } from "../../auth/authorized-parties.enum"; | |||
| import { | |||
There was a problem hiding this comment.
Formatter is showing error because of additional space here for me. Are other devs seeing it?
There was a problem hiding this comment.
Yes, same for me @dheepak-aot. When I save the file it is fixed.
@ann-aot is VS Code formatting on save?
There was a problem hiding this comment.
ya @andrewsignori-aot. When i was working on this file, VS code was slow and i forced shutdown, probably at that time I have missed it. I have fixed it now
| } | ||
|
|
||
| /** | ||
| * Get a student application exception detected after the student application was | ||
| * submitted, for instance, when there are documents to be reviewed. | ||
| * @param applicationId application associated with the exception. | ||
| * @param status statuses to be filtered. | ||
| * @param studentId student id. |
There was a problem hiding this comment.
The order of comments studentId and status are reversed.
There was a problem hiding this comment.
I had no idea, that we were following the order in the comments. I have seen comments without following the order. did we decide it lately?
There was a problem hiding this comment.
We never enforced it on PR reviews but I would say that naturally they are usually ordered.
Is it good to have them ordered? IMO, yes.
When we create a new method, VS Code will do it for us and I believe that we should try to keep it, but not as a blocker.
There was a problem hiding this comment.
I agree with that, its good to have, when i suddenly i saw these comment, I thought we had an agreement on it and i might have missed the discussion. I have updated it
| ><template #body="slotProps">{{ | ||
| dateOnlyLongString(slotProps.data.submittedDate) | ||
| }}</template></Column | ||
| > | ||
| <Column field="requestType" header="Type" sortable="true"></Column> | ||
| <Column header="Request form" sortable="false" | ||
| <Column field="requestType" header="Type" :sortable="true"></Column> |
| @@ -77,6 +77,9 @@ export const InstitutionRoutesConst = { | |||
| STUDENT_FILE_UPLOADS: Symbol(), | |||
| STUDENT_OVERAWARDS: Symbol(), | |||
| STUDENT_NOTES: Symbol(), | |||
| ASSESSMENTS_SUMMARY: Symbol(), | |||
| APPLICATION_EXCEPTION: Symbol(), | |||
| STUDENT_APPEAL_REQUEST: Symbol(), | |||
There was a problem hiding this comment.
Same route is present in student routes const as STUDENT_APPEAL_REQUESTS. One or the other way we can follow the same in both the places.
There was a problem hiding this comment.
ya, its part of the student appeal right? i will update in next PR,
|
Great work @ann-aot. Added few minor comments. |
| @@ -0,0 +1,75 @@ | |||
| <template> | |||
| <student-appeal-requests-approval | |||
There was a problem hiding this comment.
its is used inside the component,
| <header-navigator | ||
| title="Student applications" | ||
| :routeLocation="{ | ||
| name: InstitutionRoutesConst.STUDENT_APPLICATIONS, |
There was a problem hiding this comment.
The whole route can be returned to template as variable like in StudentAppealRequestsApproval.vue
| @@ -20,13 +20,13 @@ | |||
| <Column | |||
| field="submittedDate" | |||
| header="Submitted date" | |||
| sortable="true" | |||
| :sortable="true" | |||
|
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
|
controller.servicefilecaniuse-liteapplicationDetails/-
UPDATE- got an update from @JasonCTang to hide the note, I will be hiding it in my next PR.Screenshots,
Next PRs
PR 2. Remaining request form views and NOA views
PR 3. e2e tests