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

[Reporting] Move "common" types and constants into common #83198

Merged
merged 1 commit into from
Nov 12, 2020

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Nov 11, 2020

Summary

This code has been pulled from PR: #82091 to make 1 large PR into 2 atomic PRs.

See https://github.com/tsullivan/kibana/blob/reporting/feature-control-example/examples/reporting_example/public/components/app.tsx#L35 for an example of how an outside plugin can use the Reporting contract.

Problem addressed by this PR: Applications and plugins that require Reporting integration should be able to list the reporting dependency, and access modules from the Reporting Service via their start/stop methods of the Kibana Platform. Unfortunately, not all the modules they would want are accessible in UI code: they're useful, but they are:

  • incomplete and hard-to-find: in reporting/index, reporting/public, or reporting/common
  • not accessible: in server/lib code

This PR moves a lot of the constants and types that were originally defined in "server" into "common". It also makes a few constants, and helper functions available on the "public" contract. Those are needed in a followup PR to implement an example plugin for Reporting.

List of Changes

  • Added ReportingStart and ReportingSetup interfaces for plugin contract
  • Removed reporting/constants.ts (ambiguous purpose) and put declarations in reporting/common/constants.ts
  • Moved TS declarations from reporting/public/index.ts to reporting/common/types.ts
  • Moved TS declarations from reporting/server/lib/layouts and reporting/server/lib/store into reporting/common/types
  • Removed a few disabling comments of @kbn/eslint/no-restricted-paths related to hacky exporting in previous code

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@tsullivan tsullivan added (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Team:AppArch labels Nov 11, 2020
@tsullivan tsullivan force-pushed the reporting/public-contract branch from b232cd2 to d6e6b14 Compare November 11, 2020 18:36
import { PluginSetupContract as FeaturesPluginSetup } from '../../features/server';
import { LicensingPluginSetup } from '../../licensing/server';
import { AuthenticatedUser, SecurityPluginSetup } from '../../security/server';
import { SpacesPluginSetup } from '../../spaces/server';
Copy link
Member Author

Choose a reason for hiding this comment

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

VS Code auto-import rules have definitely changed 😿

Hopefully this type of noise can be disregarded.

@tsullivan tsullivan marked this pull request as ready for review November 11, 2020 20:33
@elasticmachine
Copy link
Contributor

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

@tsullivan tsullivan requested review from joelgriffith and a team November 11, 2020 20:33
@tsullivan tsullivan added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0 labels Nov 11, 2020
@tsullivan tsullivan force-pushed the reporting/public-contract branch 2 times, most recently from fdc135c to 7a59edb Compare November 11, 2020 21:00
@tsullivan tsullivan changed the title [Reporting] Move "common" types and constants for cross-integrations [Reporting] Move "common" types and constants into common Nov 11, 2020
@tsullivan tsullivan force-pushed the reporting/public-contract branch from f1a97f2 to 3456f60 Compare November 11, 2020 21:33
@tsullivan tsullivan marked this pull request as draft November 11, 2020 21:42
@tsullivan tsullivan force-pushed the reporting/public-contract branch 4 times, most recently from a918db6 to 9e83e9a Compare November 11, 2020 22:15
@tsullivan tsullivan force-pushed the reporting/public-contract branch from cfe7c1e to daba279 Compare November 11, 2020 22:20
Copy link
Member Author

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

Ready for review

@tsullivan tsullivan marked this pull request as ready for review November 11, 2020 22:22
import { ReportingAPIClient } from './lib/reporting_api_client';
import { ReportingNotifierStreamHandler as StreamHandler } from './lib/stream_handler';
import { GetCsvReportPanelAction } from './panel_actions/get_csv_panel_action';
import { csvReportingProvider } from './share_context_menu/register_csv_reporting';
import { reportingPDFPNGProvider } from './share_context_menu/register_pdf_png_reporting';

export interface ClientConfigType {
poll: ReportingConfigType['poll'];
Copy link
Member Author

Choose a reason for hiding this comment

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

This type comes from the server code, and can not easily be moved to common

@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
reporting 81.0KB 46.9KB -34.1KB

Distributable file count

id before after diff
default 42777 42770 -7

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
reporting 64.8KB 99.7KB +35.0KB
Unknown metric groups

async chunk count

id before after diff
reporting 3 1 -2

History

  • 💚 Build #87468 succeeded 44beab18553da8de1b0fde118916ad18e30bdd8c
  • 💚 Build #87447 succeeded fdc135ca3545e4662f65743af59fc0a3f186f0e5
  • 💔 Build #87451 failed 7a59edb191d5c2ad30238bb08fcd94e02d3831cc
  • 💚 Build #87416 succeeded d6e6b14eb1cc59c3bb26e3370121db0be41ebf25

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

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.

Code changes LGTM.

@tsullivan tsullivan merged commit 4932dc5 into elastic:master Nov 12, 2020
tsullivan added a commit to tsullivan/kibana that referenced this pull request Nov 12, 2020
…ntegration (elastic#83198)

# Conflicts:
#	x-pack/plugins/reporting/server/export_types/printable_pdf/create_job/index.ts
#	x-pack/plugins/reporting/server/types.ts
@tsullivan tsullivan deleted the reporting/public-contract branch November 12, 2020 16:34
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 12, 2020
* master:
  [Ingest Manager] Lift up registry/{stream,extract} functions (elastic#83239)
  [Reporting] Move "common" types and constants to allow cross-plugin integration (elastic#83198)
  [Lens] Add suffix formatter (elastic#82852)
  [App Search] Version documentation links (elastic#83245)
  Use saved object references for dashboard drilldowns (elastic#82602)
  Btsymbala/registered av (elastic#81910)
  [APM] Errors table for service overview (elastic#83065)
tsullivan added a commit that referenced this pull request Nov 12, 2020
…ugin integration (#83198) (#83303)

* [Reporting] Move "common" types and constants to allow cross-plugin integration (#83198)

# Conflicts:
#	x-pack/plugins/reporting/server/export_types/printable_pdf/create_job/index.ts
#	x-pack/plugins/reporting/server/types.ts

* fix eslint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants