From 3ad2b336fc7f2ddf9185fed9f6e4fc10a1b800bd Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Tue, 7 Jan 2025 17:51:41 -0800 Subject: [PATCH 01/25] Detecting changes and testing UI --- .../application.aest.controller.ts | 11 +- .../application.controller.service.ts | 9 ++ .../application.institutions.controller.ts | 1 + .../application/models/application.dto.ts | 8 ++ .../models/confirmation-of-enrollment.dto.ts | 3 +- .../application/application.service.ts | 37 ++++++- .../sims-db/src/entities/application.model.ts | 3 +- .../unit/application-data-comparison.spec.ts | 103 ++++++++++++++++++ .../application-data-comparison.models.ts | 12 ++ .../application-data-comparison.ts | 97 +++++++++++++++++ .../backend/libs/utilities/src/index.ts | 2 + .../web/src/assets/css/formio-shared.scss | 5 + .../components/common/StudentApplication.vue | 9 ++ .../web/src/composables/useFormioUtils.ts | 25 ++++- .../src/services/http/dto/Application.dto.ts | 8 ++ sources/packages/web/src/types/formio.ts | 13 ++- .../src/views/aest/StudentApplicationView.vue | 75 ++++++++++++- 17 files changed, 407 insertions(+), 14 deletions(-) create mode 100644 sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts create mode 100644 sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts create mode 100644 sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts index bfda505be6..5922742ab1 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts @@ -82,15 +82,24 @@ export class ApplicationAESTController extends BaseController { ); } + const originalApplicationData = application.data; + const previousApplicationVersion = + await this.applicationService.getPreviousApplicationDataVersion( + applicationId, + application.applicationNumber, + ); + if (loadDynamicData) { application.data = await this.applicationControllerService.generateApplicationFormData( - application.data, + originalApplicationData, ); } return this.applicationControllerService.transformToApplicationDTO( application, + originalApplicationData, + previousApplicationVersion.data, ); } diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts index cfd4ea7d6f..c4d7b3fa5d 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts @@ -26,6 +26,7 @@ import { ApplicationProgressDetailsAPIOutDTO, CompletedApplicationDetailsAPIOutDTO, InProgressApplicationDetailsAPIOutDTO, + ApplicationDataChangeAPIOutDTO, } from "./models/application.dto"; import { credentialTypeToDisplay, @@ -54,6 +55,7 @@ import { ApiProcessError } from "../../types"; import { ACTIVE_STUDENT_RESTRICTION } from "../../constants"; import { ECertPreValidationService } from "@sims/integrations/services/disbursement-schedule/e-cert-calculation"; import { AssessmentSequentialProcessingService } from "@sims/services"; +import { compareApplicationData } from "@sims/utilities"; /** * This service controller is a provider which is created to extract the implementation of @@ -445,7 +447,13 @@ export class ApplicationControllerService { */ async transformToApplicationDTO( application: Application, + originalApplicationData: unknown, + previousData?: unknown, ): Promise { + let changes: ApplicationDataChangeAPIOutDTO[] = []; + if (previousData) { + changes = compareApplicationData(previousData, originalApplicationData); + } return { data: application.data, id: application.id, @@ -461,6 +469,7 @@ export class ApplicationControllerService { applicationEndDate: application.currentAssessment?.offering?.studyEndDate, applicationInstitutionName: application.location?.institution.legalOperatingName, + changes, }; } diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts index 23e1a7faaa..cb765778ec 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts @@ -53,6 +53,7 @@ export class ApplicationInstitutionsController extends BaseController { ); return this.applicationControllerService.transformToApplicationDTO( application, + application.data, ); } } diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts b/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts index 85a6ec712a..f5ec304ff4 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts @@ -113,12 +113,20 @@ export class ApplicationDataAPIOutDTO extends ApplicationBaseAPIOutDTO { submittedDate?: Date; } +export class ApplicationDataChangeAPIOutDTO { + key?: string; + index?: number; + itemsRemoved?: boolean; + changes: ApplicationDataChangeAPIOutDTO[]; +} + export class ApplicationSupplementalDataAPIOutDTO extends ApplicationBaseAPIOutDTO { studentFullName: string; applicationOfferingIntensity?: OfferingIntensity; applicationStartDate?: string; applicationEndDate?: string; applicationInstitutionName?: string; + changes: ApplicationDataChangeAPIOutDTO[]; } export class ApplicationWithProgramYearAPIOutDTO { diff --git a/sources/packages/backend/apps/api/src/route-controllers/confirmation-of-enrollment/models/confirmation-of-enrollment.dto.ts b/sources/packages/backend/apps/api/src/route-controllers/confirmation-of-enrollment/models/confirmation-of-enrollment.dto.ts index e5607cf333..e7fe70decc 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/confirmation-of-enrollment/models/confirmation-of-enrollment.dto.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/confirmation-of-enrollment/models/confirmation-of-enrollment.dto.ts @@ -3,7 +3,6 @@ import { COEStatus, DisabilityStatus, ProgramInfoStatus } from "@sims/sims-db"; import { COEApprovalPeriodStatus } from "../../../services/disbursement-schedule/disbursement-schedule.models"; import { MONEY_VALUE_FOR_UNKNOWN_MAX_VALUE } from "../../../utilities"; import { COE_DENIED_REASON_OTHER_ID } from "@sims/utilities"; -import { YesNoOptions } from "apps/api/src/services/education-program-offering/education-program-offering-import-csv.models"; export class ApplicationDetailsForCOEAPIOutDTO { applicationProgramName: string; @@ -35,7 +34,7 @@ export class ApplicationDetailsForCOEAPIOutDTO { maxTuitionRemittanceAllowed: number; hasOverawardBalance: boolean; disabilityProfileStatus: DisabilityStatus; - disabilityApplicationStatus: YesNoOptions; + disabilityApplicationStatus: string; } export class COEDeniedReasonAPIOutDTO { diff --git a/sources/packages/backend/apps/api/src/services/application/application.service.ts b/sources/packages/backend/apps/api/src/services/application/application.service.ts index 909486f87f..f18d151c4e 100644 --- a/sources/packages/backend/apps/api/src/services/application/application.service.ts +++ b/sources/packages/backend/apps/api/src/services/application/application.service.ts @@ -1,5 +1,12 @@ import { Injectable } from "@nestjs/common"; -import { DataSource, In, Not, Brackets, EntityManager } from "typeorm"; +import { + DataSource, + In, + Not, + Brackets, + EntityManager, + LessThan, +} from "typeorm"; import { LoggerService, InjectLogger } from "@sims/utilities/logger"; import { RecordDataModelService, @@ -665,6 +672,34 @@ export class ApplicationService extends RecordDataModelService { }); } + async getPreviousApplicationDataVersion( + referenceApplicationId: number, + applicationNumber: string, + options?: { + studentId?: number; + institutionId?: number; + }, + ): Promise { + return this.repo.findOne({ + select: { + id: true, + data: true as unknown, + }, + where: { + id: LessThan(referenceApplicationId), + applicationNumber, + applicationStatus: ApplicationStatus.Overwritten, + student: { + id: options?.studentId, + }, + location: { institution: { id: options?.institutionId } }, + }, + order: { + id: "DESC", + }, + }); + } + /** * Get all student applications. * @param studentId student id. diff --git a/sources/packages/backend/libs/sims-db/src/entities/application.model.ts b/sources/packages/backend/libs/sims-db/src/entities/application.model.ts index f3d677ebf8..331d4b5cd6 100644 --- a/sources/packages/backend/libs/sims-db/src/entities/application.model.ts +++ b/sources/packages/backend/libs/sims-db/src/entities/application.model.ts @@ -29,7 +29,6 @@ import { Student } from "./student.model"; import { ProgramYear } from "./program-year.model"; import { StudentAssessment } from "./student-assessment.model"; import { ApplicationException } from "./application-exceptions.model"; -import { YesNoOptions } from "@sims/test-utils"; export const APPLICATION_NUMBER_LENGTH = 10; export const SIN_NUMBER_LENGTH = 9; @@ -429,7 +428,7 @@ export interface ApplicationData { /** * Application PD/PPD Status. */ - applicationPDPPDStatus?: YesNoOptions; + applicationPDPPDStatus?: string; /** * Indigenous person status. */ diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts new file mode 100644 index 0000000000..f1dc057d98 --- /dev/null +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts @@ -0,0 +1,103 @@ +// Target module +import { compareApplicationData } from "../../application-data-comparison"; +import * as util from "util"; + +describe("Detect differences between student application data.", () => { + it("Should detected changes when there are changes.", () => { + // Arrange + const dataA = { + bcResident: "yes", + dependants: [ + { + fullName: "a", + dateOfBirth: "2024-10-01", + declaredOnTaxes: "no", + attendingPostSecondarySchool: "no", + }, + ], + citizenship: "canadianCitizen", + courseDetails: [ + { + courseCode: "A", + courseName: "a", + courseEndDate: "2024-10-31", + courseStartDate: "2024-10-01", + }, + ], + hasDependents: "yes", + calculatedTaxYear: 2023, + isFullTimeAllowed: "", + studentGivenNames: null, + myProgramNotListed: null, + programYearEndDate: "2025-07-31", + studentInfoConfirmed: true, + showParentInformation: null, + applicationPDPPDStatus: "no", + myStudyPeriodIsntListed: { + offeringnotListed: false, + }, + selectedStudyEndDateBeforeSixWeeksFromToday: false, + }; + const dataB = { + bcResident: "yes", + dependants: [ + { + fullName: "a", + dateOfBirth: "2024-10-01", + declaredOnTaxes: "no", + attendingPostSecondarySchool: "no", + }, + { + fullName: "b", + dateOfBirth: "2024-10-01", + declaredOnTaxes: "yes", + attendingPostSecondarySchool: "no", + }, + ], + citizenship: "canadianCitizen", + courseDetails: [ + { + courseCode: "A", + courseName: "a", + courseEndDate: "2024-10-31", + courseStartDate: "2024-10-01", + }, + ], + hasDependents: "yes", + calculatedTaxYear: 2023, + isFullTimeAllowed: "", + studentGivenNames: null, + myProgramNotListed: null, + programYearEndDate: "2025-07-31", + studentInfoConfirmed: true, + showParentInformation: null, + applicationPDPPDStatus: "no", + myStudyPeriodIsntListed: { + offeringnotListed: false, + }, + selectedStudyEndDateBeforeSixWeeksFromToday: false, + }; + // Act + const result = compareApplicationData(dataA, dataB); + // Assert + console.log(JSON.stringify(result, null, 2)); + expect(result).toEqual([ + { + key: "dependants", + changes: [ + { + changes: [ + { + key: "declaredOnTaxes", + newValue: "no", + oldValue: "yes", + index: 0, + changes: [], + }, + ], + }, + ], + }, + ]); + }); +}); diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts new file mode 100644 index 0000000000..9baaa2cc75 --- /dev/null +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts @@ -0,0 +1,12 @@ +export class ApplicationDataChange { + constructor( + readonly key?: string, + readonly newValue?: unknown, + readonly oldValue?: unknown, + readonly index?: number, + ) { + this.changes = []; + } + itemsRemoved?: boolean; + changes: ApplicationDataChange[]; +} diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts new file mode 100644 index 0000000000..44a723d7b0 --- /dev/null +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts @@ -0,0 +1,97 @@ +import { ApplicationDataChange } from "./application-data-comparison.models"; + +export function compareApplicationData( + currentData: unknown, + previousData: unknown, +): ApplicationDataChange[] { + // Initial object to allow the recursive function to add changes. + const initialChanges = new ApplicationDataChange(); + compareApplicationDataRecursive(currentData, previousData, initialChanges); + const [root] = initialChanges.changes; + return root.changes; +} + +function compareApplicationDataRecursive( + currentData: unknown, + previousData: unknown, + parentChange: ApplicationDataChange, + options?: { propertyKey?: string; index?: number }, +): void { + // Same data, no need to compare. + if (isEqual(currentData, previousData)) { + return; + } + // Property has a null or undefined value but had a value before or vice versa. + if ( + (currentData == null && previousData != null) || + (currentData != null && previousData == null) + ) { + const newValueChange = new ApplicationDataChange( + options?.propertyKey, + currentData, + previousData, + options?.index, + ); + parentChange.changes.push(newValueChange); + return; + } + // Check array items. + if (Array.isArray(currentData) && Array.isArray(previousData)) { + let arrayItemChange: ApplicationDataChange; + if (!isEqual(currentData, previousData)) { + arrayItemChange = new ApplicationDataChange(options?.propertyKey); + parentChange.changes.push(arrayItemChange); + if (currentData.length > previousData.length) { + arrayItemChange.itemsRemoved = true; + } + } + // Property is an array that should have its items checked. + for (let index = 0; index < currentData.length; index++) { + const currentDataArrayItem = currentData[index]; + const previousDataArrayItem = previousData[index]; + if (!isEqual(currentDataArrayItem, previousDataArrayItem)) { + compareApplicationDataRecursive( + currentDataArrayItem, + previousDataArrayItem, + arrayItemChange, + { index }, + ); + } + } + return; + } + // Check object properties. + if (typeof currentData === "object") { + // Property is a object that should have its properties checked. + if (!isEqual(currentData, previousData)) { + const objectPropertyChange = new ApplicationDataChange( + options?.propertyKey, + undefined, + undefined, + options?.index, + ); + parentChange.changes.push(objectPropertyChange); + for (const propertyKey of Object.keys(currentData)) { + compareApplicationDataRecursive( + currentData[propertyKey], + previousData[propertyKey], + objectPropertyChange, + { propertyKey }, + ); + } + } + return; + } + // Property is a changed leaf property. + const newChange = new ApplicationDataChange( + options.propertyKey, + currentData, + previousData, + options?.index, + ); + parentChange.changes.push(newChange); +} + +function isEqual(a: unknown, b: unknown): boolean { + return JSON.stringify(a) === JSON.stringify(b); +} diff --git a/sources/packages/backend/libs/utilities/src/index.ts b/sources/packages/backend/libs/utilities/src/index.ts index 7d1754ebe8..6c55f7e78d 100644 --- a/sources/packages/backend/libs/utilities/src/index.ts +++ b/sources/packages/backend/libs/utilities/src/index.ts @@ -16,3 +16,5 @@ export * from "./specialized-string-builder"; export * from "./string-utils"; export * from "./address-utils"; export * from "./compressed-file-utils"; +export * from "./application-data-comparison/application-data-comparison"; +export * from "./application-data-comparison/application-data-comparison.models"; diff --git a/sources/packages/web/src/assets/css/formio-shared.scss b/sources/packages/web/src/assets/css/formio-shared.scss index 821bd78a52..a682a3e223 100644 --- a/sources/packages/web/src/assets/css/formio-shared.scss +++ b/sources/packages/web/src/assets/css/formio-shared.scss @@ -750,3 +750,8 @@ td .formio-custom-tooltip:hover .tooltip-text { .bi-question-circle::before { content: "\f504" !important; } + +.changed-value { + color: #dc2525; + font-style: italic !important; +} diff --git a/sources/packages/web/src/components/common/StudentApplication.vue b/sources/packages/web/src/components/common/StudentApplication.vue index c2f6bda2f0..855fa5a33c 100644 --- a/sources/packages/web/src/components/common/StudentApplication.vue +++ b/sources/packages/web/src/components/common/StudentApplication.vue @@ -6,6 +6,7 @@ @loaded="formLoaded" @changed="formChanged" @submitted="submitted" + @render="formRender" @customEvent="customEvent" > { + context.emit("render", form); + }; + const formLoaded = async (form: any) => { showNav.value = true; // Emit formLoadedCallback event to the parent, so that parent can @@ -216,6 +223,7 @@ export default defineComponent({ }; formInstance.on("prevPage", prevNextNavigation); formInstance.on("nextPage", prevNextNavigation); + await loadFormDependencies(); }; @@ -355,6 +363,7 @@ export default defineComponent({ wizardGoPrevious, formChanged, formLoaded, + formRender, isFirstPage, isLastPage, submitted, diff --git a/sources/packages/web/src/composables/useFormioUtils.ts b/sources/packages/web/src/composables/useFormioUtils.ts index 1f6ee064e5..a29e6b4c4e 100644 --- a/sources/packages/web/src/composables/useFormioUtils.ts +++ b/sources/packages/web/src/composables/useFormioUtils.ts @@ -7,7 +7,7 @@ import { Utils } from "@formio/js"; */ export function useFormioUtils() { // Get a component in a form definition once it is loaded. - const getComponent = (form: any, componentKey: string): any => { + const getComponent = (form: any, componentKey: string): FormIOComponent => { return Utils.getComponent(form.components, componentKey, true); }; @@ -23,7 +23,7 @@ export function useFormioUtils() { ): FormIOComponent => { const [firstComponentFound] = recursiveSearch( form, - (component) => component.key === componentKey, + (component) => component.component.key === componentKey, { stopOnFirstMatch: true }, ); return firstComponentFound; @@ -71,6 +71,8 @@ export function useFormioUtils() { ) => { const stopOnFirstMatch = options?.stopOnFirstMatch ?? false; components.forEach((component: any) => { + // console.log(component.key); + // console.log(component.component.key); if (stopOnFirstMatch && matchedComponents.length) { // If only the first match is needed, and one was found, stop searching. return; @@ -117,6 +119,24 @@ export function useFormioUtils() { return matchedComponents; }; + const searchByKey = ( + components: FormIOComponent[], + componentKey: string, + options?: { + stopOnFirstMatch: boolean; + }, + ): FormIOComponent[] => { + const defaultOptions = { stopOnFirstMatch: true }; + const matchedComponents: any[] = []; + internalRecursiveSearch( + components, + matchedComponents, + (component) => component.key === componentKey, + options ?? defaultOptions, + ); + return matchedComponents; + }; + // Search for components of a specific type. const getComponentsOfType = (form: any, type: string): any[] => { return recursiveSearch(form, (component) => component.type === type); @@ -229,5 +249,6 @@ export function useFormioUtils() { resetCheckBox, checkFormioValidity, excludeExtraneousValues, + searchByKey, }; } diff --git a/sources/packages/web/src/services/http/dto/Application.dto.ts b/sources/packages/web/src/services/http/dto/Application.dto.ts index 07d88c1d50..9bcaf1eea3 100644 --- a/sources/packages/web/src/services/http/dto/Application.dto.ts +++ b/sources/packages/web/src/services/http/dto/Application.dto.ts @@ -86,6 +86,13 @@ export interface ApplicationDataAPIOutDTO extends ApplicationBaseAPIOutDTO { submittedDate?: Date; } +export interface ApplicationDataChangeAPIOutDTO { + key?: string; + index?: number; + itemsRemoved?: boolean; + changes: ApplicationDataChangeAPIOutDTO[]; +} + /** * DTO for application data */ @@ -96,6 +103,7 @@ export interface ApplicationSupplementalDataAPIOutDTO applicationStartDate?: string; applicationEndDate?: string; applicationInstitutionName?: string; + changes: ApplicationDataChangeAPIOutDTO[]; } /** diff --git a/sources/packages/web/src/types/formio.ts b/sources/packages/web/src/types/formio.ts index b09375e6d2..e72e38751e 100644 --- a/sources/packages/web/src/types/formio.ts +++ b/sources/packages/web/src/types/formio.ts @@ -79,6 +79,14 @@ export enum FormIOCustomEventTypes { ReissueMSFAA = "reissueMSFAA", } +export interface FormIOComponentInternal { + key: string; + customClass: string; + options: unknown; + values: unknown; + data: { values: unknown[] }; +} + export interface FormIOComponent { id: string; key: string; @@ -86,6 +94,9 @@ export interface FormIOComponent { components: FormIOComponent[]; selectOptions: unknown[]; _visible: boolean; - component: any; + component: FormIOComponentInternal; + customClass: string; redraw: any; + disabled: boolean; + setValue: (value: unknown) => void; } diff --git a/sources/packages/web/src/views/aest/StudentApplicationView.vue b/sources/packages/web/src/views/aest/StudentApplicationView.vue index bb8bca41dd..f9b5ca6852 100644 --- a/sources/packages/web/src/views/aest/StudentApplicationView.vue +++ b/sources/packages/web/src/views/aest/StudentApplicationView.vue @@ -16,6 +16,8 @@ {{ emptyStringFiller(applicationDetail.applicationNumber) }} import { onMounted, ref, defineComponent } from "vue"; import { AESTRoutesConst } from "@/constants/routes/RouteConstants"; -import { ApplicationBaseAPIOutDTO } from "@/services/http/dto"; +import { + ApplicationDataChangeAPIOutDTO, + ApplicationSupplementalDataAPIOutDTO, +} from "@/services/http/dto"; import { ApplicationService } from "@/services/ApplicationService"; import { useFormatters } from "@/composables/useFormatters"; import StudentApplication from "@/components/common/StudentApplication.vue"; +import { useFormioUtils } from "@/composables"; +import { FormIOComponent } from "@/types"; export default defineComponent({ components: { @@ -48,23 +55,81 @@ export default defineComponent({ }, setup(props) { const { emptyStringFiller } = useFormatters(); - const applicationDetail = ref({} as ApplicationBaseAPIOutDTO); + const { searchByKey } = useFormioUtils(); + + const applicationDetail = ref({} as ApplicationSupplementalDataAPIOutDTO); const initialData = ref({}); const selectedForm = ref(); + let applicationWizard: any; + + const loadForm = async (form: any) => { + applicationWizard = form; + }; + + const formRender = async () => { + highlightChanges(); + }; onMounted(async () => { + const application = await ApplicationService.shared.getApplicationDetail( + props.applicationId, + ); applicationDetail.value = - await ApplicationService.shared.getApplicationDetail( - props.applicationId, - ); + application as ApplicationSupplementalDataAPIOutDTO; selectedForm.value = applicationDetail.value.applicationFormName; initialData.value = { ...applicationDetail.value.data, isReadOnly: true, }; + highlightChanges(); }); + function highlightChanges() { + if (!applicationWizard || !applicationDetail.value.changes.length) { + return; + } + highlightChangesRecursive( + applicationDetail.value.changes, + applicationWizard, + ); + } + + function highlightChangesRecursive( + changes: ApplicationDataChangeAPIOutDTO[], + parentComponent: FormIOComponent, + ) { + for (const change of changes) { + let searchComponent: FormIOComponent | undefined; + if (change.key) { + searchComponent = parentComponent; + } else if (change.index != undefined) { + searchComponent = parentComponent.components[change.index]; + highlightChangesRecursive(change.changes, searchComponent); + } else { + throw new Error("Invalid change object."); + } + if (!change.key) { + continue; + } + const [component] = searchByKey(searchComponent.components, change.key); + if (component) { + applyChangedValueStyleClass(component); + highlightChangesRecursive(change.changes, component); + } + } + } + + function applyChangedValueStyleClass(component: FormIOComponent) { + const htmlElement = document.getElementById(component.id); + if (!htmlElement || htmlElement.classList.contains("changed-value")) { + return; + } + document.getElementById(component.id)?.classList.add("changed-value"); + } + return { + loadForm, + formRender, applicationDetail, initialData, selectedForm, From ece720248f7ba8ed8a1ed0026856f031a7be1d3b Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Wed, 8 Jan 2025 13:45:47 -0800 Subject: [PATCH 02/25] Changes detection working close to expected --- .../application.controller.service.ts | 2 +- .../application-data-comparison.ts | 110 ++++++++++++++++++ .../web/src/assets/css/formio-shared.scss | 34 +++++- sources/packages/web/src/types/formio.ts | 17 +++ .../src/views/aest/StudentApplicationView.vue | 53 ++++++--- 5 files changed, 197 insertions(+), 19 deletions(-) create mode 100644 sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts index c4d7b3fa5d..30b063a80f 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts @@ -452,7 +452,7 @@ export class ApplicationControllerService { ): Promise { let changes: ApplicationDataChangeAPIOutDTO[] = []; if (previousData) { - changes = compareApplicationData(previousData, originalApplicationData); + changes = compareApplicationData(originalApplicationData, previousData); } return { data: application.data, diff --git a/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts new file mode 100644 index 0000000000..58fde1f6a9 --- /dev/null +++ b/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts @@ -0,0 +1,110 @@ +import { diff, IChange } from "json-diff-ts"; + +export class ApplicationChange { + constructor( + readonly key?: string, + readonly newValue?: unknown, + readonly oldValue?: unknown, + readonly index?: number, + ) { + this.changes = []; + } + changes: ApplicationChange[]; +} + +export function compareApplicationData( + newData: unknown, + previousData: unknown, +): ApplicationChange[] { + const rootChanges = new ApplicationChange(); + compareApplicationDataRecursive(newData, previousData, rootChanges); + const [root] = rootChanges.changes; + return root.changes; +} + +function compareApplicationDataRecursive( + newData: unknown, + previousData: unknown, + parentChange: ApplicationChange, + propertyKey?: string, + index?: number, +) { + // Same data, no need to compare. + if (isEqual(newData, previousData)) { + return; + } + // Property has a null or undefined value but had a value before or vice versa. + if ( + (newData == null && previousData != null) || + (newData != null && previousData == null) + ) { + const newValueChange = new ApplicationChange( + propertyKey, + newData, + previousData, + index, + ); + parentChange.changes.push(newValueChange); + return; + } + // Check array items. + if (Array.isArray(newData) && Array.isArray(previousData)) { + let arrayItemChange: ApplicationChange; + if (!isEqual(newData, previousData)) { + arrayItemChange = new ApplicationChange(propertyKey); + parentChange.changes.push(arrayItemChange); + } + // Property is an array that should have its items checked. + for (let i = 0; i < newData.length; i++) { + const newDataArrayItem = newData[i]; + const previousDataArrayItem = previousData[i]; + if (!isEqual(newDataArrayItem, previousDataArrayItem)) { + compareApplicationDataRecursive( + newDataArrayItem, + previousDataArrayItem, + arrayItemChange, + undefined, + i, + ); + } + } + return; + } + // Check object properties. + if (typeof newData === "object") { + // Property is a object that should have its properties checked. + if (!isEqual(newData, previousData)) { + const objectPropertyChange = new ApplicationChange(propertyKey); + parentChange.changes.push(objectPropertyChange); + for (const key of Object.keys(newData)) { + compareApplicationDataRecursive( + newData[key], + previousData[key], + objectPropertyChange, + key, + index, + ); + } + } + return; + } + // Property is a changed leaf property. + const newChange = new ApplicationChange( + propertyKey, + newData, + previousData, + index, + ); + parentChange.changes.push(newChange); +} + +export function compareApplicationDataLib( + dataA: unknown, + dataB: unknown, +): IChange[] { + return diff(dataA, dataB); +} + +function isEqual(a: unknown, b: unknown): boolean { + return JSON.stringify(a) === JSON.stringify(b); +} diff --git a/sources/packages/web/src/assets/css/formio-shared.scss b/sources/packages/web/src/assets/css/formio-shared.scss index a682a3e223..ab62b2c63c 100644 --- a/sources/packages/web/src/assets/css/formio-shared.scss +++ b/sources/packages/web/src/assets/css/formio-shared.scss @@ -751,7 +751,37 @@ td .formio-custom-tooltip:hover .tooltip-text { content: "\f504" !important; } -.changed-value { +// Styles related to the application changes +// between the current and previous application versions. +.changed-value-base { color: #dc2525; - font-style: italic !important; + border-radius: 5px; + border: 1px solid #dc2525; + padding: 4px; +} + +.changed-value-content-base { + @extend .label-small-bold; + background: #dc2525; + color: white; + border-radius: 5px; + display: block; +} + +.changed-value { + @extend .changed-value-base; +} + +.changed-value::before { + @extend .changed-value-content-base; + content: "\00a0\00a0Updated"; +} + +.changed-list-length { + @extend .changed-value-base; +} + +.changed-list-length::before { + @extend .changed-value-content-base; + content: "\00a0\00a0Items were removed from this list"; } diff --git a/sources/packages/web/src/types/formio.ts b/sources/packages/web/src/types/formio.ts index e72e38751e..de92df1e3e 100644 --- a/sources/packages/web/src/types/formio.ts +++ b/sources/packages/web/src/types/formio.ts @@ -79,6 +79,22 @@ export enum FormIOCustomEventTypes { ReissueMSFAA = "reissueMSFAA", } +/** + * Form IO component types. + * This does not represent all the types available in Form.IO, + * please expand the list as needed. + */ +export enum FromIOComponentTypes { + Hidden = "hidden", + Datagrid = "datagrid", + Button = "button", + Select = "select", + Panel = "panel", + EditGrid = "editgrid", + Radio = "radio", + Calendar = "calendar", +} + export interface FormIOComponentInternal { key: string; customClass: string; @@ -98,5 +114,6 @@ export interface FormIOComponent { customClass: string; redraw: any; disabled: boolean; + type: FromIOComponentTypes; setValue: (value: unknown) => void; } diff --git a/sources/packages/web/src/views/aest/StudentApplicationView.vue b/sources/packages/web/src/views/aest/StudentApplicationView.vue index f9b5ca6852..97012f7095 100644 --- a/sources/packages/web/src/views/aest/StudentApplicationView.vue +++ b/sources/packages/web/src/views/aest/StudentApplicationView.vue @@ -37,7 +37,7 @@ import { ApplicationService } from "@/services/ApplicationService"; import { useFormatters } from "@/composables/useFormatters"; import StudentApplication from "@/components/common/StudentApplication.vue"; import { useFormioUtils } from "@/composables"; -import { FormIOComponent } from "@/types"; +import { FormIOComponent, FromIOComponentTypes } from "@/types"; export default defineComponent({ components: { @@ -66,7 +66,12 @@ export default defineComponent({ applicationWizard = form; }; + /** + * Happens when all the form components are rendered, including lists. + */ const formRender = async () => { + // Highlight changes in the form after all the components are rendered. + // List components are ready only after the form is rendered. highlightChanges(); }; @@ -84,47 +89,63 @@ export default defineComponent({ highlightChanges(); }); + /** + * Check if the application has changes to be highlighted. + * Changes are expected after applications are edited after submitted at least once. + */ function highlightChanges() { if (!applicationWizard || !applicationDetail.value.changes.length) { return; } highlightChangesRecursive( - applicationDetail.value.changes, applicationWizard, + applicationDetail.value.changes, ); } + /** + * Apply the style class to the components that have changes. + * @param parentComponent component to have the changes highlighted. + * @param changes list of changes to be highlighted. + */ function highlightChangesRecursive( - changes: ApplicationDataChangeAPIOutDTO[], parentComponent: FormIOComponent, + changes: ApplicationDataChangeAPIOutDTO[], ) { for (const change of changes) { let searchComponent: FormIOComponent | undefined; - if (change.key) { - searchComponent = parentComponent; - } else if (change.index != undefined) { + if (typeof change.index === "number") { searchComponent = parentComponent.components[change.index]; - highlightChangesRecursive(change.changes, searchComponent); - } else { - throw new Error("Invalid change object."); + highlightChangesRecursive(searchComponent, change.changes); + } else if (change.key) { + searchComponent = parentComponent; } - if (!change.key) { + if (!change.key || !searchComponent) { continue; } const [component] = searchByKey(searchComponent.components, change.key); if (component) { - applyChangedValueStyleClass(component); - highlightChangesRecursive(change.changes, component); + applyChangedValueStyleClass(component, change.itemsRemoved); + highlightChangesRecursive(component, change.changes); } } } - function applyChangedValueStyleClass(component: FormIOComponent) { - const htmlElement = document.getElementById(component.id); - if (!htmlElement || htmlElement.classList.contains("changed-value")) { + /** + * Apply the proper highlight style to a changed component. + * @param component component to received the style. + * @param itemRemoved indicates if the component had an item removed, + * which would required a different style to be applied. + */ + function applyChangedValueStyleClass( + component: FormIOComponent, + itemRemoved?: boolean, + ) { + if (component.type === FromIOComponentTypes.Hidden) { return; } - document.getElementById(component.id)?.classList.add("changed-value"); + const cssClass = itemRemoved ? "changed-list-length" : "changed-value"; + document.getElementById(component.id)?.classList.add(cssClass); } return { From 45d09ac1bcf44081fc1729f616f565e4283a37ea Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Wed, 8 Jan 2025 14:54:06 -0800 Subject: [PATCH 03/25] Adjusting read-only --- .../application-data-comparison.models.ts | 11 ++ .../application-data-comparison.ts | 109 ++++++++++-------- .../application-data-comparison.ts | 107 ++++++++++------- .../src/views/aest/StudentApplicationView.vue | 13 ++- 4 files changed, 148 insertions(+), 92 deletions(-) create mode 100644 sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.models.ts diff --git a/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.models.ts b/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.models.ts new file mode 100644 index 0000000000..16c2a92533 --- /dev/null +++ b/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.models.ts @@ -0,0 +1,11 @@ +export class ApplicationChange { + constructor( + readonly key?: string, + readonly newValue?: unknown, + readonly oldValue?: unknown, + readonly index?: number, + ) { + this.changes = []; + } + changes: ApplicationChange[]; +} diff --git a/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts index 58fde1f6a9..3773694650 100644 --- a/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts +++ b/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts @@ -1,16 +1,4 @@ -import { diff, IChange } from "json-diff-ts"; - -export class ApplicationChange { - constructor( - readonly key?: string, - readonly newValue?: unknown, - readonly oldValue?: unknown, - readonly index?: number, - ) { - this.changes = []; - } - changes: ApplicationChange[]; -} +import { ApplicationChange } from "./application-data-comparison.models"; export function compareApplicationData( newData: unknown, @@ -48,44 +36,16 @@ function compareApplicationDataRecursive( return; } // Check array items. + // previousData is checked also to enforce the type as an array. In case + // it is null or undefined it will be handled at the previous scenario. if (Array.isArray(newData) && Array.isArray(previousData)) { - let arrayItemChange: ApplicationChange; - if (!isEqual(newData, previousData)) { - arrayItemChange = new ApplicationChange(propertyKey); - parentChange.changes.push(arrayItemChange); - } - // Property is an array that should have its items checked. - for (let i = 0; i < newData.length; i++) { - const newDataArrayItem = newData[i]; - const previousDataArrayItem = previousData[i]; - if (!isEqual(newDataArrayItem, previousDataArrayItem)) { - compareApplicationDataRecursive( - newDataArrayItem, - previousDataArrayItem, - arrayItemChange, - undefined, - i, - ); - } - } + checkArrayChanges(newData, previousData, parentChange, propertyKey); return; } // Check object properties. if (typeof newData === "object") { // Property is a object that should have its properties checked. - if (!isEqual(newData, previousData)) { - const objectPropertyChange = new ApplicationChange(propertyKey); - parentChange.changes.push(objectPropertyChange); - for (const key of Object.keys(newData)) { - compareApplicationDataRecursive( - newData[key], - previousData[key], - objectPropertyChange, - key, - index, - ); - } - } + checkObjectChanges(newData, previousData, parentChange, propertyKey, index); return; } // Property is a changed leaf property. @@ -98,13 +58,62 @@ function compareApplicationDataRecursive( parentChange.changes.push(newChange); } -export function compareApplicationDataLib( - dataA: unknown, - dataB: unknown, -): IChange[] { - return diff(dataA, dataB); +function checkArrayChanges( + newData: unknown[], + previousData: unknown[], + parentChange: ApplicationChange, + propertyKey?: string, +): void { + let arrayItemChange: ApplicationChange; + if (!isEqual(newData, previousData)) { + arrayItemChange = new ApplicationChange(propertyKey); + parentChange.changes.push(arrayItemChange); + } + // Property is an array that should have its items checked. + for (let i = 0; i < newData.length; i++) { + const newDataArrayItem = newData[i]; + const previousDataArrayItem = previousData[i]; + if (!isEqual(newDataArrayItem, previousDataArrayItem)) { + compareApplicationDataRecursive( + newDataArrayItem, + previousDataArrayItem, + arrayItemChange, + undefined, + i, + ); + } + } +} + +function checkObjectChanges( + newData: unknown, + previousData: unknown, + parentChange: ApplicationChange, + propertyKey?: string, + index?: number, +): void { + if (!isEqual(newData, previousData)) { + const objectPropertyChange = new ApplicationChange(propertyKey); + parentChange.changes.push(objectPropertyChange); + for (const key of Object.keys(newData)) { + compareApplicationDataRecursive( + newData[key], + previousData[key], + objectPropertyChange, + key, + index, + ); + } + } } +/** + * Define if two objects can be considered the same + * based on their JSON representation. + * @param a first object to be compared. + * @param b second object to be compared. + * @returns true if objects are considered the same. + */ function isEqual(a: unknown, b: unknown): boolean { return JSON.stringify(a) === JSON.stringify(b); } diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts index 44a723d7b0..38a800ce86 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts @@ -36,53 +36,18 @@ function compareApplicationDataRecursive( return; } // Check array items. + // previousData is checked also to enforce the type as an array. In case + // it is null or undefined it will be handled at the previous scenario. if (Array.isArray(currentData) && Array.isArray(previousData)) { - let arrayItemChange: ApplicationDataChange; - if (!isEqual(currentData, previousData)) { - arrayItemChange = new ApplicationDataChange(options?.propertyKey); - parentChange.changes.push(arrayItemChange); - if (currentData.length > previousData.length) { - arrayItemChange.itemsRemoved = true; - } - } - // Property is an array that should have its items checked. - for (let index = 0; index < currentData.length; index++) { - const currentDataArrayItem = currentData[index]; - const previousDataArrayItem = previousData[index]; - if (!isEqual(currentDataArrayItem, previousDataArrayItem)) { - compareApplicationDataRecursive( - currentDataArrayItem, - previousDataArrayItem, - arrayItemChange, - { index }, - ); - } - } + checkArrayChanges(currentData, previousData, parentChange, options); return; } // Check object properties. if (typeof currentData === "object") { - // Property is a object that should have its properties checked. - if (!isEqual(currentData, previousData)) { - const objectPropertyChange = new ApplicationDataChange( - options?.propertyKey, - undefined, - undefined, - options?.index, - ); - parentChange.changes.push(objectPropertyChange); - for (const propertyKey of Object.keys(currentData)) { - compareApplicationDataRecursive( - currentData[propertyKey], - previousData[propertyKey], - objectPropertyChange, - { propertyKey }, - ); - } - } + checkObjectChanges(currentData, previousData, parentChange, options); return; } - // Property is a changed leaf property. + // Property was changed and it is a leaf property. const newChange = new ApplicationDataChange( options.propertyKey, currentData, @@ -92,6 +57,68 @@ function compareApplicationDataRecursive( parentChange.changes.push(newChange); } +function checkArrayChanges( + currentData: unknown[], + previousData: unknown[], + parentChange: ApplicationDataChange, + options?: { propertyKey?: string; index?: number }, +): void { + let arrayItemChange: ApplicationDataChange; + if (!isEqual(currentData, previousData)) { + arrayItemChange = new ApplicationDataChange(options?.propertyKey); + parentChange.changes.push(arrayItemChange); + if (currentData.length > previousData.length) { + arrayItemChange.itemsRemoved = true; + } + } + // Property is an array that should have its items checked. + for (let index = 0; index < currentData.length; index++) { + const currentDataArrayItem = currentData[index]; + const previousDataArrayItem = previousData[index]; + if (!isEqual(currentDataArrayItem, previousDataArrayItem)) { + compareApplicationDataRecursive( + currentDataArrayItem, + previousDataArrayItem, + arrayItemChange, + { index }, + ); + } + } +} + +function checkObjectChanges( + currentData: unknown, + previousData: unknown, + parentChange: ApplicationDataChange, + options?: { propertyKey?: string; index?: number }, +): void { + // Property is a object that should have its properties checked. + if (!isEqual(currentData, previousData)) { + const objectPropertyChange = new ApplicationDataChange( + options?.propertyKey, + undefined, + undefined, + options?.index, + ); + parentChange.changes.push(objectPropertyChange); + for (const propertyKey of Object.keys(currentData)) { + compareApplicationDataRecursive( + currentData[propertyKey], + previousData[propertyKey], + objectPropertyChange, + { propertyKey }, + ); + } + } +} + +/** + * Define if two objects can be considered the same + * based on their JSON representation. + * @param a first object to be compared. + * @param b second object to be compared. + * @returns true if objects are considered the same. + */ function isEqual(a: unknown, b: unknown): boolean { return JSON.stringify(a) === JSON.stringify(b); } diff --git a/sources/packages/web/src/views/aest/StudentApplicationView.vue b/sources/packages/web/src/views/aest/StudentApplicationView.vue index 97012f7095..2fcbf70337 100644 --- a/sources/packages/web/src/views/aest/StudentApplicationView.vue +++ b/sources/packages/web/src/views/aest/StudentApplicationView.vue @@ -120,7 +120,11 @@ export default defineComponent({ } else if (change.key) { searchComponent = parentComponent; } - if (!change.key || !searchComponent) { + if ( + !change.key || + !searchComponent || + !searchComponent.components?.length + ) { continue; } const [component] = searchByKey(searchComponent.components, change.key); @@ -141,7 +145,12 @@ export default defineComponent({ component: FormIOComponent, itemRemoved?: boolean, ) { - if (component.type === FromIOComponentTypes.Hidden) { + // TODO: should we check for changes in the read-only version? + // TODO: should we check only read-only versions? + if ( + component.type === FromIOComponentTypes.Hidden || + component._visible === false + ) { return; } const cssClass = itemRemoved ? "changed-list-length" : "changed-value"; From d969f2eb7fcffbf59cc7e47f5b7d68344064f7b5 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Wed, 8 Jan 2025 15:42:29 -0800 Subject: [PATCH 04/25] Adjusted comparison for read-only --- .../application.aest.controller.ts | 31 ++--- .../application.controller.service.ts | 8 +- .../application-data-comparison.models.ts | 11 -- .../application-data-comparison.ts | 119 ------------------ 4 files changed, 21 insertions(+), 148 deletions(-) delete mode 100644 sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.models.ts delete mode 100644 sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts index 5922742ab1..651c1cb083 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts @@ -19,6 +19,7 @@ import { CompletedApplicationDetailsAPIOutDTO, EnrolmentApplicationDetailsAPIOutDTO, InProgressApplicationDetailsAPIOutDTO, + ApplicationFormData, } from "./models/application.dto"; import { AllowAuthorizedParty, @@ -81,25 +82,27 @@ export class ApplicationAESTController extends BaseController { `Application id ${applicationId} was not found.`, ); } - - const originalApplicationData = application.data; - const previousApplicationVersion = - await this.applicationService.getPreviousApplicationDataVersion( - applicationId, - application.applicationNumber, - ); - + let currentReadOnlyData: ApplicationFormData; + let previousReadOnlyData: ApplicationFormData; if (loadDynamicData) { - application.data = - await this.applicationControllerService.generateApplicationFormData( - originalApplicationData, + const previousApplicationVersion = + await this.applicationService.getPreviousApplicationDataVersion( + applicationId, + application.applicationNumber, ); + [currentReadOnlyData, previousReadOnlyData] = await Promise.all([ + this.applicationControllerService.generateApplicationFormData( + application.data, + ), + this.applicationControllerService.generateApplicationFormData( + previousApplicationVersion.data, + ), + ]); + application.data = currentReadOnlyData; } - return this.applicationControllerService.transformToApplicationDTO( application, - originalApplicationData, - previousApplicationVersion.data, + previousReadOnlyData, ); } diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts index 30b063a80f..f38fbca904 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts @@ -442,17 +442,17 @@ export class ApplicationControllerService { /** * Transformation util for Application. - * @param application - * @returns Application DTO + * @param application application to be converted to the DTO. + * @param previousData previous application to allow changes detection. + * @returns application DTO. */ async transformToApplicationDTO( application: Application, - originalApplicationData: unknown, previousData?: unknown, ): Promise { let changes: ApplicationDataChangeAPIOutDTO[] = []; if (previousData) { - changes = compareApplicationData(originalApplicationData, previousData); + changes = compareApplicationData(application.data, previousData); } return { data: application.data, diff --git a/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.models.ts b/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.models.ts deleted file mode 100644 index 16c2a92533..0000000000 --- a/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.models.ts +++ /dev/null @@ -1,11 +0,0 @@ -export class ApplicationChange { - constructor( - readonly key?: string, - readonly newValue?: unknown, - readonly oldValue?: unknown, - readonly index?: number, - ) { - this.changes = []; - } - changes: ApplicationChange[]; -} diff --git a/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts deleted file mode 100644 index 3773694650..0000000000 --- a/sources/packages/backend/apps/api/src/utilities/application-data-comparison/application-data-comparison.ts +++ /dev/null @@ -1,119 +0,0 @@ -import { ApplicationChange } from "./application-data-comparison.models"; - -export function compareApplicationData( - newData: unknown, - previousData: unknown, -): ApplicationChange[] { - const rootChanges = new ApplicationChange(); - compareApplicationDataRecursive(newData, previousData, rootChanges); - const [root] = rootChanges.changes; - return root.changes; -} - -function compareApplicationDataRecursive( - newData: unknown, - previousData: unknown, - parentChange: ApplicationChange, - propertyKey?: string, - index?: number, -) { - // Same data, no need to compare. - if (isEqual(newData, previousData)) { - return; - } - // Property has a null or undefined value but had a value before or vice versa. - if ( - (newData == null && previousData != null) || - (newData != null && previousData == null) - ) { - const newValueChange = new ApplicationChange( - propertyKey, - newData, - previousData, - index, - ); - parentChange.changes.push(newValueChange); - return; - } - // Check array items. - // previousData is checked also to enforce the type as an array. In case - // it is null or undefined it will be handled at the previous scenario. - if (Array.isArray(newData) && Array.isArray(previousData)) { - checkArrayChanges(newData, previousData, parentChange, propertyKey); - return; - } - // Check object properties. - if (typeof newData === "object") { - // Property is a object that should have its properties checked. - checkObjectChanges(newData, previousData, parentChange, propertyKey, index); - return; - } - // Property is a changed leaf property. - const newChange = new ApplicationChange( - propertyKey, - newData, - previousData, - index, - ); - parentChange.changes.push(newChange); -} - -function checkArrayChanges( - newData: unknown[], - previousData: unknown[], - parentChange: ApplicationChange, - propertyKey?: string, -): void { - let arrayItemChange: ApplicationChange; - if (!isEqual(newData, previousData)) { - arrayItemChange = new ApplicationChange(propertyKey); - parentChange.changes.push(arrayItemChange); - } - // Property is an array that should have its items checked. - for (let i = 0; i < newData.length; i++) { - const newDataArrayItem = newData[i]; - const previousDataArrayItem = previousData[i]; - if (!isEqual(newDataArrayItem, previousDataArrayItem)) { - compareApplicationDataRecursive( - newDataArrayItem, - previousDataArrayItem, - arrayItemChange, - undefined, - i, - ); - } - } -} - -function checkObjectChanges( - newData: unknown, - previousData: unknown, - parentChange: ApplicationChange, - propertyKey?: string, - index?: number, -): void { - if (!isEqual(newData, previousData)) { - const objectPropertyChange = new ApplicationChange(propertyKey); - parentChange.changes.push(objectPropertyChange); - for (const key of Object.keys(newData)) { - compareApplicationDataRecursive( - newData[key], - previousData[key], - objectPropertyChange, - key, - index, - ); - } - } -} - -/** - * Define if two objects can be considered the same - * based on their JSON representation. - * @param a first object to be compared. - * @param b second object to be compared. - * @returns true if objects are considered the same. - */ -function isEqual(a: unknown, b: unknown): boolean { - return JSON.stringify(a) === JSON.stringify(b); -} From ddc7a3dfffb83b7fdf0df16d4727d3bcbbfe8359 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Wed, 8 Jan 2025 16:00:31 -0800 Subject: [PATCH 05/25] Code refactor for PR preparation --- .../application.institutions.controller.ts | 1 - .../models/confirmation-of-enrollment.dto.ts | 3 ++- .../sims-db/src/entities/application.model.ts | 3 ++- .../components/common/StudentApplication.vue | 15 +++++++----- .../web/src/composables/useFormioUtils.ts | 11 +++++++-- sources/packages/web/src/types/formio.ts | 2 +- .../src/views/aest/StudentApplicationView.vue | 23 ++++++------------- 7 files changed, 30 insertions(+), 28 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts index cb765778ec..23e1a7faaa 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.institutions.controller.ts @@ -53,7 +53,6 @@ export class ApplicationInstitutionsController extends BaseController { ); return this.applicationControllerService.transformToApplicationDTO( application, - application.data, ); } } diff --git a/sources/packages/backend/apps/api/src/route-controllers/confirmation-of-enrollment/models/confirmation-of-enrollment.dto.ts b/sources/packages/backend/apps/api/src/route-controllers/confirmation-of-enrollment/models/confirmation-of-enrollment.dto.ts index e7fe70decc..e5607cf333 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/confirmation-of-enrollment/models/confirmation-of-enrollment.dto.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/confirmation-of-enrollment/models/confirmation-of-enrollment.dto.ts @@ -3,6 +3,7 @@ import { COEStatus, DisabilityStatus, ProgramInfoStatus } from "@sims/sims-db"; import { COEApprovalPeriodStatus } from "../../../services/disbursement-schedule/disbursement-schedule.models"; import { MONEY_VALUE_FOR_UNKNOWN_MAX_VALUE } from "../../../utilities"; import { COE_DENIED_REASON_OTHER_ID } from "@sims/utilities"; +import { YesNoOptions } from "apps/api/src/services/education-program-offering/education-program-offering-import-csv.models"; export class ApplicationDetailsForCOEAPIOutDTO { applicationProgramName: string; @@ -34,7 +35,7 @@ export class ApplicationDetailsForCOEAPIOutDTO { maxTuitionRemittanceAllowed: number; hasOverawardBalance: boolean; disabilityProfileStatus: DisabilityStatus; - disabilityApplicationStatus: string; + disabilityApplicationStatus: YesNoOptions; } export class COEDeniedReasonAPIOutDTO { diff --git a/sources/packages/backend/libs/sims-db/src/entities/application.model.ts b/sources/packages/backend/libs/sims-db/src/entities/application.model.ts index 331d4b5cd6..f3d677ebf8 100644 --- a/sources/packages/backend/libs/sims-db/src/entities/application.model.ts +++ b/sources/packages/backend/libs/sims-db/src/entities/application.model.ts @@ -29,6 +29,7 @@ import { Student } from "./student.model"; import { ProgramYear } from "./program-year.model"; import { StudentAssessment } from "./student-assessment.model"; import { ApplicationException } from "./application-exceptions.model"; +import { YesNoOptions } from "@sims/test-utils"; export const APPLICATION_NUMBER_LENGTH = 10; export const SIN_NUMBER_LENGTH = 9; @@ -428,7 +429,7 @@ export interface ApplicationData { /** * Application PD/PPD Status. */ - applicationPDPPDStatus?: string; + applicationPDPPDStatus?: YesNoOptions; /** * Indigenous person status. */ diff --git a/sources/packages/web/src/components/common/StudentApplication.vue b/sources/packages/web/src/components/common/StudentApplication.vue index 855fa5a33c..9183b4d6ef 100644 --- a/sources/packages/web/src/components/common/StudentApplication.vue +++ b/sources/packages/web/src/components/common/StudentApplication.vue @@ -125,7 +125,7 @@ export default defineComponent({ () => !props.notDraft && !isFirstPage.value && !props.processing, ); - const getSelectedId = (form: any) => { + const getSelectedId = (form: FormIOForm) => { return formioUtils.getComponentValueByKey(form, LOCATIONS_DROPDOWN_KEY); }; @@ -183,11 +183,14 @@ export default defineComponent({ } }; - const formRender = async (form: any) => { + /** + * The form is done rendering and has completed the attach phase. + */ + const formRender = async (form: FormIOForm) => { context.emit("render", form); }; - const formLoaded = async (form: any) => { + const formLoaded = async (form: FormIOForm) => { showNav.value = true; // Emit formLoadedCallback event to the parent, so that parent can // perform the parent specific logic inside parent on @@ -227,7 +230,7 @@ export default defineComponent({ await loadFormDependencies(); }; - const getOfferingDetails = async (form: any, locationId: number) => { + const getOfferingDetails = async (form: FormIOForm, locationId: number) => { const selectedIntensity: OfferingIntensity = formioUtils.getComponentValueByKey(form, OFFERING_INTENSITY_KEY); const educationProgramIdFromForm: number = @@ -344,11 +347,11 @@ export default defineComponent({ formInstance.nextPage(); }; - const submitted = (args: any, form: any) => { + const submitted = (args: any, form: FormIOForm) => { context.emit("submitApplication", args, form); }; - const customEvent = (form: any, event: FormIOCustomEvent) => { + const customEvent = (form: FormIOForm, event: FormIOCustomEvent) => { context.emit("customEventCallback", form, event); }; diff --git a/sources/packages/web/src/composables/useFormioUtils.ts b/sources/packages/web/src/composables/useFormioUtils.ts index a29e6b4c4e..fe0a69cbbc 100644 --- a/sources/packages/web/src/composables/useFormioUtils.ts +++ b/sources/packages/web/src/composables/useFormioUtils.ts @@ -71,8 +71,6 @@ export function useFormioUtils() { ) => { const stopOnFirstMatch = options?.stopOnFirstMatch ?? false; components.forEach((component: any) => { - // console.log(component.key); - // console.log(component.component.key); if (stopOnFirstMatch && matchedComponents.length) { // If only the first match is needed, and one was found, stop searching. return; @@ -119,6 +117,15 @@ export function useFormioUtils() { return matchedComponents; }; + /** + * Search recursively by a component and returns + * all the matches with the same key. + * @param components components to be checked. + * @param componentKey key to be matched. + * @param options search options. + * - `stopOnFirstMatch` indicates if should stop at the first match, default true. + * @returns components found. + */ const searchByKey = ( components: FormIOComponent[], componentKey: string, diff --git a/sources/packages/web/src/types/formio.ts b/sources/packages/web/src/types/formio.ts index de92df1e3e..f62794e8fc 100644 --- a/sources/packages/web/src/types/formio.ts +++ b/sources/packages/web/src/types/formio.ts @@ -2,7 +2,7 @@ * FormIO form. Methods available can be checked on * https://help.form.io/developers/form-renderer. */ -export interface FormIOForm { +export interface FormIOForm extends FormIOComponent { data: T; nosubmit: boolean; checkValidity: ( diff --git a/sources/packages/web/src/views/aest/StudentApplicationView.vue b/sources/packages/web/src/views/aest/StudentApplicationView.vue index 2fcbf70337..ba7a1b1483 100644 --- a/sources/packages/web/src/views/aest/StudentApplicationView.vue +++ b/sources/packages/web/src/views/aest/StudentApplicationView.vue @@ -16,7 +16,6 @@ {{ emptyStringFiller(applicationDetail.applicationNumber) }} { - applicationWizard = form; - }; - + let applicationWizard: FormIOForm; /** * Happens when all the form components are rendered, including lists. */ - const formRender = async () => { + const formRender = async (form: FormIOForm) => { + applicationWizard = form; // Highlight changes in the form after all the components are rendered. // List components are ready only after the form is rendered. highlightChanges(); }; - + /** + * Loads the initial application data. + */ onMounted(async () => { const application = await ApplicationService.shared.getApplicationDetail( props.applicationId, @@ -88,7 +84,6 @@ export default defineComponent({ }; highlightChanges(); }); - /** * Check if the application has changes to be highlighted. * Changes are expected after applications are edited after submitted at least once. @@ -102,7 +97,6 @@ export default defineComponent({ applicationDetail.value.changes, ); } - /** * Apply the style class to the components that have changes. * @param parentComponent component to have the changes highlighted. @@ -145,8 +139,6 @@ export default defineComponent({ component: FormIOComponent, itemRemoved?: boolean, ) { - // TODO: should we check for changes in the read-only version? - // TODO: should we check only read-only versions? if ( component.type === FromIOComponentTypes.Hidden || component._visible === false @@ -158,7 +150,6 @@ export default defineComponent({ } return { - loadForm, formRender, applicationDetail, initialData, From 5012f5777cfbb552fe0e0cc47388c4ba4ae26bb0 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Wed, 8 Jan 2025 16:43:19 -0800 Subject: [PATCH 06/25] Enabled debug for unit tests --- .vscode/launch.json | 19 +++ sources/packages/backend/jest-unit-tests.json | 46 ++++++ .../unit/application-data-comparison.spec.ts | 143 +++++++++--------- .../application-data-comparison.ts | 8 +- sources/packages/backend/package.json | 56 +------ 5 files changed, 149 insertions(+), 123 deletions(-) create mode 100644 sources/packages/backend/jest-unit-tests.json diff --git a/.vscode/launch.json b/.vscode/launch.json index 8f2eb0b275..3ce137f26a 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -241,6 +241,25 @@ "ENVIRONMENT": "test", "TZ": "UTC" } + }, + { + "type": "node", + "request": "launch", + "name": "Unit tests - Current test file", + "program": "${workspaceFolder}/sources/packages/backend/node_modules/.bin/jest", + "args": [ + "${fileBasenameNoExtension}", + "--runInBand", + "--config", + "./sources/packages/backend/jest-unit-tests.json", + "--forceExit" + ], + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen", + "disableOptimisticBPs": true, + "windows": { + "program": "${workspaceFolder}/sources/packages/backend/node_modules/jest/bin/jest" + } } ] } diff --git a/sources/packages/backend/jest-unit-tests.json b/sources/packages/backend/jest-unit-tests.json new file mode 100644 index 0000000000..5db4b0e6ba --- /dev/null +++ b/sources/packages/backend/jest-unit-tests.json @@ -0,0 +1,46 @@ +{ + "moduleFileExtensions": [ + "js", + "json", + "ts" + ], + "rootDir": ".", + "testRegex": ".*\\.spec\\.ts$", + "transform": { + "^.+\\.(t|j)s$": "ts-jest" + }, + "collectCoverageFrom": [ + "**/*.(t|j)s" + ], + "coveragePathIgnorePatterns": [ + "node_modules", + "db-migrations", + "backend/workflow", + "_tests_", + "main.ts", + ".module.ts", + "coverage", + "test-utils", + "testHelpers", + ".e2e-spec.ts", + ".models.ts", + ".model.ts", + ".dto.ts", + ".controller.ts", + ".mock.ts" + ], + "coverageDirectory": "../../tests/coverage/unit-tests", + "testEnvironment": "node", + "roots": [ + "/libs/", + "/apps/" + ], + "moduleNameMapper": { + "^@sims/sims-db(|/.*)$": "/libs/sims-db/src/$1", + "^@sims/services(|/.*)$": "/libs/services/src/$1", + "^@sims/utilities(|/.*)$": "/libs/utilities/src/$1", + "^@sims/test-utils(|/.*)$": "/libs/test-utils/src/$1", + "^@sims/integrations(|/.*)$": "/libs/integrations/src/$1", + "^@sims/auth(|/.*)$": "/libs/auth/src/$1" + } +} \ No newline at end of file diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts index f1dc057d98..07b10e9fef 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts @@ -1,73 +1,31 @@ -// Target module import { compareApplicationData } from "../../application-data-comparison"; -import * as util from "util"; describe("Detect differences between student application data.", () => { - it("Should detected changes when there are changes.", () => { + it("Should return an empty array when no changes were detected.", () => { // Arrange - const dataA = { + const current = { bcResident: "yes", - dependants: [ - { - fullName: "a", - dateOfBirth: "2024-10-01", - declaredOnTaxes: "no", - attendingPostSecondarySchool: "no", - }, - ], - citizenship: "canadianCitizen", - courseDetails: [ - { - courseCode: "A", - courseName: "a", - courseEndDate: "2024-10-31", - courseStartDate: "2024-10-01", - }, - ], - hasDependents: "yes", + hasDependents: "no", calculatedTaxYear: 2023, - isFullTimeAllowed: "", - studentGivenNames: null, - myProgramNotListed: null, - programYearEndDate: "2025-07-31", - studentInfoConfirmed: true, - showParentInformation: null, - applicationPDPPDStatus: "no", myStudyPeriodIsntListed: { offeringnotListed: false, }, - selectedStudyEndDateBeforeSixWeeksFromToday: false, }; - const dataB = { + // Act + const result = compareApplicationData(current, current); + // Assert + console.log(JSON.stringify(result, null, 2)); + expect(result).toHaveLength(0); + }); + + it("Should detected changes when the changes happened in primitive values at the first level including null and undefined.", () => { + // Arrange + const current = { bcResident: "yes", - dependants: [ - { - fullName: "a", - dateOfBirth: "2024-10-01", - declaredOnTaxes: "no", - attendingPostSecondarySchool: "no", - }, - { - fullName: "b", - dateOfBirth: "2024-10-01", - declaredOnTaxes: "yes", - attendingPostSecondarySchool: "no", - }, - ], - citizenship: "canadianCitizen", - courseDetails: [ - { - courseCode: "A", - courseName: "a", - courseEndDate: "2024-10-31", - courseStartDate: "2024-10-01", - }, - ], - hasDependents: "yes", + hasDependents: "no", calculatedTaxYear: 2023, isFullTimeAllowed: "", studentGivenNames: null, - myProgramNotListed: null, programYearEndDate: "2025-07-31", studentInfoConfirmed: true, showParentInformation: null, @@ -75,26 +33,73 @@ describe("Detect differences between student application data.", () => { myStudyPeriodIsntListed: { offeringnotListed: false, }, - selectedStudyEndDateBeforeSixWeeksFromToday: false, + }; + const previous = { + bcResident: "changed yes", + hasDependents: "changed no", + calculatedTaxYear: 2000, + // isFullTimeAllowed was removed from the properties and should be detected as changed. + studentGivenNames: undefined, + programYearEndDate: "2025-07-31", + studentInfoConfirmed: true, + showParentInformation: true, + applicationPDPPDStatus: "changed no", + myStudyPeriodIsntListed: { + offeringnotListed: true, + }, }; // Act - const result = compareApplicationData(dataA, dataB); + const result = compareApplicationData(current, previous); // Assert - console.log(JSON.stringify(result, null, 2)); expect(result).toEqual([ { - key: "dependants", + key: "bcResident", + newValue: "yes", + oldValue: "changed yes", + changes: [], + }, + { + key: "hasDependents", + newValue: "no", + oldValue: "changed no", + changes: [], + }, + { + key: "calculatedTaxYear", + newValue: 2023, + oldValue: 2000, + changes: [], + }, + { + key: "isFullTimeAllowed", + newValue: "", + changes: [], + }, + { + key: "studentGivenNames", + newValue: null, + changes: [], + }, + { + key: "showParentInformation", + newValue: null, + oldValue: true, + changes: [], + }, + { + key: "applicationPDPPDStatus", + newValue: "no", + oldValue: "changed no", + changes: [], + }, + { + key: "myStudyPeriodIsntListed", changes: [ { - changes: [ - { - key: "declaredOnTaxes", - newValue: "no", - oldValue: "yes", - index: 0, - changes: [], - }, - ], + key: "offeringnotListed", + newValue: false, + oldValue: true, + changes: [], }, ], }, diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts index 38a800ce86..7bbdd5e4a4 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts @@ -8,7 +8,7 @@ export function compareApplicationData( const initialChanges = new ApplicationDataChange(); compareApplicationDataRecursive(currentData, previousData, initialChanges); const [root] = initialChanges.changes; - return root.changes; + return root?.changes ?? []; } function compareApplicationDataRecursive( @@ -23,8 +23,10 @@ function compareApplicationDataRecursive( } // Property has a null or undefined value but had a value before or vice versa. if ( - (currentData == null && previousData != null) || - (currentData != null && previousData == null) + (currentData === null && previousData !== null) || + (currentData !== null && previousData === null) || + (currentData === undefined && previousData !== undefined) || + (currentData !== undefined && previousData === undefined) ) { const newValueChange = new ApplicationDataChange( options?.propertyKey, diff --git a/sources/packages/backend/package.json b/sources/packages/backend/package.json index 13acf35885..d041aae051 100644 --- a/sources/packages/backend/package.json +++ b/sources/packages/backend/package.json @@ -21,10 +21,10 @@ "docker:start:queue-consumers": "node --enable-source-maps --max-old-space-size=550 dist/apps/queue-consumers/main.js", "start:prod:load-test-gateway": "node --enable-source-maps dist/apps/load-test-gateway/main.js", "docker:start:load-test-gateway": "node --enable-source-maps dist/apps/load-test-gateway/main.js", - "test": "cross-env ENVIRONMENT=test jest --verbose --forceExit", - "test:watch": "jest --watch ", - "test:cov": "cross-env ENVIRONMENT=test jest --coverage --forceExit", - "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand", + "test": "cross-env ENVIRONMENT=test jest --verbose --forceExit --config ./jest-unit-tests.json", + "test:watch": "jest --watch --config ./jest-unit-tests.json", + "test:cov": "cross-env ENVIRONMENT=test jest --coverage --forceExit --config ./jest-unit-tests.json", + "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand --config ./jest-unit-tests.json", "test:e2e:api": "npm run db:seed:test filter=CreateInstitutionsAndAuthenticationUsers,CreateAESTUsers,CreateStudentUsers,CreateRestrictions && cross-env ENVIRONMENT=test jest --collect-coverage --verbose --config ./apps/api/test/jest-e2e.json --forceExit", "test:e2e:api:local": "cross-env ENVIRONMENT=test TZ=UTC jest --config ./apps/api/test/jest-e2e.json --forceExit", "test:e2e:workers": "npm run migration:run && cross-env ENVIRONMENT=test jest --collect-coverage --verbose --config ./apps/workers/test/jest-e2e.json --forceExit", @@ -132,51 +132,5 @@ "ts-node": "^10.9.1", "tsconfig-paths": "^3.9.0", "typescript": "^4.5.5" - }, - "jest": { - "moduleFileExtensions": [ - "js", - "json", - "ts" - ], - "rootDir": ".", - "testRegex": ".*\\.spec\\.ts$", - "transform": { - "^.+\\.(t|j)s$": "ts-jest" - }, - "collectCoverageFrom": [ - "**/*.(t|j)s" - ], - "coveragePathIgnorePatterns": [ - "node_modules", - "db-migrations", - "backend/workflow", - "_tests_", - "main.ts", - ".module.ts", - "coverage", - "test-utils", - "testHelpers", - ".e2e-spec.ts", - ".models.ts", - ".model.ts", - ".dto.ts", - ".controller.ts", - ".mock.ts" - ], - "coverageDirectory": "../../tests/coverage/unit-tests", - "testEnvironment": "node", - "roots": [ - "/libs/", - "/apps/" - ], - "moduleNameMapper": { - "^@sims/sims-db(|/.*)$": "/libs/sims-db/src/$1", - "^@sims/services(|/.*)$": "/libs/services/src/$1", - "^@sims/utilities(|/.*)$": "/libs/utilities/src/$1", - "^@sims/test-utils(|/.*)$": "/libs/test-utils/src/$1", - "^@sims/integrations(|/.*)$": "/libs/integrations/src/$1", - "^@sims/auth(|/.*)$": "/libs/auth/src/$1" - } } -} +} \ No newline at end of file From 20f09e95cc0a3a09fc6f028625aea898f44efd21 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Thu, 9 Jan 2025 12:44:23 -0800 Subject: [PATCH 07/25] Added unit tests and DTO conversion --- .../application.controller.service.ts | 37 +++++- .../unit/application-data-comparison.spec.ts | 113 +++++++++++++++++- .../application-data-comparison.ts | 38 +++++- 3 files changed, 183 insertions(+), 5 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts index f38fbca904..18be4f143b 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts @@ -36,7 +36,7 @@ import { getPIRDeniedReason, getUserFullName, } from "../../utilities"; -import { getDateOnlyFormat } from "@sims/utilities"; +import { ApplicationDataChange, getDateOnlyFormat } from "@sims/utilities"; import { Application, ApplicationData, @@ -450,9 +450,13 @@ export class ApplicationControllerService { application: Application, previousData?: unknown, ): Promise { - let changes: ApplicationDataChangeAPIOutDTO[] = []; + const changes: ApplicationDataChangeAPIOutDTO[] = []; if (previousData) { - changes = compareApplicationData(application.data, previousData); + const applicationDataChanges = compareApplicationData( + application.data, + previousData, + ); + this.transformToApplicationChangesDTO(applicationDataChanges, changes); } return { data: application.data, @@ -473,6 +477,33 @@ export class ApplicationControllerService { }; } + /** + * Recursively converts the {@link ApplicationDataChange} service model to the + * DTO model {@link ApplicationDataChangeAPIOutDTO} which will ensure + * that only required properties will be returned from the API also preventing + * that future changes in the service model will not be directly returned. + * @param applicationDataChanges service model application changes. + * @param applicationDataChangeAPIOutDTO converted API DTO model. + */ + transformToApplicationChangesDTO( + applicationDataChanges: ApplicationDataChange[], + applicationDataChangeAPIOutDTO: ApplicationDataChangeAPIOutDTO[], + ): void { + applicationDataChanges.forEach((dataChange) => { + const dataChangeDTO: ApplicationDataChangeAPIOutDTO = { + key: dataChange.key, + index: dataChange.index, + itemsRemoved: dataChange.itemsRemoved, + changes: [], + }; + applicationDataChangeAPIOutDTO.push(dataChangeDTO); + this.transformToApplicationChangesDTO( + dataChange.changes, + dataChangeDTO.changes, + ); + }); + } + /** * Convert disbursements into the enrolment DTO with information * about first and second disbursements. diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts index 07b10e9fef..55244a70c3 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts @@ -18,7 +18,7 @@ describe("Detect differences between student application data.", () => { expect(result).toHaveLength(0); }); - it("Should detected changes when the changes happened in primitive values at the first level including null and undefined.", () => { + it("Should detected changes when the changes happened in primitive values at the first level including null to undefined or vice versa.", () => { // Arrange const current = { bcResident: "yes", @@ -105,4 +105,115 @@ describe("Detect differences between student application data.", () => { }, ]); }); + + it("Should detected items removed in an array when the current data array has fewer items than the previous.", () => { + // Arrange + const current = { + dependants: [ + { + fullName: "My son 1", + dateOfBirth: "2025-01-02", + }, + ], + }; + const previous = { + dependants: [ + { + fullName: "My son 1", + dateOfBirth: "2025-01-02", + }, + { + fullName: "My son 2", + dateOfBirth: "2025-01-02", + }, + ], + }; + // Act + const result = compareApplicationData(current, previous); + // Assert + expect(result).toEqual([ + { + key: "dependants", + changes: [], + itemsRemoved: true, + }, + ]); + }); + + it("Should detected items changed in an array at any level when the current data has at least one different property from the previous data.", () => { + // Arrange + const current = { + dependants: [ + { + fullName: "My son 1", + dateOfBirth: "2025-01-02", + someFilesArray: [ + { + url: "student/files/FileNameA-3370396b-8460-41d3-bb47-198ee84cc528.txt", + name: "FileNameA-3370396b-8460-41d3-bb47-198ee84cc528.txt", + originalName: "FileNameA.txt", + }, + { + url: "student/files/FileNameA-3370396b-8460-41d3-bb47-198ee84cc528.txt", + name: "FileNameA-3370396b-8460-41d3-bb47-198ee84cc528_SOMETHING_CHANGED.txt", + originalName: "FileNameA.txt", + }, + ], + }, + ], + }; + const previous = { + dependants: [ + { + fullName: "My son 1", + dateOfBirth: "2025-01-02", + someFilesArray: [ + { + url: "student/files/FileNameA-3370396b-8460-41d3-bb47-198ee84cc528.txt", + name: "FileNameA-3370396b-8460-41d3-bb47-198ee84cc528.txt", + originalName: "FileNameA.txt", + }, + { + url: "student/files/FileNameA-3370396b-8460-41d3-bb47-198ee84cc528.txt", + name: "FileNameA-3370396b-8460-41d3-bb47-198ee84cc528.txt", + originalName: "FileNameA.txt", + }, + ], + }, + ], + }; + // Act + const result = compareApplicationData(current, previous); + // Assert + expect(result).toEqual([ + { + key: "dependants", + changes: [ + { + index: 0, + changes: [ + { + key: "someFilesArray", + changes: [ + { + index: 1, + changes: [ + { + key: "name", + newValue: + "FileNameA-3370396b-8460-41d3-bb47-198ee84cc528_SOMETHING_CHANGED.txt", + oldValue: + "FileNameA-3370396b-8460-41d3-bb47-198ee84cc528.txt", + changes: [], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ]); + }); }); diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts index 7bbdd5e4a4..24eb2d4213 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts @@ -1,5 +1,14 @@ import { ApplicationDataChange } from "./application-data-comparison.models"; +/** + * Compares two sets of student application data and returns an array of changes. + * Identifies differences between the current and previous + * data, such as changes in values, additions, or deletions, and represents + * them as an array of {@link ApplicationDataChange} objects. + * @param currentData current state of the application data. + * @param previousData previous state of the application data. + * @returns array of {@link ApplicationDataChange} objects representing the detected changes. + */ export function compareApplicationData( currentData: unknown, previousData: unknown, @@ -11,6 +20,15 @@ export function compareApplicationData( return root?.changes ?? []; } +/** + * Recursively compares two sets of student application data and detect the differences. + * @param currentData current state of the student application data. + * @param previousData previous state of the student application data. + * @param parentChange parent change to add the new changes detected. + * @param options options to support the new change creation. + * - `propertyKey`: key of the property that changed. + * - `index`: index of the array item that changed. + */ function compareApplicationDataRecursive( currentData: unknown, previousData: unknown, @@ -59,6 +77,15 @@ function compareApplicationDataRecursive( parentChange.changes.push(newChange); } +/** + * Checks if an array has changes comparing the currentData and previousData arrays. + * @param currentData current data array. + * @param previousData previous data array. + * @param parentChange parent change that will have the new {@link ApplicationDataChange} added. + * @param options options to support the new change creation. + * - `propertyKey`: key of the property that changed. + * - `index`: index of the array item that changed. + */ function checkArrayChanges( currentData: unknown[], previousData: unknown[], @@ -69,7 +96,7 @@ function checkArrayChanges( if (!isEqual(currentData, previousData)) { arrayItemChange = new ApplicationDataChange(options?.propertyKey); parentChange.changes.push(arrayItemChange); - if (currentData.length > previousData.length) { + if (currentData.length < previousData.length) { arrayItemChange.itemsRemoved = true; } } @@ -88,6 +115,15 @@ function checkArrayChanges( } } +/** + * Checks if an object has changes comparing the currentData and previousData objects. + * @param currentData current data object. + * @param previousData previous data object. + * @param parentChange parent change that will have the new {@link ApplicationDataChange} added. + * @param options options to support the new change creation. + * - `propertyKey`: key of the property that changed. + * - `index`: index of the array item that changed. + */ function checkObjectChanges( currentData: unknown, previousData: unknown, From d7ec5131a013e2d3f47f7f8546433db9f3adf28e Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Thu, 9 Jan 2025 14:18:40 -0800 Subject: [PATCH 08/25] E2E tests --- ...ntroller.getApplicationDetails.e2e-spec.ts | 51 ++++++++++++++++++- .../application.aest.controller.ts | 20 +++++--- .../application.controller.service.ts | 12 +++-- .../application/models/application.dto.ts | 2 +- .../test-utils/src/factories/application.ts | 3 ++ 5 files changed, 75 insertions(+), 13 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts b/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts index 86a3718336..78325331b1 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts @@ -11,7 +11,12 @@ import { E2EDataSources, saveFakeApplication, } from "@sims/test-utils"; -import { EducationProgramOffering, OfferingIntensity } from "@sims/sims-db"; +import { + ApplicationData, + ApplicationStatus, + EducationProgramOffering, + OfferingIntensity, +} from "@sims/sims-db"; import { getUserFullName } from "../../../utilities"; import { addDays, getISODateOnlyString } from "@sims/utilities"; @@ -110,6 +115,50 @@ describe("ApplicationAESTController(e2e)-getApplicationDetails", () => { }); }); + it("Should get the student application changes when the application has a previous version and its dynamic data was changed.", async () => { + // Arrange + const previousApplication = await saveFakeApplication( + db.dataSource, + {}, + { + applicationStatus: ApplicationStatus.Overwritten, + applicationData: { + studystartDate: "2000-01-01", + studyendDate: "2000-01-31", + selectedOffering: 1, + studentNumber: "1234567", + } as ApplicationData, + }, + ); + const currentApplication = await saveFakeApplication( + db.dataSource, + {}, + { + applicationStatus: ApplicationStatus.Completed, + applicationNumber: previousApplication.applicationNumber, + applicationData: { + studystartDate: "2000-12-01", + studyendDate: "2000-12-31", + selectedOffering: 4, + } as ApplicationData, + }, + ); + const token = await getAESTToken(AESTGroups.BusinessAdministrators); + const endpoint = `/aest/application/${currentApplication.id}`; + + // Act/Assert + const response = await request(app.getHttpServer()) + .get(endpoint) + .auth(token, BEARER_AUTH_TYPE) + .expect(HttpStatus.OK); + expect(response.body.changes).toStrictEqual([ + { key: "studyendDate", changes: [] }, + { key: "studystartDate", changes: [] }, + { key: "selectedOffering", changes: [] }, + { key: "selectedOfferingName", changes: [] }, + ]); + }); + afterAll(async () => { await app?.close(); }); diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts index 651c1cb083..978acaf7ee 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts @@ -85,24 +85,32 @@ export class ApplicationAESTController extends BaseController { let currentReadOnlyData: ApplicationFormData; let previousReadOnlyData: ApplicationFormData; if (loadDynamicData) { + const currentReadOnlyDataPromise = + this.applicationControllerService.generateApplicationFormData( + application.data, + ); + // Check if a previous application exists. const previousApplicationVersion = await this.applicationService.getPreviousApplicationDataVersion( applicationId, application.applicationNumber, ); - [currentReadOnlyData, previousReadOnlyData] = await Promise.all([ - this.applicationControllerService.generateApplicationFormData( - application.data, - ), + // If there is a previous application, generate its read-only data. + const previousReadOnlyDataPromise = + previousApplicationVersion && this.applicationControllerService.generateApplicationFormData( previousApplicationVersion.data, - ), + ); + // Wait for both promises to resolve. + [currentReadOnlyData, previousReadOnlyData] = await Promise.all([ + currentReadOnlyDataPromise, + previousReadOnlyDataPromise, ]); application.data = currentReadOnlyData; } return this.applicationControllerService.transformToApplicationDTO( application, - previousReadOnlyData, + { previousData: previousReadOnlyData }, ); } diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts index 18be4f143b..82994363f8 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts @@ -443,19 +443,21 @@ export class ApplicationControllerService { /** * Transformation util for Application. * @param application application to be converted to the DTO. - * @param previousData previous application to allow changes detection. + * @param options additional options. + * - `previousData` previous application to allow changes detection. * @returns application DTO. */ async transformToApplicationDTO( application: Application, - previousData?: unknown, + options?: { previousData?: unknown }, ): Promise { - const changes: ApplicationDataChangeAPIOutDTO[] = []; - if (previousData) { + let changes: ApplicationDataChangeAPIOutDTO[]; + if (options?.previousData) { const applicationDataChanges = compareApplicationData( application.data, - previousData, + options.previousData, ); + changes = []; this.transformToApplicationChangesDTO(applicationDataChanges, changes); } return { diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts b/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts index f5ec304ff4..f3cb4c1b53 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts @@ -126,7 +126,7 @@ export class ApplicationSupplementalDataAPIOutDTO extends ApplicationBaseAPIOutD applicationStartDate?: string; applicationEndDate?: string; applicationInstitutionName?: string; - changes: ApplicationDataChangeAPIOutDTO[]; + changes?: ApplicationDataChangeAPIOutDTO[]; } export class ApplicationWithProgramYearAPIOutDTO { diff --git a/sources/packages/backend/libs/test-utils/src/factories/application.ts b/sources/packages/backend/libs/test-utils/src/factories/application.ts index 9b19df87a6..41b25db785 100644 --- a/sources/packages/backend/libs/test-utils/src/factories/application.ts +++ b/sources/packages/backend/libs/test-utils/src/factories/application.ts @@ -239,6 +239,7 @@ export async function saveFakeApplicationDisbursements( * - `programYear` related program year. * @param options additional options: * - `applicationStatus` application status for the application. + * - `applicationNumber` application number for the application. * - `offeringIntensity` if provided sets the offering intensity for the created fakeApplication, otherwise sets it to fulltime by default. * - `applicationData` related application data. * - `pirStatus` program info status. @@ -259,6 +260,7 @@ export async function saveFakeApplication( }, options?: { applicationStatus?: ApplicationStatus; + applicationNumber?: string; offeringIntensity?: OfferingIntensity; applicationData?: ApplicationData; pirStatus?: ProgramInfoStatus; @@ -294,6 +296,7 @@ export async function saveFakeApplication( { initialValue: { data: options?.applicationData, + applicationNumber: options?.applicationNumber, pirStatus: options?.pirStatus, isArchived: options?.isArchived ? options?.isArchived : false, }, From 168f042d9b53ba4f4694d322d028cacf9ed50a68 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Thu, 9 Jan 2025 14:20:24 -0800 Subject: [PATCH 09/25] Sonarcloud issues --- .../application/application.controller.service.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts index 82994363f8..d112571b54 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts @@ -36,7 +36,11 @@ import { getPIRDeniedReason, getUserFullName, } from "../../utilities"; -import { ApplicationDataChange, getDateOnlyFormat } from "@sims/utilities"; +import { + ApplicationDataChange, + getDateOnlyFormat, + compareApplicationData, +} from "@sims/utilities"; import { Application, ApplicationData, @@ -55,7 +59,6 @@ import { ApiProcessError } from "../../types"; import { ACTIVE_STUDENT_RESTRICTION } from "../../constants"; import { ECertPreValidationService } from "@sims/integrations/services/disbursement-schedule/e-cert-calculation"; import { AssessmentSequentialProcessingService } from "@sims/services"; -import { compareApplicationData } from "@sims/utilities"; /** * This service controller is a provider which is created to extract the implementation of From 5c39f11c3d6cf3c10d65100cbc2388186b87a535 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Thu, 9 Jan 2025 14:49:41 -0800 Subject: [PATCH 10/25] Reused method from another open PR --- .../application.aest.controller.ts | 6 +- .../application/application.service.ts | 61 ++++++++++--------- .../test-utils/src/factories/application.ts | 1 + 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts index 978acaf7ee..999d45d85d 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts @@ -90,10 +90,10 @@ export class ApplicationAESTController extends BaseController { application.data, ); // Check if a previous application exists. - const previousApplicationVersion = - await this.applicationService.getPreviousApplicationDataVersion( + const [previousApplicationVersion] = + await this.applicationService.getPreviousApplicationVersions( applicationId, - application.applicationNumber, + { loadDynamicData: true, limit: 1 }, ); // If there is a previous application, generate its read-only data. const previousReadOnlyDataPromise = diff --git a/sources/packages/backend/apps/api/src/services/application/application.service.ts b/sources/packages/backend/apps/api/src/services/application/application.service.ts index f18d151c4e..de2e0c847d 100644 --- a/sources/packages/backend/apps/api/src/services/application/application.service.ts +++ b/sources/packages/backend/apps/api/src/services/application/application.service.ts @@ -672,34 +672,6 @@ export class ApplicationService extends RecordDataModelService { }); } - async getPreviousApplicationDataVersion( - referenceApplicationId: number, - applicationNumber: string, - options?: { - studentId?: number; - institutionId?: number; - }, - ): Promise { - return this.repo.findOne({ - select: { - id: true, - data: true as unknown, - }, - where: { - id: LessThan(referenceApplicationId), - applicationNumber, - applicationStatus: ApplicationStatus.Overwritten, - student: { - id: options?.studentId, - }, - location: { institution: { id: options?.institutionId } }, - }, - order: { - id: "DESC", - }, - }); - } - /** * Get all student applications. * @param studentId student id. @@ -1872,6 +1844,39 @@ export class ApplicationService extends RecordDataModelService { }); } + /** + * Get previous application versions for an application. + * @param applicationId application id. + * @param options method options. + * - `limit` number of previous application versions to be returned. + * - `loadDynamicData` optionally loads the dynamic data. + * @returns previous application versions. + */ + async getPreviousApplicationVersions( + applicationId: number, + options?: { loadDynamicData?: boolean; limit?: number }, + ): Promise { + const application = await this.repo.findOne({ + select: { applicationNumber: true, submittedDate: true }, + where: { id: applicationId }, + }); + return this.repo.find({ + select: { + id: true, + submittedDate: true, + data: !!options?.loadDynamicData as unknown, + }, + where: { + submittedDate: LessThan(application.submittedDate), + applicationNumber: application.applicationNumber, + }, + order: { + submittedDate: "DESC", + }, + take: options?.limit, + }); + } + @InjectLogger() logger: LoggerService; } diff --git a/sources/packages/backend/libs/test-utils/src/factories/application.ts b/sources/packages/backend/libs/test-utils/src/factories/application.ts index 41b25db785..7c02ee5a8d 100644 --- a/sources/packages/backend/libs/test-utils/src/factories/application.ts +++ b/sources/packages/backend/libs/test-utils/src/factories/application.ts @@ -341,6 +341,7 @@ export async function saveFakeApplication( fakeOriginalAssessment, ); savedApplication.currentAssessment = savedOriginalAssessment; + savedApplication.submittedDate = new Date(); } else { savedApplication.location = null; } From ea1c3e86ec46db9d1424826e0c6fe41e91caa8c1 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Thu, 9 Jan 2025 15:38:49 -0800 Subject: [PATCH 11/25] Allow changes array to be null --- ...ntroller.getApplicationDetails.e2e-spec.ts | 24 +++++++++++++++---- .../application.controller.service.ts | 14 +++++++---- .../application/models/application.dto.ts | 2 +- .../src/services/http/dto/Application.dto.ts | 4 ++-- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts b/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts index 78325331b1..653a034207 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts @@ -127,6 +127,12 @@ describe("ApplicationAESTController(e2e)-getApplicationDetails", () => { studyendDate: "2000-01-31", selectedOffering: 1, studentNumber: "1234567", + courseDetails: [ + { + courseName: "courseName", + courseCode: "courseCode", + }, + ], } as ApplicationData, }, ); @@ -140,6 +146,12 @@ describe("ApplicationAESTController(e2e)-getApplicationDetails", () => { studystartDate: "2000-12-01", studyendDate: "2000-12-31", selectedOffering: 4, + courseDetails: [ + { + courseName: "courseName updated", + courseCode: "courseCode", + }, + ], } as ApplicationData, }, ); @@ -152,10 +164,14 @@ describe("ApplicationAESTController(e2e)-getApplicationDetails", () => { .auth(token, BEARER_AUTH_TYPE) .expect(HttpStatus.OK); expect(response.body.changes).toStrictEqual([ - { key: "studyendDate", changes: [] }, - { key: "studystartDate", changes: [] }, - { key: "selectedOffering", changes: [] }, - { key: "selectedOfferingName", changes: [] }, + { key: "studyendDate" }, + { + key: "courseDetails", + changes: [{ index: 0, changes: [{ key: "courseName" }] }], + }, + { key: "studystartDate" }, + { key: "selectedOffering" }, + { key: "selectedOfferingName" }, ]); }); diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts index d112571b54..31ff031622 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts @@ -460,8 +460,10 @@ export class ApplicationControllerService { application.data, options.previousData, ); - changes = []; - this.transformToApplicationChangesDTO(applicationDataChanges, changes); + if (applicationDataChanges.length) { + changes = []; + this.transformToApplicationChangesDTO(applicationDataChanges, changes); + } } return { data: application.data, @@ -493,15 +495,19 @@ export class ApplicationControllerService { transformToApplicationChangesDTO( applicationDataChanges: ApplicationDataChange[], applicationDataChangeAPIOutDTO: ApplicationDataChangeAPIOutDTO[], - ): void { + ) { applicationDataChanges.forEach((dataChange) => { const dataChangeDTO: ApplicationDataChangeAPIOutDTO = { key: dataChange.key, index: dataChange.index, itemsRemoved: dataChange.itemsRemoved, - changes: [], }; applicationDataChangeAPIOutDTO.push(dataChangeDTO); + // Check if there are nested changes + if (!dataChange.changes.length) { + return; + } + dataChangeDTO.changes = []; this.transformToApplicationChangesDTO( dataChange.changes, dataChangeDTO.changes, diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts b/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts index f3cb4c1b53..b0705dcd3e 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts @@ -117,7 +117,7 @@ export class ApplicationDataChangeAPIOutDTO { key?: string; index?: number; itemsRemoved?: boolean; - changes: ApplicationDataChangeAPIOutDTO[]; + changes?: ApplicationDataChangeAPIOutDTO[]; } export class ApplicationSupplementalDataAPIOutDTO extends ApplicationBaseAPIOutDTO { diff --git a/sources/packages/web/src/services/http/dto/Application.dto.ts b/sources/packages/web/src/services/http/dto/Application.dto.ts index 9bcaf1eea3..a54dd54174 100644 --- a/sources/packages/web/src/services/http/dto/Application.dto.ts +++ b/sources/packages/web/src/services/http/dto/Application.dto.ts @@ -90,7 +90,7 @@ export interface ApplicationDataChangeAPIOutDTO { key?: string; index?: number; itemsRemoved?: boolean; - changes: ApplicationDataChangeAPIOutDTO[]; + changes?: ApplicationDataChangeAPIOutDTO[]; } /** @@ -103,7 +103,7 @@ export interface ApplicationSupplementalDataAPIOutDTO applicationStartDate?: string; applicationEndDate?: string; applicationInstitutionName?: string; - changes: ApplicationDataChangeAPIOutDTO[]; + changes?: ApplicationDataChangeAPIOutDTO[]; } /** From 1648afd4a5dbb9f6581d581f6946c3755e333cf1 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Thu, 9 Jan 2025 16:55:39 -0800 Subject: [PATCH 12/25] Removed empty arrays from API result --- .../application.controller.service.ts | 2 +- .../application/models/application.dto.ts | 3 +- .../unit/application-data-comparison.spec.ts | 82 ++++++++++++++++++- .../application-data-comparison.models.ts | 40 +++++++-- .../application-data-comparison.ts | 46 ++++++----- .../src/services/http/dto/Application.dto.ts | 3 +- .../contracts/ApplicationDataComparison.ts | 14 ++++ sources/packages/web/src/types/index.ts | 1 + .../src/views/aest/StudentApplicationView.vue | 31 ++++--- 9 files changed, 179 insertions(+), 43 deletions(-) create mode 100644 sources/packages/web/src/types/contracts/ApplicationDataComparison.ts diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts index 31ff031622..754dd0a0b3 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts @@ -500,7 +500,7 @@ export class ApplicationControllerService { const dataChangeDTO: ApplicationDataChangeAPIOutDTO = { key: dataChange.key, index: dataChange.index, - itemsRemoved: dataChange.itemsRemoved, + changeType: dataChange.changeType, }; applicationDataChangeAPIOutDTO.push(dataChangeDTO); // Check if there are nested changes diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts b/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts index b0705dcd3e..5bf657fb5e 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/models/application.dto.ts @@ -19,6 +19,7 @@ import { import { JsonMaxSize } from "../../../utilities/class-validation"; import { JSON_20KB } from "../../../constants"; import { ECertFailedValidation } from "@sims/integrations/services/disbursement-schedule/disbursement-schedule.models"; +import { ChangeTypes } from "@sims/utilities"; export class SaveApplicationAPIInDTO { /** @@ -116,7 +117,7 @@ export class ApplicationDataAPIOutDTO extends ApplicationBaseAPIOutDTO { export class ApplicationDataChangeAPIOutDTO { key?: string; index?: number; - itemsRemoved?: boolean; + changeType: ChangeTypes; changes?: ApplicationDataChangeAPIOutDTO[]; } diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts index 55244a70c3..087de05b2a 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts @@ -11,8 +11,10 @@ describe("Detect differences between student application data.", () => { offeringnotListed: false, }, }; + // Act const result = compareApplicationData(current, current); + // Assert console.log(JSON.stringify(result, null, 2)); expect(result).toHaveLength(0); @@ -48,57 +50,81 @@ describe("Detect differences between student application data.", () => { offeringnotListed: true, }, }; + // Act const result = compareApplicationData(current, previous); + // Assert expect(result).toEqual([ { key: "bcResident", newValue: "yes", oldValue: "changed yes", + index: undefined, + changeType: "updated", changes: [], }, { key: "hasDependents", newValue: "no", oldValue: "changed no", + index: undefined, + changeType: "updated", changes: [], }, { key: "calculatedTaxYear", newValue: 2023, oldValue: 2000, + index: undefined, + changeType: "updated", changes: [], }, { key: "isFullTimeAllowed", newValue: "", + oldValue: undefined, + index: undefined, + changeType: "updated", changes: [], }, { key: "studentGivenNames", newValue: null, + oldValue: undefined, + index: undefined, + changeType: "updated", changes: [], }, { key: "showParentInformation", newValue: null, oldValue: true, + index: undefined, + changeType: "updated", changes: [], }, { key: "applicationPDPPDStatus", newValue: "no", oldValue: "changed no", + index: undefined, + changeType: "updated", changes: [], }, { key: "myStudyPeriodIsntListed", + newValue: undefined, + oldValue: undefined, + index: undefined, + changeType: "updated", changes: [ { key: "offeringnotListed", newValue: false, oldValue: true, + index: undefined, + changeType: "updated", changes: [], }, ], @@ -128,14 +154,16 @@ describe("Detect differences between student application data.", () => { }, ], }; + // Act const result = compareApplicationData(current, previous); + // Assert expect(result).toEqual([ { key: "dependants", + changeType: "itemsRemoved", changes: [], - itemsRemoved: true, }, ]); }); @@ -182,21 +210,27 @@ describe("Detect differences between student application data.", () => { }, ], }; + // Act const result = compareApplicationData(current, previous); + // Assert expect(result).toEqual([ { key: "dependants", + changeType: "updated", changes: [ { index: 0, + changeType: "updated", changes: [ { key: "someFilesArray", + changeType: "updated", changes: [ { index: 1, + changeType: "updated", changes: [ { key: "name", @@ -204,6 +238,7 @@ describe("Detect differences between student application data.", () => { "FileNameA-3370396b-8460-41d3-bb47-198ee84cc528_SOMETHING_CHANGED.txt", oldValue: "FileNameA-3370396b-8460-41d3-bb47-198ee84cc528.txt", + changeType: "updated", changes: [], }, ], @@ -216,4 +251,49 @@ describe("Detect differences between student application data.", () => { }, ]); }); + + it("Should detected an object with removed properties in the current data when these objects belongs to an array.", () => { + // Arrange + const current = { + dependants: [ + { + fullName: "My son 1", + dateOfBirth: "2025-01-01", + }, + { + fullName: "My son 2", + }, + ], + }; + const previous = { + dependants: [ + { + fullName: "My son 1", + dateOfBirth: "2025-01-01", + }, + { + fullName: "My son 2", + dateOfBirth: "2025-01-02", + }, + ], + }; + + // Act + const result = compareApplicationData(current, previous); + + // Assert + expect(result).toEqual([ + { + key: "dependants", + changeType: "updated", + changes: [ + { + index: 1, + changeType: "propertiesRemoved", + changes: [], + }, + ], + }, + ]); + }); }); diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts index 9baaa2cc75..bbbc10e941 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts @@ -1,12 +1,38 @@ +export enum ChangeTypes { + /** + * Indicates that a new value was added or updated. + */ + Update = "updated", + /** + * An array had at least one property removed. + */ + ItemsRemoved = "itemsRemoved", + /** + * An array had at least one property removed. + */ + PropertiesRemoved = "propertiesRemoved", +} + export class ApplicationDataChange { - constructor( - readonly key?: string, - readonly newValue?: unknown, - readonly oldValue?: unknown, - readonly index?: number, - ) { + readonly key?: string; + readonly newValue?: unknown; + readonly oldValue?: unknown; + readonly index?: number; + + constructor(options?: { + key?: string; + newValue?: unknown; + oldValue?: unknown; + index?: number; + }) { + this.key = options?.key; + this.newValue = options?.newValue; + this.oldValue = options?.oldValue; + this.index = options?.index; + this.changeType = ChangeTypes.Update; this.changes = []; } - itemsRemoved?: boolean; + + changeType: ChangeTypes; changes: ApplicationDataChange[]; } diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts index 24eb2d4213..d67a6c6cf8 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts @@ -1,4 +1,7 @@ -import { ApplicationDataChange } from "./application-data-comparison.models"; +import { + ApplicationDataChange, + ChangeTypes, +} from "./application-data-comparison.models"; /** * Compares two sets of student application data and returns an array of changes. @@ -46,12 +49,12 @@ function compareApplicationDataRecursive( (currentData === undefined && previousData !== undefined) || (currentData !== undefined && previousData === undefined) ) { - const newValueChange = new ApplicationDataChange( - options?.propertyKey, - currentData, - previousData, - options?.index, - ); + const newValueChange = new ApplicationDataChange({ + key: options?.propertyKey, + newValue: currentData, + oldValue: previousData, + index: options?.index, + }); parentChange.changes.push(newValueChange); return; } @@ -68,12 +71,12 @@ function compareApplicationDataRecursive( return; } // Property was changed and it is a leaf property. - const newChange = new ApplicationDataChange( - options.propertyKey, - currentData, - previousData, - options?.index, - ); + const newChange = new ApplicationDataChange({ + key: options?.propertyKey, + newValue: currentData, + oldValue: previousData, + index: options?.index, + }); parentChange.changes.push(newChange); } @@ -94,10 +97,10 @@ function checkArrayChanges( ): void { let arrayItemChange: ApplicationDataChange; if (!isEqual(currentData, previousData)) { - arrayItemChange = new ApplicationDataChange(options?.propertyKey); + arrayItemChange = new ApplicationDataChange({ key: options?.propertyKey }); parentChange.changes.push(arrayItemChange); if (currentData.length < previousData.length) { - arrayItemChange.itemsRemoved = true; + arrayItemChange.changeType = ChangeTypes.ItemsRemoved; } } // Property is an array that should have its items checked. @@ -132,12 +135,13 @@ function checkObjectChanges( ): void { // Property is a object that should have its properties checked. if (!isEqual(currentData, previousData)) { - const objectPropertyChange = new ApplicationDataChange( - options?.propertyKey, - undefined, - undefined, - options?.index, - ); + const objectPropertyChange = new ApplicationDataChange({ + key: options?.propertyKey, + index: options?.index, + }); + if (Object.keys(currentData).length < Object.keys(previousData).length) { + objectPropertyChange.changeType = ChangeTypes.PropertiesRemoved; + } parentChange.changes.push(objectPropertyChange); for (const propertyKey of Object.keys(currentData)) { compareApplicationDataRecursive( diff --git a/sources/packages/web/src/services/http/dto/Application.dto.ts b/sources/packages/web/src/services/http/dto/Application.dto.ts index a54dd54174..f10f8d3cf3 100644 --- a/sources/packages/web/src/services/http/dto/Application.dto.ts +++ b/sources/packages/web/src/services/http/dto/Application.dto.ts @@ -13,6 +13,7 @@ import { ApplicationOfferingChangeRequestStatus, StudentAssessmentStatus, ECertFailedValidation, + ChangeTypes, } from "@/types"; export interface InProgressApplicationDetailsAPIOutDTO { @@ -89,7 +90,7 @@ export interface ApplicationDataAPIOutDTO extends ApplicationBaseAPIOutDTO { export interface ApplicationDataChangeAPIOutDTO { key?: string; index?: number; - itemsRemoved?: boolean; + changeType: ChangeTypes; changes?: ApplicationDataChangeAPIOutDTO[]; } diff --git a/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts b/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts new file mode 100644 index 0000000000..9398625b1c --- /dev/null +++ b/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts @@ -0,0 +1,14 @@ +export enum ChangeTypes { + /** + * Indicates that a new value was added or updated. + */ + Update = "updated", + /** + * An array had at least one property removed. + */ + ItemsRemoved = "itemsRemoved", + /** + * An array had at least one property removed. + */ + PropertiesRemoved = "propertiesRemoved", +} diff --git a/sources/packages/web/src/types/index.ts b/sources/packages/web/src/types/index.ts index b33213034d..5d355f2963 100644 --- a/sources/packages/web/src/types/index.ts +++ b/sources/packages/web/src/types/index.ts @@ -46,3 +46,4 @@ export * from "@/types/contracts/StatusChip"; export * from "@/types/contracts/ApplicationOfferingChangeRequest"; export * from "@/types/contracts/ECertFailedValidation"; export * from "@/types/contracts/RestrictionBypassContracts"; +export * from "@/types/contracts/ApplicationDataComparison"; diff --git a/sources/packages/web/src/views/aest/StudentApplicationView.vue b/sources/packages/web/src/views/aest/StudentApplicationView.vue index ba7a1b1483..24f31d9da0 100644 --- a/sources/packages/web/src/views/aest/StudentApplicationView.vue +++ b/sources/packages/web/src/views/aest/StudentApplicationView.vue @@ -36,7 +36,12 @@ import { ApplicationService } from "@/services/ApplicationService"; import { useFormatters } from "@/composables/useFormatters"; import StudentApplication from "@/components/common/StudentApplication.vue"; import { useFormioUtils } from "@/composables"; -import { FormIOComponent, FormIOForm, FromIOComponentTypes } from "@/types"; +import { + ChangeTypes, + FormIOComponent, + FormIOForm, + FromIOComponentTypes, +} from "@/types"; export default defineComponent({ components: { @@ -89,7 +94,7 @@ export default defineComponent({ * Changes are expected after applications are edited after submitted at least once. */ function highlightChanges() { - if (!applicationWizard || !applicationDetail.value.changes.length) { + if (!applicationWizard || !applicationDetail.value.changes?.length) { return; } highlightChangesRecursive( @@ -104,8 +109,13 @@ export default defineComponent({ */ function highlightChangesRecursive( parentComponent: FormIOComponent, - changes: ApplicationDataChangeAPIOutDTO[], + changes?: ApplicationDataChangeAPIOutDTO[], ) { + if (!changes?.length) { + // Since the method is called recursively, the check is executed at this level + // instead of every time this method is called. + return; + } for (const change of changes) { let searchComponent: FormIOComponent | undefined; if (typeof change.index === "number") { @@ -114,16 +124,12 @@ export default defineComponent({ } else if (change.key) { searchComponent = parentComponent; } - if ( - !change.key || - !searchComponent || - !searchComponent.components?.length - ) { + if (!change.key || !searchComponent?.components?.length) { continue; } const [component] = searchByKey(searchComponent.components, change.key); if (component) { - applyChangedValueStyleClass(component, change.itemsRemoved); + applyChangedValueStyleClass(component, change.changeType); highlightChangesRecursive(component, change.changes); } } @@ -137,7 +143,7 @@ export default defineComponent({ */ function applyChangedValueStyleClass( component: FormIOComponent, - itemRemoved?: boolean, + changeType: ChangeTypes, ) { if ( component.type === FromIOComponentTypes.Hidden || @@ -145,7 +151,10 @@ export default defineComponent({ ) { return; } - const cssClass = itemRemoved ? "changed-list-length" : "changed-value"; + const cssClass = + changeType === ChangeTypes.ItemsRemoved + ? "changed-list-length" + : "changed-value"; document.getElementById(component.id)?.classList.add(cssClass); } From 094f568c2960d6e9877d4540fa74145e192e27cb Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Fri, 10 Jan 2025 08:33:41 -0800 Subject: [PATCH 13/25] Minor unit test renaming --- ... application-data-comparison.compareApplicationData.spec.ts} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/{application-data-comparison.spec.ts => application-data-comparison.compareApplicationData.spec.ts} (99%) diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts similarity index 99% rename from sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts rename to sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts index 087de05b2a..bc8119b104 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.spec.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts @@ -1,6 +1,6 @@ import { compareApplicationData } from "../../application-data-comparison"; -describe("Detect differences between student application data.", () => { +describe("compareApplicationData", () => { it("Should return an empty array when no changes were detected.", () => { // Arrange const current = { From 69a87f3dc8b1005087f109506e653cdf7f507e3a Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Fri, 10 Jan 2025 08:57:14 -0800 Subject: [PATCH 14/25] Fix E2E test result --- ...ntroller.getApplicationDetails.e2e-spec.ts | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts b/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts index 653a034207..a10c20d20c 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/_tests_/application.aest.controller.getApplicationDetails.e2e-spec.ts @@ -164,14 +164,38 @@ describe("ApplicationAESTController(e2e)-getApplicationDetails", () => { .auth(token, BEARER_AUTH_TYPE) .expect(HttpStatus.OK); expect(response.body.changes).toStrictEqual([ - { key: "studyendDate" }, { + changeType: "updated", + key: "studyendDate", + }, + { + changeType: "updated", + changes: [ + { + changeType: "updated", + changes: [ + { + changeType: "updated", + key: "courseName", + }, + ], + index: 0, + }, + ], key: "courseDetails", - changes: [{ index: 0, changes: [{ key: "courseName" }] }], }, - { key: "studystartDate" }, - { key: "selectedOffering" }, - { key: "selectedOfferingName" }, + { + changeType: "updated", + key: "studystartDate", + }, + { + changeType: "updated", + key: "selectedOffering", + }, + { + changeType: "updated", + key: "selectedOfferingName", + }, ]); }); From 19be56b487e3a5937c2e1332d534034cb0296259 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Fri, 10 Jan 2025 09:18:39 -0800 Subject: [PATCH 15/25] Pre-review adjustments --- ...-data-comparison.compareApplicationData.spec.ts | 14 -------------- .../application-data-comparison.models.ts | 2 +- .../application-data-comparison.ts | 2 +- .../packages/web/src/assets/css/formio-shared.scss | 6 +++--- .../web/src/assets/css/global-style-variables.scss | 1 + .../types/contracts/ApplicationDataComparison.ts | 2 +- 6 files changed, 7 insertions(+), 20 deletions(-) diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts index bc8119b104..1ae849f58b 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts @@ -16,7 +16,6 @@ describe("compareApplicationData", () => { const result = compareApplicationData(current, current); // Assert - console.log(JSON.stringify(result, null, 2)); expect(result).toHaveLength(0); }); @@ -60,7 +59,6 @@ describe("compareApplicationData", () => { key: "bcResident", newValue: "yes", oldValue: "changed yes", - index: undefined, changeType: "updated", changes: [], }, @@ -68,7 +66,6 @@ describe("compareApplicationData", () => { key: "hasDependents", newValue: "no", oldValue: "changed no", - index: undefined, changeType: "updated", changes: [], }, @@ -76,23 +73,18 @@ describe("compareApplicationData", () => { key: "calculatedTaxYear", newValue: 2023, oldValue: 2000, - index: undefined, changeType: "updated", changes: [], }, { key: "isFullTimeAllowed", newValue: "", - oldValue: undefined, - index: undefined, changeType: "updated", changes: [], }, { key: "studentGivenNames", newValue: null, - oldValue: undefined, - index: undefined, changeType: "updated", changes: [], }, @@ -100,7 +92,6 @@ describe("compareApplicationData", () => { key: "showParentInformation", newValue: null, oldValue: true, - index: undefined, changeType: "updated", changes: [], }, @@ -108,22 +99,17 @@ describe("compareApplicationData", () => { key: "applicationPDPPDStatus", newValue: "no", oldValue: "changed no", - index: undefined, changeType: "updated", changes: [], }, { key: "myStudyPeriodIsntListed", - newValue: undefined, - oldValue: undefined, - index: undefined, changeType: "updated", changes: [ { key: "offeringnotListed", newValue: false, oldValue: true, - index: undefined, changeType: "updated", changes: [], }, diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts index bbbc10e941..431c7993f8 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts @@ -8,7 +8,7 @@ export enum ChangeTypes { */ ItemsRemoved = "itemsRemoved", /** - * An array had at least one property removed. + * An object had at least one property removed. */ PropertiesRemoved = "propertiesRemoved", } diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts index d67a6c6cf8..95340f5ac2 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts @@ -133,7 +133,7 @@ function checkObjectChanges( parentChange: ApplicationDataChange, options?: { propertyKey?: string; index?: number }, ): void { - // Property is a object that should have its properties checked. + // Property is an object that should have its properties checked. if (!isEqual(currentData, previousData)) { const objectPropertyChange = new ApplicationDataChange({ key: options?.propertyKey, diff --git a/sources/packages/web/src/assets/css/formio-shared.scss b/sources/packages/web/src/assets/css/formio-shared.scss index ab62b2c63c..6258506082 100644 --- a/sources/packages/web/src/assets/css/formio-shared.scss +++ b/sources/packages/web/src/assets/css/formio-shared.scss @@ -754,15 +754,15 @@ td .formio-custom-tooltip:hover .tooltip-text { // Styles related to the application changes // between the current and previous application versions. .changed-value-base { - color: #dc2525; + color: $application-changed-value-color; border-radius: 5px; - border: 1px solid #dc2525; + border: 1px solid $application-changed-value-color; padding: 4px; } .changed-value-content-base { @extend .label-small-bold; - background: #dc2525; + background: $application-changed-value-color; color: white; border-radius: 5px; display: block; diff --git a/sources/packages/web/src/assets/css/global-style-variables.scss b/sources/packages/web/src/assets/css/global-style-variables.scss index 767ccb2090..8e3b49242e 100644 --- a/sources/packages/web/src/assets/css/global-style-variables.scss +++ b/sources/packages/web/src/assets/css/global-style-variables.scss @@ -27,6 +27,7 @@ $secondary-header-color-lt: #8692a4; $app-content-color: #333a47; $panel-warning-color: #f3b229; $panel-error-color: #dc2525; +$application-changed-value-color: #dc2525; $button-font-color: #ffffff; /** SCSS Font Size variables **/ diff --git a/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts b/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts index 9398625b1c..8cd889502a 100644 --- a/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts +++ b/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts @@ -8,7 +8,7 @@ export enum ChangeTypes { */ ItemsRemoved = "itemsRemoved", /** - * An array had at least one property removed. + * An object had at least one property removed. */ PropertiesRemoved = "propertiesRemoved", } From 4356d2d0d77b957863b3f9a021832840e17aaab4 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Fri, 10 Jan 2025 11:27:20 -0800 Subject: [PATCH 16/25] Fix highlight logic --- .../web/src/views/aest/StudentApplicationView.vue | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sources/packages/web/src/views/aest/StudentApplicationView.vue b/sources/packages/web/src/views/aest/StudentApplicationView.vue index 24f31d9da0..f5a404d986 100644 --- a/sources/packages/web/src/views/aest/StudentApplicationView.vue +++ b/sources/packages/web/src/views/aest/StudentApplicationView.vue @@ -108,18 +108,20 @@ export default defineComponent({ * @param changes list of changes to be highlighted. */ function highlightChangesRecursive( - parentComponent: FormIOComponent, + parentComponent?: FormIOComponent, changes?: ApplicationDataChangeAPIOutDTO[], ) { - if (!changes?.length) { - // Since the method is called recursively, the check is executed at this level + if (!parentComponent || !changes?.length) { + // Since the method is called recursively, the checks are executed at this level // instead of every time this method is called. return; } for (const change of changes) { let searchComponent: FormIOComponent | undefined; if (typeof change.index === "number") { - searchComponent = parentComponent.components[change.index]; + searchComponent = parentComponent?.components?.length + ? parentComponent.components[change.index] + : undefined; highlightChangesRecursive(searchComponent, change.changes); } else if (change.key) { searchComponent = parentComponent; From 32e42a4f666103f3b0d59269a085250ed7d129aa Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Fri, 10 Jan 2025 16:19:49 -0800 Subject: [PATCH 17/25] Changed behavior for child list items --- .../application-data-comparison.models.ts | 4 ++ .../application-data-comparison.ts | 2 + .../web/src/assets/css/formio-shared.scss | 13 ++++++- .../contracts/ApplicationDataComparison.ts | 4 ++ .../src/views/aest/StudentApplicationView.vue | 39 ++++++++++++------- 5 files changed, 47 insertions(+), 15 deletions(-) diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts index 431c7993f8..e7a464d14e 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts @@ -7,6 +7,10 @@ export enum ChangeTypes { * An array had at least one property removed. */ ItemsRemoved = "itemsRemoved", + /** + * Items were added to the end of the array. + */ + ItemsAppended = "itemsAppended", /** * An object had at least one property removed. */ diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts index 95340f5ac2..ddf39ea7f9 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts @@ -101,6 +101,8 @@ function checkArrayChanges( parentChange.changes.push(arrayItemChange); if (currentData.length < previousData.length) { arrayItemChange.changeType = ChangeTypes.ItemsRemoved; + } else if (currentData.length > previousData.length) { + arrayItemChange.changeType = ChangeTypes.ItemsAppended; } } // Property is an array that should have its items checked. diff --git a/sources/packages/web/src/assets/css/formio-shared.scss b/sources/packages/web/src/assets/css/formio-shared.scss index 6258506082..6c84782443 100644 --- a/sources/packages/web/src/assets/css/formio-shared.scss +++ b/sources/packages/web/src/assets/css/formio-shared.scss @@ -777,11 +777,20 @@ td .formio-custom-tooltip:hover .tooltip-text { content: "\00a0\00a0Updated"; } -.changed-list-length { +.changed-list-item-removed { @extend .changed-value-base; } -.changed-list-length::before { +.changed-list-item-removed::before { @extend .changed-value-content-base; content: "\00a0\00a0Items were removed from this list"; } + +.changed-list-item-appended { + @extend .changed-value-base; +} + +.changed-list-item-appended::before { + @extend .changed-value-content-base; + content: "\00a0\00a0Items were appended to the list"; +} diff --git a/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts b/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts index 8cd889502a..578c552472 100644 --- a/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts +++ b/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts @@ -7,6 +7,10 @@ export enum ChangeTypes { * An array had at least one property removed. */ ItemsRemoved = "itemsRemoved", + /** + * Items were added to the end of the array. + */ + ItemsAppended = "itemsAppended", /** * An object had at least one property removed. */ diff --git a/sources/packages/web/src/views/aest/StudentApplicationView.vue b/sources/packages/web/src/views/aest/StudentApplicationView.vue index f5a404d986..b4cc3830aa 100644 --- a/sources/packages/web/src/views/aest/StudentApplicationView.vue +++ b/sources/packages/web/src/views/aest/StudentApplicationView.vue @@ -108,21 +108,23 @@ export default defineComponent({ * @param changes list of changes to be highlighted. */ function highlightChangesRecursive( - parentComponent?: FormIOComponent, - changes?: ApplicationDataChangeAPIOutDTO[], + parentComponent: FormIOComponent, + changes: ApplicationDataChangeAPIOutDTO[], ) { - if (!parentComponent || !changes?.length) { - // Since the method is called recursively, the checks are executed at this level - // instead of every time this method is called. - return; - } for (const change of changes) { let searchComponent: FormIOComponent | undefined; if (typeof change.index === "number") { searchComponent = parentComponent?.components?.length ? parentComponent.components[change.index] : undefined; - highlightChangesRecursive(searchComponent, change.changes); + if (searchComponent && change.changes) { + // Should check further for nested changes. + highlightChangesRecursive(searchComponent, change.changes); + } else if (searchComponent) { + // searchComponent has a change, but no nested changes. + // It also does not have a key because it is a child in a list. + applyChangedValueStyleClass(searchComponent, change.changeType); + } } else if (change.key) { searchComponent = parentComponent; } @@ -132,7 +134,10 @@ export default defineComponent({ const [component] = searchByKey(searchComponent.components, change.key); if (component) { applyChangedValueStyleClass(component, change.changeType); - highlightChangesRecursive(component, change.changes); + if (change.changes) { + // Should check further for nested changes. + highlightChangesRecursive(component, change.changes); + } } } } @@ -153,10 +158,18 @@ export default defineComponent({ ) { return; } - const cssClass = - changeType === ChangeTypes.ItemsRemoved - ? "changed-list-length" - : "changed-value"; + let cssClass: string; + switch (changeType) { + case ChangeTypes.ItemsAppended: + cssClass = "changed-list-item-appended"; + break; + case ChangeTypes.ItemsRemoved: + cssClass = "changed-list-item-removed"; + break; + default: + cssClass = "changed-value"; + break; + } document.getElementById(component.id)?.classList.add(cssClass); } From 40ac7c9db0614302700875b5db7a9ad0801a2a80 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Mon, 13 Jan 2025 08:54:25 -0800 Subject: [PATCH 18/25] Added void --- .../application/application.controller.service.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts index 754dd0a0b3..5f93075d84 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.controller.service.ts @@ -360,7 +360,7 @@ export class ApplicationControllerService { private async processSelectedLocation( data: ApplicationData, additionalFormData: ApplicationFormData, - ) { + ): Promise { if (data.selectedLocation) { const designatedLocation = await this.locationService.getLocation( data.selectedLocation, @@ -383,7 +383,7 @@ export class ApplicationControllerService { private async processSelectedProgram( data: ApplicationData, additionalFormData: ApplicationFormData, - ) { + ): Promise { if (data.selectedProgram) { const selectedProgram = await this.programService.getProgramById( data.selectedProgram, @@ -429,7 +429,7 @@ export class ApplicationControllerService { private async processSelectedOffering( data: ApplicationData, additionalFormData: ApplicationFormData, - ) { + ): Promise { if (data.selectedOffering) { const selectedOffering = await this.offeringService.getOfferingById( data.selectedOffering, @@ -495,7 +495,7 @@ export class ApplicationControllerService { transformToApplicationChangesDTO( applicationDataChanges: ApplicationDataChange[], applicationDataChangeAPIOutDTO: ApplicationDataChangeAPIOutDTO[], - ) { + ): void { applicationDataChanges.forEach((dataChange) => { const dataChangeDTO: ApplicationDataChangeAPIOutDTO = { key: dataChange.key, From 87cc4ddb34847ce6bb698a42009853e4ea90a4a2 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Mon, 13 Jan 2025 09:19:20 -0800 Subject: [PATCH 19/25] Post merge fixes and sonarcloud issue --- .../application/application.service.ts | 8 ---- .../test-utils/src/factories/application.ts | 1 - .../application-data-comparison.ts | 9 ++-- .../src/views/aest/StudentApplicationView.vue | 48 ++++++++++++++----- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/sources/packages/backend/apps/api/src/services/application/application.service.ts b/sources/packages/backend/apps/api/src/services/application/application.service.ts index a8ad67becd..8acc44f16b 100644 --- a/sources/packages/backend/apps/api/src/services/application/application.service.ts +++ b/sources/packages/backend/apps/api/src/services/application/application.service.ts @@ -7,14 +7,6 @@ import { EntityManager, LessThan, } from "typeorm"; -import { - DataSource, - In, - Not, - Brackets, - EntityManager, - LessThan, -} from "typeorm"; import { LoggerService, InjectLogger } from "@sims/utilities/logger"; import { RecordDataModelService, diff --git a/sources/packages/backend/libs/test-utils/src/factories/application.ts b/sources/packages/backend/libs/test-utils/src/factories/application.ts index 23ddc39d38..7df5a3df0c 100644 --- a/sources/packages/backend/libs/test-utils/src/factories/application.ts +++ b/sources/packages/backend/libs/test-utils/src/factories/application.ts @@ -301,7 +301,6 @@ export async function saveFakeApplication( applicationNumber: options?.applicationNumber, pirStatus: options?.pirStatus, isArchived: options?.isArchived ? options?.isArchived : false, - applicationNumber: options?.applicationNumber, submittedDate: options?.submittedDate, }, }, diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts index ddf39ea7f9..eba88c4b7a 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts @@ -44,10 +44,11 @@ function compareApplicationDataRecursive( } // Property has a null or undefined value but had a value before or vice versa. if ( - (currentData === null && previousData !== null) || - (currentData !== null && previousData === null) || - (currentData === undefined && previousData !== undefined) || - (currentData !== undefined && previousData === undefined) + (currentData === null || + currentData === undefined || + previousData === null || + previousData === undefined) && + currentData !== previousData ) { const newValueChange = new ApplicationDataChange({ key: options?.propertyKey, diff --git a/sources/packages/web/src/views/aest/StudentApplicationView.vue b/sources/packages/web/src/views/aest/StudentApplicationView.vue index 493b6a95c3..639b3f6f26 100644 --- a/sources/packages/web/src/views/aest/StudentApplicationView.vue +++ b/sources/packages/web/src/views/aest/StudentApplicationView.vue @@ -68,6 +68,7 @@ export default defineComponent({ const initialData = ref({}); const selectedForm = ref(); let applicationWizard: FormIOForm; + /** * Happens when all the form components are rendered, including lists. */ @@ -77,6 +78,7 @@ export default defineComponent({ // List components are ready only after the form is rendered. highlightChanges(); }; + /** * Loads the initial application data. */ @@ -95,6 +97,7 @@ export default defineComponent({ }; highlightChanges(); }); + /** * Check if the application has changes to be highlighted. * Changes are expected after applications are edited after submitted at least once. @@ -108,6 +111,7 @@ export default defineComponent({ applicationDetail.value.changes, ); } + /** * Apply the style class to the components that have changes. * @param parentComponent component to have the changes highlighted. @@ -120,18 +124,14 @@ export default defineComponent({ for (const change of changes) { let searchComponent: FormIOComponent | undefined; if (typeof change.index === "number") { - searchComponent = parentComponent?.components?.length - ? parentComponent.components[change.index] - : undefined; - if (searchComponent && change.changes) { - // Should check further for nested changes. - highlightChangesRecursive(searchComponent, change.changes); - } else if (searchComponent) { - // searchComponent has a change, but no nested changes. - // It also does not have a key because it is a child in a list. - applyChangedValueStyleClass(searchComponent, change.changeType); - } + // The item to be processed is an array item. + searchComponent = processArrayItem( + parentComponent, + change.index, + change, + ); } else if (change.key) { + // The item to be processed is a component. searchComponent = parentComponent; } if (!change.key || !searchComponent?.components?.length) { @@ -148,6 +148,32 @@ export default defineComponent({ } } + /** + * Process an array item component. + * @param parentComponent component that contains the array item. + * @param changeIndex index of the array item that has the change. + * @param change change object that contains the change details. + * @returns component if it exists, otherwise undefined. + */ + function processArrayItem( + parentComponent: FormIOComponent, + changeIndex: number, + change: ApplicationDataChangeAPIOutDTO, + ): FormIOComponent | undefined { + const searchComponent = parentComponent?.components?.length + ? parentComponent.components[changeIndex] + : undefined; + if (searchComponent && change.changes) { + // Should check further for nested changes. + highlightChangesRecursive(searchComponent, change.changes); + } else if (searchComponent) { + // searchComponent has a change, but no nested changes. + // It also does not have a key because it is a child in a list. + applyChangedValueStyleClass(searchComponent, change.changeType); + } + return searchComponent; + } + /** * Apply the proper highlight style to a changed component. * @param component component to received the style. From 7c561848f99a41e9164cefb91cffa8e9c7b34d29 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Mon, 13 Jan 2025 10:36:44 -0800 Subject: [PATCH 20/25] Data comparison improvement --- .../application-data-comparison.ts | 49 +++++++++---------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts index eba88c4b7a..d5a561558f 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.ts @@ -96,15 +96,14 @@ function checkArrayChanges( parentChange: ApplicationDataChange, options?: { propertyKey?: string; index?: number }, ): void { - let arrayItemChange: ApplicationDataChange; - if (!isEqual(currentData, previousData)) { - arrayItemChange = new ApplicationDataChange({ key: options?.propertyKey }); - parentChange.changes.push(arrayItemChange); - if (currentData.length < previousData.length) { - arrayItemChange.changeType = ChangeTypes.ItemsRemoved; - } else if (currentData.length > previousData.length) { - arrayItemChange.changeType = ChangeTypes.ItemsAppended; - } + const arrayItemChange = new ApplicationDataChange({ + key: options?.propertyKey, + }); + parentChange.changes.push(arrayItemChange); + if (currentData.length < previousData.length) { + arrayItemChange.changeType = ChangeTypes.ItemsRemoved; + } else if (currentData.length > previousData.length) { + arrayItemChange.changeType = ChangeTypes.ItemsAppended; } // Property is an array that should have its items checked. for (let index = 0; index < currentData.length; index++) { @@ -137,23 +136,21 @@ function checkObjectChanges( options?: { propertyKey?: string; index?: number }, ): void { // Property is an object that should have its properties checked. - if (!isEqual(currentData, previousData)) { - const objectPropertyChange = new ApplicationDataChange({ - key: options?.propertyKey, - index: options?.index, - }); - if (Object.keys(currentData).length < Object.keys(previousData).length) { - objectPropertyChange.changeType = ChangeTypes.PropertiesRemoved; - } - parentChange.changes.push(objectPropertyChange); - for (const propertyKey of Object.keys(currentData)) { - compareApplicationDataRecursive( - currentData[propertyKey], - previousData[propertyKey], - objectPropertyChange, - { propertyKey }, - ); - } + const objectPropertyChange = new ApplicationDataChange({ + key: options?.propertyKey, + index: options?.index, + }); + if (Object.keys(currentData).length < Object.keys(previousData).length) { + objectPropertyChange.changeType = ChangeTypes.PropertiesRemoved; + } + parentChange.changes.push(objectPropertyChange); + for (const propertyKey of Object.keys(currentData)) { + compareApplicationDataRecursive( + currentData[propertyKey], + previousData[propertyKey], + objectPropertyChange, + { propertyKey }, + ); } } From 69dad6be8266d65a5a747f705ca58a2f45b0f108 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Mon, 13 Jan 2025 11:29:54 -0800 Subject: [PATCH 21/25] Unit test for itemsAppended and PR comment fix --- ...-comparison.compareApplicationData.spec.ts | 43 +++++++++++++++++++ .../application-data-comparison.models.ts | 4 +- .../contracts/ApplicationDataComparison.ts | 2 +- 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts index 1ae849f58b..e3eb50beb7 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/_tests_/unit/application-data-comparison.compareApplicationData.spec.ts @@ -154,6 +154,49 @@ describe("compareApplicationData", () => { ]); }); + it("Should detected items appended to an array when the current data array has more items than the previous.", () => { + // Arrange + const current = { + dependants: [ + { + fullName: "My son 1", + dateOfBirth: "2025-01-02", + }, + { + fullName: "My son 2", + dateOfBirth: "2025-01-02", + }, + ], + }; + const previous = { + dependants: [ + { + fullName: "My son 1", + dateOfBirth: "2025-01-02", + }, + ], + }; + + // Act + const result = compareApplicationData(current, previous); + + // Assert + expect(result).toEqual([ + { + key: "dependants", + changeType: "itemsAppended", + changes: [ + { + newValue: { fullName: "My son 2", dateOfBirth: "2025-01-02" }, + index: 1, + changeType: "updated", + changes: [], + }, + ], + }, + ]); + }); + it("Should detected items changed in an array at any level when the current data has at least one different property from the previous data.", () => { // Arrange const current = { diff --git a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts index e7a464d14e..dca6f57e8c 100644 --- a/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts +++ b/sources/packages/backend/libs/utilities/src/application-data-comparison/application-data-comparison.models.ts @@ -2,7 +2,7 @@ export enum ChangeTypes { /** * Indicates that a new value was added or updated. */ - Update = "updated", + Updated = "updated", /** * An array had at least one property removed. */ @@ -33,7 +33,7 @@ export class ApplicationDataChange { this.newValue = options?.newValue; this.oldValue = options?.oldValue; this.index = options?.index; - this.changeType = ChangeTypes.Update; + this.changeType = ChangeTypes.Updated; this.changes = []; } diff --git a/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts b/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts index 578c552472..1d491cfb85 100644 --- a/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts +++ b/sources/packages/web/src/types/contracts/ApplicationDataComparison.ts @@ -2,7 +2,7 @@ export enum ChangeTypes { /** * Indicates that a new value was added or updated. */ - Update = "updated", + Updated = "updated", /** * An array had at least one property removed. */ From 524fe442da8936a7a5583de1338c2a64eb027a1c Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Mon, 13 Jan 2025 11:42:28 -0800 Subject: [PATCH 22/25] E2E fix --- .../backend/libs/test-utils/src/factories/application.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sources/packages/backend/libs/test-utils/src/factories/application.ts b/sources/packages/backend/libs/test-utils/src/factories/application.ts index 7df5a3df0c..2141ab2186 100644 --- a/sources/packages/backend/libs/test-utils/src/factories/application.ts +++ b/sources/packages/backend/libs/test-utils/src/factories/application.ts @@ -344,7 +344,10 @@ export async function saveFakeApplication( fakeOriginalAssessment, ); savedApplication.currentAssessment = savedOriginalAssessment; - savedApplication.submittedDate = new Date(); + if (!savedApplication.submittedDate) { + // Ensures a non-draft application will have a submitted date. + savedApplication.submittedDate = new Date(); + } } else { savedApplication.location = null; } From a88feb5a7434ba1942acc279192b86ee8defee1e Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Mon, 13 Jan 2025 11:53:48 -0800 Subject: [PATCH 23/25] PR review fixes --- .../packages/web/src/views/aest/StudentApplicationView.vue | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sources/packages/web/src/views/aest/StudentApplicationView.vue b/sources/packages/web/src/views/aest/StudentApplicationView.vue index 639b3f6f26..0e981a405e 100644 --- a/sources/packages/web/src/views/aest/StudentApplicationView.vue +++ b/sources/packages/web/src/views/aest/StudentApplicationView.vue @@ -95,7 +95,6 @@ export default defineComponent({ ...applicationDetail.value.data, isReadOnly: true, }; - highlightChanges(); }); /** @@ -177,8 +176,8 @@ export default defineComponent({ /** * Apply the proper highlight style to a changed component. * @param component component to received the style. - * @param itemRemoved indicates if the component had an item removed, - * which would required a different style to be applied. + * @param changeType indicates the operation that has changed the value, + * which may required a different style to be applied. */ function applyChangedValueStyleClass( component: FormIOComponent, From 4039fb6dcdb3287620bd6fa5e2e3486763e9a396 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Mon, 13 Jan 2025 14:13:03 -0800 Subject: [PATCH 24/25] PR review comment --- .../application/application.aest.controller.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts index fd92d50a4f..bcf0660ea8 100644 --- a/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts +++ b/sources/packages/backend/apps/api/src/route-controllers/application/application.aest.controller.ts @@ -86,16 +86,16 @@ export class ApplicationAESTController extends BaseController { let currentReadOnlyData: ApplicationFormData; let previousReadOnlyData: ApplicationFormData; if (loadDynamicData) { - const currentReadOnlyDataPromise = - this.applicationControllerService.generateApplicationFormData( - application.data, - ); // Check if a previous application exists. const [previousApplicationVersion] = await this.applicationService.getPreviousApplicationVersions( applicationId, { loadDynamicData: true, limit: 1 }, ); + const currentReadOnlyDataPromise = + this.applicationControllerService.generateApplicationFormData( + application.data, + ); // If there is a previous application, generate its read-only data. const previousReadOnlyDataPromise = previousApplicationVersion && From a9f371a7d1f9530cb3d502d968dddbbeffde8718 Mon Sep 17 00:00:00 2001 From: Andrew Boni Signori Date: Mon, 13 Jan 2025 14:35:26 -0800 Subject: [PATCH 25/25] CSS improvement as Dheepak's suggestion --- sources/packages/web/src/assets/css/formio-shared.scss | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sources/packages/web/src/assets/css/formio-shared.scss b/sources/packages/web/src/assets/css/formio-shared.scss index 6c84782443..c7e16bbdee 100644 --- a/sources/packages/web/src/assets/css/formio-shared.scss +++ b/sources/packages/web/src/assets/css/formio-shared.scss @@ -766,6 +766,7 @@ td .formio-custom-tooltip:hover .tooltip-text { color: white; border-radius: 5px; display: block; + white-space: pre; } .changed-value { @@ -774,7 +775,7 @@ td .formio-custom-tooltip:hover .tooltip-text { .changed-value::before { @extend .changed-value-content-base; - content: "\00a0\00a0Updated"; + content: " Updated"; } .changed-list-item-removed { @@ -783,7 +784,7 @@ td .formio-custom-tooltip:hover .tooltip-text { .changed-list-item-removed::before { @extend .changed-value-content-base; - content: "\00a0\00a0Items were removed from this list"; + content: " Items were removed from this list"; } .changed-list-item-appended { @@ -792,5 +793,5 @@ td .formio-custom-tooltip:hover .tooltip-text { .changed-list-item-appended::before { @extend .changed-value-content-base; - content: "\00a0\00a0Items were appended to the list"; + content: " Items were appended to the list"; }