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

Security/Spaces - cleanup react warnings #53287

Merged
merged 2 commits into from
Dec 19, 2019

Conversation

legrego
Copy link
Member

@legrego legrego commented Dec 17, 2019

Summary

Addresses React warnings in the Spaces and Security plugins. Most changes were test related, with a couple of changes from componentWillMount to componentDidMount.

There is one outstanding warning thrown when running our test suite, but it is thrown by FormattedRelative, which is part of our @kbn/i18n package as a result of an older version of react-intl. This should be resolved once #38642 is addressed.

@legrego legrego added chore v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.6.0 Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Dec 17, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@legrego legrego marked this pull request as ready for review December 17, 2019 15:47
@legrego legrego requested a review from a team as a code owner December 17, 2019 15:47
@legrego
Copy link
Member Author

legrego commented Dec 18, 2019

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

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

@legrego legrego requested a review from azasypkin December 18, 2019 14:40
@azasypkin
Copy link
Member

ACK: will review today or tomorrow CET morning.

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM!

await Promise.resolve();

wrapper.update();
await act(async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Ah, that's much better!

Copy link
Member Author

Choose a reason for hiding this comment

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

I came across that utility when writing tests for the role management UI. I thought you'd appreciate that change!

@legrego legrego merged commit 3b57f0a into elastic:master Dec 19, 2019
@legrego legrego deleted the security/cleanup-react-warnings branch December 19, 2019 12:19
legrego added a commit that referenced this pull request Dec 19, 2019
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
mbondyra added a commit to mbondyra/kibana that referenced this pull request Dec 19, 2019
…f github.com:mbondyra/kibana into IS-51910_share-lens-change-index-pattern-in-discover

* 'IS-51910_share-lens-change-index-pattern-in-discover' of github.com:mbondyra/kibana:
  [Discover] Remove angular field filter template code (elastic#53513)
  [APM] Improve table and other panel loading states (elastic#53459)
  Security/Spaces - cleanup react warnings (elastic#53287)
  Revert "NP licensing add functional tests (elastic#53002)" (elastic#53577)
  NP licensing add functional tests (elastic#53002)
  fix onLicenseInfoChange callback to be called on update (elastic#53559)
  Document how to extend request handler context (elastic#53271)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore release_note:skip Skip the PR/issue when compiling release notes Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants