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 link from dashboard saved search to Discover #92937

Merged
merged 9 commits into from
Mar 3, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe('Doc Table', () => {
setServices({
uiSettings: core.uiSettings,
filterManager: dataMock.query.filterManager,
addBasePath: (path) => path,
});

setDocViewsRegistry({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,16 @@ export function createTableRowDirective($compile: ng.ICompileService) {
$scope.row._id,
$scope.indexPattern.id,
$scope.columns,
getServices().filterManager
getServices().filterManager,
getServices().addBasePath
);
};

$scope.getSingleDocHref = () => {
return getServices().addBasePath(
`/app/discover#/doc/${$scope.indexPattern.id}/${
$scope.row._index
}?id=${encodeURIComponent($scope.row._id)}`
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
<a
class="euiLink"
data-test-subj="docTableRowAction"
ng-href="#/doc/{{indexPattern.id}}/{{row._index}}?id={{uriEncodedId}}"
ng-href="{{ getSingleDocHref() }}"
i18n-id="discover.docTable.tableRow.viewSingleDocumentLinkText"
i18n-default-message="View single document"
></a>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
</div>
</div>
<div class="kbnDocTable__container kbnDocTable__padBottom">
<table class="kbnDocTable table" ng-if="indexPattern">
<table class="kbnDocTable table" ng-if="indexPattern" data-test-subj="docTable">
<thead
kbn-table-header
columns="columns"
Expand All @@ -44,6 +44,7 @@
index-pattern="indexPattern"
filter="filter"
class="kbnDocTable__row"
data-test-subj="docTableRow{{ row['$$_isAnchor'] ? ' docTableAnchorRow' : ''}}"
Copy link
Member

Choose a reason for hiding this comment

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

Nit: docTableAnchorRow should not be necessary here, it's just used in context view and there infinite-scroll is set to true. It's here added to the paginated doc table that's used in the embeddable.

on-add-column="onAddColumn"
on-remove-column="onRemoveColumn"
use-new-fields-api="useNewFieldsApi"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ describe('Discover flyout', function () {
onClose={onClose}
onFilter={jest.fn()}
onRemoveColumn={jest.fn()}
services={({ filterManager: createFilterManagerMock() } as unknown) as DiscoverServices}
services={
({
filterManager: createFilterManagerMock(),
addBasePath: (path: string) => path,
} as unknown) as DiscoverServices
}
/>
);

Expand All @@ -53,17 +58,22 @@ describe('Discover flyout', function () {
onClose={onClose}
onFilter={jest.fn()}
onRemoveColumn={jest.fn()}
services={({ filterManager: createFilterManagerMock() } as unknown) as DiscoverServices}
services={
({
filterManager: createFilterManagerMock(),
addBasePath: (path: string) => `/base${path}`,
} as unknown) as DiscoverServices
}
/>
);

const actions = findTestSubject(component, 'docTableRowAction');
expect(actions.length).toBe(2);
expect(actions.first().prop('href')).toMatchInlineSnapshot(
`"#/doc/index-pattern-with-timefield-id/i?id=1"`
`"/base#/doc/index-pattern-with-timefield-id/i?id=1"`
);
expect(actions.last().prop('href')).toMatchInlineSnapshot(
`"#/context/index-pattern-with-timefield-id/1?_g=(filters:!())&_a=(columns:!(date),filters:!())"`
`"/base/app/discover#/context/index-pattern-with-timefield-id/1?_g=(filters:!())&_a=(columns:!(date),filters:!())"`
);
findTestSubject(component, 'euiFlyoutCloseButton').simulate('click');
expect(onClose).toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ export function DiscoverGridFlyout({
<EuiButtonEmpty
size="xs"
iconType="document"
href={`#/doc/${indexPattern.id}/${hit._index}?id=${encodeURIComponent(
hit._id as string
)}`}
href={services.addBasePath(
`#/doc/${indexPattern.id}/${hit._index}?id=${encodeURIComponent(
hit._id as string
)}`
)}
data-test-subj="docTableRowAction"
>
{i18n.translate('discover.grid.tableRow.viewSingleDocumentLinkTextSimple', {
Expand All @@ -96,7 +98,13 @@ export function DiscoverGridFlyout({
<EuiButtonEmpty
size="xs"
iconType="documents"
href={getContextUrl(hit._id, indexPattern.id, columns, services.filterManager)}
href={getContextUrl(
hit._id,
indexPattern.id,
columns,
services.filterManager,
services.addBasePath
)}
data-test-subj="docTableRowAction"
>
{i18n.translate('discover.grid.tableRow.viewSurroundingDocumentsLinkTextSimple', {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export function DocViewer(renderProps: DocViewRenderProps) {
}

return (
<div className="kbnDocViewer">
<div className="kbnDocViewer" data-test-subj="kbnDocViewer">
<EuiTabbedContent size="s" tabs={tabs} />
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,32 @@ const filterManager = ({
getGlobalFilters: () => [],
getAppFilters: () => [],
} as unknown) as FilterManager;
const addBasePath = (path: string) => `/base/${path}`;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: /base/${path} -> /base${path} and the expected url would be /base/app/....


describe('Get context url', () => {
test('returning a valid context url', async () => {
const url = await getContextUrl('docId', 'ipId', ['test1', 'test2'], filterManager);
const url = await getContextUrl(
'docId',
'ipId',
['test1', 'test2'],
filterManager,
addBasePath
);
expect(url).toMatchInlineSnapshot(
`"#/context/ipId/docId?_g=(filters:!())&_a=(columns:!(test1,test2),filters:!())"`
`"/base//app/discover#/context/ipId/docId?_g=(filters:!())&_a=(columns:!(test1,test2),filters:!())"`
);
});

test('returning a valid context url when docId contains whitespace', async () => {
const url = await getContextUrl('doc Id', 'ipId', ['test1', 'test2'], filterManager);
const url = await getContextUrl(
'doc Id',
'ipId',
['test1', 'test2'],
filterManager,
addBasePath
);
expect(url).toMatchInlineSnapshot(
`"#/context/ipId/doc%20Id?_g=(filters:!())&_a=(columns:!(test1,test2),filters:!())"`
`"/base//app/discover#/context/ipId/doc%20Id?_g=(filters:!())&_a=(columns:!(test1,test2),filters:!())"`
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { stringify } from 'query-string';
import rison from 'rison-node';
import { url } from '../../../../kibana_utils/common';
import { esFilters, FilterManager } from '../../../../data/public';
import { DiscoverServices } from '../../build_services';

/**
* Helper function to generate an URL to a document in Discover's context view
Expand All @@ -18,7 +19,8 @@ export function getContextUrl(
documentId: string,
indexPatternId: string,
columns: string[],
filterManager: FilterManager
filterManager: FilterManager,
addBasePath: DiscoverServices['addBasePath']
) {
const globalFilters = filterManager.getGlobalFilters();
const appFilters = filterManager.getAppFilters();
Expand All @@ -36,7 +38,9 @@ export function getContextUrl(
{ encode: false, sort: false }
);

return `#/context/${encodeURIComponent(indexPatternId)}/${encodeURIComponent(
documentId
)}?${hash}`;
return addBasePath(
`/app/discover#/context/${encodeURIComponent(indexPatternId)}/${encodeURIComponent(
documentId
)}?${hash}`
);
}
35 changes: 34 additions & 1 deletion test/functional/apps/context/_discover_navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,18 @@ export default function ({ getService, getPageObjects }) {
const retry = getService('retry');
const docTable = getService('docTable');
const filterBar = getService('filterBar');
const PageObjects = getPageObjects(['common', 'discover', 'timePicker', 'context']);
const PageObjects = getPageObjects([
'common',
'discover',
'timePicker',
'settings',
'dashboard',
'context',
'header',
]);
const testSubjects = getService('testSubjects');
const dashboardAddPanel = getService('dashboardAddPanel');
const browser = getService('browser');

describe('context link in discover', () => {
before(async () => {
Expand Down Expand Up @@ -94,5 +104,28 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.discover.waitForDiscoverAppOnScreen();
await PageObjects.discover.waitForDocTableLoadingComplete();
});

it('navigates to doc view from embeddable', async () => {
await PageObjects.common.navigateToApp('discover');
await PageObjects.discover.saveSearch('my search');
await PageObjects.header.waitUntilLoadingHasFinished();

await PageObjects.common.navigateToApp('dashboard');
await PageObjects.dashboard.gotoDashboardLandingPage();
await PageObjects.dashboard.clickNewDashboard();

await dashboardAddPanel.addSavedSearch('my search');
await PageObjects.header.waitUntilLoadingHasFinished();

await docTable.clickRowToggle({ rowIndex: 0 });
const rowActions = await docTable.getRowActions({ rowIndex: 0 });
await rowActions[1].click();
await PageObjects.common.sleep(250);
// accept alert if it pops up
const alert = await browser.getAlert();
await alert?.accept();
expect(await browser.getCurrentUrl()).to.contain('#/doc');
expect(await PageObjects.discover.isShowingDocViewer()).to.be(true);
});
});
}
35 changes: 34 additions & 1 deletion test/functional/apps/discover/_data_grid_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,19 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const filterBar = getService('filterBar');
const dataGrid = getService('dataGrid');
const docTable = getService('docTable');
const PageObjects = getPageObjects(['common', 'discover', 'timePicker', 'settings']);
const PageObjects = getPageObjects([
'common',
'discover',
'timePicker',
'settings',
'dashboard',
'header',
]);
const defaultSettings = { defaultIndex: 'logstash-*', 'doc_table:legacy': false };
const kibanaServer = getService('kibanaServer');
const esArchiver = getService('esArchiver');
const dashboardAddPanel = getService('dashboardAddPanel');
const browser = getService('browser');

describe('discover data grid context tests', () => {
before(async () => {
Expand Down Expand Up @@ -78,5 +87,29 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
}
expect(disabledFilterCounter).to.be(TEST_FILTER_COLUMN_NAMES.length);
});

it('navigates to context view from embeddable', async () => {
await PageObjects.common.navigateToApp('discover');
await PageObjects.discover.saveSearch('my search');
await PageObjects.header.waitUntilLoadingHasFinished();

await PageObjects.common.navigateToApp('dashboard');
await PageObjects.dashboard.gotoDashboardLandingPage();
await PageObjects.dashboard.clickNewDashboard();

await dashboardAddPanel.addSavedSearch('my search');
await PageObjects.header.waitUntilLoadingHasFinished();

await dataGrid.clickRowToggle({ rowIndex: 0 });
const rowActions = await dataGrid.getRowActions({ rowIndex: 0 });
await rowActions[1].click();
await PageObjects.common.sleep(250);
// accept alert if it pops up
const alert = await browser.getAlert();
await alert?.accept();
expect(await browser.getCurrentUrl()).to.contain('#/context');
await PageObjects.header.waitUntilLoadingHasFinished();
expect(await docTable.getRowsText()).to.have.length(6);
});
});
}
4 changes: 4 additions & 0 deletions test/functional/page_objects/discover_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,10 @@ export function DiscoverPageProvider({ getService, getPageObjects }: FtrProvider
await find.clickByCssSelector('.fa-sort-up');
}

public async isShowingDocViewer() {
return await testSubjects.exists('kbnDocViewer');
}

public async getMarks() {
const table = await docTable.getTable();
const $ = await table.parseDomContent();
Expand Down