Temporary fix for fetchAllSavedQueriesForSelectedTab() to run saved queries #8805

riysaxen-amzn · 2024-11-04T18:47:45Z

Copied from draft PR -> #8796

Description

Issues Resolved

Screenshot

Testing the changes

 yarn test:jest src/plugins/data/public/ui/saved_query_flyouts/open_saved_query_flyout.test.tsx
yarn run v1.22.22
$ scripts/use_node scripts/jest src/plugins/data/public/ui/saved_query_flyouts/open_saved_query_flyout.test.tsx
jest-haste-map: duplicate manual mock found: index
  The following files share their name; please delete one of them:
    * <rootDir>/src/plugins/vis_type_vega/public/expressions/__mocks__/index.ts
    * <rootDir>/src/core/server/saved_objects/import/__mocks__/index.ts

  console.error
    Warning: Failed prop type: The prop `textToCopy` is marked as required in `EuiCopy`, but its value is `undefined`.
        in EuiCopy (created by SavedQueryCard)
        in SavedQueryCard (created by OpenSavedQueryFlyout)
        in div (created by EuiTabbedContent)
        in div (created by EuiTabbedContent)
        in EuiTabbedContent (created by OpenSavedQueryFlyout)
        in div (created by EuiFlyoutBody)
        in div (created by EuiFlyoutBody)
        in div (created by EuiFlyoutBody)
        in EuiFlyoutBody (created by OpenSavedQueryFlyout)
        in div (created by EuiFlyout)
        in EuiFlyout (created by OpenSavedQueryFlyout)
        in OpenSavedQueryFlyout

      164 |           </EuiFlexItem>
      165 |           <EuiFlexItem grow={false} style={{ marginTop: 0 }}>
    > 166 |             <EuiCopy textToCopy={savedQuery.attributes.query.query as string}>
          |             ^
      167 |               {(copy) => (
      168 |                 <EuiButtonIcon
      169 |                   onClick={copy}

      at console.error (node_modules/@testing-library/react/dist/act-compat.js:55:34)
      at printWarning (node_modules/prop-types/checkPropTypes.js:20:15)
      at checkPropTypes (node_modules/prop-types/checkPropTypes.js:83:11)
      at validatePropTypes (node_modules/react/cjs/react.development.js:1714:7)
      at Object.createElement (node_modules/react/cjs/react.development.js:1813:5)
      at SavedQueryCard (src/plugins/data/public/ui/saved_query_flyouts/saved_query_card.tsx:166:13)
      at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:14803:18)
      at mountIndeterminateComponent (node_modules/react-dom/cjs/react-dom.development.js:17482:13)
      at beginWork (node_modules/react-dom/cjs/react-dom.development.js:18596:16)
      at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:23179:14)
      at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:22157:12)
      at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:22130:22)
      at performSyncWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:21756:9)
      at node_modules/react-dom/cjs/react-dom.development.js:11089:24
      at unstable_runWithPriority (node_modules/scheduler/cjs/scheduler.development.js:653:12)
      at runWithPriority$1 (node_modules/react-dom/cjs/react-dom.development.js:11039:10)
      at flushSyncCallbackQueueImpl (node_modules/react-dom/cjs/react-dom.development.js:11084:7)
      at flushSyncCallbackQueue (node_modules/react-dom/cjs/react-dom.development.js:11072:3)
      at scheduleUpdateOnFiber (node_modules/react-dom/cjs/react-dom.development.js:21199:9)
      at setHasTemplateQueries (node_modules/react-dom/cjs/react-dom.development.js:15660:5)
      at src/plugins/data/public/ui/saved_query_flyouts/open_saved_query_flyout.tsx:74:5

  console.warn
    EuiButtonIcon requires aria-label or aria-labelledby to be specified because icon-only
          buttons are screen-reader-inaccessible without them.

      72 |     );
      73 |     setSavedQueries(queriesForSelectedTab);
    > 74 |     setHasTemplateQueries(templateQueriesPresent);
         |     ^
      75 |   }, [savedQueryService, selectedTabId, setSavedQueries]);
      76 |
      77 |   const updatePageIndex = useCallback((index: number) => {

      at EuiButtonIcon (node_modules/@elastic/eui/test-env/eui_components/button/button_icon/button_icon.js:110:13)
      at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:14803:18)
      at mountIndeterminateComponent (node_modules/react-dom/cjs/react-dom.development.js:17482:13)
      at beginWork (node_modules/react-dom/cjs/react-dom.development.js:18596:16)
      at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:23179:14)
      at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:22157:12)
      at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:22130:22)
      at performSyncWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:21756:9)
      at node_modules/react-dom/cjs/react-dom.development.js:11089:24
      at unstable_runWithPriority (node_modules/scheduler/cjs/scheduler.development.js:653:12)
      at runWithPriority$1 (node_modules/react-dom/cjs/react-dom.development.js:11039:10)
      at flushSyncCallbackQueueImpl (node_modules/react-dom/cjs/react-dom.development.js:11084:7)
      at flushSyncCallbackQueue (node_modules/react-dom/cjs/react-dom.development.js:11072:3)
      at scheduleUpdateOnFiber (node_modules/react-dom/cjs/react-dom.development.js:21199:9)
      at setHasTemplateQueries (node_modules/react-dom/cjs/react-dom.development.js:15660:5)
      at src/plugins/data/public/ui/saved_query_flyouts/open_saved_query_flyout.tsx:74:5

  console.warn
    EuiButtonIcon requires aria-label or aria-labelledby to be specified because icon-only
          buttons are screen-reader-inaccessible without them.

      72 |     );
      73 |     setSavedQueries(queriesForSelectedTab);
    > 74 |     setHasTemplateQueries(templateQueriesPresent);
         |     ^
      75 |   }, [savedQueryService, selectedTabId, setSavedQueries]);
      76 |
      77 |   const updatePageIndex = useCallback((index: number) => {

      at EuiButtonIcon (node_modules/@elastic/eui/test-env/eui_components/button/button_icon/button_icon.js:110:13)
      at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:14803:18)
      at mountIndeterminateComponent (node_modules/react-dom/cjs/react-dom.development.js:17482:13)
      at beginWork (node_modules/react-dom/cjs/react-dom.development.js:18596:16)
      at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:23179:14)
      at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:22157:12)
      at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:22130:22)
      at performSyncWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:21756:9)
      at node_modules/react-dom/cjs/react-dom.development.js:11089:24
      at unstable_runWithPriority (node_modules/scheduler/cjs/scheduler.development.js:653:12)
      at runWithPriority$1 (node_modules/react-dom/cjs/react-dom.development.js:11039:10)
      at flushSyncCallbackQueueImpl (node_modules/react-dom/cjs/react-dom.development.js:11084:7)
      at flushSyncCallbackQueue (node_modules/react-dom/cjs/react-dom.development.js:11072:3)
      at scheduleUpdateOnFiber (node_modules/react-dom/cjs/react-dom.development.js:21199:9)
      at setHasTemplateQueries (node_modules/react-dom/cjs/react-dom.development.js:15660:5)
      at src/plugins/data/public/ui/saved_query_flyouts/open_saved_query_flyout.tsx:74:5

 PASS  src/plugins/data/public/ui/saved_query_flyouts/open_saved_query_flyout.test.tsx
  OpenSavedQueryFlyout
    ✓ renders correctly and displays saved queries (225 ms)
    ✓ fetches saved queries on mount (59 ms)

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        5.594 s
Ran all test suites matching /src\/plugins\/data\/public\/ui\/saved_query_flyouts\/open_saved_query_flyout.test.tsx/i.
✨  Done in 10.75s.

