fix: Dev tools console autocomplete issue #5567 #5568

kishor82 · 2023-12-03T01:23:33Z

Description

Resolves a bug in the Console Plugin Dev Tool where the retrieveSettings function was incorrectly called with 'fields' instead of 'indices'. for retrieveAliases function.

Issues Resolved

closes #5567

Screenshot

Check List

codecov · 2023-12-03T01:36:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9ba5cf5) 66.98% compared to head (2500f83) 67.02%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5568      +/-   ##
==========================================
+ Coverage   66.98%   67.02%   +0.03%     
==========================================
  Files        3293     3294       +1     
  Lines       63294    63296       +2     
  Branches    10067    10066       -1     
==========================================
+ Hits        42396    42422      +26     
+ Misses      18458    18431      -27     
- Partials     2440     2443       +3

Flag	Coverage Δ
Linux_1	`35.24% <ø> (ø)`
Linux_2	`55.17% <ø> (ø)`
Linux_3	`43.87% <100.00%> (+0.07%)`	⬆️
Linux_4	`35.34% <ø> (ø)`
Windows_1	`35.27% <ø> (ø)`
Windows_2	`55.14% <ø> (ø)`
Windows_3	`43.89% <100.00%> (+0.08%)`	⬆️
Windows_4	`35.34% <ø> (ø)`

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.

joshuarrrr

@kishor82 Thanks for the fix! Just a couple minor questions and suggestions.

joshuarrrr · 2023-12-08T01:38:48Z

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

+interface SettingKeyToPathMap {
+  fields: '_mapping';
+  indices: '_aliases';
+  templates: '_template';
+}


Why do we need to define this as an interface? It seems like it would be simpler to just define the const here, instead of within the retrieveSettings scope:

Suggested change

interface SettingKeyToPathMap {

fields: '_mapping';

indices: '_aliases';

templates: '_template';

}

const SETTING_KEY_TO_PATH_MAP = {

fields: '_mapping';

indices: '_aliases';

templates: '_template';

} as const;

Yes, it would be simpler to define it as a constant. I initially set it up for potential usage in tests and other functions, but it turned out to be unnecessary.

