Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lens] Last used Index pattern is saved to and retrieved from local storage #69511

Merged
merged 3 commits into from
Jun 26, 2020
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export function getIndexPatternDatasource({
state,
savedObjectsClient: await savedObjectsClient,
defaultIndexPatternId: core.uiSettings.get('defaultIndex'),
storage,
});
},

Expand Down Expand Up @@ -207,6 +208,7 @@ export function getIndexPatternDatasource({
setState,
savedObjectsClient,
onError: onIndexPatternLoadError,
storage,
});
}}
data={data}
Expand Down Expand Up @@ -290,6 +292,7 @@ export function getIndexPatternDatasource({
layerId: props.layerId,
onError: onIndexPatternLoadError,
replaceIfPossible: true,
storage,
});
}}
{...props}
Expand Down
88 changes: 88 additions & 0 deletions x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ import { documentField } from './document_field';

jest.mock('./operations');

const createMockStorage = (lastData?: Record<string, string>) => {
return {
get: jest.fn().mockImplementation(() => lastData),
set: jest.fn(),
remove: jest.fn(),
clear: jest.fn(),
};
};

const sampleIndexPatterns = {
a: {
id: 'a',
Expand Down Expand Up @@ -269,8 +278,10 @@ describe('loader', () => {

describe('loadInitialState', () => {
it('should load a default state', async () => {
const storage = createMockStorage();
const state = await loadInitialState({
savedObjectsClient: mockClient(),
storage,
});

expect(state).toMatchObject({
Expand All @@ -285,12 +296,61 @@ describe('loader', () => {
layers: {},
showEmptyFields: false,
});
expect(storage.set).toHaveBeenCalledWith('lens-settings', {
indexPatternId: 'a',
});
});

it('should load a default state when lastUsedIndexPatternId is not found in indexPatternRefs', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it also write back to localStorage overwriting the missing index pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! I'll add it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, that made me think

  1. (scenario you mentioned) lastUsedIndexPatternId is not found in kibana - we write back to localStorage overwriting the missing index pattern
  2. (second scenario) we open saved visualization - should we also write back to localStorage? I think so 🤔 -for now we only do it on index pattern change

const storage = createMockStorage({ indexPatternId: 'c' });
const state = await loadInitialState({
savedObjectsClient: mockClient(),
storage,
});

expect(state).toMatchObject({
currentIndexPatternId: 'a',
indexPatternRefs: [
{ id: 'a', title: sampleIndexPatterns.a.title },
{ id: 'b', title: sampleIndexPatterns.b.title },
],
indexPatterns: {
a: sampleIndexPatterns.a,
},
layers: {},
showEmptyFields: false,
});
expect(storage.set).toHaveBeenCalledWith('lens-settings', {
indexPatternId: 'a',
});
});

it('should load lastUsedIndexPatternId if in localStorage', async () => {
const state = await loadInitialState({
savedObjectsClient: mockClient(),
storage: createMockStorage({ indexPatternId: 'b' }),
});

expect(state).toMatchObject({
currentIndexPatternId: 'b',
indexPatternRefs: [
{ id: 'a', title: sampleIndexPatterns.a.title },
{ id: 'b', title: sampleIndexPatterns.b.title },
],
indexPatterns: {
b: sampleIndexPatterns.b,
},
layers: {},
showEmptyFields: false,
});
});

it('should use the default index pattern id, if provided', async () => {
const storage = createMockStorage();
const state = await loadInitialState({
defaultIndexPatternId: 'b',
savedObjectsClient: mockClient(),
storage,
});

expect(state).toMatchObject({
Expand All @@ -305,6 +365,9 @@ describe('loader', () => {
layers: {},
showEmptyFields: false,
});
expect(storage.set).toHaveBeenCalledWith('lens-settings', {
indexPatternId: 'b',
});
});

it('should initialize from saved state', async () => {
Expand Down Expand Up @@ -336,9 +399,11 @@ describe('loader', () => {
},
},
};
const storage = createMockStorage({ indexPatternId: 'a' });
const state = await loadInitialState({
state: savedState,
savedObjectsClient: mockClient(),
storage,
});

expect(state).toMatchObject({
Expand All @@ -353,6 +418,10 @@ describe('loader', () => {
layers: savedState.layers,
showEmptyFields: false,
});

expect(storage.set).toHaveBeenCalledWith('lens-settings', {
indexPatternId: 'b',
});
});
});

Expand All @@ -367,13 +436,15 @@ describe('loader', () => {
layers: {},
showEmptyFields: true,
};
const storage = createMockStorage({ indexPatternId: 'b' });

await changeIndexPattern({
state,
setState,
id: 'a',
savedObjectsClient: mockClient(),
onError: jest.fn(),
storage,
});

expect(setState).toHaveBeenCalledTimes(1);
Expand All @@ -383,6 +454,9 @@ describe('loader', () => {
a: sampleIndexPatterns.a,
},
});
expect(storage.set).toHaveBeenCalledWith('lens-settings', {
indexPatternId: 'a',
});
});

it('handles errors', async () => {
Expand All @@ -398,6 +472,8 @@ describe('loader', () => {
showEmptyFields: true,
};

const storage = createMockStorage({ indexPatternId: 'b' });

await changeIndexPattern({
state,
setState,
Expand All @@ -409,9 +485,11 @@ describe('loader', () => {
}),
},
onError,
storage,
});

expect(setState).not.toHaveBeenCalled();
expect(storage.set).not.toHaveBeenCalled();
expect(onError).toHaveBeenCalledWith(err);
});
});
Expand Down Expand Up @@ -452,13 +530,16 @@ describe('loader', () => {
showEmptyFields: true,
};

const storage = createMockStorage({ indexPatternId: 'a' });

await changeLayerIndexPattern({
state,
setState,
indexPatternId: 'b',
layerId: 'l1',
savedObjectsClient: mockClient(),
onError: jest.fn(),
storage,
});

expect(setState).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -492,6 +573,9 @@ describe('loader', () => {
},
},
});
expect(storage.set).toHaveBeenCalledWith('lens-settings', {
indexPatternId: 'b',
});
});

it('handles errors', async () => {
Expand All @@ -515,6 +599,8 @@ describe('loader', () => {
showEmptyFields: true,
};

const storage = createMockStorage({ indexPatternId: 'b' });

await changeLayerIndexPattern({
state,
setState,
Expand All @@ -527,9 +613,11 @@ describe('loader', () => {
}),
},
onError,
storage,
});

expect(setState).not.toHaveBeenCalled();
expect(storage.set).not.toHaveBeenCalled();
expect(onError).toHaveBeenCalledWith(err);
});
});
Expand Down
29 changes: 27 additions & 2 deletions x-pack/plugins/lens/public/indexpattern_datasource/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import _ from 'lodash';
import { IStorageWrapper } from 'src/plugins/kibana_utils/public';
import { SavedObjectsClientContract, SavedObjectAttributes, HttpSetup } from 'kibana/public';
import { SimpleSavedObject } from 'kibana/public';
import { StateSetter } from '../types';
Expand All @@ -24,6 +25,7 @@ import {
IFieldType,
IndexPatternTypeMeta,
} from '../../../../../src/plugins/data/public';
import { readFromStorage, writeToStorage } from '../settings_storage';