Changelog

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

…ueries Signed-off-by: Riya Saxena <[email protected]>

github-actions · 2024-11-04T18:48:16Z

ℹ️ Manual Changeset Creation Reminder

Please ensure manual commit for changeset file 8805.yml under folder changelogs/fragments to complete this PR.

If you want to use the available OpenSearch Changeset Bot App to avoid manual creation of changeset file you can install it in your forked repository following this link.

For more information about formatting of changeset files, please visit OpenSearch Auto Changeset and Release Notes Tool.

Signed-off-by: Riya Saxena <[email protected]>

codecov · 2024-11-04T19:08:15Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.85%. Comparing base (78204a0) to head (67178ca).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8805      +/-   ##
==========================================
+ Coverage   60.78%   60.85%   +0.06%     
==========================================
  Files        3798     3798              
  Lines       90701    90701              
  Branches    14284    14284              
==========================================
+ Hits        55135    55195      +60     
+ Misses      32067    32000      -67     
- Partials     3499     3506       +7

Flag	Coverage Δ
Linux_1	`29.05% <ø> (ø)`
Linux_2	`56.39% <ø> (ø)`
Linux_3	`37.76% <100.00%> (+0.10%)`	⬆️
Linux_4	`29.82% <ø> (ø)`
Windows_1	`29.06% <ø> (ø)`
Windows_2	`56.34% <ø> (ø)`
Windows_3	`37.76% <100.00%> (+0.10%)`	⬆️
Windows_4	`29.82% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ashwin-pc · 2024-11-04T21:13:54Z

@riysaxen-amzn can you add detials about why this change is needed and what the issue is?

riysaxen-amzn · 2024-11-04T21:19:54Z

@riysaxen-amzn can you add detials about why this change is needed and what the issue is?

The existing logic to get all the saved queries is broken in NeoApp so we need this temp fix until a permanent fix is found

AMoo-Miki · 2024-11-04T22:05:10Z

The existing logic to get all the saved queries is broken in NeoApp so we need this temp fix until a permanent fix is found

Can you please explain what it is that is broken? Looking at the source code, findSavedQueries is more restrictive (based on supported languages of the app) than getAllSavedQueries. Is that restriction what you need?

virajsanghvi · 2024-11-04T23:32:31Z

src/plugins/data/public/ui/saved_query_flyouts/open_saved_query_flyout.tsx

-    const allQueries = await savedQueryService.getAllSavedQueries();
+    // const allQueries = await savedQueryService.getAllSavedQueries();
+    // TODO: Mitigation until getAllSavedQueries gets fixed
+    const { queries: allQueries } = await savedQueryService.findSavedQueries('', 10000);


Is this behind a feature flag or would it be used in either case? Is there a risk with the 10000 limit (I'm assuming)?

It is behind flag and we will get it fixed as a fast follow and we don't see a risk with count as 10k
That's the default upper limit

riysaxen-amzn · 2024-11-04T23:43:21Z

The existing logic to get all the saved queries is broken in NeoApp so we need this temp fix until a permanent fix is found

Can you please explain what it is that is broken? Looking at the source code, findSavedQueries is more restrictive (based on supported languages of the app) than getAllSavedQueries. Is that restriction what you need?

The getAllSavedQueries returns 0 as total if we pass per page as 0

joshuali925 · 2024-11-04T23:57:57Z

src/plugins/data/public/ui/saved_query_flyouts/open_saved_query_flyout.tsx

@@ -61,7 +61,9 @@ export function OpenSavedQueryFlyout({
  const [searchQuery, setSearchQuery] = useState(EuiSearchBar.Query.MATCH_ALL);

  const fetchAllSavedQueriesForSelectedTab = useCallback(async () => {
-    const allQueries = await savedQueryService.getAllSavedQueries();
+    // const allQueries = await savedQueryService.getAllSavedQueries();
+    // TODO: Mitigation until getAllSavedQueries gets fixed


what is the issue with getAllSavedQueries?

AMoo-Miki · 2024-11-05T00:15:49Z

The getAllSavedQueries returns 0 as total if we pass per page as 0

Then let's not pass 0. Changing this to 1 should be enough:

OpenSearch-Dashboards/src/plugins/data/public/query/saved_query/saved_query_service.ts

Line 210 in 48bfe2c

perPage: 0,

riysaxen-amzn · 2024-11-05T01:43:10Z

closing in favor of https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8808/commits

Temporary fix for fetchAllSavedQueriesForSelectedTab() to run saved q…

4bea096

…ueries Signed-off-by: Riya Saxena <[email protected]>

github-actions bot added the first-time-contributor label Nov 4, 2024

github-actions bot added the failed changeset label Nov 4, 2024

riysaxen-amzn and others added 2 commits November 4, 2024 10:49

changelogs added

256e300

Signed-off-by: Riya Saxena <[email protected]>

Merge branch 'main' into fetchAllSavedQueriesForSelectedTab

f34b495

github-actions bot removed the failed changeset label Nov 4, 2024

virajsanghvi reviewed Nov 4, 2024

View reviewed changes

Merge branch 'main' into fetchAllSavedQueriesForSelectedTab

67178ca

riysaxen-amzn requested a review from virajsanghvi November 4, 2024 23:49

joshuali925 reviewed Nov 4, 2024

View reviewed changes

riysaxen-amzn closed this Nov 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Temporary fix for fetchAllSavedQueriesForSelectedTab() to run saved queries #8805

Temporary fix for fetchAllSavedQueriesForSelectedTab() to run saved queries #8805

riysaxen-amzn commented Nov 4, 2024 •

edited

Loading

github-actions bot commented Nov 4, 2024

codecov bot commented Nov 4, 2024 •

edited

Loading

ashwin-pc commented Nov 4, 2024

riysaxen-amzn commented Nov 4, 2024

AMoo-Miki commented Nov 4, 2024

virajsanghvi Nov 4, 2024

riysaxen-amzn Nov 4, 2024

riysaxen-amzn commented Nov 4, 2024

joshuali925 Nov 4, 2024

AMoo-Miki commented Nov 5, 2024

riysaxen-amzn commented Nov 5, 2024

Temporary fix for fetchAllSavedQueriesForSelectedTab() to run saved queries #8805

Temporary fix for fetchAllSavedQueriesForSelectedTab() to run saved queries #8805

Conversation

riysaxen-amzn commented Nov 4, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

github-actions bot commented Nov 4, 2024

ℹ️ Manual Changeset Creation Reminder

codecov bot commented Nov 4, 2024 • edited Loading

Codecov Report

ashwin-pc commented Nov 4, 2024

riysaxen-amzn commented Nov 4, 2024

AMoo-Miki commented Nov 4, 2024

virajsanghvi Nov 4, 2024

Choose a reason for hiding this comment

riysaxen-amzn Nov 4, 2024

Choose a reason for hiding this comment

riysaxen-amzn commented Nov 4, 2024

joshuali925 Nov 4, 2024

Choose a reason for hiding this comment

AMoo-Miki commented Nov 5, 2024

riysaxen-amzn commented Nov 5, 2024

riysaxen-amzn commented Nov 4, 2024 •

edited

Loading

codecov bot commented Nov 4, 2024 •

edited

Loading