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] Fix csv export with relative time filter from discover main view #123206

Merged
merged 21 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
6f282a7
[Discover] fix relative time filter for csv export from discover main…
dimaanj Jan 18, 2022
b1c777d
[Discover] fix array assignment
dimaanj Jan 19, 2022
403132c
[Discover] fix functional test
dimaanj Jan 20, 2022
b5c36d9
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Jan 20, 2022
cb16297
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Jan 24, 2022
5666be9
Merge branch 'main' of https://github.com/elastic/kibana into fix-csv…
dimaanj Jan 25, 2022
22c12c4
[Discover] add test coverage for the issue
dimaanj Jan 25, 2022
1199e7d
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Jan 26, 2022
c914287
[Discover] add debug points for functional test
dimaanj Jan 26, 2022
0565dd7
[Discover] try to get clipboard permissions
dimaanj Jan 26, 2022
762d51c
[Discover] fix functional test
dimaanj Jan 27, 2022
b2c7fbb
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Jan 27, 2022
23f21ef
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Jan 31, 2022
26b0493
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Feb 14, 2022
25a87bb
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Feb 22, 2022
2831c1c
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Mar 9, 2022
2040d86
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Mar 14, 2022
17927ed
[Discover] apply suggestion
dimaanj Mar 14, 2022
d1a345a
[Discover] apply suggestion
dimaanj Mar 15, 2022
135fb18
[Discover] apply naming suggestion
dimaanj Mar 16, 2022
4198cb8
Merge branch 'main' into fix-csv-for-relative-timefilter
kibanamachine Mar 16, 2022
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 @@ -315,7 +315,7 @@ export function setState(stateContainer: ReduxLikeStateContainer<AppState>, newS
/**
* Helper function to compare 2 different filter states
*/
export function isEqualFilters(filtersA: Filter[], filtersB: Filter[]) {
export function isEqualFilters(filtersA?: Filter[] | Filter, filtersB?: Filter[] | Filter) {
if (!filtersA && !filtersB) {
return true;
} else if (!filtersA || !filtersB) {
Expand Down
19 changes: 14 additions & 5 deletions src/plugins/discover/public/utils/get_sharing_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from '../../common';
import type { SavedSearch, SortOrder } from '../services/saved_searches';
import { getSortForSearchSource } from '../components/doc_table';
import { AppState } from '../application/main/services/discover_state';
import { AppState, isEqualFilters } from '../application/main/services/discover_state';

/**
* Preparing data to share the current state as link or CSV/Report
Expand All @@ -31,7 +31,7 @@ export async function getSharingData(
const { uiSettings: config, data } = services;
const searchSource = currentSearchSource.createCopy();
const index = searchSource.getField('index')!;
const existingFilter = searchSource.getField('filter');
let existingFilter = searchSource.getField('filter') as Filter[] | Filter | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

@ppisljar we could simplify consumer code by always returning Filter[] from getField('filter') instead of Filter[] | Filter | (() => Filter[] | Filter | undefined). WDYT?

It may have to be Filter[] | (() => Filter[]). Still, currently seems like a lot of things to check 😅


searchSource.setField(
'sort',
Expand Down Expand Up @@ -59,11 +59,20 @@ export async function getSharingData(
}
}

const absoluteTimeFilter = data.query.timefilter.timefilter.createFilter(index);
const relativeTimeFilter = data.query.timefilter.timefilter.createRelativeFilter(index);
return {
getSearchSource: (absoluteTime?: boolean): SerializedSearchSourceFields => {
const timeFilter = absoluteTime
? data.query.timefilter.timefilter.createFilter(index)
: data.query.timefilter.timefilter.createRelativeFilter(index);
const timeFilter = absoluteTime ? absoluteTimeFilter : relativeTimeFilter;

// remove timeFilter from existing filter
if (Array.isArray(existingFilter)) {
existingFilter = existingFilter.filter(
(current) => !isEqualFilters(current, absoluteTimeFilter)
);
} else if (isEqualFilters(existingFilter, absoluteTimeFilter)) {
existingFilter = undefined;
}

if (existingFilter && timeFilter) {
searchSource.setField(
Expand Down
54 changes: 53 additions & 1 deletion x-pack/test/functional/apps/discover/reporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import expect from '@kbn/expect';
import { Key } from 'selenium-webdriver';
import moment from 'moment';
import { FtrProviderContext } from '../../ftr_provider_context';

Expand All @@ -17,8 +18,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const kibanaServer = getService('kibanaServer');
const browser = getService('browser');
const retry = getService('retry');
const PageObjects = getPageObjects(['reporting', 'common', 'discover', 'timePicker']);
const PageObjects = getPageObjects(['reporting', 'common', 'discover', 'timePicker', 'share']);
const filterBar = getService('filterBar');
const find = getService('find');
const testSubjects = getService('testSubjects');

const setFieldsFromSource = async (setValue: boolean) => {
await kibanaServer.uiSettings.update({ 'discover:searchFieldsFromSource': setValue });
Expand Down Expand Up @@ -76,6 +79,55 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await PageObjects.discover.selectIndexPattern('ecommerce');
});

it('generates a report with single timefilter', async () => {
await PageObjects.discover.clickNewSearchButton();
await PageObjects.timePicker.setCommonlyUsedTime('Last_24 hours');
await PageObjects.discover.saveSearch('single-timefilter-search');

// get shared URL value
await PageObjects.share.clickShareTopNavButton();
const sharedURL = await PageObjects.share.getSharedUrl();

// click 'Copy POST URL'
await PageObjects.share.clickShareTopNavButton();
await PageObjects.reporting.openCsvReportingPanel();
const advOpt = await find.byXPath(`//button[descendant::*[text()='Advanced options']]`);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should really try to avoid this kind of selector in tests because it is tied to copy which can change

Copy link
Member

Choose a reason for hiding this comment

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

We'll make this easier for tests in a separate issue: #127842

await advOpt.click();
const postUrl = await find.byXPath(`//button[descendant::*[text()='Copy POST URL']]`);
await postUrl.click();

// get clipboard value using field search input, since
// 'browser.getClipboardValue()' doesn't work, due to permissions
const pastToElement = await testSubjects.find('fieldFilterSearchInput');
Copy link
Contributor

Choose a reason for hiding this comment

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

passedToElement? Might be simpler to just call it textInput?

Copy link
Member

Choose a reason for hiding this comment

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

We should come up with a better way to expose the POST URL content for functional tests like this, and expose a simple method to access it in https://github.com/elastic/kibana/blob/main/x-pack/test/reporting_functional/services/scenarios.ts

Let's do that within the Reporting team, in a separate issue.

await pastToElement.click();
await browser.getActions().keyDown(Key.CONTROL).perform();
await browser.getActions().keyDown('v').perform();

const reportURL = decodeURIComponent(await pastToElement.getAttribute('value'));

// get number of filters in URLs
const timeFiltersNumberInReportURL =
reportURL.split('query:(range:(order_date:(format:strict_date_optional_time').length - 1;
const timeFiltersNumberInSharedURL = sharedURL.split('time:').length - 1;

expect(timeFiltersNumberInSharedURL).to.be(1);
expect(sharedURL.includes('time:(from:now-24h%2Fh,to:now))')).to.be(true);

expect(timeFiltersNumberInReportURL).to.be(1);
expect(
reportURL.includes(
'query:(range:(order_date:(format:strict_date_optional_time,gte:now-24h/h,lte:now))))'
)
).to.be(true);

// return keyboard state
await browser.getActions().keyUp(Key.CONTROL).perform();
await browser.getActions().keyUp('v').perform();

// return field search input state
await pastToElement.clearValue();
});

it('generates a report from a new search with data: default', async () => {
await PageObjects.discover.clickNewSearchButton();
await PageObjects.reporting.setTimepickerInEcommerceDataRange();
Expand Down