interface SavedIndexPatternAttributes extends SavedObjectAttributes {
title: string;
Expand Down Expand Up @@ -68,31 +70,48 @@ export async function loadIndexPatterns({
);
}

const getLastUsedIndexPatternId = (
storage: IStorageWrapper,
indexPatternRefs: IndexPatternRef[]
) => {
const indexPattern = readFromStorage(storage, 'indexPatternId');
return indexPattern && indexPatternRefs.find((i) => i.id === indexPattern)?.id;
};

const setLastUsedIndexPatternId = (storage: IStorageWrapper, value: string) => {
writeToStorage(storage, 'indexPatternId', value);
};

export async function loadInitialState({
state,
savedObjectsClient,
defaultIndexPatternId,
storage,
}: {
state?: IndexPatternPersistedState;
savedObjectsClient: SavedObjectsClient;
defaultIndexPatternId?: string;
storage: IStorageWrapper;
}): Promise<IndexPatternPrivateState> {
const indexPatternRefs = await loadIndexPatternRefs(savedObjectsClient);
const lastUsedIndexPatternId = getLastUsedIndexPatternId(storage, indexPatternRefs);

const requiredPatterns = _.unique(
state
? Object.values(state.layers)
.map((l) => l.indexPatternId)
.concat(state.currentIndexPatternId)
: [defaultIndexPatternId || indexPatternRefs[0].id]
: [lastUsedIndexPatternId || defaultIndexPatternId || indexPatternRefs[0].id]
);

const currentIndexPatternId = requiredPatterns[0];
setLastUsedIndexPatternId(storage, currentIndexPatternId);

const indexPatterns = await loadIndexPatterns({
savedObjectsClient,
cache: {},
patterns: requiredPatterns,
});

if (state) {
return {
...state,
Expand Down Expand Up @@ -120,12 +139,14 @@ export async function changeIndexPattern({
state,
setState,
onError,
storage,
}: {
id: string;
savedObjectsClient: SavedObjectsClient;
state: IndexPatternPrivateState;
setState: SetState;
onError: ErrorHandler;
storage: IStorageWrapper;
}) {
try {
const indexPatterns = await loadIndexPatterns({
Expand All @@ -145,6 +166,7 @@ export async function changeIndexPattern({
},
currentIndexPatternId: id,
}));
setLastUsedIndexPatternId(storage, id);
} catch (err) {
onError(err);
}
Expand All @@ -158,6 +180,7 @@ export async function changeLayerIndexPattern({
setState,
onError,
replaceIfPossible,
storage,
}: {
indexPatternId: string;
layerId: string;
Expand All @@ -166,6 +189,7 @@ export async function changeLayerIndexPattern({
setState: SetState;
onError: ErrorHandler;
replaceIfPossible?: boolean;
storage: IStorageWrapper;
}) {
try {
const indexPatterns = await loadIndexPatterns({
Expand All @@ -186,6 +210,7 @@ export async function changeLayerIndexPattern({
},
currentIndexPatternId: replaceIfPossible ? indexPatternId : s.currentIndexPatternId,
}));
setLastUsedIndexPatternId(storage, indexPatternId);
} catch (err) {
onError(err);
}
Expand Down
17 changes: 17 additions & 0 deletions x-pack/plugins/lens/public/settings_storage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { IStorageWrapper } from 'src/plugins/kibana_utils/public';

const STORAGE_KEY = 'lens-settings';

export const readFromStorage = (storage: IStorageWrapper, key: string) => {
const data = storage.get(STORAGE_KEY);
return data && data[key];
};
export const writeToStorage = (storage: IStorageWrapper, key: string, value: string) => {
storage.set(STORAGE_KEY, { [key]: value });
};