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

[Alerting UI] Fix console error when setting connector params #83333

Merged
merged 11 commits into from
Nov 17, 2020

Conversation

ymao1
Copy link
Contributor

@ymao1 ymao1 commented Nov 12, 2020

Resolves #83078

Summary

This is fixed by setting a default to an empty string in the TextAreaWithMessageVariables and TextFieldWithMessageVariables components when the input is undefined. If the initial input is undefined, the form control is uncontrolled, then setting a value changes the input to controlled, causing the console error seen in the issue. The issue specifically called out the PagerDuty connector but I also saw this behavior in other connector param forms.

To verify:

  1. Create and save a PagerDuty connector
  2. Click on the newly created connector to Edit and open a browser console
  3. Navigate to Test tab and enter text to Summary field
  4. Verify that no console error shows up
  5. (If desired) repeat for other connector types

@@ -72,10 +72,8 @@ export const ConnectorAddFlyout = ({
const [isSaving, setIsSaving] = useState<boolean>(false);

const closeFlyout = useCallback(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the following lines because I was seeing another console error when saving a connector. With this PR #82126, the connector flyouts are destroyed when closed instead of hidden, so setting these states on closing was causing a Warning: Can't perform a React state update on an unmounted component. browser error.

@ymao1 ymao1 changed the title Console error/pagerduty [Alerting UI] Fix console error when testing connector Nov 12, 2020
@ymao1 ymao1 changed the title [Alerting UI] Fix console error when testing connector [Alerting UI] Fix console error when setting connector params Nov 16, 2020
@ymao1 ymao1 self-assigned this Nov 16, 2020
@ymao1 ymao1 added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.11.0 v8.0.0 labels Nov 16, 2020
@ymao1 ymao1 marked this pull request as ready for review November 16, 2020 15:56
@ymao1 ymao1 requested a review from a team as a code owner November 16, 2020 15:56
@elasticmachine
Copy link
Contributor

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

@apmmachine
Copy link
Contributor

💚 Build Succeeded

@kibanamachine
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
triggersActionsUi 1.4MB 1.4MB -45.0B

History

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

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM! Tested locally and it works like a magic 👍

Copy link
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, tested locally 👍

@ymao1 ymao1 merged commit ee81b5f into elastic:master Nov 17, 2020
ymao1 added a commit to ymao1/kibana that referenced this pull request Nov 17, 2020
…c#83333)

* Fixing console errors

* Setting defaults for undefined inputs in text area/field with message variables

* Cleanup

* Cleanup

* Fixing pagerduty timestamp validation

* Fixing test

* Pagerduty params

* Reverting unnecessary changes
ymao1 added a commit that referenced this pull request Nov 17, 2020
#83537)

* Fixing console errors

* Setting defaults for undefined inputs in text area/field with message variables

* Cleanup

* Cleanup

* Fixing pagerduty timestamp validation

* Fixing test

* Pagerduty params

* Reverting unnecessary changes
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 17, 2020
* master: (51 commits)
  [ML] Persisted URL state for the Data frame analytics jobs and models pages (elastic#83439)
  adds xpack.security.authc.selector.enabled setting (elastic#83551)
  skip flaky suite (elastic#77279)
  [ML] Improve support for script and aggregation fields in anomaly detection jobs (elastic#81923)
  [Workplace Search] Migrate SourcesLogic from ent-search (elastic#83544)
  [ML] Add UI test for feature importance features (elastic#82677)
  [Maps] Improve icons for all layer types (elastic#83503)
  Replace experimental badge with Beta (elastic#83468)
  [Fleet][EPM] Unified install and archive (elastic#83384)
  Move src/legacy/server/keystore to src/cli (elastic#83483)
  Used SO for saving the API key IDs that should be deleted (elastic#82211)
  [Uptime] Mock implementation to account for math flakiness test (elastic#83535)
  [Workplace Search] Enable check for org context based on URL (elastic#83487)
  [App Search] Added all Document related routes and logic (elastic#83324)
  [Alerting UI] Fix console error when setting connector params (elastic#83333)
  [Discover] Allow custom name for fields via index pattern field management (elastic#70039)
  [Uptime] Fix monitor list down histogram (elastic#83411)
  remove headers timeout hack, rely on nodejs timeouts (elastic#83419)
  [ML] Update console autocomplete for ML data frame evaluate API (elastic#83151)
  [Lens] Color in dimension trigger (elastic#76871)
  ...
@ymao1 ymao1 deleted the console-error/pagerduty branch February 4, 2021 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Alerting UI] Console error on edit fileds in Testing tab of PagerDuty connector
6 participants