Skip to content

Commit

Permalink
[bugfix] Check that oldData is defined before trying to migrate (#7537)
Browse files Browse the repository at this point in the history
<!--- Hello Dagster contributor! It's great to have you with us! -->
<!-- Make sure to read https://docs.dagster.io/community/contributing -->

## Summary
<!-- Describe your changes here, include the motivation/context, test coverage, -->
<!-- the type of change i.e. breaking change, new feature, or bug fix -->
<!-- and related GitHub issue or screenshots (if applicable). -->


FIxes issue where launchpad config gets wiped out whenever you load the launchpad.

## Test Plan
<!--- Please describe the tests you have added and your testing environment (if applicable). -->
 
Made a change to launchpad config. Reload the page. See that the launchpad config sticks around.



## Checklist
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask in our Slack. -->

- [ ] My change requires a change to the documentation and I have updated the documentation accordingly.
- [ ] I have added tests to cover my changes.
  • Loading branch information
salazarm committed Apr 22, 2022
1 parent 45db8cd commit 3f1f0fd
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ export function useExecutionSessionStorage(

// TODO: Remove this migration logic in a few patches when we know the old namespace is likely no longer being used
const oldDataMigrated = React.useRef(false);
if (!oldDataMigrated.current) {
if (oldData && !oldDataMigrated.current) {
onSave(oldData);
window.localStorage.removeItem(oldNamespace);
window.localStorage.removeItem(getKey(oldNamespace));
oldDataMigrated.current = true;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import {render} from '@testing-library/react';
import * as React from 'react';

import {TestProvider} from '../../testing/TestProvider';
import {AppContext} from '../AppContext';
import {useExecutionSessionStorage} from '../ExecutionSessionStorage';

function mockedLocalStorage() {
let store = {};
return {
setItem(key: string, item: any) {
store[key] = item;
},
getItem(key: string) {
return store[key];
},
removeItem(key: string) {
delete store[key];
},
clear() {
store = {};
},
get length() {
return Object.keys(store).length;
},
key(n: number) {
return Object.keys(store)[n];
},
};
}

let BASE_PATH = '';
const REPO_ADDRESS = {
name: 'test-name',
location: 'test-location',
};

const PIPELINE = 'test-pipeline';

const oldKeyFormat = (repoAddress: typeof REPO_ADDRESS, pipelineOrJobName: string) => {
return `dagit.v2.${repoAddress.name}.${pipelineOrJobName}`;
};

const newKeyFormat = (
basePath: string,
repoAddress: typeof REPO_ADDRESS,
pipelineOrJobName: string,
) => {
return `dagit.v2.${basePath}-${repoAddress.location}-${repoAddress.name}-${pipelineOrJobName}`;
};

describe('ExecutionSessionStorage', () => {
const originalLocalStorage = window.localStorage;
beforeEach(() => {
window.localStorage = mockedLocalStorage();
jest.resetModules();
});
afterEach(() => {
window.localStorage = originalLocalStorage;
});

it('Migrates old localStorage data from old format', () => {
const testData = {sessions: {test: 'test'}, current: 'test'};
const oldFormat = oldKeyFormat(REPO_ADDRESS, PIPELINE);
const newFormat = newKeyFormat(BASE_PATH, REPO_ADDRESS, PIPELINE);

window.localStorage.setItem(oldFormat, JSON.stringify(testData));

function TestComponent() {
const context = React.useContext(AppContext);
BASE_PATH = context.basePath;
const [data] = useExecutionSessionStorage(REPO_ADDRESS, PIPELINE);
expect(data).toEqual(testData);
return <div />;
}

render(
<TestProvider>
<TestComponent />
</TestProvider>,
);

// Its at the new key
expect(JSON.parse(window.localStorage.getItem(newFormat) as any)).toEqual(testData);

// old key is deleted
expect(window.localStorage.getItem(oldFormat)).toEqual(null);
});
});

0 comments on commit 3f1f0fd

Please sign in to comment.