Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mappings editor] Add code improvements for source field #201188

Merged

Conversation

ElenaStoeva
Copy link
Contributor

@ElenaStoeva ElenaStoeva commented Nov 21, 2024

Closes #200769

Summary

This PR is a follow-up to #199854 and it adds the following code improvements:

  • Replaces Mappings-editor-context-level property hasEnterpriceLicense with plugin-context-level canUseSyntheticSource property
  • Adds jest tests to check if the synthetic option is correctly displayed based on license
  • Improves readability of serializer logic for the source field

How to test:
The same test instructions from #199854 can be followed with a focus on checking that the synthetic option is only available in Enterprise license.

@ElenaStoeva ElenaStoeva added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes Feature:Mappings Editor Index mappings editor UI v9.0.0 backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) v8.17.0 v8.18.0 labels Nov 21, 2024
@ElenaStoeva ElenaStoeva self-assigned this Nov 21, 2024
@ElenaStoeva ElenaStoeva marked this pull request as ready for review November 21, 2024 18:04
@ElenaStoeva ElenaStoeva requested a review from a team as a code owner November 21, 2024 18:04
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-management (Team:Kibana Management)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
indexManagement 698.8KB 698.5KB -352.0B

Page load bundle

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

id before after diff
indexManagement 46.6KB 46.9KB +325.0B

History

cc @ElenaStoeva

Copy link
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for addressing these, changes lgtm! 🚀

@ElenaStoeva ElenaStoeva merged commit 762bb7f into elastic:main Nov 25, 2024
21 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12010323249

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 25, 2024
)

Closes elastic#200769

## Summary

This PR is a follow-up to elastic#199854
and it adds the following code improvements:

- Replaces Mappings-editor-context-level property `hasEnterpriceLicense`
with plugin-context-level `canUseSyntheticSource` property
- Adds jest tests to check if the synthetic option is correctly
displayed based on license
- Improves readability of serializer logic for the source field

**How to test:**
The same test instructions from
elastic#199854 can be followed with a
focus on checking that the synthetic option is only available in
Enterprise license.

(cherry picked from commit 762bb7f)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 25, 2024
)

Closes elastic#200769

## Summary

This PR is a follow-up to elastic#199854
and it adds the following code improvements:

- Replaces Mappings-editor-context-level property `hasEnterpriceLicense`
with plugin-context-level `canUseSyntheticSource` property
- Adds jest tests to check if the synthetic option is correctly
displayed based on license
- Improves readability of serializer logic for the source field

**How to test:**
The same test instructions from
elastic#199854 can be followed with a
focus on checking that the synthetic option is only available in
Enterprise license.