joshuarrrr · 2023-12-08T01:44:04Z

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

  Promise.allSettled([
    retrieveMappings(http, settingsToRetrieve, dataSourceId),
    retrieveAliases(http, settingsToRetrieve, dataSourceId),
    retrieveTemplates(http, settingsToRetrieve, dataSourceId),
-  ]).then(() => {
+  ]).then((res) => {


do we need this? If so, I think our style would generally be

Suggested change

]).then((res) => {

]).then((_res) => {

to hint that it's unused.

We no longer require this. I added a console while testing and forgot to remove it.

joshuarrrr · 2023-12-08T01:45:51Z

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

@@ -330,13 +336,8 @@ function retrieveSettings(
  if (settingsToRetrieve[settingsKey] === true) {
    return opensearch.send(http, 'GET', settingKeyToPathMap[settingsKey], null, dataSourceId);
  } else {
-    if (settingsToRetrieve[settingsKey] === false) {
-      // If the user doesn't want autocomplete suggestions, then clear any that exist
-      return Promise.resolve({});


Why did we want to change this response shape?

The check settingsToRetrieve[settingsKey] === false is unnecessary, so I have removed it. Additionally, in the else condition, the response body doesn't matter, whether it's undefined or {}, as long as it's not an instance of HttpResponse.

joshuarrrr · 2023-12-08T01:46:18Z

src/plugins/console/public/lib/opensearch/http_response.mock.ts

+  headers: Array<[string, string]>
+): Response => {
+  return {
+    // headers: {} as Headers,


remove

Suggested change

// headers: {} as Headers,

joshuarrrr

@kishor82 Thanks for all the fixes! I pulled it down locally and reproduced your demo - LGTM!

Signed-off-by: Kishor Rathva <kishorrathva8298@gmail.com>

ananzh · 2023-12-14T18:25:37Z

Also verified on 1.3.
1.3 has no issue. No backport to 1.3.

* fix: Dev tools console autocomplete issue #5567 * Added CHANGELOG * Added test for retrieveAutoCompleteInfo * Refactored tests * Added suggested changes. --------- Signed-off-by: Kishor Rathva <kishorrathva8298@gmail.com> (cherry picked from commit a5c45a3) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* fix: Dev tools console autocomplete issue #5567 * Added CHANGELOG * Added test for retrieveAutoCompleteInfo * Refactored tests * Added suggested changes. --------- (cherry picked from commit a5c45a3) Signed-off-by: Kishor Rathva <kishorrathva8298@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

kishor82 requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, kristenTian, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, BSFishy, curq and bandinib-amzn as code owners December 3, 2023 01:23

github-actions bot added the first-time-contributor label Dec 3, 2023

kishor82 force-pushed the fix/5567 branch 6 times, most recently from f6a4bdd to b3c82b4 Compare December 7, 2023 05:32

joshuarrrr reviewed Dec 8, 2023

View reviewed changes

joshuarrrr added v2.12.0 backport 2.x labels Dec 8, 2023

kishor82 requested a review from joshuarrrr December 8, 2023 07:00

kishor82 force-pushed the fix/5567 branch from 74635be to 94fda7a Compare December 12, 2023 19:02

kishor82 requested a review from SuZhou-Joe as a code owner December 12, 2023 19:02

kishor82 force-pushed the fix/5567 branch from 94fda7a to 2677796 Compare December 12, 2023 20:47

ananzh assigned joshuarrrr Dec 12, 2023

joshuarrrr approved these changes Dec 13, 2023

View reviewed changes

kishor82 force-pushed the fix/5567 branch 3 times, most recently from 62afa55 to 8a244f4 Compare December 13, 2023 23:24

kishor82 added 5 commits December 14, 2023 11:21

fix: Dev tools console autocomplete issue opensearch-project#5567

11ef6ce

Signed-off-by: Kishor Rathva <kishorrathva8298@gmail.com>

Added CHANGELOG

8ea5dda

Signed-off-by: Kishor Rathva <kishorrathva8298@gmail.com>

Added test for retrieveAutoCompleteInfo

8f3396e

Signed-off-by: Kishor Rathva <kishorrathva8298@gmail.com>

Refactored tests

310614f

Signed-off-by: Kishor Rathva <kishorrathva8298@gmail.com>

Added suggested changes.

2500f83

Signed-off-by: Kishor Rathva <kishorrathva8298@gmail.com>

kishor82 force-pushed the fix/5567 branch from 8a244f4 to 2500f83 Compare December 14, 2023 05:51

ananzh approved these changes Dec 14, 2023

View reviewed changes

ananzh added the bug Something isn't working label Dec 14, 2023

ananzh merged commit a5c45a3 into opensearch-project:main Dec 14, 2023
69 of 70 checks passed

opensearch-trigger-bot bot mentioned this pull request Dec 14, 2023

[Backport 2.x] fix: Dev tools console autocomplete issue #5567 #5614

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Dev tools console autocomplete issue #5567 #5568

fix: Dev tools console autocomplete issue #5567 #5568

kishor82 commented Dec 3, 2023 •

edited

Loading

codecov bot commented Dec 3, 2023 •

edited

Loading

joshuarrrr left a comment

joshuarrrr Dec 8, 2023

kishor82 Dec 8, 2023

joshuarrrr Dec 8, 2023

kishor82 Dec 8, 2023

joshuarrrr Dec 8, 2023

kishor82 Dec 8, 2023

joshuarrrr Dec 8, 2023

joshuarrrr left a comment •

edited

Loading

ananzh commented Dec 14, 2023

fix: Dev tools console autocomplete issue #5567 #5568

fix: Dev tools console autocomplete issue #5567 #5568

Conversation

kishor82 commented Dec 3, 2023 • edited Loading

Description

Issues Resolved

Screenshot

Check List

codecov bot commented Dec 3, 2023 • edited Loading

Codecov Report

joshuarrrr left a comment

Choose a reason for hiding this comment

joshuarrrr Dec 8, 2023

Choose a reason for hiding this comment

kishor82 Dec 8, 2023

Choose a reason for hiding this comment

joshuarrrr Dec 8, 2023

Choose a reason for hiding this comment

kishor82 Dec 8, 2023

Choose a reason for hiding this comment

joshuarrrr Dec 8, 2023

Choose a reason for hiding this comment

kishor82 Dec 8, 2023

Choose a reason for hiding this comment

joshuarrrr Dec 8, 2023

Choose a reason for hiding this comment

joshuarrrr left a comment • edited Loading

Choose a reason for hiding this comment

ananzh commented Dec 14, 2023

kishor82 commented Dec 3, 2023 •

edited

Loading

codecov bot commented Dec 3, 2023 •

edited

Loading

joshuarrrr left a comment •

edited

Loading