Skip to content

Commit

Permalink
fix issue with double fetch
Browse files Browse the repository at this point in the history
  • Loading branch information
Dosant committed Oct 29, 2020
1 parent 8986867 commit 368cedf
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ export class SearchEmbeddable
private prevTimeRange?: TimeRange;
private prevFilters?: Filter[];
private prevQuery?: Query;
private prevSearchSessionId?: string;

constructor(
{
Expand Down Expand Up @@ -332,8 +331,7 @@ export class SearchEmbeddable
!esFilters.onlyDisabledFiltersChanged(this.input.filters, this.prevFilters) ||
!_.isEqual(this.prevQuery, this.input.query) ||
!_.isEqual(this.prevTimeRange, this.input.timeRange) ||
!_.isEqual(searchScope.sort, this.input.sort || this.savedSearch.sort) ||
this.prevSearchSessionId !== this.input.searchSessionId;
!_.isEqual(searchScope.sort, this.input.sort || this.savedSearch.sort);

// If there is column or sort data on the panel, that means the original columns or sort settings have
// been overridden in a dashboard.
Expand All @@ -350,7 +348,6 @@ export class SearchEmbeddable
this.prevFilters = this.input.filters;
this.prevQuery = this.input.query;
this.prevTimeRange = this.input.timeRange;
this.prevSearchSessionId = this.input.searchSessionId;
} else if (this.searchScope) {
// trigger a digest cycle to make sure non-fetch relevant changes are propagated
this.searchScope.$applyAsync();
Expand Down
15 changes: 15 additions & 0 deletions src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { EmbeddableOutput, EmbeddableInput } from './i_embeddable';
import { ViewMode } from '../types';
import { ContactCardEmbeddable } from '../test_samples/embeddables/contact_card/contact_card_embeddable';
import { FilterableEmbeddable } from '../test_samples/embeddables/filterable_embeddable';
import type { Filter } from '../../../../data/public';

class TestClass {
constructor() {}
Expand Down Expand Up @@ -79,6 +80,20 @@ test('Embeddable reload is called if lastReloadRequest input time changes', asyn
expect(hello.reload).toBeCalledTimes(1);
});

test('Embeddable reload is called if lastReloadRequest input time changed and new input is used', async () => {
const hello = new FilterableEmbeddable({ id: '123', filters: [], lastReloadRequestTime: 0 });

const aFilter = ({} as unknown) as Filter;
hello.reload = jest.fn(() => {
// when reload is called embeddable already has new input
expect(hello.getInput().filters).toEqual([aFilter]);
});

hello.updateInput({ lastReloadRequestTime: 1, filters: [aFilter] });

expect(hello.reload).toBeCalledTimes(1);
});

test('Embeddable reload is not called if lastReloadRequest input time does not change', async () => {
const hello = new FilterableEmbeddable({ id: '123', filters: [], lastReloadRequestTime: 1 });

Expand Down
3 changes: 2 additions & 1 deletion src/plugins/embeddable/public/lib/embeddables/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,13 @@ export abstract class Embeddable<

private onResetInput(newInput: TEmbeddableInput) {
if (!isEqual(this.input, newInput)) {
const oldLastReloadRequestTime = this.input.lastReloadRequestTime;
this.input = newInput;
this.input$.next(newInput);
this.updateOutput({
title: getPanelTitle(this.input, this.output),
} as Partial<TEmbeddableOutput>);
if (this.input.lastReloadRequestTime !== newInput.lastReloadRequestTime) {
if (oldLastReloadRequestTime !== newInput.lastReloadRequestTime) {
this.reload();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ export class VisualizeEmbeddable
private timeRange?: TimeRange;
private query?: Query;
private filters?: Filter[];
private searchSessionId?: string;
private visCustomizations?: Pick<VisualizeInput, 'vis' | 'table'>;
private subscriptions: Subscription[] = [];
private expression: string = '';
Expand Down Expand Up @@ -244,11 +243,6 @@ export class VisualizeEmbeddable
dirty = true;
}

if (this.searchSessionId !== this.input.searchSessionId) {
this.searchSessionId = this.input.searchSessionId;
dirty = true;
}

if (this.vis.description && this.domNode) {
this.domNode.setAttribute('data-description', this.vis.description);
}
Expand Down Expand Up @@ -380,7 +374,7 @@ export class VisualizeEmbeddable
query: this.input.query,
filters: this.input.filters,
},
searchSessionId: this.searchSessionId,
searchSessionId: this.input.searchSessionId,
uiState: this.vis.uiState,
inspectorAdapters: this.inspectorAdapters,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ export class Embeddable
timeRange?: TimeRange;
query?: Query;
filters?: Filter[];
searchSessionId?: string;
lastReloadRequestTime?: number;
} = {};

Expand Down Expand Up @@ -150,14 +149,12 @@ export class Embeddable
if (
!_.isEqual(containerState.timeRange, this.externalSearchContext.timeRange) ||
!_.isEqual(containerState.query, this.externalSearchContext.query) ||
!_.isEqual(cleanedFilters, this.externalSearchContext.filters) ||
containerState.searchSessionId !== this.externalSearchContext.searchSessionId
!_.isEqual(cleanedFilters, this.externalSearchContext.filters)
) {
this.externalSearchContext = {
timeRange: containerState.timeRange,
query: containerState.query,
lastReloadRequestTime: this.externalSearchContext.lastReloadRequestTime,
searchSessionId: containerState.searchSessionId,
filters: cleanedFilters,
};

Expand All @@ -180,7 +177,7 @@ export class Embeddable
ExpressionRenderer={this.expressionRenderer}
expression={this.expression || null}
searchContext={this.getMergedSearchContext()}
searchSessionId={this.externalSearchContext.searchSessionId}
searchSessionId={this.input.searchSessionId}
handleEvent={this.handleEvent}
/>,
domNode
Expand Down

0 comments on commit 368cedf

Please sign in to comment.