diff --git a/packages/evolution-backend/src/services/audits/__tests__/SurveyObjectParsers.test.ts b/packages/evolution-backend/src/services/audits/__tests__/SurveyObjectParsers.test.ts index 146f08274..8e447e6b8 100644 --- a/packages/evolution-backend/src/services/audits/__tests__/SurveyObjectParsers.test.ts +++ b/packages/evolution-backend/src/services/audits/__tests__/SurveyObjectParsers.test.ts @@ -186,6 +186,7 @@ describe('SurveyObjectParsers Integration', () => { await populatePersonsForHousehold( surveyObjectsWithErrors, surveyObjectsWithErrors.household!, + undefined, correctedResponse, surveyObjectsRegistry ); diff --git a/packages/evolution-backend/src/services/surveyObjects/PersonFactory.ts b/packages/evolution-backend/src/services/surveyObjects/PersonFactory.ts index 008987738..5b24afae4 100644 --- a/packages/evolution-backend/src/services/surveyObjects/PersonFactory.ts +++ b/packages/evolution-backend/src/services/surveyObjects/PersonFactory.ts @@ -5,20 +5,27 @@ * License text available at https://opensource.org/licenses/MIT */ +import _omit from 'lodash/omit'; + import { SurveyObjectsWithErrors } from 'evolution-common/lib/services/baseObjects/types'; import { CorrectedResponse } from 'evolution-common/lib/services/questionnaire/types'; import { Person } from 'evolution-common/lib/services/baseObjects/Person'; import { isOk } from 'evolution-common/lib/types/Result.type'; import { Household } from 'evolution-common/lib/services/baseObjects/Household'; +import { Home } from 'evolution-common/lib/services/baseObjects/Home'; +import { Optional } from 'evolution-common/lib/types/Optional.type'; import projectConfig from '../../config/projectConfig'; import { SurveyObjectsRegistry } from 'evolution-common/lib/services/baseObjects/SurveyObjectsRegistry'; import { compareSequenceThenUuid } from 'evolution-common/lib/services/baseObjects/sequenceUtils'; +import { populateJourneysForPerson } from './JourneyFactory'; +import { ExtendedPersonAttributes } from 'evolution-common/lib/services/baseObjects/Person'; /** * Generate persons * Populate members for a household from the household's persons attributes * @param {SurveyObjectsWithErrors} surveyObjectsWithErrors - Container for created objects with errors * @param {Household} household - The household to add the members to + * @param {Home} home - The home object for geography assignment, needed by nested journeys * @param {CorrectedResponse} correctedResponse - corrected response * @param {SurveyObjectsRegistry} surveyObjectsRegistry - SurveyObjectsRegistry * @returns {Promise} @@ -26,6 +33,7 @@ import { compareSequenceThenUuid } from 'evolution-common/lib/services/baseObjec export async function populatePersonsForHousehold( surveyObjectsWithErrors: SurveyObjectsWithErrors, household: Household, + home: Optional, correctedResponse: CorrectedResponse, surveyObjectsRegistry: SurveyObjectsRegistry ): Promise { @@ -56,13 +64,30 @@ export async function populatePersonsForHousehold( ? projectConfig.surveyObjectParsers.person(originalCorrectedPersonAttributes, correctedResponse) : originalCorrectedPersonAttributes; - const personResult = Person.create(personAttributes as { [key: string]: unknown }, surveyObjectsRegistry); + // Omit journeys as they will be populated separately in the next step (populateJourneysForPerson) + const personResult = Person.create( + _omit(personAttributes as { [key: string]: unknown }, ['journeys']) as ExtendedPersonAttributes, + surveyObjectsRegistry + ); if (isOk(personResult)) { // Assign color to person personResult.result.assignColor(personIndex); household.members.push(personResult.result); personIndex++; + + // Create journeys for this person (includes visited places, trips, and segments) + await populateJourneysForPerson( + surveyObjectsWithErrors, + personResult.result, + personAttributes as ExtendedPersonAttributes, + home, + correctedResponse, + surveyObjectsRegistry + ); + + // Setup work and school places after all visited places are created + personResult.result.setupWorkAndSchoolPlaces(); } else { console.log( ` ==== Person ${personUuid} creation failed with errors count: ${personResult.errors?.length || 0} ====` diff --git a/packages/evolution-backend/src/services/surveyObjects/SurveyObjectsFactory.ts b/packages/evolution-backend/src/services/surveyObjects/SurveyObjectsFactory.ts index 350247dc8..b71102be6 100644 --- a/packages/evolution-backend/src/services/surveyObjects/SurveyObjectsFactory.ts +++ b/packages/evolution-backend/src/services/surveyObjects/SurveyObjectsFactory.ts @@ -13,8 +13,6 @@ import { isOk } from 'evolution-common/lib/types/Result.type'; import { SurveyObjectsWithErrors } from 'evolution-common/lib/services/baseObjects/types'; import { CorrectedResponse } from 'evolution-common/lib/services/questionnaire/types'; import { populatePersonsForHousehold } from './PersonFactory'; -import { populateJourneysForPerson } from './JourneyFactory'; -import { ExtendedPersonAttributes } from 'evolution-common/lib/services/baseObjects/Person'; import projectConfig from '../../config/projectConfig'; import { Home } from 'evolution-common/lib/services/baseObjects/Home'; import { ExtendedHouseholdAttributes, Household } from 'evolution-common/lib/services/baseObjects/Household'; @@ -177,37 +175,15 @@ export class SurveyObjectsFactory { // Continue with persons, journeys, etc. if household and home were created if (household && householdAttributes) { - // For now, we'll keep the existing factory functions for persons/journeys - // These can be refactored later in the same way + // populatePersonsForHousehold also creates journeys (and their visited places, + // trips, and segments) for each successfully created person await populatePersonsForHousehold( surveyObjectsWithErrors, household, + home, correctedResponse, surveyObjectsRegistry ); - - const personsAttributes = householdAttributes.persons || {}; - const persons = household.members || []; - - // Loop through each person - for (let i = 0, count = persons.length; i < count; i++) { - const person = persons[i]; - const personUuid = person._uuid!; - const personAttributes = personsAttributes[personUuid] as ExtendedPersonAttributes; - - // Generate all journeys for this person (includes visited places, trips, and segments) - await populateJourneysForPerson( - surveyObjectsWithErrors, - person, - personAttributes, - home, - correctedResponse, - surveyObjectsRegistry - ); - - // Setup work and school places after all visited places are created - person.setupWorkAndSchoolPlaces(); - } } return surveyObjectsWithErrors; diff --git a/packages/evolution-backend/src/services/surveyObjects/__tests__/PersonFactory.test.ts b/packages/evolution-backend/src/services/surveyObjects/__tests__/PersonFactory.test.ts index 4385cc377..94e960c2e 100644 --- a/packages/evolution-backend/src/services/surveyObjects/__tests__/PersonFactory.test.ts +++ b/packages/evolution-backend/src/services/surveyObjects/__tests__/PersonFactory.test.ts @@ -7,6 +7,7 @@ import { v4 as uuidV4 } from 'uuid'; import { populatePersonsForHousehold } from '../PersonFactory'; +import { populateJourneysForPerson } from '../JourneyFactory'; import { SurveyObjectsWithErrors } from 'evolution-common/lib/services/baseObjects/types'; import { Household } from 'evolution-common/lib/services/baseObjects/Household'; import { Person } from 'evolution-common/lib/services/baseObjects/Person'; @@ -20,7 +21,11 @@ jest.mock('evolution-common/lib/services/baseObjects/Person', () => ({ create: jest.fn() } })); +jest.mock('../JourneyFactory'); const MockedPerson = Person as jest.MockedClass; +const mockedPopulateJourneysForPerson = populateJourneysForPerson as jest.MockedFunction< + typeof populateJourneysForPerson +>; describe('PersonFactory', () => { let surveyObjectsRegistry: SurveyObjectsRegistry; @@ -78,6 +83,7 @@ describe('PersonFactory', () => { // Clear all mocks jest.clearAllMocks(); + mockedPopulateJourneysForPerson.mockResolvedValue(); }); describe('populatePersonsForHousehold', () => { @@ -86,20 +92,22 @@ describe('PersonFactory', () => { const mockPerson1 = { _uuid: 'person-1', age: 30, - assignColor: jest.fn() + assignColor: jest.fn(), + setupWorkAndSchoolPlaces: jest.fn() } as unknown as Person; const mockPerson2 = { _uuid: 'person-2', age: 25, - assignColor: jest.fn() + assignColor: jest.fn(), + setupWorkAndSchoolPlaces: jest.fn() } as unknown as Person; (MockedPerson.create as jest.Mock) .mockReturnValueOnce(createOk(mockPerson1)) .mockReturnValueOnce(createOk(mockPerson2)); - await populatePersonsForHousehold(surveyObjectsWithErrors, household, correctedResponse, surveyObjectsRegistry); + await populatePersonsForHousehold(surveyObjectsWithErrors, household, undefined, correctedResponse, surveyObjectsRegistry); // Verify Person.create was called with correct attributes expect(MockedPerson.create).toHaveBeenCalledTimes(2); @@ -135,6 +143,112 @@ describe('PersonFactory', () => { expect(surveyObjectsWithErrors.errorsByObject.personsByUuid).toEqual({}); }); + it('should omit journeys from Person.create attributes', async () => { + correctedResponse.household!.persons = { + 'person-1': { + _uuid: 'person-1', + _sequence: 1, + age: 30, + journeys: { + 'journey-1': { _uuid: 'journey-1', _sequence: 1 } + } + } + }; + + (MockedPerson.create as jest.Mock).mockReturnValue(createOk({ + _uuid: 'person-1', + assignColor: jest.fn(), + setupWorkAndSchoolPlaces: jest.fn() + } as unknown as Person)); + + await populatePersonsForHousehold(surveyObjectsWithErrors, household, undefined, correctedResponse, surveyObjectsRegistry); + + expect(MockedPerson.create).toHaveBeenCalledWith( + expect.objectContaining({ + _uuid: 'person-1', + _sequence: 1, + age: 30 + }), + surveyObjectsRegistry + ); + expect(MockedPerson.create).toHaveBeenCalledWith( + expect.not.objectContaining({ + journeys: expect.anything() + }), + surveyObjectsRegistry + ); + expect(household.members).toHaveLength(1); + }); + + it('should populate journeys and setup work/school places for each successfully created person', async () => { + const home = { _uuid: 'home-uuid' } as any; + const mockPerson1 = { + _uuid: 'person-1', + assignColor: jest.fn(), + setupWorkAndSchoolPlaces: jest.fn() + } as unknown as Person; + const mockPerson2 = { + _uuid: 'person-2', + assignColor: jest.fn(), + setupWorkAndSchoolPlaces: jest.fn() + } as unknown as Person; + + (MockedPerson.create as jest.Mock) + .mockReturnValueOnce(createOk(mockPerson1)) + .mockReturnValueOnce(createOk(mockPerson2)); + + await populatePersonsForHousehold(surveyObjectsWithErrors, household, home, correctedResponse, surveyObjectsRegistry); + + expect(mockedPopulateJourneysForPerson).toHaveBeenCalledTimes(2); + expect(mockedPopulateJourneysForPerson).toHaveBeenNthCalledWith( + 1, + surveyObjectsWithErrors, + mockPerson1, + expect.objectContaining({ _uuid: 'person-1' }), + home, + correctedResponse, + surveyObjectsRegistry + ); + expect(mockedPopulateJourneysForPerson).toHaveBeenNthCalledWith( + 2, + surveyObjectsWithErrors, + mockPerson2, + expect.objectContaining({ _uuid: 'person-2' }), + home, + correctedResponse, + surveyObjectsRegistry + ); + + expect(mockPerson1.setupWorkAndSchoolPlaces).toHaveBeenCalledTimes(1); + expect(mockPerson2.setupWorkAndSchoolPlaces).toHaveBeenCalledTimes(1); + }); + + it('should not populate journeys or setup work/school places for persons that failed to be created', async () => { + const errors = [new Error('Invalid age')]; + const mockPerson2 = { + _uuid: 'person-2', + assignColor: jest.fn(), + setupWorkAndSchoolPlaces: jest.fn() + } as unknown as Person; + + (MockedPerson.create as jest.Mock) + .mockReturnValueOnce(createErrors(errors)) + .mockReturnValueOnce(createOk(mockPerson2)); + + await populatePersonsForHousehold(surveyObjectsWithErrors, household, undefined, correctedResponse, surveyObjectsRegistry); + + expect(mockedPopulateJourneysForPerson).toHaveBeenCalledTimes(1); + expect(mockedPopulateJourneysForPerson).toHaveBeenCalledWith( + surveyObjectsWithErrors, + mockPerson2, + expect.objectContaining({ _uuid: 'person-2' }), + undefined, + correctedResponse, + surveyObjectsRegistry + ); + expect(mockPerson2.setupWorkAndSchoolPlaces).toHaveBeenCalledTimes(1); + }); + it('should handle person creation errors', async () => { const errors = [new Error('Invalid age')]; @@ -142,10 +256,11 @@ describe('PersonFactory', () => { .mockReturnValueOnce(createErrors(errors)) .mockReturnValueOnce(createOk({ _uuid: 'person-2', - assignColor: jest.fn() + assignColor: jest.fn(), + setupWorkAndSchoolPlaces: jest.fn() } as unknown as Person)); - await populatePersonsForHousehold(surveyObjectsWithErrors, household, correctedResponse, surveyObjectsRegistry); + await populatePersonsForHousehold(surveyObjectsWithErrors, household, undefined, correctedResponse, surveyObjectsRegistry); // Verify error was stored expect(surveyObjectsWithErrors.errorsByObject.personsByUuid['person-1']).toEqual(errors); @@ -171,10 +286,11 @@ describe('PersonFactory', () => { (MockedPerson.create as jest.Mock).mockReturnValue(createOk({ _uuid: 'person-1', - assignColor: jest.fn() + assignColor: jest.fn(), + setupWorkAndSchoolPlaces: jest.fn() } as unknown as Person)); - await populatePersonsForHousehold(surveyObjectsWithErrors, household, correctedResponse, surveyObjectsRegistry); + await populatePersonsForHousehold(surveyObjectsWithErrors, household, undefined, correctedResponse, surveyObjectsRegistry); // Should only create one person (skip undefined) expect(MockedPerson.create).toHaveBeenCalledTimes(1); @@ -184,7 +300,7 @@ describe('PersonFactory', () => { it('should handle missing household', async () => { surveyObjectsWithErrors.household = undefined; - await populatePersonsForHousehold(surveyObjectsWithErrors, household, correctedResponse, surveyObjectsRegistry); + await populatePersonsForHousehold(surveyObjectsWithErrors, household, undefined, correctedResponse, surveyObjectsRegistry); expect(MockedPerson.create).not.toHaveBeenCalled(); }); @@ -192,7 +308,7 @@ describe('PersonFactory', () => { it('should handle missing persons attributes', async () => { correctedResponse.household!.persons = undefined; - await populatePersonsForHousehold(surveyObjectsWithErrors, household, correctedResponse, surveyObjectsRegistry); + await populatePersonsForHousehold(surveyObjectsWithErrors, household, undefined, correctedResponse, surveyObjectsRegistry); expect(MockedPerson.create).not.toHaveBeenCalled(); expect(household.members).toHaveLength(0); @@ -219,11 +335,11 @@ describe('PersonFactory', () => { }; (MockedPerson.create as jest.Mock) - .mockReturnValueOnce(createOk({ _uuid: 'person-1', assignColor: jest.fn() } as unknown as Person)) - .mockReturnValueOnce(createOk({ _uuid: 'person-2', assignColor: jest.fn() } as unknown as Person)) - .mockReturnValueOnce(createOk({ _uuid: 'person-3', assignColor: jest.fn() } as unknown as Person)); + .mockReturnValueOnce(createOk({ _uuid: 'person-1', assignColor: jest.fn(), setupWorkAndSchoolPlaces: jest.fn() } as unknown as Person)) + .mockReturnValueOnce(createOk({ _uuid: 'person-2', assignColor: jest.fn(), setupWorkAndSchoolPlaces: jest.fn() } as unknown as Person)) + .mockReturnValueOnce(createOk({ _uuid: 'person-3', assignColor: jest.fn(), setupWorkAndSchoolPlaces: jest.fn() } as unknown as Person)); - await populatePersonsForHousehold(surveyObjectsWithErrors, household, correctedResponse, surveyObjectsRegistry); + await populatePersonsForHousehold(surveyObjectsWithErrors, household, undefined, correctedResponse, surveyObjectsRegistry); // Verify persons were created in sequence order (1, 2, 3) expect(MockedPerson.create).toHaveBeenNthCalledWith(1, expect.objectContaining({ _sequence: 1 }), surveyObjectsRegistry); diff --git a/packages/evolution-backend/src/services/surveyObjects/__tests__/SurveyObjectsFactory.test.ts b/packages/evolution-backend/src/services/surveyObjects/__tests__/SurveyObjectsFactory.test.ts index d9eac05a3..10022f8be 100644 --- a/packages/evolution-backend/src/services/surveyObjects/__tests__/SurveyObjectsFactory.test.ts +++ b/packages/evolution-backend/src/services/surveyObjects/__tests__/SurveyObjectsFactory.test.ts @@ -7,7 +7,6 @@ import { SurveyObjectsFactory } from '../SurveyObjectsFactory'; import { populatePersonsForHousehold } from '../PersonFactory'; -import { populateJourneysForPerson } from '../JourneyFactory'; import { InterviewAttributes, CorrectedResponse } from 'evolution-common/lib/services/questionnaire/types'; import { create as createInterviewObject } from 'evolution-common/lib/services/baseObjects/interview/InterviewUnserializer'; import { Home } from 'evolution-common/lib/services/baseObjects/Home'; @@ -17,7 +16,6 @@ import { SurveyObjectsRegistry } from 'evolution-common/lib/services/baseObjects // Mock dependencies jest.mock('../PersonFactory'); -jest.mock('../JourneyFactory'); jest.mock('evolution-common/lib/services/baseObjects/interview/InterviewUnserializer'); jest.mock('evolution-common/lib/services/baseObjects/Home', () => ({ Home: { @@ -31,7 +29,6 @@ jest.mock('evolution-common/lib/services/baseObjects/Household', () => ({ })); const mockedpopulatePersonsForHousehold = populatePersonsForHousehold as jest.MockedFunction; -const mockedPopulateJourneysForPerson = populateJourneysForPerson as jest.MockedFunction; const mockedCreateInterviewObject = createInterviewObject as jest.MockedFunction; const MockedHome = Home as jest.MockedClass; const MockedHousehold = Household as jest.MockedClass; @@ -86,7 +83,6 @@ describe('SurveyObjectsFactory', () => { (MockedHome.create as jest.Mock).mockReturnValue(createOk(mockHome as any)); (MockedHousehold.create as jest.Mock).mockReturnValue(createOk(mockHousehold as any)); mockedpopulatePersonsForHousehold.mockResolvedValue(); - mockedPopulateJourneysForPerson.mockResolvedValue(); const result = await factory.createAllObjectsWithErrors(interviewAttributes); @@ -114,25 +110,15 @@ describe('SurveyObjectsFactory', () => { expect(result.home).toBe(mockHome); expect(result.household).toBe(mockHousehold); - // Verify person and journey factories were called + // Verify person factory was called expect(mockedpopulatePersonsForHousehold).toHaveBeenCalledWith( result, mockHousehold, - interviewAttributes.corrected_response, - expect.any(SurveyObjectsRegistry) - ); - expect(mockedPopulateJourneysForPerson).toHaveBeenCalledWith( - result, - mockHousehold.members[0], - interviewAttributes.corrected_response!.household!.persons!['person-1'], mockHome, interviewAttributes.corrected_response, expect.any(SurveyObjectsRegistry) ); - // Verify setupWorkAndSchoolPlaces was called - expect(mockHousehold.members[0].setupWorkAndSchoolPlaces).toHaveBeenCalled(); - // Verify no errors expect(result.errorsByObject.interview).toEqual([]); expect(result.errorsByObject.home).toEqual([]); @@ -189,9 +175,8 @@ describe('SurveyObjectsFactory', () => { expect(result.household).toBeUndefined(); expect(result.errorsByObject.household).toBe(errors); - // Should not call person/journey factories if household creation fails + // Should not call the person factory if household creation fails expect(mockedpopulatePersonsForHousehold).not.toHaveBeenCalled(); - expect(mockedPopulateJourneysForPerson).not.toHaveBeenCalled(); }); it('should default home _uuid to the interview uuid when not explicitly set', async () => { @@ -279,12 +264,13 @@ describe('SurveyObjectsFactory', () => { expect(MockedHousehold.create).not.toHaveBeenCalled(); }); - it('should process multiple persons', async () => { + it('should call populatePersonsForHousehold with the created home and household', async () => { + const mockHome = { _uuid: 'home-uuid' }; const mockHousehold = { _uuid: 'household-uuid', members: [ - { _uuid: 'person-1', _sequence: 1, setupWorkAndSchoolPlaces: jest.fn() }, - { _uuid: 'person-2', _sequence: 2, setupWorkAndSchoolPlaces: jest.fn() } + { _uuid: 'person-1', _sequence: 1 }, + { _uuid: 'person-2', _sequence: 2 } ] }; @@ -294,35 +280,22 @@ describe('SurveyObjectsFactory', () => { } as any; mockedCreateInterviewObject.mockReturnValue(createOk({} as any)); - (MockedHome.create as jest.Mock).mockReturnValue(createOk({} as any)); + (MockedHome.create as jest.Mock).mockReturnValue(createOk(mockHome as any)); (MockedHousehold.create as jest.Mock).mockReturnValue(createOk(mockHousehold as any)); mockedpopulatePersonsForHousehold.mockResolvedValue(); - mockedPopulateJourneysForPerson.mockResolvedValue(); const result = await factory.createAllObjectsWithErrors(interviewAttributes); - // Should call journey factory for each person - expect(mockedPopulateJourneysForPerson).toHaveBeenCalledTimes(2); - expect(mockedPopulateJourneysForPerson).toHaveBeenCalledWith( - result, - mockHousehold.members[0], - interviewAttributes.corrected_response!.household!.persons!['person-1'], - result.home, - interviewAttributes.corrected_response, - expect.any(SurveyObjectsRegistry) - ); - expect(mockedPopulateJourneysForPerson).toHaveBeenCalledWith( + // populatePersonsForHousehold (mocked) is responsible for creating journeys and + // calling setupWorkAndSchoolPlaces per person; this is covered by PersonFactory.test.ts + expect(mockedpopulatePersonsForHousehold).toHaveBeenCalledTimes(1); + expect(mockedpopulatePersonsForHousehold).toHaveBeenCalledWith( result, - mockHousehold.members[1], - interviewAttributes.corrected_response!.household!.persons!['person-2'], - result.home, + mockHousehold, + mockHome, interviewAttributes.corrected_response, expect.any(SurveyObjectsRegistry) ); - - // Should call setupWorkAndSchoolPlaces for each person - expect(mockHousehold.members[0].setupWorkAndSchoolPlaces).toHaveBeenCalled(); - expect(mockHousehold.members[1].setupWorkAndSchoolPlaces).toHaveBeenCalled(); }); it('should handle empty persons object', async () => { @@ -338,10 +311,10 @@ describe('SurveyObjectsFactory', () => { (MockedHousehold.create as jest.Mock).mockReturnValue(createOk(mockHousehold as any)); mockedpopulatePersonsForHousehold.mockResolvedValue(); - await factory.createAllObjectsWithErrors(interviewAttributes); + const result = await factory.createAllObjectsWithErrors(interviewAttributes); - // Should not call journey factory if no persons - expect(mockedPopulateJourneysForPerson).not.toHaveBeenCalled(); + expect(mockedpopulatePersonsForHousehold).toHaveBeenCalledTimes(1); + expect(result.household).toBe(mockHousehold); }); it('should initialize with correct factory options', () => {