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

[BUG][Discover] Updating a filter for a saved search does not show up when sharing #6838

Closed
huyaboo opened this issue May 26, 2024 · 4 comments
Assignees
Labels
bug Something isn't working discover for discover reinvent v2.16.0

Comments

@huyaboo
Copy link
Member

huyaboo commented May 26, 2024

Describe the bug

In 2.11 Discover, when a user modifies a saved search with new filters (but does not save the changes) and goes to share the link, the shared link does not show the updated filters. OSD 2.7 allowed this.

To Reproduce
Steps to reproduce the behavior:

  1. Go to Discover and select an Index Pattern (in the recording, I used opensearch_dashboards_sample_data_flights
  2. Click + Add filter and add some filter (in the recording, I used DestCountry is US
  3. Click Save at the top right to save the search and do not navigate away from the page
  4. Click + Add filter and add another filter (in the recording, I used OriginCountry is US
  5. Click Share and Copy link (this will also be reproduced when the Short URL is toggled)
  6. Navigate to the new link and the filter added in step 4 will not be present

Expected behavior
The filters should persist when sharing the link

OpenSearch Version
OS 2.11.1

Dashboards Version
OSD 2.11.1

Plugins
Only core plugins

Screenshots

Screen.Recording.2024-05-26.at.10.54.40.AM.mov

Host/Environment (please complete the following information):

  • OS: MacOS (Apple Silicon)
  • Browser and version: Firefox

Additional Context
I checked the links at several stages

Link with original filters before saving changes

http://localhost:5601/app/data-explorer/discover#?_a=(discover:(columns:!(_source),isDirty:!f,sort:!()),metadata:(indexPattern:d3d7af60-4c81-11e8-b3d7-01146121b73d,view:discover))&_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:now-1d,to:now))&_q=(filters:!(('$state':(store:appState),meta:(alias:!n,disabled:!f,index:d3d7af60-4c81-11e8-b3d7-01146121b73d,key:DestCountry,negate:!f,params:(query:US),type:phrase),query:(match_phrase:(DestCountry:US)))),query:(language:kuery,query:''))

Link of the saved search with original filters

http://localhost:5601/app/data-explorer/discover/#/view/f6feda30-19ef-11ef-aaf9-9d70eec92071?_q=(filters:!(('$state':(store:appState),meta:(alias:!n,disabled:!f,index:d3d7af60-4c81-11e8-b3d7-01146121b73d,key:DestCountry,negate:!f,params:(query:US),type:phrase),query:(match_phrase:(DestCountry:US)))),query:(language:kuery,query:''))&_a=(discover:(columns:!(_source),isDirty:!f,savedSearch:f6feda30-19ef-11ef-aaf9-9d70eec92071,sort:!()),metadata:(indexPattern:d3d7af60-4c81-11e8-b3d7-01146121b73d,view:discover))&_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:now-1d,to:now))

Shared link

http://localhost:5601/app/data-explorer/discover/#/view/f6feda30-19ef-11ef-aaf9-9d70eec92071?_q=(filters:!(('$state':(store:appState),meta:(alias:!n,disabled:!f,index:d3d7af60-4c81-11e8-b3d7-01146121b73d,key:DestCountry,negate:!f,params:(query:US),type:phrase),query:(match_phrase:(DestCountry:US))),('$state':(store:appState),meta:(alias:!n,disabled:!f,index:d3d7af60-4c81-11e8-b3d7-01146121b73d,key:OriginCountry,negate:!f,params:(query:US),type:phrase),query:(match_phrase:(OriginCountry:US)))),query:(language:kuery,query:''))&_a=(discover:(columns:!(_source),isDirty:!f,savedSearch:f6feda30-19ef-11ef-aaf9-9d70eec92071,sort:!()),metadata:(indexPattern:d3d7af60-4c81-11e8-b3d7-01146121b73d,view:discover))&_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:now-1d,to:now))

# Shortened url
http://localhost:5601/goto/1971a5625e978ddfb67dc89d84c0bb5c
@huyaboo huyaboo added bug Something isn't working untriaged labels May 26, 2024
@ananzh
Copy link
Member

ananzh commented Jun 4, 2024

When add a new filter to a saved search, then refresh the url: If saved object id exist, in the following effect

useEffect(() => {
    (async () => {
      const savedSearchInstance = await getSavedSearchById(savedSearchId);
      setSavedSearch(savedSearchInstance);

      // sync initial app filters from savedObject to filterManager
      const filters = cloneDeep(savedSearchInstance.searchSource.getOwnField('filter'));
      const query = savedSearchInstance.searchSource.getField('query') || data.query.queryString.getDefaultQuery();
      const actualFilters = [];
      if (filters !== undefined) {
        const result = typeof filters === 'function' ? filters() : filters;
        if (result !== undefined) {
          actualFilters.push(...(Array.isArray(result) ? result : [result]));
        }
      }
      filterManager.setAppFilters(actualFilters);
      data.query.queryString.setQuery(query);
      if (savedSearchInstance !== null && savedSearchInstance !== void 0 && savedSearchInstance.id) {
        chrome.recentlyAccessed.add(savedSearchInstance.getFullPath(), savedSearchInstance.title, savedSearchInstance.id);
      }
    })();
    return () => {};
    // This effect will only run when getSavedSearchById is called, which is
    // only called when the component is first mounted.
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [getSavedSearchById, savedSearchId]);

data.query.queryString.setQuery(query); will reset the filter using the saved filter in data service which will wrap out the data filter.

@ananzh ananzh removed the untriaged label Jun 4, 2024
@ananzh
Copy link
Member

ananzh commented Jun 4, 2024

We could modify the logic in use_search to

useEffect(() => {
    (async () => {
      const savedSearchInstance = await getSavedSearchById(savedSearchId);
      setSavedSearch(savedSearchInstance);

      // sync initial app filters from savedObject to filterManager
      const savedFilters = cloneDeep(savedSearchInstance.searchSource.getOwnField('filter'));
      const savedQuery = savedSearchInstance.searchSource.getField('query');
      // data.query.queryString.getDefaultQuery();
      const actualFilters = [];

      // Load the filters from the filter manager
      const filterManagerFilters = data.query.filterManager.getFilters();
      const queryStringQuery = data.query.queryString.getDefaultQuery();

      // Determine which set of filters to use
      let selectedFilters;
      let selectedQuery: Query | undefined;

      if (!filterManagerFilters || filterManagerFilters.length === 0) {
        // when first load, use saved object's filters
        selectedFilters = savedFilters;
        selectedQuery = savedQuery;
      } else {
        // allow update filters after load a saved search
        selectedFilters = filterManagerFilters;
        selectedQuery = queryStringQuery;
      }

      if (selectedFilters !== undefined) {
        const result = typeof selectedFilters === 'function' ? selectedFilters() : selectedFilters;
        if (result !== undefined) {
          actualFilters.push(...(Array.isArray(result) ? result : [result]));
        }
      }

      filterManager.setAppFilters(actualFilters);
      data.query.queryString.setQuery(selectedQuery);

      if (savedSearchInstance?.id) {
        chrome.recentlyAccessed.add(
          savedSearchInstance.getFullPath(),
          savedSearchInstance.title,
          savedSearchInstance.id
        );
      }
    })();

    return () => {};
    // This effect will only run when getSavedSearchById is called, which is
    // only called when the component is first mounted.
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [getSavedSearchById, savedSearchId]);
  1. when we load the url with id, filterManager should be empty. we just need to capture the saved filter.
  2. when we modify the filter later, we need to capture the filter from data service.

@ananzh
Copy link
Member

ananzh commented Jun 4, 2024

The query should have the same issue and we need to modify it as well

@abbyhu2000
Copy link
Member

This is fixed by #8168

Screen.Recording.2024-09-16.at.11.52.51.AM.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working discover for discover reinvent v2.16.0
Projects
None yet
Development

No branches or pull requests

3 participants