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

[BeatsCM] Cleanup and refactor #26425

Closed
wants to merge 53 commits into from

Conversation

mattapperson
Copy link
Contributor

@mattapperson mattapperson commented Nov 29, 2018

This PR is a major step to cleaning up the code Beats CM codebase to better align to the arch docs, but also to add additional structure to the UI side that was lacking before.

This PR also:

  • cuts nearly in half the network calls we were making previously.
  • fixes a number of client side errors
  • render performance is drastically improved
  • KQL query bar now works on the tags list

Note: SOOO many files were touched here, extensive testing will be required to ensure no regressions were caused

# Conflicts:
#	x-pack/plugins/beats_management/public/lib/adapters/framework/adapter_types.ts
#	x-pack/plugins/beats_management/public/lib/adapters/framework/kibana_framework_adapter.ts
#	x-pack/plugins/beats_management/public/lib/lib.ts
#	x-pack/plugins/beats_management/public/router.tsx
# Conflicts:
#	x-pack/plugins/beats_management/index.ts
# Conflicts:
#	x-pack/plugins/beats_management/public/index.tsx
#	x-pack/plugins/beats_management/public/pages/main/walkthrough_review.tsx
#	x-pack/plugins/beats_management/public/pages/overview/finish_walkthrough.tsx
#	x-pack/plugins/beats_management/public/pages/overview/walkthrough_review.tsx
#	x-pack/plugins/beats_management/public/pages/tag.tsx
#	x-pack/plugins/beats_management/public/router.tsx
# Conflicts:
#	x-pack/plugins/beats_management/public/index.tsx
#	x-pack/plugins/beats_management/public/lib/adapters/framework/kibana_framework_adapter.ts
#	x-pack/plugins/beats_management/public/lib/compose/kibana.ts
#	x-pack/plugins/beats_management/public/pages/beat/index.tsx
#	x-pack/plugins/beats_management/public/pages/overview/create_tag_fragment.tsx
#	x-pack/plugins/beats_management/public/pages/overview/enroll_fragment.tsx
#	x-pack/plugins/beats_management/public/pages/overview/enrolled_beats.tsx
#	x-pack/plugins/beats_management/public/pages/overview/index.tsx
#	x-pack/plugins/beats_management/public/pages/overview/tag_configurations.tsx
#	x-pack/plugins/beats_management/public/pages/walkthrough/initial/finish.tsx
#	x-pack/plugins/beats_management/public/router.tsx
#	x-pack/plugins/beats_management/server/lib/adapters/framework/adapter_types.ts
#	x-pack/plugins/beats_management/server/lib/adapters/framework/hapi_framework_adapter.ts
#	x-pack/plugins/canvas/scripts/peg_build.js
# Conflicts:
#	x-pack/plugins/beats_management/public/router.tsx
# Conflicts:
#	x-pack/plugins/beats_management/common/constants/index.ts
# Conflicts:
#	x-pack/plugins/canvas/scripts/start.js
…apters

# Conflicts:
#	x-pack/package.json
#	x-pack/plugins/beats_management/public/index.tsx
#	x-pack/plugins/beats_management/public/pages/main/index.tsx
@mattapperson
Copy link
Contributor Author

Breadcrumbs now working, also they work in-line with #25887

@mattapperson
Copy link
Contributor Author

Translations pulled in with the last merge... now time for review and bug squashing 😊

@elastic elastic deleted a comment from elasticmachine Nov 30, 2018
@elastic elastic deleted a comment from elasticmachine Nov 30, 2018
@elastic elastic deleted a comment from elasticmachine Nov 30, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@mattapperson mattapperson requested review from ph and removed request for chrisronline December 3, 2018 16:52
@elasticmachine
Copy link
Contributor

💔 Build Failed

const DOM_ELEMENT_NAME = this.PLUGIN_ID.replace('_', '-');
const adapter = this;
this.routes.when(
`${path}${[...Array(6)].map((e, n) => `/:arg${n}?`).join('')}`, // Hack because angular 1 does not support wildcards
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add FIXME before this comment, that angular 1 reference bring back so much memories. :D

@ph
Copy link
Contributor

ph commented Dec 3, 2018

@mattapperson I did a quick look at the code, I cannot do a full review because I am not too familiar with typescript and how Kibana works I will leave that to @justinkambic.

One thing I saw is we have removed a few test files that are testing the behavior of the API is this intended? Like enroll, tokens and assign tags tests file.

@mattapperson
Copy link
Contributor Author

@mattapperson I did a quick look at the code, I cannot do a full review because I am not too familiar with typescript and how Kibana works I will leave that to @justinkambic.

One thing I saw is we have removed a few test files that are testing the behavior of the API is this intended? Like enroll, tokens and assign tags tests file.

Those files were merged and/or were empty. Additional tests will be added in a later PR so as to not make this one bigger then need be

@elastic elastic deleted a comment from elasticmachine Dec 4, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@mattapperson
Copy link
Contributor Author

Closing in favor of #26636

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants