-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Search] Use session service on a dashboard #81297
Changes from 12 commits
8f4b9ac
de93ecf
45055e0
7c8660b
05aca66
b1663a6
8fda2ee
7fe2507
7df03d1
6b7db17
923ca31
020f809
fda39c4
1960561
45f9981
8986867
368cedf
0bbeb44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,7 +139,7 @@ export class DashboardAppController { | |
dashboardCapabilities, | ||
scopedHistory, | ||
embeddableCapabilities: { visualizeCapabilities, mapsCapabilities }, | ||
data: { query: queryService }, | ||
data: { query: queryService, search: searchService }, | ||
core: { | ||
notifications, | ||
overlays, | ||
|
@@ -412,8 +412,9 @@ export class DashboardAppController { | |
>(DASHBOARD_CONTAINER_TYPE); | ||
|
||
if (dashboardFactory) { | ||
const searchSessionId = searchService.session.start(); | ||
dashboardFactory | ||
.create(getDashboardInput()) | ||
.create({ ...getDashboardInput(), searchSessionId }) | ||
.then((container: DashboardContainer | ErrorEmbeddable | undefined) => { | ||
if (container && !isErrorEmbeddable(container)) { | ||
dashboardContainer = container; | ||
|
@@ -572,7 +573,7 @@ export class DashboardAppController { | |
differences.filters = appStateDashboardInput.filters; | ||
} | ||
|
||
Object.keys(_.omit(containerInput, ['filters'])).forEach((key) => { | ||
Object.keys(_.omit(containerInput, ['filters', 'searchSessionId'])).forEach((key) => { | ||
const containerValue = (containerInput as { [key: string]: unknown })[key]; | ||
const appStateValue = ((appStateDashboardInput as unknown) as { [key: string]: unknown })[ | ||
key | ||
|
@@ -590,7 +591,8 @@ export class DashboardAppController { | |
const refreshDashboardContainer = () => { | ||
const changes = getChangesFromAppStateForContainerState(); | ||
if (changes && dashboardContainer) { | ||
dashboardContainer.updateInput(changes); | ||
const searchSessionId = searchService.session.start(); | ||
dashboardContainer.updateInput({ ...changes, searchSessionId }); | ||
} | ||
}; | ||
|
||
|
@@ -1109,12 +1111,6 @@ export class DashboardAppController { | |
$scope.model.filters = filterManager.getFilters(); | ||
$scope.model.query = queryStringManager.getQuery(); | ||
dashboardStateManager.applyFilters($scope.model.query, $scope.model.filters); | ||
if (dashboardContainer) { | ||
dashboardContainer.updateInput({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not needed. After removing all dashboard container updates are handled in single place right now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm in the middle of completely re-doing this file as part of deangularization. It will be a functional component - I may end up pinging you for some input at some point. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ThomThomson, that's great news 👍 Will be glad to chime in. |
||
filters: $scope.model.filters, | ||
query: $scope.model.query, | ||
}); | ||
} | ||
}, | ||
}); | ||
|
||
|
@@ -1159,6 +1155,7 @@ export class DashboardAppController { | |
if (dashboardContainer) { | ||
dashboardContainer.destroy(); | ||
} | ||
searchService.session.clear(); | ||
}); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -153,6 +153,21 @@ export class RequestsViewComponent extends Component<InspectorViewProps, Request | |
</EuiText> | ||
)} | ||
|
||
{this.state.request && this.state.request.searchSessionId && ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this only for the functional tests, or do you think there is value in showing this parameter to the user? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would also be useful for debugging and support purposes |
||
<EuiText size="xs"> | ||
<p | ||
data-test-subj={'inspectorRequestSearchSessionId'} | ||
data-search-session-id={this.state.request.searchSessionId} | ||
> | ||
<FormattedMessage | ||
id="inspector.requests.searchSessionId" | ||
defaultMessage="Search session id: {searchSessionId}" | ||
values={{ searchSessionId: this.state.request.searchSessionId }} | ||
/> | ||
</p> | ||
</EuiText> | ||
)} | ||
|
||
<EuiSpacer size="m" /> | ||
|
||
{this.state.request && <RequestDetails request={this.state.request} />} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a major nit, but do we really want to have the name be so specific?
I think sessions can and will be used for more than search.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
sessionId
is way too vague for dashboards / embeddable level, but OK on search level.sessionId
on dashboard could mean anything:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, if this terminology is for using in solutions :)