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.

Sign up for GitHub

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

Jump to bottom

Advanced Settings management app to kibana platform plugin #56931

Merged
merged 35 commits into from
Feb 11, 2020
Merged

Advanced Settings management app to kibana platform plugin #56931

merged 35 commits into from
Feb 11, 2020

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Feb 5, 2020 •
edited
Loading

Summary

Move Advanced Settings to its own kibana platform plugin.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@mattkime
Copy link
Contributor Author

mattkime commented Feb 5, 2020

@elasticmachine merge upstream

@mattkime
Copy link
Contributor Author

mattkime commented Feb 6, 2020

@elasticmachine merge upstream

@elastic elastic deleted a comment from kibanamachine Feb 6, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@elastic elastic deleted a comment from elasticmachine Feb 9, 2020
@mattkime mattkime changed the title Adv settings mgmt app to kibana platform Advanced Settings management app to kibana platform plugin Feb 9, 2020
@mattkime mattkime added release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v7.7.0 labels Feb 9, 2020
mattkime
mattkime commented Feb 9, 2020
View reviewed changes
src/plugins/advanced_settings/public/management_app/advanced_settings.tsx Outdated
const { queryText } = this.props;
const parsedQuery = Query.parse(queryText ? `ariaName:"${getAriaName(queryText)}"` : '');
// const parsedQuery = Query.parse(queryText ? `ariaName:"${getAriaName(queryText)}"` : '');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to figure out what this was for. Not functional in master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems this was useful minus including ariaName

@mattkime mattkime marked this pull request as ready for review February 9, 2020 16:24
@mattkime mattkime requested review from a team as code owners February 9, 2020 16:24
pgayvallet
pgayvallet approved these changes Feb 10, 2020
View reviewed changes
Copy link
Contributor

@pgayvallet pgayvallet 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 platform change once comments are addressed

src/core/public/ui_settings/ui_settings_service.mock.ts Outdated Show resolved Hide resolved
cchaos
cchaos approved these changes Feb 10, 2020
View reviewed changes
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

The SASS file moves are fine, though there really should be an _index.scss file within the new kibana/src/plugins/advanced_settings/public/ folder that then does all the imports of it's individual SASS files.

streamich
streamich approved these changes Feb 10, 2020
View reviewed changes
Copy link
Contributor

@streamich streamich 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 on Mac/Chrome. I just have two comments about tests below.

...lugins/advanced_settings/public/management_app/__snapshots__/advanced_settings.test.tsx.snap Outdated Show resolved Hide resolved
...ins/advanced_settings/public/management_app/components/form/__snapshots__/form.test.tsx.snap Outdated Show resolved Hide resolved
src/plugins/advanced_settings/public/management_app/components/form/form.test.tsx Outdated Show resolved Hide resolved
src/plugins/advanced_settings/public/management_app/components/form/form.test.tsx Show resolved Hide resolved
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@mattkime mattkime merged commit 0d30327 into elastic:master Feb 11, 2020
@mattkime mattkime mentioned this pull request Feb 11, 2020
Merged
mattkime added a commit to mattkime/kibana that referenced this pull request Feb 11, 2020
@mattkime
Advanced Settings management app to kibana platform plugin (elastic#5…
355e7f9 
…6931)

* advanced settings component registry to new platform
mattkime added a commit that referenced this pull request Feb 11, 2020
@mattkime
[7.x] Advanced Settings management app to kibana platform plugin (#56931
d3abab0 
) (#57341)

* Advanced Settings management app to kibana platform plugin (#56931)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 11, 2020
@gmmorris
Merge branch 'master' into alerting/alert-instances
7695820 
* master: (27 commits)
  Include actions new platform plugin for codeowners (elastic#57252)
  [APM][docs] 7.6 documentation updates (elastic#57124)
  Expressions refactor (elastic#54342)
  [ML] New Platform server shim: update annotation routes to use new platform router  (elastic#57067)
  Remove injected ui app vars from Canvas (elastic#56190)
  update max_anomaly_score route schema to handle possible undefined values (elastic#57339)
  [Add panel flyout] Moving create new to the top of SavedObjectFinder (elastic#56428)
  Add mock of a legacy ui api to re-enable Canvas storybook (elastic#56673)
  [monitoring] Removes noisy event received log (elastic#57275)
  Remove use of copied MANAGEMENT_BREADCRUMBS and use `setBreadcrumbs` from management section's mount (elastic#57324)
  Advanced Settings management app to kibana platform plugin (elastic#56931)
  [ML] New Platform server shim: update recognize modules routes to use new platform router (elastic#57206)
  [ML] Fix overall stats for saved search on the Data Visualizer page (elastic#57312)
  [ML] [NP] Removing ui imports (elastic#56358)
  [SIEM] Fixes failing Cypress tests (elastic#57202)
  Create observability CODEOWNERS reference (elastic#57109)
  fix results service schema (elastic#57217)
  don't register a wrapper if browser side function exists. (elastic#57196)
  Ui Actions explorer example (elastic#57006)
  Fix update alert API to still work when AAD is out of sync (elastic#57039)
  ...
@mattkime mattkime mentioned this pull request Feb 12, 2020
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cchaos cchaos cchaos approved these changes

@streamich streamich streamich approved these changes

@pgayvallet pgayvallet pgayvallet approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mattkime @elasticmachine @kibanamachine @cchaos @pgayvallet @streamich