Skip to content

[Reporting] server side code clean up#106940

Merged
tsullivan merged 17 commits intoelastic:masterfrom
tsullivan:reporting/legacy-cleanups
Aug 11, 2021
Merged

[Reporting] server side code clean up#106940
tsullivan merged 17 commits intoelastic:masterfrom
tsullivan:reporting/legacy-cleanups

Conversation

@tsullivan
Copy link
Copy Markdown
Member

@tsullivan tsullivan commented Jul 27, 2021 •
edited
Loading

Summary

Cleans up code in Reporting that uses leftover patterns from the old platform. This code used an unnecessary amount of abstraction that made it difficult to jump to function definitions.

Closes #96253

Checklist

For maintainers

@tsullivan tsullivan changed the title Reporting/legacy cleanups [Reporting] server side code clean up Jul 28, 2021
@tsullivan tsullivan requested review from a team July 28, 2021 00:17
@tsullivan tsullivan added release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v8.0.0 labels Jul 28, 2021
@tsullivan tsullivan marked this pull request as ready for review July 28, 2021 00:18
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jul 28, 2021

Pinging @elastic/kibana-app-services (Team:AppServices)

@tsullivan
Copy link
Copy Markdown
Member Author

tsullivan commented Jul 29, 2021

@elasticmachine merge upstream

@tsullivan tsullivan mentioned this pull request Jul 29, 2021
Merged
2 tasks
tsullivan
tsullivan commented Jul 29, 2021
View reviewed changes
x-pack/plugins/reporting/server/lib/enqueue_job.ts
): EnqueueJobFn {
): Promise<Report> {
const logger = parentLogger.clone(['createJob']);
return async function enqueueJob(
Copy link
Copy Markdown
Member Author

@tsullivan tsullivan Jul 29, 2021

Choose a reason for hiding this comment

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

It's probably easier to review changes like this with the feature in Github that lets you ignore whitespace changes: add ?w=1 to the github URL.

@tsullivan
Copy link
Copy Markdown
Member Author

tsullivan commented Jul 30, 2021

@elasticmachine merge upstream

@tsullivan
Copy link
Copy Markdown
Member Author

tsullivan commented Aug 2, 2021

@elasticmachine merge upstream

dokmic
dokmic approved these changes Aug 4, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@dokmic dokmic left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Nice cleanup removing an unnecessary layer.

jloleysens
jloleysens approved these changes Aug 9, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Changes look good to me! Thanks a lot for doing this 🍻

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Aug 11, 2021

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

API count

id before after diff
reporting 140 139 -1

API count missing comments

id before after diff
reporting 139 138 -1

Non-exported public API item count

id before after diff
reporting 15 14 -1

History

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

@tsullivan tsullivan merged commit e4e22ab into elastic:master Aug 11, 2021
@tsullivan tsullivan deleted the reporting/legacy-cleanups branch August 11, 2021 05:34
@tsullivan tsullivan mentioned this pull request Aug 11, 2021
Merged
tsullivan added a commit to tsullivan/kibana that referenced this pull request Aug 11, 2021
@tsullivan @kibanamachine
[Reporting] server side code clean up (elastic#106940)
5745654 
* clean up the enqueue job function

* clean up the screenshots observable

* clean up authorized user pre routing

* clean up get_user

* fix download job response handlers

* clean up jobs query factory repetition

* clean up setup deps made available from plugin.ts

* update test for screenshots observable

* Revert "clean up setup deps made available from plugin.ts"

This reverts commit 91de680.

* revert renames

* minor rename

* fix test after rename

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 11, 2021
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into reporting/new…
66273c7 
…-png-pdf-report-type

* 'master' of github.com:elastic/kibana: (101 commits)
  [ML] APM Latency Correlations: Field/value candidates prioritization (elastic#107370)
  [Reporting] Add lenience to a test on the order of asserted logs (elastic#108135)
  [Lens] fix do not submit invalid query in filtered metric (elastic#107542)
  skip flaky test (elastic#108043)
  fix newly introduced type error (elastic#107593)
  [Reporting] server side code clean up (elastic#106940)
  [build_ts_refs] improve caches, allow building a subset of projects (elastic#107981)
  [APM] Add new ftr_e2e to kibana CI and remove current e2e tests. (elastic#107593)
  add manage rules link to alerts dropdown (elastic#107950)
  [ML] Enable Index data visualizer document count chart to update time range query (elastic#106438)
  [Security Solutions][Detection Engine] Fixes "undefined" crash for author field by adding a migration for it (elastic#107230)
  [Actions UI] Fixed Jira Api token label. (elastic#107776)
  [Alerting UI] Fixed display permissions for edit/delete buttons when user has read only access. (elastic#107996)
  [Maps] fix code owners (elastic#108106)
  Update EMS landing page url (elastic#108102)
  Do not render page header for loading domains (elastic#108078)
  Update dependency @elastic/charts to v33.2.2 (elastic#107939)
  [APM] Display throughput as tps (instead of tpm) when bucket size < 60 seconds (elastic#107850)
  [Fleet] Fix all category count (elastic#108089)
  [Security Solution][Bug] - Disable alert table RBAC until fields sorted (elastic#108034)
  ...

# Conflicts:
#	x-pack/plugins/reporting/server/export_types/common/generate_png.ts
#	x-pack/plugins/reporting/server/lib/screenshots/index.ts
#	x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts
#	x-pack/plugins/reporting/server/lib/screenshots/observable.ts
tsullivan added a commit that referenced this pull request Aug 11, 2021
@tsullivan @kibanamachine
[7.x] [Reporting] server side code clean up (#106940) (#108140)
18c376f 
* [Reporting] server side code clean up (#106940)

* clean up the enqueue job function

* clean up the screenshots observable

* clean up authorized user pre routing

* clean up get_user

* fix download job response handlers

* clean up jobs query factory repetition

* clean up setup deps made available from plugin.ts

* update test for screenshots observable

* Revert "clean up setup deps made available from plugin.ts"

This reverts commit 91de680.

* revert renames

* minor rename

* fix test after rename

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* fix legacy

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@tsullivan tsullivan mentioned this pull request Aug 12, 2021
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jloleysens jloleysens jloleysens approved these changes

@dokmic dokmic dokmic approved these changes

Assignees

No one assigned

Labels

release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Reporting/Tech Debt] Simplify Factory functions in server code

5 participants

@tsullivan @elasticmachine @kibanamachine @jloleysens @dokmic