Skip to content

Removed a possibility to define two different names for Alert types on API and UI level.#86236

Merged
YulNaumenko merged 8 commits intoelastic:masterfrom
YulNaumenko:alerts-types-can-have-two-names
Dec 20, 2020
Merged

Removed a possibility to define two different names for Alert types on API and UI level.#86236
YulNaumenko merged 8 commits intoelastic:masterfrom
YulNaumenko:alerts-types-can-have-two-names

Conversation

@YulNaumenko
Copy link
Copy Markdown
Contributor

@YulNaumenko YulNaumenko commented Dec 17, 2020

Resolve #61695

@YulNaumenko YulNaumenko added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.12.0 labels Dec 17, 2020
@YulNaumenko YulNaumenko self-assigned this Dec 17, 2020
@YulNaumenko YulNaumenko requested review from a team as code owners December 17, 2020 05:54
@YulNaumenko YulNaumenko requested a review from a team December 17, 2020 05:54
@YulNaumenko YulNaumenko requested a review from a team as a code owner December 17, 2020 05:54
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 17, 2020

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Dec 17, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 17, 2020

Pinging @elastic/uptime (Team:uptime)

@mikecote mikecote self-requested a review December 17, 2020 12:50
mikecote
mikecote reviewed Dec 17, 2020
View reviewed changes
x-pack/plugins/triggers_actions_ui/public/types.ts Outdated
| React.LazyExoticComponent<ComponentType<AlertTypeParamsExpressionProps<AlertParamsType>>>;
requiresAppContext: boolean;
defaultActionMessage?: string;
name?: JSX.Element;
Copy link
Copy Markdown
Contributor

@mikecote mikecote Dec 17, 2020

Choose a reason for hiding this comment

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

I took a look at the places that still use this name property. It seems they're using it for string values. Can these be pulled from the server side now with the changes you've done?

Copy link
Copy Markdown
Contributor Author

@YulNaumenko YulNaumenko Dec 17, 2020

Choose a reason for hiding this comment

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

Yes, I've just missed some of the places, but fortunately we have a great tests coverage to catch this as well!

Copy link
Copy Markdown
Contributor

@mikecote mikecote Dec 17, 2020

Choose a reason for hiding this comment

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

I saw one place using <FormattedMessage ... />. I think we'll be able to remove this line after the cleanup?

Suggested change
name?: JSX.Element;

Copy link
Copy Markdown
Contributor Author

@YulNaumenko YulNaumenko Dec 17, 2020

Choose a reason for hiding this comment

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

Good catch! I didn't realized at first it is a FormattedMessage :-)

chrisronline
chrisronline approved these changes Dec 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

LGTM for Stack Monitoring

@YulNaumenko YulNaumenko requested a review from a team December 17, 2020 17:20
@botelastic botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Dec 17, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 17, 2020

Pinging @elastic/apm-ui (Team:apm)

@YulNaumenko YulNaumenko requested a review from mikecote December 17, 2020 17:44
ogupte
ogupte approved these changes Dec 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@ogupte ogupte left a comment

Choose a reason for hiding this comment

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

APM change LGTM!

mikecote
mikecote approved these changes Dec 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

Changes LGTM!

ymao1
ymao1 approved these changes Dec 18, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM!

andrewvc
andrewvc approved these changes Dec 18, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Dec 19, 2020

💚 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
monitoring 979.5KB 979.0KB -505.0B
triggersActionsUi 1.6MB 1.6MB -267.0B
uptime 1.0MB 1.0MB +12.0B
total -760.0B

Page load bundle

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

id before after diff
apm 49.2KB 48.6KB -586.0B
infra 180.2KB 179.8KB -405.0B
stackAlerts 17.3KB 16.9KB -449.0B
uptime 24.7KB 24.3KB -399.0B
total -1.8KB

History

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

@YulNaumenko YulNaumenko merged commit 396018f into elastic:master Dec 20, 2020
@YulNaumenko YulNaumenko mentioned this pull request Dec 20, 2020
Merged
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Dec 20, 2020
@YulNaumenko
Removed a possibility to define two different names for Alert types o…
cefbb33 
…n API and UI level. (elastic#86236)

* Removed a possibility to define two different names for Alert types on API and UI level

* fixed typechecks

* fixed typechecks

* fixed due to comments

* fixed typechecks

* fixed jest tests

* fixed typechecks
YulNaumenko added a commit that referenced this pull request Dec 20, 2020
@YulNaumenko
Removed a possibility to define two different names for Alert types o…
4737c73 
…n API and UI level. (#86236) (#86605)

* Removed a possibility to define two different names for Alert types on API and UI level

* fixed typechecks

* fixed typechecks

* fixed due to comments

* fixed typechecks

* fixed jest tests

* fixed typechecks
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 21, 2020
@gmmorris
Merge branch 'master' into alerts/remove-generics-defaults-redux
561ebb1 
* master: (48 commits)
  Fix request with disabled aggregation (elastic#85696)
  [Security Solution][Detections][Threshold Rules] Threshold Rule Bug Fixes (elastic#84918)
  Removed a possibility to define two different names for Alert types on API and UI level. (elastic#86236)
  Bump Node.js from version 14.15.2 to 14.15.3 (elastic#86593)
  [index patterns] Fleep app - Keep saved object field list until field caps provides fields (elastic#85370)
  [Security Solutions] fix timeline tabs + layout (elastic#86581)
  Upgrade to hapi version 20 (elastic#85406)
  App Services: Remove remaining uiActions, expressions, data, embeddable circular dependencies. (elastic#82791)
  Rename chartLibrary setting to legacyChartsLibrary (elastic#86529)
  [CI] TeamCity updates (elastic#85843)
  [Maps] Use Json for mvt-tests (elastic#86492)
  [Rollup Jobs] Added autofocus to cron editor (elastic#86324)
  [Monitoring][Alerting] CCR read exceptions alert (elastic#85908)
  [CI] Bump memory for main CI workers (elastic#86541)
  Explicitly set Elasticsearch heap size during CI and local development (elastic#86513)
  [App Search] Updates to results on the documents view (elastic#86181)
  [Discover] Change default sort handling  (elastic#85561)
  [App Search] Convert DocumentCreationModal to DocumentCreationFlyout (elastic#86508)
  [App Search] Sample Engines should have access to the Crawler (elastic#86502)
  Fixed duplication of create new modal (elastic#86489)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andrewvc andrewvc andrewvc approved these changes

@mikecote mikecote mikecote approved these changes

@ymao1 ymao1 ymao1 approved these changes

+2 more reviewers

@chrisronline chrisronline chrisronline approved these changes

@ogupte ogupte ogupte approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@YulNaumenko YulNaumenko

Labels

Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.12.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Alert types can have two names

8 participants

@YulNaumenko @elasticmachine @kibanamachine @chrisronline @andrewvc @ogupte @mikecote @ymao1