Created language manager inside query string service #7756

abbyhu2000 · 2024-08-19T20:17:03Z

Description

Get rid of settings class
Created a language manager class inside query string service to handle language related logics
Use language manager class to handle query enhancement logics
Update query enhancement interface to be:

export interface QueryEnhancement {
  language: string;
  search: SearchInterceptor;
  editor: (
    collapsedProps: any,
    expandedProps: any,
    bodyProps: any
  ) => EditorInstance<any, any, any>;
  meta?: {
    queryStringInput: {
      // will replace '<data_source>' with the data source name
      initialValue: string;
    };
  };
  fields?: {
    visualizable?: boolean;
  };
  // List of supported app names that this enhancement should be enabled for,
  // if not provided it will be enabled for all apps
  supportedAppNames?: string[];
}

Use query enhancement to receive query editor props and render query editor

Issues Resolved

Screenshot

Testing the changes

Changelog

feat: Created language manager inside query string service

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

codecov · 2024-08-19T20:37:49Z

Codecov Report

Attention: Patch coverage is 6.45161% with 29 lines in your changes missing coverage. Please review.

Project coverage is 52.56%. Comparing base (9d3ca79) to head (04e0dc8).
Report is 30 commits behind head on main.

Files	Patch %	Lines
.../query_string/language_manager/language_manager.ts	0.00%	15 Missing ⚠️
.../public/ui/dataset_navigator/dataset_navigator.tsx	0.00%	4 Missing ⚠️
src/plugins/data/public/plugin.ts	0.00%	3 Missing ⚠️
src/plugins/data/public/search/search_service.ts	0.00%	3 Missing ⚠️
src/plugins/data/public/query/query_service.ts	0.00%	2 Missing ⚠️
.../public/query/query_string/query_string_manager.ts	0.00%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (9d3ca79) and HEAD (04e0dc8). Click for more details.

HEAD has 4 uploads less than BASE

Flag BASE (9d3ca79) HEAD (04e0dc8)

Linux_4 1 0

Linux_3 1 0

Linux_1 1 0

Windows_3 1 0

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #7756       +/-   ##
===========================================
- Coverage   63.83%   52.56%   -11.27%     
===========================================
  Files        3655     3147      -508     
  Lines       81126    69225    -11901     
  Branches    12923    10747     -2176     
===========================================
- Hits        51783    36387    -15396     
- Misses      26165    30994     +4829     
+ Partials     3178     1844     -1334

Flag	Coverage Δ
Linux_1	`?`
Linux_2	`55.87% <ø> (ø)`
Linux_3	`?`
Linux_4	`?`
Windows_1	`29.25% <6.45%> (-0.85%)`	⬇️
Windows_2	`55.82% <ø> (ø)`
Windows_3	`?`
Windows_4	`27.76% <6.45%> (-3.53%)`	⬇️

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.

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

kavilla

nothing blocking (besides test failures).

some comments. also do we not want to register a default languages using the language manager?

i know we discussed before but putting for context.

kavilla · 2024-08-21T22:31:25Z

src/plugins/data/public/query/query_string/language_manager/language_manager.ts

@@ -22,21 +21,49 @@ export interface DataSettings {
  };
 }

