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

[Dashboard] searchSessionId not updated when pinned filter changes #151219

Closed
nreese opened this issue Feb 14, 2023 · 1 comment · Fixed by #151390
Closed

[Dashboard] searchSessionId not updated when pinned filter changes #151219

nreese opened this issue Feb 14, 2023 · 1 comment · Fixed by #151390
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:Filters impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@nreese
Copy link
Contributor

nreese commented Feb 14, 2023

startDashboardSearchSessionIntegration dispatches setSearchSessionId when there is new query state. However, startDashboardSearchSessionIntegration uses getUnsavedChanges which uses dashboardDiffingFunctions to check for changes. dashboardDiffingFunctions removes pinned filters from the check since pinned filters are not part of application state.

https://github.com/elastic/kibana/blob/main/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/search_sessions/start_dashboard_search_session_integration.ts#L128

    // listen to and compare states to determine when to launch a new session.
    this.getInput$()
      .pipe(pairwise(), debounceTime(CHANGE_CHECK_DEBOUNCE))
      .subscribe(async (states) => {
        const [previous, current] = states as DashboardContainerByValueInput[];
        const changes = await getUnsavedChanges.bind(this)(previous, current, refetchKeys);
        const shouldRefetch = Object.keys(changes).length > 0;
        if (!shouldRefetch) return;

        const {
          getState,
          dispatch,
          actions: { setSearchSessionId },
        } = this.getReduxEmbeddableTools();
        const currentSearchSessionId = getState().explicitInput.searchSessionId;

        const updatedSearchSessionId: string | undefined = (() => {
          // do not update session id if this is irrelevant state change to prevent excessive searches
          if (!shouldRefetch) return;

          let searchSessionIdFromURL = getSearchSessionIdFromURL();
          if (searchSessionIdFromURL) {
            if (session.isRestore() && session.isCurrentSession(searchSessionIdFromURL)) {
              // we had previously been in a restored session but have now changed state so remove the session id from the URL.
              removeSessionIdFromUrl();
              searchSessionIdFromURL = undefined;
            } else {
              session.restore(searchSessionIdFromURL);
            }
          }
          return searchSessionIdFromURL ?? session.start();
        })();

        if (updatedSearchSessionId && updatedSearchSessionId !== currentSearchSessionId) {
          dispatch(setSearchSessionId(updatedSearchSessionId));
        }
      });
});

https://github.com/elastic/kibana/blob/main/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_functions.ts#L84

filters: ({ currentValue, lastValue }) =>
    compareFilters(
      currentValue.filter((f) => !isFilterPinned(f)),
      lastValue.filter((f) => !isFilterPinned(f)),
      COMPARE_ALL_OPTIONS
    ),
@nreese nreese added bug Fixes for quality problems that affect the customer experience Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Feb 14, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@nreese nreese changed the title [Dashboard] searchSessionId not updated when pinned filter changed [Dashboard] searchSessionId not updated when pinned filter changes Feb 14, 2023
@ThomThomson ThomThomson added Feature:Dashboard Dashboard related features Feature:Filters labels Feb 14, 2023
@nreese nreese self-assigned this Feb 15, 2023
nreese added a commit that referenced this issue Feb 21, 2023
#151390)

Fixes #151219 and
#151224

PR separates shouldRefresh logic from unsavedChanges logic to account
for difference in filter check.

shouldRefresh filter check:
* includes pinned filters
* excludes disabled filters
* excludes $state so pinning/unpinning a filter does not cause a
refresh.

---------

Co-authored-by: kibanamachine <[email protected]>
kibanamachine pushed a commit that referenced this issue Feb 21, 2023
#151390)

Fixes #151219 and
#151224

PR separates shouldRefresh logic from unsavedChanges logic to account
for difference in filter check.

shouldRefresh filter check:
* includes pinned filters
* excludes disabled filters
* excludes $state so pinning/unpinning a filter does not cause a
refresh.

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit cd910be)
kibanamachine added a commit that referenced this issue Feb 21, 2023
…changes (#151390) (#151742)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[Dashboard] fix searchSessionId not updated when pinned filter
changes (#151390)](#151390)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Nathan
Reese","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-02-21T16:31:06Z","message":"[Dashboard]
fix searchSessionId not updated when pinned filter changes
(#151390)\n\nFixes #151219
and\r\nhttps://github.com//issues/151224\r\n\r\nPR
separates shouldRefresh logic from unsavedChanges logic to
account\r\nfor difference in filter check.\r\n\r\nshouldRefresh filter
check:\r\n* includes pinned filters\r\n* excludes disabled filters\r\n*
excludes $state so pinning/unpinning a filter does not cause
a\r\nrefresh.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"cd910bee1cb062111e094c2744f153010e6b2e2c","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Dashboard","release_note:fix","Team:Presentation","loe:hours","impact:medium","auto-backport","v8.7.0","v8.8.0"],"number":151390,"url":"https://github.com/elastic/kibana/pull/151390","mergeCommit":{"message":"[Dashboard]
fix searchSessionId not updated when pinned filter changes
(#151390)\n\nFixes #151219
and\r\nhttps://github.com//issues/151224\r\n\r\nPR
separates shouldRefresh logic from unsavedChanges logic to
account\r\nfor difference in filter check.\r\n\r\nshouldRefresh filter
check:\r\n* includes pinned filters\r\n* excludes disabled filters\r\n*
excludes $state so pinning/unpinning a filter does not cause
a\r\nrefresh.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"cd910bee1cb062111e094c2744f153010e6b2e2c"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/151390","number":151390,"mergeCommit":{"message":"[Dashboard]
fix searchSessionId not updated when pinned filter changes
(#151390)\n\nFixes #151219
and\r\nhttps://github.com//issues/151224\r\n\r\nPR
separates shouldRefresh logic from unsavedChanges logic to
account\r\nfor difference in filter check.\r\n\r\nshouldRefresh filter
check:\r\n* includes pinned filters\r\n* excludes disabled filters\r\n*
excludes $state so pinning/unpinning a filter does not cause
a\r\nrefresh.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"cd910bee1cb062111e094c2744f153010e6b2e2c"}}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:Filters impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants