From fb107d9bcddae4416f60188661a12994058c3d39 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Date: Tue, 15 Oct 2024 15:29:44 -0700 Subject: [PATCH 1/5] #3222 - Provide Ministry the ability to edit basic bceid profile info (e2e tests) --- ...oller.updateProfileInformation.e2e-spec.ts | 181 ++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts diff --git a/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts new file mode 100644 index 0000000000..49a7f677dc --- /dev/null +++ b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts @@ -0,0 +1,181 @@ +import { HttpStatus, INestApplication } from "@nestjs/common"; +import * as request from "supertest"; +import { + AESTGroups, + BEARER_AUTH_TYPE, + createTestingAppModule, + getAESTToken, +} from "../../../../testHelpers"; +import { + E2EDataSources, + createE2EDataSources, + saveFakeStudent, +} from "@sims/test-utils"; +import { IdentityProviders } from "@sims/sims-db"; + +describe("StudentAESTController(e2e)-getUploadedFile", () => { + let app: INestApplication; + let db: E2EDataSources; + + beforeAll(async () => { + const { nestApplication, dataSource } = await createTestingAppModule(); + app = nestApplication; + db = createE2EDataSources(dataSource); + }); + + it("Should allow the student profile update when the student is a Basic BCeID user.", async () => { + // Arrange + const student = await saveFakeStudent(db.dataSource); + student.user.identityProviderType = IdentityProviders.BCeIDBasic; + await db.student.save(student); + const studentProfileUpdateInfo = { + givenNames: "SCOOBY", + lastName: "DOOBY-DOO", + birthdate: "1980-03-24", + email: "dummy@some.domain", + noteDescription: "Some dummy note.", + }; + const token = await getAESTToken(AESTGroups.OperationsAdministrators); + let endpoint = `/aest/student/${student.id}`; + + // Act/Assert + await request(app.getHttpServer()) + .patch(endpoint) + .send(studentProfileUpdateInfo) + .auth(token, BEARER_AUTH_TYPE) + .expect(HttpStatus.OK); + + // Arrange + endpoint = `/aest/student/${student.id}`; + // Act/Assert + const response = await request(app.getHttpServer()) + .get(endpoint) + .auth(token, BEARER_AUTH_TYPE) + .expect(HttpStatus.OK); + expect(response.body).toEqual( + expect.objectContaining({ + firstName: "SCOOBY", + lastName: "DOOBY-DOO", + dateOfBirth: "1980-03-24", + email: "dummy@some.domain", + }), + ); + }); + + it("Should throw a HTTP Unprocessable Entity (422) error when the student is not a Basic BCeID user.", async () => { + // Arrange + const student = await saveFakeStudent(db.dataSource); + student.user.identityProviderType = IdentityProviders.BCeIDBusiness; + await db.student.save(student); + const studentProfileUpdateInfo = { + givenNames: "SCOOBY", + lastName: "DOOBY-DOO", + birthdate: "1980-03-24", + email: "dummy@some.domain", + noteDescription: "Some dummy note.", + }; + const token = await getAESTToken(AESTGroups.OperationsAdministrators); + const endpoint = `/aest/student/${student.id}`; + + // Act/Assert + const response = await request(app.getHttpServer()) + .patch(endpoint) + .send(studentProfileUpdateInfo) + .auth(token, BEARER_AUTH_TYPE) + .expect(HttpStatus.UNPROCESSABLE_ENTITY); + expect(response.body).toStrictEqual({ + error: "Unprocessable Entity", + message: "Not possible to update a non-basic-BCeID student.", + statusCode: HttpStatus.UNPROCESSABLE_ENTITY, + }); + }); + + it("Should throw a HTTP Forbidden (403) error when the ministry user performing the profile update does not have the associated role.", async () => { + // Arrange + const token = await getAESTToken(AESTGroups.MOFOperations); + const endpoint = "/aest/student/9999"; + + // Act/Assert + await request(app.getHttpServer()) + .patch(endpoint) + .send({}) + .auth(token, BEARER_AUTH_TYPE) + .expect(HttpStatus.FORBIDDEN); + }); + + it("Should throw a HTTP Unprocessable Entity (422) error when the student is a Basic BCeID user but none of the user profile information that needs to be updated has changed.", async () => { + // Arrange + const student = await saveFakeStudent(db.dataSource); + student.user.identityProviderType = IdentityProviders.BCeIDBasic; + await db.student.save(student); + const studentProfileUpdateInfo = { + givenNames: student.user.firstName?.toUpperCase(), + lastName: student.user.lastName.toUpperCase(), + birthdate: student.birthDate, + email: student.user.email.toUpperCase(), + noteDescription: "Some other dummy note.", + }; + const token = await getAESTToken(AESTGroups.BusinessAdministrators); + const endpoint = `/aest/student/${student.id}`; + + // Act/Assert + const response = await request(app.getHttpServer()) + .patch(endpoint) + .send(studentProfileUpdateInfo) + .auth(token, BEARER_AUTH_TYPE) + .expect(HttpStatus.UNPROCESSABLE_ENTITY); + expect(response.body).toStrictEqual({ + error: "Unprocessable Entity", + message: "No profile data updated because no changes were detected.", + statusCode: HttpStatus.UNPROCESSABLE_ENTITY, + }); + }); + + it("Should throw a HTTP Not Found (404) error when the student is a Basic BCeID user but none of the user profile information that needs to be updated has changed.", async () => { + // Arrange + const studentProfileUpdateInfo = { + givenNames: "SCOOBY", + lastName: "DOOBY-DOO", + birthdate: "1980-03-24", + email: "dummy@some.domain", + noteDescription: "Some dummy note.", + }; + const token = await getAESTToken(AESTGroups.BusinessAdministrators); + const endpoint = "/aest/student/9999"; + + // Act/Assert + const response = await request(app.getHttpServer()) + .patch(endpoint) + .send(studentProfileUpdateInfo) + .auth(token, BEARER_AUTH_TYPE) + .expect(HttpStatus.NOT_FOUND); + expect(response.body).toStrictEqual({ + error: "Not Found", + message: "Student does not exist.", + statusCode: HttpStatus.NOT_FOUND, + }); + }); + + it("Should throw a HTTP Bad Request error when some mandatory profile update information is missing from the payload.", async () => { + // Arrange + const studentProfileUpdateInfo = { + givenNames: "SCOOBY", + birthdate: "1980-03-24", + email: "dummy@some.domain", + noteDescription: "Some dummy note.", + }; + const token = await getAESTToken(AESTGroups.OperationsAdministrators); + const endpoint = "/aest/student/9999"; + + // Act/Assert + await request(app.getHttpServer()) + .patch(endpoint) + .send(studentProfileUpdateInfo) + .auth(token, BEARER_AUTH_TYPE) + .expect(HttpStatus.BAD_REQUEST); + }); + + afterAll(async () => { + await app?.close(); + }); +}); From 4c908a7fc7775830d7130c1168619ea25d9a4ab1 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Date: Tue, 15 Oct 2024 15:44:31 -0700 Subject: [PATCH 2/5] Code Update: Review Comments --- ...student.aest.controller.updateProfileInformation.e2e-spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts index 49a7f677dc..504a893b29 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts @@ -131,7 +131,7 @@ describe("StudentAESTController(e2e)-getUploadedFile", () => { }); }); - it("Should throw a HTTP Not Found (404) error when the student is a Basic BCeID user but none of the user profile information that needs to be updated has changed.", async () => { + it("Should throw a HTTP Not Found (404) error when the student does not exist.", async () => { // Arrange const studentProfileUpdateInfo = { givenNames: "SCOOBY", From 5640cd72f12747acd316dd73d4a4652d76bb6956 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Date: Tue, 15 Oct 2024 17:39:12 -0700 Subject: [PATCH 3/5] Code Update: Review Comments --- ...oller.updateProfileInformation.e2e-spec.ts | 77 +++++++++---------- 1 file changed, 36 insertions(+), 41 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts index 504a893b29..6d22fb68e6 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts @@ -12,29 +12,39 @@ import { saveFakeStudent, } from "@sims/test-utils"; import { IdentityProviders } from "@sims/sims-db"; +import * as faker from "faker"; -describe("StudentAESTController(e2e)-getUploadedFile", () => { +interface StudentProfileUpdateInfo { + givenNames: string; + lastName: string; + birthdate: string; + email: string; + noteDescription: string; +} + +describe("StudentAESTController(e2e)-updateProfileInformation", () => { let app: INestApplication; let db: E2EDataSources; + let studentProfileUpdateInfo: StudentProfileUpdateInfo; beforeAll(async () => { const { nestApplication, dataSource } = await createTestingAppModule(); app = nestApplication; db = createE2EDataSources(dataSource); + studentProfileUpdateInfo = { + givenNames: faker.name.firstName(), + lastName: faker.name.lastName(), + birthdate: "1990-01-15", + email: faker.internet.email(), + noteDescription: faker.lorem.text(), + }; }); it("Should allow the student profile update when the student is a Basic BCeID user.", async () => { // Arrange const student = await saveFakeStudent(db.dataSource); student.user.identityProviderType = IdentityProviders.BCeIDBasic; - await db.student.save(student); - const studentProfileUpdateInfo = { - givenNames: "SCOOBY", - lastName: "DOOBY-DOO", - birthdate: "1980-03-24", - email: "dummy@some.domain", - noteDescription: "Some dummy note.", - }; + await db.user.save(student.user); const token = await getAESTToken(AESTGroups.OperationsAdministrators); let endpoint = `/aest/student/${student.id}`; @@ -54,26 +64,19 @@ describe("StudentAESTController(e2e)-getUploadedFile", () => { .expect(HttpStatus.OK); expect(response.body).toEqual( expect.objectContaining({ - firstName: "SCOOBY", - lastName: "DOOBY-DOO", - dateOfBirth: "1980-03-24", - email: "dummy@some.domain", + firstName: studentProfileUpdateInfo.givenNames, + lastName: studentProfileUpdateInfo.lastName, + dateOfBirth: studentProfileUpdateInfo.birthdate, + email: studentProfileUpdateInfo.email, }), ); }); - it("Should throw a HTTP Unprocessable Entity (422) error when the student is not a Basic BCeID user.", async () => { + it("Should throw an HTTP Unprocessable Entity (422) error when the student is not a Basic BCeID user.", async () => { // Arrange const student = await saveFakeStudent(db.dataSource); student.user.identityProviderType = IdentityProviders.BCeIDBusiness; - await db.student.save(student); - const studentProfileUpdateInfo = { - givenNames: "SCOOBY", - lastName: "DOOBY-DOO", - birthdate: "1980-03-24", - email: "dummy@some.domain", - noteDescription: "Some dummy note.", - }; + await db.user.save(student.user); const token = await getAESTToken(AESTGroups.OperationsAdministrators); const endpoint = `/aest/student/${student.id}`; @@ -90,7 +93,7 @@ describe("StudentAESTController(e2e)-getUploadedFile", () => { }); }); - it("Should throw a HTTP Forbidden (403) error when the ministry user performing the profile update does not have the associated role.", async () => { + it("Should throw an HTTP Forbidden (403) error when the ministry user performing the profile update does not have the associated role.", async () => { // Arrange const token = await getAESTToken(AESTGroups.MOFOperations); const endpoint = "/aest/student/9999"; @@ -103,11 +106,11 @@ describe("StudentAESTController(e2e)-getUploadedFile", () => { .expect(HttpStatus.FORBIDDEN); }); - it("Should throw a HTTP Unprocessable Entity (422) error when the student is a Basic BCeID user but none of the user profile information that needs to be updated has changed.", async () => { + it("Should throw an HTTP Unprocessable Entity (422) error when the student is a Basic BCeID user but none of the user profile information that needs to be updated has changed.", async () => { // Arrange const student = await saveFakeStudent(db.dataSource); student.user.identityProviderType = IdentityProviders.BCeIDBasic; - await db.student.save(student); + await db.user.save(student.user); const studentProfileUpdateInfo = { givenNames: student.user.firstName?.toUpperCase(), lastName: student.user.lastName.toUpperCase(), @@ -131,15 +134,8 @@ describe("StudentAESTController(e2e)-getUploadedFile", () => { }); }); - it("Should throw a HTTP Not Found (404) error when the student does not exist.", async () => { + it("Should throw an HTTP Not Found (404) error when the student does not exist.", async () => { // Arrange - const studentProfileUpdateInfo = { - givenNames: "SCOOBY", - lastName: "DOOBY-DOO", - birthdate: "1980-03-24", - email: "dummy@some.domain", - noteDescription: "Some dummy note.", - }; const token = await getAESTToken(AESTGroups.BusinessAdministrators); const endpoint = "/aest/student/9999"; @@ -156,21 +152,20 @@ describe("StudentAESTController(e2e)-getUploadedFile", () => { }); }); - it("Should throw a HTTP Bad Request error when some mandatory profile update information is missing from the payload.", async () => { + it("Should throw an HTTP Bad Request error when some mandatory profile update information is missing from the payload.", async () => { // Arrange - const studentProfileUpdateInfo = { - givenNames: "SCOOBY", - birthdate: "1980-03-24", - email: "dummy@some.domain", - noteDescription: "Some dummy note.", - }; const token = await getAESTToken(AESTGroups.OperationsAdministrators); const endpoint = "/aest/student/9999"; // Act/Assert await request(app.getHttpServer()) .patch(endpoint) - .send(studentProfileUpdateInfo) + .send({ + givenNames: faker.name.firstName(), + birthdate: "1990-01-15", + email: faker.internet.email(), + noteDescription: faker.lorem.text(), + }) .auth(token, BEARER_AUTH_TYPE) .expect(HttpStatus.BAD_REQUEST); }); From fbdac956aa9982ffc7f01b35ab87b32390e71938 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Date: Wed, 16 Oct 2024 12:30:23 -0700 Subject: [PATCH 4/5] Code Update: Review Comments --- ...oller.updateProfileInformation.e2e-spec.ts | 162 ++++++++++++++---- .../student/models/student.dto.ts | 2 +- .../src/services/student/student.service.ts | 6 +- 3 files changed, 132 insertions(+), 38 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts index 6d22fb68e6..92271a0f98 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts @@ -36,7 +36,7 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { lastName: faker.name.lastName(), birthdate: "1990-01-15", email: faker.internet.email(), - noteDescription: faker.lorem.text(), + noteDescription: faker.datatype.uuid(), }; }); @@ -46,7 +46,7 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { student.user.identityProviderType = IdentityProviders.BCeIDBasic; await db.user.save(student.user); const token = await getAESTToken(AESTGroups.OperationsAdministrators); - let endpoint = `/aest/student/${student.id}`; + const endpoint = `/aest/student/${student.id}`; // Act/Assert await request(app.getHttpServer()) @@ -55,21 +55,47 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { .auth(token, BEARER_AUTH_TYPE) .expect(HttpStatus.OK); - // Arrange - endpoint = `/aest/student/${student.id}`; - // Act/Assert - const response = await request(app.getHttpServer()) - .get(endpoint) - .auth(token, BEARER_AUTH_TYPE) - .expect(HttpStatus.OK); - expect(response.body).toEqual( - expect.objectContaining({ + const updatedStudent = await db.student.findOne({ + select: { + id: true, + birthDate: true, + user: { + id: true, + firstName: true, + lastName: true, + email: true, + createdAt: true, + identityProviderType: true, + isActive: true, + updatedAt: true, + userName: true, + }, + sinValidation: { id: true }, + }, + relations: { user: true, sinValidation: true }, + where: { + id: student.id, + notes: { description: studentProfileUpdateInfo.noteDescription }, + }, + }); + expect(updatedStudent).toEqual({ + id: updatedStudent.id, + birthDate: studentProfileUpdateInfo.birthdate, + sinValidation: { id: updatedStudent.sinValidation.id }, + user: { + // Validating all the properties since eager is set to true for the User in the Student model resulting in all properties being fetched. + email: studentProfileUpdateInfo.email, firstName: studentProfileUpdateInfo.givenNames, lastName: studentProfileUpdateInfo.lastName, - dateOfBirth: studentProfileUpdateInfo.birthdate, - email: studentProfileUpdateInfo.email, - }), - ); + id: updatedStudent.user.id, + createdAt: updatedStudent.user.createdAt, + identityProviderType: updatedStudent.user.identityProviderType, + isActive: updatedStudent.user.isActive, + updatedAt: updatedStudent.user.updatedAt, + userName: updatedStudent.user.userName, + }, + }); + expect(updatedStudent.sinValidation.id).not.toBe(student.sinValidation.id); }); it("Should throw an HTTP Unprocessable Entity (422) error when the student is not a Basic BCeID user.", async () => { @@ -81,16 +107,16 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { const endpoint = `/aest/student/${student.id}`; // Act/Assert - const response = await request(app.getHttpServer()) + await request(app.getHttpServer()) .patch(endpoint) .send(studentProfileUpdateInfo) .auth(token, BEARER_AUTH_TYPE) - .expect(HttpStatus.UNPROCESSABLE_ENTITY); - expect(response.body).toStrictEqual({ - error: "Unprocessable Entity", - message: "Not possible to update a non-basic-BCeID student.", - statusCode: HttpStatus.UNPROCESSABLE_ENTITY, - }); + .expect(HttpStatus.UNPROCESSABLE_ENTITY) + .expect({ + error: "Unprocessable Entity", + message: "Not possible to update a non-basic-BCeID student.", + statusCode: HttpStatus.UNPROCESSABLE_ENTITY, + }); }); it("Should throw an HTTP Forbidden (403) error when the ministry user performing the profile update does not have the associated role.", async () => { @@ -122,16 +148,16 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { const endpoint = `/aest/student/${student.id}`; // Act/Assert - const response = await request(app.getHttpServer()) + await request(app.getHttpServer()) .patch(endpoint) .send(studentProfileUpdateInfo) .auth(token, BEARER_AUTH_TYPE) - .expect(HttpStatus.UNPROCESSABLE_ENTITY); - expect(response.body).toStrictEqual({ - error: "Unprocessable Entity", - message: "No profile data updated because no changes were detected.", - statusCode: HttpStatus.UNPROCESSABLE_ENTITY, - }); + .expect(HttpStatus.UNPROCESSABLE_ENTITY) + .expect({ + error: "Unprocessable Entity", + message: "No profile data updated because no changes were detected.", + statusCode: HttpStatus.UNPROCESSABLE_ENTITY, + }); }); it("Should throw an HTTP Not Found (404) error when the student does not exist.", async () => { @@ -140,16 +166,16 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { const endpoint = "/aest/student/9999"; // Act/Assert - const response = await request(app.getHttpServer()) + await request(app.getHttpServer()) .patch(endpoint) .send(studentProfileUpdateInfo) .auth(token, BEARER_AUTH_TYPE) - .expect(HttpStatus.NOT_FOUND); - expect(response.body).toStrictEqual({ - error: "Not Found", - message: "Student does not exist.", - statusCode: HttpStatus.NOT_FOUND, - }); + .expect(HttpStatus.NOT_FOUND) + .expect({ + error: "Not Found", + message: "Student does not exist.", + statusCode: HttpStatus.NOT_FOUND, + }); }); it("Should throw an HTTP Bad Request error when some mandatory profile update information is missing from the payload.", async () => { @@ -170,6 +196,70 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { .expect(HttpStatus.BAD_REQUEST); }); + it("Should allow the student profile update when the student is a Basic BCeID user and the givenNames is not provided.", async () => { + // Arrange + const student = await saveFakeStudent(db.dataSource); + student.user.identityProviderType = IdentityProviders.BCeIDBasic; + await db.user.save(student.user); + const updatedStudentProfile = { + lastName: faker.name.lastName(), + birthdate: "1990-01-16", + email: faker.internet.email(), + noteDescription: faker.datatype.uuid(), + }; + const token = await getAESTToken(AESTGroups.OperationsAdministrators); + const endpoint = `/aest/student/${student.id}`; + + // Act/Assert + await request(app.getHttpServer()) + .patch(endpoint) + .send(updatedStudentProfile) + .auth(token, BEARER_AUTH_TYPE) + .expect(HttpStatus.OK); + + const updatedStudent = await db.student.findOne({ + select: { + id: true, + birthDate: true, + user: { + id: true, + firstName: true, + lastName: true, + email: true, + createdAt: true, + identityProviderType: true, + isActive: true, + updatedAt: true, + userName: true, + }, + sinValidation: { id: true }, + }, + relations: { user: true, sinValidation: true }, + where: { + id: student.id, + notes: { description: updatedStudentProfile.noteDescription }, + }, + }); + expect(updatedStudent).toEqual({ + id: updatedStudent.id, + birthDate: updatedStudentProfile.birthdate, + sinValidation: { id: updatedStudent.sinValidation.id }, + user: { + // Validating all the properties since eager is set to true for the User in the Student model resulting in all properties being fetched. + email: updatedStudentProfile.email, + firstName: null, + lastName: updatedStudentProfile.lastName, + id: updatedStudent.user.id, + createdAt: updatedStudent.user.createdAt, + identityProviderType: updatedStudent.user.identityProviderType, + isActive: updatedStudent.user.isActive, + updatedAt: updatedStudent.user.updatedAt, + userName: updatedStudent.user.userName, + }, + }); + expect(updatedStudent.sinValidation.id).not.toBe(student.sinValidation.id); + }); + afterAll(async () => { await app?.close(); }); diff --git a/sources/packages/backend/apps/api/src/route-controllers/student/models/student.dto.ts b/sources/packages/backend/apps/api/src/route-controllers/student/models/student.dto.ts index 24061956b7..ccb4cc5126 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/student/models/student.dto.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/student/models/student.dto.ts @@ -287,7 +287,7 @@ export class UpdateStudentDetailsAPIInDTO { lastName: string; @MaxLength(USER_GIVEN_NAMES_MAX_LENGTH) @ValidateIf( - (value: UpdateStudentDetailsAPIInDTO) => value.givenNames !== null, + (value: UpdateStudentDetailsAPIInDTO) => !!value.givenNames?.trim(), ) givenNames: string; @IsNotEmpty() diff --git a/sources/packages/backend/apps/api/src/services/student/student.service.ts b/sources/packages/backend/apps/api/src/services/student/student.service.ts index d7201a28b5..6905615b13 100644 --- a/sources/packages/backend/apps/api/src/services/student/student.service.ts +++ b/sources/packages/backend/apps/api/src/services/student/student.service.ts @@ -510,9 +510,13 @@ export class StudentService extends RecordDataModelService { studentUserData: StudentUserData, auditUserId: number, ): Promise { + // TODO: Remove the trim function below once the DTO sanitization is done. + studentUserData.givenNames = studentUserData.givenNames?.trim(); + studentUserData.lastName = studentUserData.lastName.trim(); + studentUserData.email = studentUserData.email.trim(); const studentToSync = await this.getStudentById(studentUserData.studentId); let mustSave = false; - if (!studentUserData.givenNames?.trim()) { + if (!studentUserData.givenNames) { studentUserData.givenNames = null; } if ( From d89d571055e9a791d4ec8f972aafc30f276ea0a0 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Date: Wed, 16 Oct 2024 13:51:02 -0700 Subject: [PATCH 5/5] Code Update: Review Comments --- ...oller.updateProfileInformation.e2e-spec.ts | 58 ++++++++----------- .../student/models/student.dto.ts | 2 +- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts index 92271a0f98..c7edfc80af 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/student/_tests_/e2e/student.aest.controller.updateProfileInformation.e2e-spec.ts @@ -40,7 +40,7 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { }; }); - it("Should allow the student profile update when the student is a Basic BCeID user.", async () => { + it("Should allow the ministry user to update a student profile when the student is a Basic BCeID user and the ministry user has the associated role.", async () => { // Arrange const student = await saveFakeStudent(db.dataSource); student.user.identityProviderType = IdentityProviders.BCeIDBasic; @@ -129,7 +129,12 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { .patch(endpoint) .send({}) .auth(token, BEARER_AUTH_TYPE) - .expect(HttpStatus.FORBIDDEN); + .expect(HttpStatus.FORBIDDEN) + .expect({ + error: "Forbidden", + message: "Forbidden resource", + statusCode: HttpStatus.FORBIDDEN, + }); }); it("Should throw an HTTP Unprocessable Entity (422) error when the student is a Basic BCeID user but none of the user profile information that needs to be updated has changed.", async () => { @@ -193,7 +198,15 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { noteDescription: faker.lorem.text(), }) .auth(token, BEARER_AUTH_TYPE) - .expect(HttpStatus.BAD_REQUEST); + .expect(HttpStatus.BAD_REQUEST) + .expect({ + error: "Bad Request", + message: [ + "lastName must be shorter than or equal to 100 characters", + "lastName should not be empty", + ], + statusCode: HttpStatus.BAD_REQUEST, + }); }); it("Should allow the student profile update when the student is a Basic BCeID user and the givenNames is not provided.", async () => { @@ -202,10 +215,8 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { student.user.identityProviderType = IdentityProviders.BCeIDBasic; await db.user.save(student.user); const updatedStudentProfile = { - lastName: faker.name.lastName(), - birthdate: "1990-01-16", - email: faker.internet.email(), - noteDescription: faker.datatype.uuid(), + ...studentProfileUpdateInfo, + givenNames: null, }; const token = await getAESTToken(AESTGroups.OperationsAdministrators); const endpoint = `/aest/student/${student.id}`; @@ -220,44 +231,21 @@ describe("StudentAESTController(e2e)-updateProfileInformation", () => { const updatedStudent = await db.student.findOne({ select: { id: true, - birthDate: true, user: { - id: true, firstName: true, lastName: true, - email: true, - createdAt: true, - identityProviderType: true, - isActive: true, - updatedAt: true, - userName: true, }, - sinValidation: { id: true }, }, - relations: { user: true, sinValidation: true }, + relations: { user: true }, where: { id: student.id, notes: { description: updatedStudentProfile.noteDescription }, }, }); - expect(updatedStudent).toEqual({ - id: updatedStudent.id, - birthDate: updatedStudentProfile.birthdate, - sinValidation: { id: updatedStudent.sinValidation.id }, - user: { - // Validating all the properties since eager is set to true for the User in the Student model resulting in all properties being fetched. - email: updatedStudentProfile.email, - firstName: null, - lastName: updatedStudentProfile.lastName, - id: updatedStudent.user.id, - createdAt: updatedStudent.user.createdAt, - identityProviderType: updatedStudent.user.identityProviderType, - isActive: updatedStudent.user.isActive, - updatedAt: updatedStudent.user.updatedAt, - userName: updatedStudent.user.userName, - }, - }); - expect(updatedStudent.sinValidation.id).not.toBe(student.sinValidation.id); + expect(updatedStudent.user.firstName).toEqual(null); + expect(updatedStudent.user.lastName).toEqual( + updatedStudentProfile.lastName, + ); }); afterAll(async () => { diff --git a/sources/packages/backend/apps/api/src/route-controllers/student/models/student.dto.ts b/sources/packages/backend/apps/api/src/route-controllers/student/models/student.dto.ts index ccb4cc5126..24061956b7 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/student/models/student.dto.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/student/models/student.dto.ts @@ -287,7 +287,7 @@ export class UpdateStudentDetailsAPIInDTO { lastName: string; @MaxLength(USER_GIVEN_NAMES_MAX_LENGTH) @ValidateIf( - (value: UpdateStudentDetailsAPIInDTO) => !!value.givenNames?.trim(), + (value: UpdateStudentDetailsAPIInDTO) => value.givenNames !== null, ) givenNames: string; @IsNotEmpty()