[UnifiedFieldList] Remove redundant server routes. Create new example plugin for unified field list components and migrate tests. #158377

jughosta · 2023-05-24T13:21:28Z

Summary

Before:

Unified Field List plugin has internal routes (wrappers for client code) which exist only to run api functional tests against them:

/api/unified_field_list/existing_fields/{dataViewId}
/api/unified_field_list/field_stats

Client code does not call these routes directly. So there is no reason in keeping and versioning them.

After:

Internal routes are removed
A new "Unified Field List Examples" page was created http://localhost:5601/app/unifiedFieldListExamples
API functional tests (which used the routes) were converted to functional tests against this new example page
Created a new unifiedFieldList page object which is used now in functional tests (methods are extracted from existing discover page object).

For testing:

Steps:

Run Kibana with examples: yarn start --run-examples
Install sample data
And navigate to Developer Examples > Unified Field List Examples page.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

…omponents

…fix'

…o 157109-remove-server-routes

…fix'

…o 157109-remove-server-routes

…r-routes

jughosta · 2023-05-26T11:48:25Z

src/plugins/unified_field_list/kibana.jsonc

@@ -5,7 +5,7 @@
  "description": "Contains functionality for the field list which can be integrated into apps",
  "plugin": {
    "id": "unifiedFieldList",
-    "server": true,
+    "server": false,


Removed the server code. Now we can convert unifiedFieldList from a plugin to a package!

Great PR removing legacy and providing documentation that can be tested! Just a question, it migrating to packages would have been possible before, but it would need 2 different packages for public and server code, much more effort right?

@kertal Yes, and a server package would consist of only routes for tests (if it's even possible to test package routes with functional API tests)

jughosta · 2023-05-26T11:52:47Z

src/plugins/discover/public/application/main/components/sidebar/discover_field.tsx

@@ -39,30 +39,25 @@ function getCommonFieldItemButtonProps({
 }: GetCommonFieldItemButtonPropsParams): {
  field: FieldItemButtonProps<DataViewField>['field'];
  isSelected: FieldItemButtonProps<DataViewField>['isSelected'];
-  dataTestSubj: FieldItemButtonProps<DataViewField>['dataTestSubj'];


It's a part of unifiedFieldList page now.

jughosta · 2023-05-26T11:53:39Z

src/plugins/unified_field_list/public/components/field_stats/field_stats.tsx

@@ -480,86 +482,91 @@ const FieldStatsComponent: React.FC<FieldStatsProps> = ({

    if (field.type === 'date') {
      return combineWithTitleAndFooter(
-        <div data-test-subj={`${dataTestSubject}-histogram`}>
-          <Chart size={{ height: 200, width: 300 - 32 }}>
+        <div data-test-subj="unifiedFieldStats-timeDistribution">


Only wrapped with an additional div with a stable data-test-subj.

jughosta · 2023-05-26T11:53:52Z

src/plugins/unified_field_list/public/components/field_stats/field_stats.tsx

+
+    if (showingHistogram || !topValues || !topValues.buckets.length) {
+      return combineWithTitleAndFooter(
+        <div data-test-subj="unifiedFieldStats-histogram">


jughosta · 2023-05-26T11:54:24Z

src/plugins/unified_field_list/public/components/field_stats/field_top_values.tsx

-      {buckets.map((bucket, index) => {
-        const fieldValue = bucket.key;
-        const formatted = formatter.convert(fieldValue);
+    <div


Same. Also only added a div wrapper.

jughosta · 2023-05-26T11:55:27Z

src/plugins/unified_field_list/public/services/field_existing/field_existing_utils.ts

-/**
- * Exported only for unit tests.
- */
-export function legacyExistingFields(docs: estypes.SearchHit[], fields: Field[]): string[] {


Found some unreachable code. Removed.

elasticmachine · 2023-05-26T13:41:06Z

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

ThomThomson

Code only review, functional test changes LGTM!

peteharverson

Edit to the transform plugin test LGTM for ML.

stratoula

Code review only, Visualizations team changes LGTM

nreese

kibana-gis changes LGTM
code review only

kertal

Code LGTM 👍 Great to have a nice example plugin, this is very helpful. And by removing / migrating those server side we don't need to migrate these APIs for server less. Win-win . Tested locally and works as expected

kibana-ci · 2023-05-31T11:00:04Z

💚 Build Succeeded

Buildkite Build
Commit: 0fb831e

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`unifiedFieldList`	267	270	+3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`discover`	421.6KB	421.5KB	-107.0B
`unifiedFieldList`	47.9KB	48.4KB	+431.0B
total			+324.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`unifiedFieldList`	52.8KB	53.1KB	+269.0B

Unknown metric groups

API count

id	before	after	diff
`unifiedFieldList`	293	296	+3

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	416	420	+4
`unifiedFieldList`	21	18	-3
total			+3

References to deprecated APIs

id	before	after	diff
`unifiedFieldList`	30	20	-10

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	500	504	+4
`unifiedFieldList`	21	18	-3
total			+3

History

💔 Build #131216 failed 284a101
💛 Build #130950 was flaky 1192c40
💚 Build #130745 succeeded f833612
💔 Build #130710 failed be5f421
💚 Build #130458 succeeded 3970974

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jughosta

jughosta and others added 22 commits May 24, 2023 15:20

[UnifiedFieldList] Create new example plugin for unified field list c…

2a92ef1

…omponents

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

94db4af

…fix'

[CI] Auto-commit changed files from 'node scripts/generate codeowners'

03fafa4

[UnifiedFieldList] Extend the example

2e69ec2

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

b2df82a

…fix'

[UnifiedFieldList] Update actions

8ab8123

[UnifiedFieldList] Fix types

04cb3dc

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

6da8214

…fix'

[UnifiedFieldList] Add a drop zone

1f76b03

Merge remote-tracking branch 'origin/157109-remove-server-routes' int…

45fe68f

…o 157109-remove-server-routes

[UnifiedFieldList] Add new page object

5f3910a

[UnifiedFieldList] Fix tests

5c74340

[UnifiedFieldList] Update more tests

4270689

[UnifiedFieldList] Update more tests

5e07ed3

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

1dbfe09

…fix'

[UnifiedFieldList] Update field existence tests

7b2720d

Merge remote-tracking branch 'origin/157109-remove-server-routes' int…

363a101

…o 157109-remove-server-routes

[UnifiedFieldList] Remove redundant code

d85cf13

[UnifiedFieldList] Rearrange the code

e68299b

[UnifiedFieldList] Revert some changes for later

27f82f3

Merge remote-tracking branch 'upstream/main' into 157109-remove-serve…

70a3a15

…r-routes

[UnifiedFieldList] Add a comment

8353c72

jughosta changed the title ~~[UnifiedFieldList] Create new example plugin for unified field list components~~ [UnifiedFieldList] Remove redundant server routes. Create new example plugin for unified field list components and migrate tests. May 26, 2023

jughosta self-assigned this May 26, 2023

jughosta commented May 26, 2023

View reviewed changes

jughosta marked this pull request as ready for review May 26, 2023 13:41

jughosta requested review from a team as code owners May 26, 2023 13:41

ThomThomson approved these changes May 26, 2023

View reviewed changes

peteharverson approved these changes May 26, 2023

View reviewed changes

jughosta added 2 commits May 28, 2023 16:06

Merge branch 'main' into 157109-remove-server-routes

be5f421

Merge branch 'main' into 157109-remove-server-routes

f833612

stratoula approved these changes May 29, 2023

View reviewed changes

[UnifiedFieldList] Use plugin id

1192c40

nreese approved these changes May 30, 2023

View reviewed changes

MichaelMarcialis approved these changes May 30, 2023

View reviewed changes

kertal approved these changes May 31, 2023

View reviewed changes

jughosta added 3 commits May 31, 2023 11:23

Merge branch 'main' into 157109-remove-server-routes

284a101

[UnifiedFieldList] Update tests

a10f893

Merge branch 'main' into 157109-remove-server-routes

0fb831e

jughosta merged commit 8d399fe into elastic:main May 31, 2023

jughosta deleted the 157109-remove-server-routes branch May 31, 2023 11:25

kibanamachine added the v8.9.0 label May 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UnifiedFieldList] Remove redundant server routes. Create new example plugin for unified field list components and migrate tests. #158377

[UnifiedFieldList] Remove redundant server routes. Create new example plugin for unified field list components and migrate tests. #158377

jughosta commented May 24, 2023 •

edited

Loading

jughosta May 26, 2023 •

edited

Loading

kertal May 30, 2023

jughosta May 30, 2023

jughosta May 26, 2023

jughosta May 26, 2023

jughosta May 26, 2023

jughosta May 26, 2023

jughosta May 26, 2023

elasticmachine commented May 26, 2023

ThomThomson left a comment

peteharverson left a comment

stratoula left a comment

nreese left a comment

kertal left a comment

kibana-ci commented May 31, 2023

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

[UnifiedFieldList] Remove redundant server routes. Create new example plugin for unified field list components and migrate tests. #158377

[UnifiedFieldList] Remove redundant server routes. Create new example plugin for unified field list components and migrate tests. #158377

Conversation

jughosta commented May 24, 2023 • edited Loading

Summary

Checklist

jughosta May 26, 2023 • edited Loading

Choose a reason for hiding this comment

kertal May 30, 2023

Choose a reason for hiding this comment

jughosta May 30, 2023

Choose a reason for hiding this comment

jughosta May 26, 2023

Choose a reason for hiding this comment

jughosta May 26, 2023

Choose a reason for hiding this comment

jughosta May 26, 2023

Choose a reason for hiding this comment

jughosta May 26, 2023

Choose a reason for hiding this comment

jughosta May 26, 2023

Choose a reason for hiding this comment

elasticmachine commented May 26, 2023

ThomThomson left a comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

nreese left a comment

Choose a reason for hiding this comment

kertal left a comment

Choose a reason for hiding this comment

kibana-ci commented May 31, 2023

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

jughosta commented May 24, 2023 •

edited

Loading

jughosta May 26, 2023 •

edited

Loading