-export class Settings {
+export class LanguageManager {
  private isEnabled = false;


i think this can be removed

kavilla · 2024-08-21T22:31:54Z

src/plugins/data/public/query/query_string/language_manager/language_manager.ts

  private isEnabled = false;
  private enabledQueryEnhancementsUpdated$ = new BehaviorSubject<boolean>(this.isEnabled);
  private enhancedAppNames: string[] = [];
+  private queryEnhancements: Map<string, QueryEnhancement>;
+  private queryEditorExtensionMap: Record<string, QueryEditorExtensionConfig>;


should be apart of the query enhancement

kavilla · 2024-08-21T22:34:16Z

src/plugins/data/public/search/search_service.ts

@@ -135,10 +135,10 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
    { fieldFormats, indexPatterns }: SearchServiceStartDependencies
  ): ISearchStart {
    const search = ((request, options) => {
+      const queryString = getQueryService().queryString;
      const selectedLanguage = getQueryService().queryString.getQuery().language;


can use the constant you create on queryString.getQuery().language

kavilla · 2024-08-21T22:36:23Z

src/plugins/data/public/ui/index.ts

@@ -46,6 +39,10 @@ export {
  QueryEditorExtensions,
  QueryEditorExtensionDependencies,
  QueryEditorExtensionConfig,
+  createEditor,


how do we intend to use this? (sorry if it's explained later in PR).
exporting components which devs can handle the state and prop or do they access it by the ui service?

kavilla · 2024-08-21T22:37:52Z

src/plugins/data/public/ui/query_editor/query_editor.tsx

 import { getQueryService, getIndexPatterns } from '../../services';
+import { DataSettings } from '../../query/query_string/language_manager/language_manager';


nit: do you have an index file defined here that exports the stuff you want to export, so you can just have ../../query/query_string/language_manager

kavilla · 2024-08-21T23:29:26Z

src/plugins/data/public/ui/query_editor/query_editor.tsx

+    // const dateRangeEnhancement = enhancement?.searchBar?.dateRange;
+    // const dateRange = dateRangeEnhancement
+    //   ? {
+    //       from: dateRangeEnhancement.initialFrom!,


i dont think we need this anymore. we did this initially because I thought quick date ranges like "Last 30 days" "Last 15 minutes" didn't work for PPL queries since I append the the date picker time filter to the query. but it should work with those quick ranges because we have a helper function to convert it

or we can double down and have a formatDateRange handler when you register a language

kavilla · 2024-08-21T23:30:24Z

src/plugins/data/public/ui/search_bar/search_bar.tsx

+      .getQueryEnhancements(this.state.query?.language!);
+
+    // if it is an enhanced language, check the filterable field
+    if (enhancement) {


kavilla · 2024-08-21T23:31:55Z

src/plugins/data/public/ui/ui_service.ts

-      },
-    };
-  }
+  public setup(core: CoreSetup, {}: UiServiceSetupDependencies): any {}


this should be IUiSetup just need to update the type IUiSetup to be what you expect now in this case empty

kavilla · 2024-08-21T23:33:06Z

src/plugins/data/public/ui/ui_service.ts

@@ -83,7 +57,6 @@ export class UiService implements Plugin<IUiSetup, IUiStart> {
      ),
      SearchBar,
      SuggestionsComponent,


Continue a comment from above, it seems like we do not expose the createEditor function from here. do people access it from the SearchBar?

github-actions bot added the distinguished-contributor label Aug 19, 2024

opensearch-changeset-bot bot added a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Aug 19, 2024

Changeset file for PR opensearch-project#7756 created/updated

1d0f28f

abbyhu2000 force-pushed the language_manager_new branch 2 times, most recently from 1ef8075 to cf615f8 Compare August 19, 2024 20:21

abbyhu2000 marked this pull request as ready for review August 19, 2024 20:22

opensearch-changeset-bot bot added a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Aug 19, 2024

Changeset file for PR opensearch-project#7756 created/updated

d62ff78

progress

56ddcc6

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

abbyhu2000 and others added 3 commits August 20, 2024 21:59

use language manager and query enhancement editor to render editor UI

9176247

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

make filter options dissapear for non-filterable languages

e00e200

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

Changeset file for PR opensearch-project#7756 created/updated

04e0dc8

abbyhu2000 force-pushed the language_manager_new branch from 1b5a865 to 04e0dc8 Compare August 20, 2024 22:08

kavilla reviewed Aug 21, 2024

View reviewed changes

kavilla added the discover-next label Aug 21, 2024

abbyhu2000 pushed a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Aug 21, 2024

Changeset file for PR opensearch-project#7756 created/updated

e02e831

kavilla closed this Aug 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Created language manager inside query string service #7756

Created language manager inside query string service #7756

abbyhu2000 commented Aug 19, 2024 •

edited

Loading

codecov bot commented Aug 19, 2024 •

edited

Loading

kavilla left a comment

kavilla Aug 21, 2024

kavilla Aug 21, 2024

kavilla Aug 21, 2024

kavilla Aug 21, 2024

kavilla Aug 21, 2024

kavilla Aug 21, 2024

kavilla Aug 21, 2024

kavilla Aug 21, 2024

kavilla Aug 21, 2024

kavilla Aug 21, 2024

		import { getQueryService, getIndexPatterns } from '../../services';
		import { DataSettings } from '../../query/query_string/language_manager/language_manager';

Created language manager inside query string service #7756

Created language manager inside query string service #7756

Conversation

abbyhu2000 commented Aug 19, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

codecov bot commented Aug 19, 2024 • edited Loading

Codecov Report

kavilla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abbyhu2000 commented Aug 19, 2024 •

edited

Loading

codecov bot commented Aug 19, 2024 •

edited

Loading