-
Notifications
You must be signed in to change notification settings - Fork 14
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.
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
#1853 - Enable Notes Tab for Public Institution User #1933
Conversation
sources/packages/backend/apps/api/src/route-controllers/note/note.institutions.controller.ts
Show resolved
Hide resolved
sources/packages/backend/apps/api/src/route-controllers/note/note.institutions.controller.ts
Show resolved
Hide resolved
* @param relations entity relations. | ||
* @returns a persisted fake student restriction. | ||
*/ | ||
export async function saveFakeStudentRestriction( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sh16011993 also created saveFakeStudentRestriction
method, probably we can have one
https://github.com/bcgov/SIMS/pull/1924/files
sources/packages/web/src/components/common/notes/StudentNotes.vue
Outdated
Show resolved
Hide resolved
sources/packages/backend/apps/api/src/route-controllers/note/note.institutions.controller.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments @andrepestana-aot
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Outdated
Show resolved
Hide resolved
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Outdated
Show resolved
Hide resolved
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Outdated
Show resolved
Hide resolved
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Outdated
Show resolved
Hide resolved
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Outdated
Show resolved
Hide resolved
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Show resolved
Hide resolved
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Outdated
Show resolved
Hide resolved
sources/packages/web/src/views/institution/student/InstitutionStudentNotes.vue
Outdated
Show resolved
Hide resolved
sources/packages/web/src/components/common/notes/StudentNotes.vue
Outdated
Show resolved
Hide resolved
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Show resolved
Hide resolved
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Show resolved
Hide resolved
.../route-controllers/note/_tests_/e2e/note.institutions.controller.getStudentNotes.e2e-spec.ts
Show resolved
Hide resolved
Good to see the student notes for institution. Please have a look at the comments. |
.expect({ | ||
statusCode: 403, | ||
message: | ||
"The institution is not allowed access to the student data of the given student.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
THis error is in a constant INSTITUTION_STUDENT_DATA_ACCESS_ERROR_MESSAGE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes. Approving with one minor comment. Please take care of the same.
Kudos, SonarCloud Quality Gate passed!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice work @andrepestana-aot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work @andrepestana-aot 👍 Thanks for doing the changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing the changes. Looks good 👍
Ministry view:

Institutions view:

Added test cases:
√ Should get all note types for a student when they are available.
√ Should throw Forbidden when student does not exist.
√ Should throw Forbidden when student exists but does not have an application for the institution.
√ Should cause a bad request when a wrong note type is provided as a filter.
√ Should get an empty student notes result when student has no notes.
√ Should get specific student notes types when student has notes and a note type filter was provided.
√ Should not get notes for student restriction and resolution notes with notification type 'no effect' when any are available.