Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion src/redux/reducers/Connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,14 @@ const initializeJobSchedule = (state, action) => {

const jobSchedule = JobSchedule.initialize(member, job, config, isComboJobsEnabled)

return { ...state, jobSchedule }
const members = member?.guid ? upsertMember(state, { payload: member }) : state.members

return {
...state,
currentMemberGuid: member?.guid ?? state.currentMemberGuid,
jobSchedule,
members,
}
}

const verifyExistingConnection = (state, action) => {
Expand Down
25 changes: 25 additions & 0 deletions src/redux/reducers/__tests__/Connect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,31 @@ describe('Connect redux store', () => {
},
])
})

test('stores the (refreshed) member as the current member without changing location', () => {
const staleMember = { guid: 'MBR-1', is_being_aggregated: false, most_recent_job_guid: null }
const freshMember = {
guid: 'MBR-1',
is_being_aggregated: true,
most_recent_job_guid: 'JOB-1',
}
const beforeState = {
...defaultState,
currentMemberGuid: 'MBR-1',
members: [staleMember],
location: [{ step: STEPS.SEARCH }, { step: STEPS.CONNECTING }],
}

const afterState = reducer(
beforeState,
initializeJobSchedule(freshMember, aggJob, { mode: AGG_MODE }),
)

expect(afterState.currentMemberGuid).toBe('MBR-1')
expect(afterState.members).toEqual([freshMember])
expect(afterState.location).toEqual(beforeState.location)
expect(afterState.jobSchedule.isInitialized).toBe(true)
})
})

describe('RETRY_OAUTH action', () => {
Expand Down
39 changes: 23 additions & 16 deletions src/utilities/JobSchedule.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,30 +69,37 @@ export const initialize = (member, recentJob, config, isComboJobsEnabled) => {
/**
* Update the schedule with the finished job.
* - Mark the finished job as DONE
* - Find and update the next PENDING JOB
* - If nothing is left ACTIVE, promote the next PENDING job
*
* The finished job is not always the ACTIVE one. Firefly starts a job of its
* own when an OAuth member is redirected back and background aggregation is disabled,
* and that job can be the one that finishes while our scheduled job is still waiting
* to run. In that case the ACTIVE job must stay ACTIVE so Connecting can start it;
* promoting a PENDING job as well would leave two ACTIVE jobs and nothing would ever
* pick up the second one.
*
* @param {Object} schedule the jobSchedule object
* @param {Object} finishedJob the job that was just finished
* @return {Object} an updated jobSchedule
*/
export const onJobFinished = (schedule, finishedJob) => {
let hasSetActiveJob = false

const updatedJobs = schedule.jobs.map((scheduledJob) => {
if (finishedJob.job_type === scheduledJob.type) {
// If the finished job's type matched the scheduled one, mark it as done
return { ...scheduledJob, status: JOB_STATUSES.DONE }
} else if (!hasSetActiveJob && scheduledJob.status === JOB_STATUSES.PENDING) {
// If we haven't set an active job and this one is pending, mark it as
// active, we only have one active job at a time.
hasSetActiveJob = true
return { ...scheduledJob, status: JOB_STATUSES.ACTIVE }
}
const jobs = schedule.jobs.map((scheduledJob) =>
finishedJob?.job_type === scheduledJob.type
? { ...scheduledJob, status: JOB_STATUSES.DONE }
: scheduledJob,
)

return scheduledJob
})
const hasActiveJob = jobs.some((job) => job.status === JOB_STATUSES.ACTIVE)

return { isInitialized: true, jobs: updatedJobs }
if (!hasActiveJob) {
const nextPendingIndex = jobs.findIndex((job) => job.status === JOB_STATUSES.PENDING)

if (nextPendingIndex !== -1) {
jobs[nextPendingIndex] = { ...jobs[nextPendingIndex], status: JOB_STATUSES.ACTIVE }
}
}

return { isInitialized: true, jobs }
}

export const areAllJobsDone = (schedule) => {
Expand Down
68 changes: 68 additions & 0 deletions src/utilities/__tests__/JobSchedule-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,4 +219,72 @@ describe('JobSchedule.onJobFinished', () => {
},
])
})

describe('when the finished job is not the active job', () => {
const verifyJob = { guid: 'JOB-2', job_type: JOB_TYPES.VERIFICATION }

test('keeps the active job active and does not promote a pending job', () => {
const prevSchedule = {
isInitialized: true,
jobs: [
{ type: JOB_TYPES.VERIFICATION, status: JOB_STATUSES.ACTIVE },
{ type: JOB_TYPES.IDENTIFICATION, status: JOB_STATUSES.PENDING },
],
}

const schedule = JobSchedule.onJobFinished(prevSchedule, aggJob)

expect(schedule.jobs).toEqual([
{ type: JOB_TYPES.VERIFICATION, status: JOB_STATUSES.ACTIVE },
{ type: JOB_TYPES.IDENTIFICATION, status: JOB_STATUSES.PENDING },
])
expect(JobSchedule.getActiveJob(schedule)).toEqual({
type: JOB_TYPES.VERIFICATION,
status: JOB_STATUSES.ACTIVE,
})
})

test('leaves the schedule alone when the finished job was already done', () => {
const prevSchedule = {
isInitialized: true,
jobs: [
{ type: JOB_TYPES.VERIFICATION, status: JOB_STATUSES.DONE },
{ type: JOB_TYPES.IDENTIFICATION, status: JOB_STATUSES.ACTIVE },
],
}

const schedule = JobSchedule.onJobFinished(prevSchedule, verifyJob)

expect(schedule.jobs).toEqual(prevSchedule.jobs)
expect(JobSchedule.areAllJobsDone(schedule)).toBe(false)
})

test('marks a pending job done if that is what finished, keeping the active one', () => {
const prevSchedule = {
isInitialized: true,
jobs: [
{ type: JOB_TYPES.AGGREGATION, status: JOB_STATUSES.ACTIVE },
{ type: JOB_TYPES.VERIFICATION, status: JOB_STATUSES.PENDING },
],
}

const schedule = JobSchedule.onJobFinished(prevSchedule, verifyJob)

expect(schedule.jobs).toEqual([
{ type: JOB_TYPES.AGGREGATION, status: JOB_STATUSES.ACTIVE },
{ type: JOB_TYPES.VERIFICATION, status: JOB_STATUSES.DONE },
])
})

test('tolerates a missing job', () => {
const prevSchedule = {
isInitialized: true,
jobs: [{ type: JOB_TYPES.VERIFICATION, status: JOB_STATUSES.ACTIVE }],
}

const schedule = JobSchedule.onJobFinished(prevSchedule, null)

expect(schedule.jobs).toEqual(prevSchedule.jobs)
})
})
})
Loading
Loading