Fix dev tool console autocomplete not loading issue #3775

zhongnansu · 2023-04-04T00:42:40Z

Description

Dev tool console dynamic autocomplete is not working, this PR is to fix it . Solution is just pass along the http all the way to the opensearch.send()

Issues Resolved

#3515

Check List

codecov-commenter · 2023-04-04T01:49:53Z

Codecov Report

Merging #3775 (aa34740) into main (6b42669) will decrease coverage by 0.01%.
The diff coverage is 60.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3775      +/-   ##
==========================================
- Coverage   66.43%   66.43%   -0.01%     
==========================================
  Files        3210     3210              
  Lines       61677    61681       +4     
  Branches     9522     9522              
==========================================
+ Hits        40977    40978       +1     
- Misses      18419    18421       +2     
- Partials     2281     2282       +1

Flag	Coverage Δ
Linux	`66.38% <60.00%> (-0.01%)`	⬇️
Windows	`66.38% <60.00%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
...ensearch/use_send_current_request_to_opensearch.ts	`68.00% <0.00%> (ø)`
...rc/plugins/console/public/lib/mappings/mappings.js	`72.78% <60.34%> (+1.46%)`	⬆️
...containers/editor/legacy/console_editor/editor.tsx	`55.00% <100.00%> (ø)`

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

joshuarrrr

@zhongnansu A couple minor questions, but this looks good. Thanks for the jquery updates, too!

src/plugins/console/public/lib/mappings/mappings.js

joshuarrrr · 2023-04-04T19:38:45Z

src/plugins/console/public/lib/mappings/mappings.js

  let ret = [].concat.apply([], indicesOrAliases);
  ret.sort();
-  let last;
-  ret = $.map(ret, function (v) {
-    const r = last === v ? null : v;
-    last = v;
-    return r;
-  });
+  ret = ret.reduce((result, value, index, array) => {
+    const last = array[index - 1];
+    if (last !== value) {
+      result.push(value);
+    }
+    return result;


nit - this is fine, but all we're really trying to do is get an array with the unique values, right? Something like

const ret = [...new Set(indicesOrAliases)]]

src/plugins/console/public/lib/mappings/mappings.js

joshuarrrr · 2023-04-04T20:00:33Z

@Nicksqain If you have a chance, please review and compare to your changes in mapping.js from #3733

After this is merged, we can rebase your PR on top.

joshuarrrr · 2023-04-04T21:07:16Z

@zhongnansu The other thing I forgot to mention in the review - what ideas do you have about the best way to add a regression test for this? It's actually a fairly significant impact to lose this data-based autocompletion, so we'd like to make sure our tests catch it if it breaks in the future. My initial thought is that it makes the most sense as a functional test, because we can make sure it's actually correctly using the data to correctly populate the fields.

Signed-off-by: Su <[email protected]>

ashwin-pc · 2023-04-05T08:04:20Z

@zhongnansu why are we skipping the changelog here?

Nicksqain · 2023-04-05T11:13:52Z

@Nicksqain If you have a chance, please review and compare to your changes in mapping.js from #3733

After this is merged, we can rebase your PR on top.

@joshuarrrr Then maybe I should just copy the modified mapping.js from @zhongnansu into my mapping.js?
Or delete mapping.js in my PR

…mplete

Signed-off-by: Su <[email protected]>

zhongnansu · 2023-04-05T17:35:15Z

@zhongnansu why are we skipping the changelog here?

@ashwin-pc my bad, I though it was an unreleased bug. Added change log

zhongnansu · 2023-04-05T17:38:20Z

@zhongnansu The other thing I forgot to mention in the review - what ideas do you have about the best way to add a regression test for this? It's actually a fairly significant impact to lose this data-based autocompletion, so we'd like to make sure our tests catch it if it breaks in the future. My initial thought is that it makes the most sense as a functional test, because we can make sure it's actually correctly using the data to correctly populate the fields.

good point, I am having similar thoughts. Maybe create an issue in functional test repo to track

AMoo-Miki · 2023-04-05T18:58:19Z

src/plugins/console/public/lib/mappings/mappings.js

  if (typeof indicesOrAliases === 'string') {
    indicesOrAliases = [indicesOrAliases];
  }


How would you feel about changing this chunk and the above condition to:

Suggested change

if (typeof indicesOrAliases === 'string') {

indicesOrAliases = [indicesOrAliases];

}

if (typeof indicesOrAliases === 'string') {

indicesOrAliases = [indicesOrAliases];

} else if (!Array.isArray(indicesOrAliases)) {

return;

}

note that unlike the previous code, this is return nothing.

Since it's a new condition comparing to previous code, can you elaborate on the use case of returning noting, when indicesOrAliases is not a string and not an Array?

This kind of change seems more suitable as part of migrating this mappings file from js to ts. @zhongnansu Do you mind opening a stub issue for that?

joshuarrrr · 2023-04-05T21:49:44Z

good point, I am having similar thoughts. Maybe create an issue in functional test repo to track

OK, cool - please create that issue and link it here.

zhongnansu · 2023-04-05T22:01:20Z

good point, I am having similar thoughts. Maybe create an issue in functional test repo to track

OK, cool - please create that issue and link it here.

@joshuarrrr issue created opensearch-project/opensearch-dashboards-functional-test#605

joshuarrrr · 2023-04-06T18:49:15Z

src/plugins/console/public/lib/mappings/mappings.js

  if (typeof indicesOrAliases === 'string') {
    indicesOrAliases = [indicesOrAliases];
  }


This kind of change seems more suitable as part of migrating this mappings file from js to ts. @zhongnansu Do you mind opening a stub issue for that?

* Fix dev tool console autocomplete issue * add changelog Signed-off-by: Su <[email protected]> --------- Signed-off-by: Su <[email protected]> (cherry picked from commit 29008fa) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md

* Fix dev tool console autocomplete issue * add changelog Signed-off-by: Su <[email protected]> --------- Signed-off-by: Su <[email protected]> (cherry picked from commit 29008fa) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Zhongnan Su <[email protected]>

…ct#3775) * Fix dev tool console autocomplete issue * add changelog Signed-off-by: Su <[email protected]> --------- Signed-off-by: Su <[email protected]> Signed-off-by: David Sinclair <[email protected]>

zhongnansu requested a review from a team as a code owner April 4, 2023 00:42

zhongnansu requested a review from zengyan-amazon April 4, 2023 00:42

github-actions bot added the all-star-contributor label Apr 4, 2023

zhongnansu added v2.7.0 backport 2.x Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Apr 4, 2023

zhongnansu mentioned this pull request Apr 4, 2023

[BUG] Dev tool console autocomplete suggestion doesn't suggest index name #3515

Closed

zhongnansu mentioned this pull request Apr 4, 2023

2.7.0 Platform Upgrades for Plugins (O11y, MDS, Geospatial,+) #3674

Closed

23 tasks

zhongnansu requested a review from joshuarrrr April 4, 2023 17:37

zhongnansu assigned joshuarrrr and zengyan-amazon Apr 4, 2023

joshuarrrr mentioned this pull request Apr 4, 2023

Replace jquery usage in console plugin with native methods #3679

Closed

4 tasks

joshuarrrr reviewed Apr 4, 2023

View reviewed changes

zhongnansu requested a review from joshuarrrr April 4, 2023 20:54

Fix dev tool console autocomplete issue

d5f0812

Signed-off-by: Su <[email protected]>

zhongnansu force-pushed the dev-tool-fix-autocomplete branch from 2a4b2e2 to d5f0812 Compare April 4, 2023 23:39

zhongnansu changed the title ~~Fix dev tool console autocomplete issue~~ Fix dev tool console autocomplete not loading issue Apr 5, 2023

zhongnansu added 2 commits April 5, 2023 10:31

Merge remote-tracking branch 'upstream/main' into dev-tool-fix-autoco…

b70648c

…mplete

add changelog

aa34740

Signed-off-by: Su <[email protected]>

zhongnansu requested a review from ashwin-pc April 5, 2023 17:35

zhongnansu removed the Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry label Apr 5, 2023

zhongnansu assigned ashwin-pc and unassigned zengyan-amazon Apr 5, 2023

ashwin-pc approved these changes Apr 5, 2023

View reviewed changes

AMoo-Miki reviewed Apr 5, 2023

View reviewed changes

zhongnansu mentioned this pull request Apr 5, 2023

[FEATURE] Add functional test for dev tool console opensearch-project/opensearch-dashboards-functional-test#605

Open

2 tasks

zhongnansu added the console label Apr 6, 2023

joshuarrrr approved these changes Apr 6, 2023

View reviewed changes

joshuarrrr merged commit 29008fa into opensearch-project:main Apr 6, 2023

zhongnansu linked an issue Apr 6, 2023 that may be closed by this pull request

[BUG] Dev tool console autocomplete suggestion doesn't suggest index name #3515

Closed

opensearch-trigger-bot bot mentioned this pull request Apr 6, 2023

[Backport 2.x] Fix dev tool console autocomplete not loading issue #3799

Merged

This was referenced Apr 6, 2023

[Console] Convert all lib/mappings files to typescript #3800

Closed

[Multiple DataSource] Integrate multiple datasource with dev tool console #3754

Merged

Nicksqain mentioned this pull request Apr 7, 2023

[CCI] Replace jquery usage in console plugin with native methods #3733

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dev tool console autocomplete not loading issue #3775

Fix dev tool console autocomplete not loading issue #3775

zhongnansu commented Apr 4, 2023 •

edited

Loading

codecov-commenter commented Apr 4, 2023 •

edited

Loading

joshuarrrr left a comment

joshuarrrr Apr 4, 2023

joshuarrrr commented Apr 4, 2023

joshuarrrr commented Apr 4, 2023

ashwin-pc commented Apr 5, 2023

Nicksqain commented Apr 5, 2023 •

edited

Loading

zhongnansu commented Apr 5, 2023

zhongnansu commented Apr 5, 2023

AMoo-Miki Apr 5, 2023

zhongnansu Apr 5, 2023

joshuarrrr Apr 6, 2023

zhongnansu Apr 6, 2023

joshuarrrr commented Apr 5, 2023

zhongnansu commented Apr 5, 2023

joshuarrrr Apr 6, 2023

Fix dev tool console autocomplete not loading issue #3775

Fix dev tool console autocomplete not loading issue #3775

Conversation

zhongnansu commented Apr 4, 2023 • edited Loading

Description

Issues Resolved

Check List

codecov-commenter commented Apr 4, 2023 • edited Loading

Codecov Report

joshuarrrr left a comment

Choose a reason for hiding this comment

joshuarrrr Apr 4, 2023

Choose a reason for hiding this comment

joshuarrrr commented Apr 4, 2023

joshuarrrr commented Apr 4, 2023

ashwin-pc commented Apr 5, 2023

Nicksqain commented Apr 5, 2023 • edited Loading

zhongnansu commented Apr 5, 2023

zhongnansu commented Apr 5, 2023

AMoo-Miki Apr 5, 2023

Choose a reason for hiding this comment

zhongnansu Apr 5, 2023

Choose a reason for hiding this comment

joshuarrrr Apr 6, 2023

Choose a reason for hiding this comment

zhongnansu Apr 6, 2023

Choose a reason for hiding this comment

joshuarrrr commented Apr 5, 2023

zhongnansu commented Apr 5, 2023

joshuarrrr Apr 6, 2023

Choose a reason for hiding this comment

zhongnansu commented Apr 4, 2023 •

edited

Loading

codecov-commenter commented Apr 4, 2023 •

edited

Loading

Nicksqain commented Apr 5, 2023 •

edited

Loading