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

[Discover] Attempt to fix onDataViewEdited flakiness by resetting dataStateContainer #199982

Closed
wants to merge 9 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,8 @@ export function getDiscoverStateContainer({
};

const onDataViewEdited = async (editedDataView: DataView) => {
console.log('onDataViewEdited', editedDataView);
dataStateContainer.reset();
if (editedDataView.isPersisted()) {
// Clear the current data view from the cache and create a new instance
// of it, ensuring we have a new object reference to trigger a re-render
Expand All @@ -421,7 +423,7 @@ export function getDiscoverStateContainer({
} else {
await updateAdHocDataViewId();
}
loadDataViewList();
await loadDataViewList();
addLog('[getDiscoverStateContainer] onDataViewEdited triggers data fetching');
fetchData();
};
Expand Down
3 changes: 1 addition & 2 deletions test/functional/apps/discover/group3/_lens_vis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
return seriesType;
}

// FLAKY: https://github.com/elastic/kibana/issues/184600
describe.skip('discover lens vis', function () {
describe('discover lens vis', function () {
before(async () => {
await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']);
await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,5 +475,6 @@ export function getMissingIndexPattern(
if (!missingIds.length) {
return [];
}
console.log('missingIds', { missingIds, currentDatasourceState, indexPatterns });
return missingIds;
}
26 changes: 22 additions & 4 deletions x-pack/plugins/lens/public/embeddable/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ import {
AddUserMessages,
UserMessagesGetter,
UserMessagesDisplayLocationId,
IndexPattern,
} from '../types';

import type {
Expand Down Expand Up @@ -475,6 +476,7 @@ export class Embeddable
},
parent
);
console.log('new Embeddable');

this.lensInspector = getLensInspectorService(deps.inspector);
this.expressionRenderer = deps.expressionRenderer;
Expand Down Expand Up @@ -632,6 +634,7 @@ export class Embeddable
};

public getUserMessages: UserMessagesGetter = (locationId, filters) => {
console.log('getUserMessages');
const userMessages: UserMessage[] = [];
userMessages.push(
...getApplicationUserMessages({
Expand Down Expand Up @@ -929,6 +932,7 @@ export class Embeddable
}

async initializeSavedVis(input: LensEmbeddableInput) {
console.log('initializeSavedVis');
const unwrapResult: LensUnwrapResult | false = await this.deps.attributeService
.unwrapAttributes(input)
.catch((e: Error) => {
Expand Down Expand Up @@ -959,6 +963,7 @@ export class Embeddable

this.expression = ast;
this.indexPatterns = indexPatterns;
console.log('initializeSavedVis', this.indexPatterns);
this.indexPatternRefs = indexPatternRefs;
this.activeVisualizationState = activeVisualizationState;
} catch {
Expand Down Expand Up @@ -1039,6 +1044,7 @@ export class Embeddable
};

private onRender: ExpressionWrapperProps['onRender$'] = () => {
console.log('onRender');
let datasourceEvents: string[] = [];
let visualizationEvents: string[] = [];

Expand Down Expand Up @@ -1552,8 +1558,20 @@ export class Embeddable
)
)
).forEach((dataView) => indexPatterns.push(dataView));

this.internalDataViews = uniqBy(indexPatterns, 'id');
const prevInternalDataViews = this.internalDataViews;

// making sure to not run into the race condition that the data source has the previous data view id
// causing to throw an exception that's not necessary
const internalDataViews = uniqBy([...indexPatterns, ...prevInternalDataViews], 'id');
console.log('Reassign internalDataViews initializeOutput', this.internalDataViews);
this.internalDataViews = internalDataViews;
// when we don't set this, this.getUserMessages, which is using it can run into a race condition
for (const dataView of internalDataViews) {
if (dataView.id && !this.indexPatterns[dataView.id]) {
// this is to prevent an exception in Discover
this.indexPatterns[dataView.id] = dataView as unknown as IndexPattern;
}
}

// passing edit url and index patterns to the output of this embeddable for
// the container to pick them up and use them to configure filter bar and
Expand All @@ -1565,7 +1583,7 @@ export class Embeddable
if (
!Boolean(this.isTextBasedLanguage()) &&
input.timeRange == null &&
indexPatterns.some((indexPattern) => indexPattern.isTimeBased())
internalDataViews.some((indexPattern) => indexPattern.isTimeBased())
) {
this.addUserMessages([
{
Expand All @@ -1582,7 +1600,7 @@ export class Embeddable
},
]);
}

// here is where the race condition happens
const blockingErrors = this.getUserMessages(blockingMessageDisplayLocations, {
severity: 'error',
});
Expand Down