(cherry picked from commit 762bb7f)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.17
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine mentioned this pull request Nov 25, 2024
2 tasks
kibanamachine added a commit that referenced this pull request Nov 25, 2024
…) (#201577)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Mappings editor] Add code improvements for source field
(#201188)](#201188)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Elena
Stoeva","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-25T12:46:05Z","message":"[Mappings
editor] Add code improvements for source field (#201188)\n\nCloses
https://github.com/elastic/kibana/issues/200769\r\n\r\n##
Summary\r\n\r\nThis PR is a follow-up to
https://github.com/elastic/kibana/pull/199854\r\nand it adds the
following code improvements:\r\n\r\n- Replaces
Mappings-editor-context-level property `hasEnterpriceLicense`\r\nwith
plugin-context-level `canUseSyntheticSource` property\r\n- Adds jest
tests to check if the synthetic option is correctly\r\ndisplayed based
on license\r\n- Improves readability of serializer logic for the source
field\r\n\r\n\r\n**How to test:**\r\nThe same test instructions
from\r\nhttps://github.com//pull/199854 can be followed
with a\r\nfocus on checking that the synthetic option is only available
in\r\nEnterprise
license.","sha":"762bb7f59d1d980aa34358c62ae0ef53e81f726e","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Kibana
Management","release_note:skip","Feature:Mappings
Editor","v9.0.0","backport:prev-minor","v8.17.0","v8.18.0"],"title":"[Mappings
editor] Add code improvements for source
field","number":201188,"url":"https://github.com/elastic/kibana/pull/201188","mergeCommit":{"message":"[Mappings
editor] Add code improvements for source field (#201188)\n\nCloses
https://github.com/elastic/kibana/issues/200769\r\n\r\n##
Summary\r\n\r\nThis PR is a follow-up to
https://github.com/elastic/kibana/pull/199854\r\nand it adds the
following code improvements:\r\n\r\n- Replaces
Mappings-editor-context-level property `hasEnterpriceLicense`\r\nwith
plugin-context-level `canUseSyntheticSource` property\r\n- Adds jest
tests to check if the synthetic option is correctly\r\ndisplayed based
on license\r\n- Improves readability of serializer logic for the source
field\r\n\r\n\r\n**How to test:**\r\nThe same test instructions
from\r\nhttps://github.com//pull/199854 can be followed
with a\r\nfocus on checking that the synthetic option is only available
in\r\nEnterprise
license.","sha":"762bb7f59d1d980aa34358c62ae0ef53e81f726e"}},"sourceBranch":"main","suggestedTargetBranches":["8.17","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201188","number":201188,"mergeCommit":{"message":"[Mappings
editor] Add code improvements for source field (#201188)\n\nCloses
https://github.com/elastic/kibana/issues/200769\r\n\r\n##
Summary\r\n\r\nThis PR is a follow-up to
https://github.com/elastic/kibana/pull/199854\r\nand it adds the
following code improvements:\r\n\r\n- Replaces
Mappings-editor-context-level property `hasEnterpriceLicense`\r\nwith
plugin-context-level `canUseSyntheticSource` property\r\n- Adds jest
tests to check if the synthetic option is correctly\r\ndisplayed based
on license\r\n- Improves readability of serializer logic for the source
field\r\n\r\n\r\n**How to test:**\r\nThe same test instructions
from\r\nhttps://github.com//pull/199854 can be followed
with a\r\nfocus on checking that the synthetic option is only available
in\r\nEnterprise
license.","sha":"762bb7f59d1d980aa34358c62ae0ef53e81f726e"}},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Elena Stoeva <[email protected]>
kibanamachine added a commit that referenced this pull request Nov 25, 2024
) (#201576)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Mappings editor] Add code improvements for source field
(#201188)](#201188)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Elena
Stoeva","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-25T12:46:05Z","message":"[Mappings
editor] Add code improvements for source field (#201188)\n\nCloses
https://github.com/elastic/kibana/issues/200769\r\n\r\n##
Summary\r\n\r\nThis PR is a follow-up to
https://github.com/elastic/kibana/pull/199854\r\nand it adds the
following code improvements:\r\n\r\n- Replaces
Mappings-editor-context-level property `hasEnterpriceLicense`\r\nwith
plugin-context-level `canUseSyntheticSource` property\r\n- Adds jest
tests to check if the synthetic option is correctly\r\ndisplayed based
on license\r\n- Improves readability of serializer logic for the source
field\r\n\r\n\r\n**How to test:**\r\nThe same test instructions
from\r\nhttps://github.com//pull/199854 can be followed
with a\r\nfocus on checking that the synthetic option is only available
in\r\nEnterprise
license.","sha":"762bb7f59d1d980aa34358c62ae0ef53e81f726e","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Kibana
Management","release_note:skip","Feature:Mappings
Editor","v9.0.0","backport:prev-minor","v8.17.0","v8.18.0"],"title":"[Mappings
editor] Add code improvements for source
field","number":201188,"url":"https://github.com/elastic/kibana/pull/201188","mergeCommit":{"message":"[Mappings
editor] Add code improvements for source field (#201188)\n\nCloses
https://github.com/elastic/kibana/issues/200769\r\n\r\n##
Summary\r\n\r\nThis PR is a follow-up to
https://github.com/elastic/kibana/pull/199854\r\nand it adds the
following code improvements:\r\n\r\n- Replaces
Mappings-editor-context-level property `hasEnterpriceLicense`\r\nwith
plugin-context-level `canUseSyntheticSource` property\r\n- Adds jest
tests to check if the synthetic option is correctly\r\ndisplayed based
on license\r\n- Improves readability of serializer logic for the source
field\r\n\r\n\r\n**How to test:**\r\nThe same test instructions
from\r\nhttps://github.com//pull/199854 can be followed
with a\r\nfocus on checking that the synthetic option is only available
in\r\nEnterprise
license.","sha":"762bb7f59d1d980aa34358c62ae0ef53e81f726e"}},"sourceBranch":"main","suggestedTargetBranches":["8.17","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201188","number":201188,"mergeCommit":{"message":"[Mappings
editor] Add code improvements for source field (#201188)\n\nCloses
https://github.com/elastic/kibana/issues/200769\r\n\r\n##
Summary\r\n\r\nThis PR is a follow-up to
https://github.com/elastic/kibana/pull/199854\r\nand it adds the
following code improvements:\r\n\r\n- Replaces
Mappings-editor-context-level property `hasEnterpriceLicense`\r\nwith
plugin-context-level `canUseSyntheticSource` property\r\n- Adds jest
tests to check if the synthetic option is correctly\r\ndisplayed based
on license\r\n- Improves readability of serializer logic for the source
field\r\n\r\n\r\n**How to test:**\r\nThe same test instructions
from\r\nhttps://github.com//pull/199854 can be followed
with a\r\nfocus on checking that the synthetic option is only available
in\r\nEnterprise
license.","sha":"762bb7f59d1d980aa34358c62ae0ef53e81f726e"}},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Elena Stoeva <[email protected]>
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024
)

Closes elastic#200769

## Summary

This PR is a follow-up to elastic#199854
and it adds the following code improvements:

- Replaces Mappings-editor-context-level property `hasEnterpriceLicense`
with plugin-context-level `canUseSyntheticSource` property
- Adds jest tests to check if the synthetic option is correctly
displayed based on license
- Improves readability of serializer logic for the source field


**How to test:**
The same test instructions from
elastic#199854 can be followed with a
focus on checking that the synthetic option is only available in
Enterprise license.
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
)

Closes elastic#200769

## Summary

This PR is a follow-up to elastic#199854
and it adds the following code improvements:

- Replaces Mappings-editor-context-level property `hasEnterpriceLicense`
with plugin-context-level `canUseSyntheticSource` property
- Adds jest tests to check if the synthetic option is correctly
displayed based on license
- Improves readability of serializer logic for the source field


**How to test:**
The same test instructions from
elastic#199854 can be followed with a
focus on checking that the synthetic option is only available in
Enterprise license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) Feature:Mappings Editor Index mappings editor UI release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.17.0 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Mappings Editor] Follow-up improvement for _source field
4 participants