Skip to content

Commit

Permalink
fix: Merge persisted state into default state by default (#6898)
Browse files Browse the repository at this point in the history
This commit updates `PersistantStore.generateDefaultState` to merge in
`persistedState` to `defaultState`, rather than just returning
`persistedState` as-is.

Addresses #6233.
  • Loading branch information
codeofdusk authored Aug 16, 2023
1 parent ba27d73 commit 2922ea0
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
7 changes: 4 additions & 3 deletions src/common/flux/persistent-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { BaseStoreImpl } from 'background/stores/base-store-impl';
import { IndexedDBAPI } from 'common/indexedDB/indexedDB';
import { Logger } from 'common/logging/logger';
import { StoreNames } from 'common/stores/store-names';
import { cloneDeep, isEqual } from 'lodash';
import { cloneDeep, isEmpty, isEqual, merge } from 'lodash';

export abstract class PersistentStore<TState> extends BaseStoreImpl<TState, Promise<void>> {
private previouslyPersistedState: TState | null;
Expand All @@ -30,13 +30,14 @@ export abstract class PersistentStore<TState> extends BaseStoreImpl<TState, Prom

// Allow specific stores to override default state behavior
protected generateDefaultState(persistedData: TState): TState {
return persistedData;
const defaultState = this.getDefaultState();
return !isEmpty(persistedData) ? merge({}, defaultState, persistedData) : defaultState;
}

public override initialize(initialState?: TState): void {
const generatedPersistedState = this.generateDefaultState(this.persistedState);

this.state = initialState || (generatedPersistedState ?? this.getDefaultState());
this.state = initialState || generatedPersistedState;

this.addActionListeners();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,9 @@ describe('AssessmentCardSelectionStore Test', () => {
function createStoreForAssessmentCardSelectionActions(
actionName: keyof AssessmentCardSelectionActions,
): StoreTester<AssessmentCardSelectionStoreData, AssessmentCardSelectionActions> {
const assessmentStoreState: AssessmentStoreData = createAssessmentStoreDataWithStatus(
ManualTestStatus.UNKNOWN,
);
const factory = (actions: AssessmentCardSelectionActions) =>
new AssessmentCardSelectionStore(
actions,
Expand All @@ -763,13 +766,19 @@ describe('AssessmentCardSelectionStore Test', () => {
null,
);

setupDataGeneratorMock(null, assessmentStoreState);

return new StoreTester(AssessmentCardSelectionActions, actionName, factory);
}

function createStoreForAssessmentActions(
actionName: keyof AssessmentActions,
assessmentsProvider?: AssessmentsProvider,
): StoreTester<AssessmentCardSelectionStoreData, AssessmentActions> {
const assessmentStoreState: AssessmentStoreData = createAssessmentStoreDataWithStatus(
ManualTestStatus.UNKNOWN,
);

const factory = (actions: AssessmentActions) =>
new AssessmentCardSelectionStore(
new AssessmentCardSelectionActions(),
Expand All @@ -784,6 +793,8 @@ describe('AssessmentCardSelectionStore Test', () => {
null,
);

setupDataGeneratorMock(null, assessmentStoreState);

return new StoreTester(AssessmentActions, actionName, factory);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('PersistentStoreTest', () => {

testObject.initialize();

expect(testObject.getState()).toBe(persistedState);
expect(testObject.getState()).toStrictEqual(persistedState);
});

test('Initialize without persisted data', async () => {
Expand Down

0 comments on commit 2922ea0

Please sign in to comment.