[Reporting/Dashboard] Update integration to use v2 reports #108553

jloleysens · 2021-08-13T15:42:45Z

Summary

Updates the dashboard-reporting integration so that dashboard uses v2 export types. This enables PNG/PDF reports to be generated without first needing to save as well as using locators instead of storing URLs.

How to test

Start Kibana + ES on basic level license
Load sample data into the cluster via the UI
Open dashboard and create a new dashboard
Add a visulization
Add a query to the dashboard
Add a filter to the dashboard
Set a custom timerange on the dashboard
Create a PDF and PNG report via the "Share" menu
Copy the POST URL for both the PDF and PNG report and execute these against local Kibana
Verify that all the reports have been generated and show the expected result in the visualisations

To reviewers

Will the "Copy POST URL" functionality cause any issues? I know URL length is a concern with Vega charts, but looking at the current implementation, is there anything missing?
I've reworked the locator to pass all state on scopedHistory.location.state instead of the URL and created a new interface called ForwardedDashboardState. This is the shape of the data dashboard should expect to find on scopedHistory.location.state.

Screenshots

Unsaved dashboard, with unsaved panel (on the left):

Release note

Dashboard PDF and PNG reports can now be generated without requiring users to first save.

TODO

Figure out how to make time filters work correctly
Tests

…ment time filters are not working correctly

jloleysens · 2021-08-16T11:28:00Z

@elasticmachine merge upstream

…ssible

…on react component

elasticmachine · 2021-08-17T10:24:45Z

Pinging @elastic/kibana-presentation (Team:Presentation)

elasticmachine · 2021-08-17T10:24:46Z

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

elasticmachine · 2021-08-17T10:24:47Z

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

… exist

values we get from it will never contain something that cannot be passed to history.push - updated types to remove some `& SerializableRecord` instances - fixed embeddable drilldown Jest tests given that we no longer expect state to be in the URL

tsullivan · 2021-08-23T16:52:31Z

src/plugins/dashboard/public/application/top_nav/show_share_modal.tsx

+        savedDashboard.title ||
+        i18n.translate('dashboard.share.defaultDashboardTitle', {
+          defaultMessage: 'Dashboard [{date}]',
+          values: { date: moment().toISOString(true) },


Does this date string need to be converted to the user's timezone and formatted per the time format in Advanced Settings?

Whether we issue a request for v1 or v2 reports depends on the jobProviderOptions that are passed in. If they contain locatorParams then we know it's a v2 report type. To answer your question: yes v1 can be accessed from this UI and depends on the values passed in from the consumer.

I decided to keep it this way because I was not sure whether other parts of Kibana (like Canvas and visualize) still depend on v1 report types based on the params they pass in. Once they've all been updated we should remove this funcitonality - and probably send out a message that Reports now requires locators. WDYT?

tsullivan · 2021-08-23T16:52:37Z

src/plugins/dashboard/public/application/top_nav/show_share_modal.tsx

+        savedDashboard.title ||
+        i18n.translate('dashboard.share.defaultDashboardTitle', {
+          defaultMessage: 'Dashboard [{date}]',
+          values: { date: moment().toISOString(true) },


Does this date string need to be converted to the user's timezone and formatted per the time format in Advanced Settings?

That's a great point @tsullivan ! Do you mind if I address this in a follow up PR as I did this same thing in discover

kibana/src/plugins/discover/public/application/apps/main/components/top_nav/get_top_nav_links.ts

Line 129 in f0bf877

values: { date: moment().toISOString(true) },

jloleysens · 2021-08-24T11:01:02Z

@elasticmachine merge upstream

tsullivan · 2021-08-24T17:58:43Z

src/plugins/dashboard/public/locator.ts

@@ -26,7 +33,8 @@ const cleanEmptyKeys = (stateObj: Record<string, unknown>) => {

 export const DASHBOARD_APP_LOCATOR = 'DASHBOARD_APP_LOCATOR';

-export interface DashboardAppLocatorParams extends SerializableRecord {
+// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
+export type DashboardAppLocatorParams = {


Nit: maybe add a comment about why this is not an interface?

tsullivan · 2021-08-24T18:01:46Z

x-pack/plugins/reporting/common/job_utils.ts

 // TODO: Remove this code once everyone is using the new PDF format, then we can also remove the legacy
 // export type entirely
 export const isJobV2Params = ({ sharingData }: { sharingData: Record<string, unknown> }): boolean =>
-  Array.isArray(sharingData.locatorParams);
+  Array.isArray(sharingData.locatorParams) || isObject(sharingData.locatorParams);


Would returning sharingData.locatorParams != null work more simply?

Happy to go with your suggestion, I was trying to be more strict in what values pass the test, but I don't think it adds much value in this case.

tsullivan

I left a few small comments, but this LGTM!

jloleysens · 2021-08-25T10:08:06Z

@elasticmachine merge upstream

…ce" declaration

kibanamachine · 2021-08-25T13:23:39Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dashboard`	266	268	+2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`dashboard`	138	132	-6
`data`	2981	2977	-4
total			-10

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`dashboard`	221.3KB	222.5KB	+1.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`dashboard`	9	10	+1

Page load bundle

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

id	before	after	diff
`dashboard`	329.2KB	329.8KB	+567.0B
`reporting`	67.4KB	67.3KB	-13.0B
total			+554.0B

Unknown metric groups

API count

id	before	after	diff
`dashboard`	161	145	-16
`data`	3498	3494	-4
total			-20

References to deprecated APIs

id	before	after	diff
`dashboard`	165	164	-1

History

💚 Build #148095 succeeded 4f2b82d
💚 Build #147807 succeeded c3a8f4f
💚 Build #147691 succeeded 566851e
💚 Build #147298 succeeded 9d97c06
💚 Build #146941 succeeded e3e7f7e

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

jloleysens · 2021-08-26T07:53:03Z

I'm going to merge this for now so that we can get a good amount of time for this in master. I think we've addressed @ppisljar's concerns regarding types (used type Name = rather than interface to avoid issues with extending SerializableRecord. We can address further concerns in a follow up PR 👍🏻

…08553) * very wip, updating dashboard integration to use v2 reports. at the moment time filters are not working correctly * added missing dependency to hook * added tests and refined ForwadedAppState interface * remove unused import * updated test because generating a report from an unsaved report is possible * migrated locator to forward state on history only, reordered methods on react component * remove unused import * update locator test and use panel index number if panelIndex does not exist * ensure locator params are serializable * - moved getSerializableRecord to locator.ts to ensure that the values we get from it will never contain something that cannot be passed to history.push - updated types to remove some `& SerializableRecord` instances - fixed embeddable drilldown Jest tests given that we no longer expect state to be in the URL * update generated api docs * remove unused variable * - removed SerializedRecord extension from dashboard locator params interface - factored out state conversion logic from the locator getLocation * updated locator jest tests and SerializableRecord types * explicitly map values to dashboardlocatorparams and export serializable params type * use serializable params type in embeddable * factored out logic for converting panels to dashboard panels map * use "type =" instead of "interface" * big update to locator params: type fixes and added options key * added comment about why we are using "type" alias instead of "interface" declaration * simplify is v2 job param check Co-authored-by: Kibana Machine <[email protected]>

…110177) * very wip, updating dashboard integration to use v2 reports. at the moment time filters are not working correctly * added missing dependency to hook * added tests and refined ForwadedAppState interface * remove unused import * updated test because generating a report from an unsaved report is possible * migrated locator to forward state on history only, reordered methods on react component * remove unused import * update locator test and use panel index number if panelIndex does not exist * ensure locator params are serializable * - moved getSerializableRecord to locator.ts to ensure that the values we get from it will never contain something that cannot be passed to history.push - updated types to remove some `& SerializableRecord` instances - fixed embeddable drilldown Jest tests given that we no longer expect state to be in the URL * update generated api docs * remove unused variable * - removed SerializedRecord extension from dashboard locator params interface - factored out state conversion logic from the locator getLocation * updated locator jest tests and SerializableRecord types * explicitly map values to dashboardlocatorparams and export serializable params type * use serializable params type in embeddable * factored out logic for converting panels to dashboard panels map * use "type =" instead of "interface" * big update to locator params: type fixes and added options key * added comment about why we are using "type" alias instead of "interface" declaration * simplify is v2 job param check Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

very wip, updating dashboard integration to use v2 reports. at the mo…

40ab51f

…ment time filters are not working correctly

jloleysens added Feature:Dashboard Dashboard related features enhancement New value added to drive a business result Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.0.0 Team:AppServices v7.16.0 labels Aug 13, 2021

jloleysens requested review from tsullivan and ThomThomson August 13, 2021 15:42

kibanamachine and others added 6 commits August 16, 2021 07:28

Merge branch 'master' into dashboard/update-report-to-use-v2

56985bd

added missing dependency to hook

47ec77a

added tests and refined ForwadedAppState interface

1cb0766

remove unused import

adee42b

updated test because generating a report from an unsaved report is po…

3a93a16

…ssible

migrated locator to forward state on history only, reordered methods …

9eacc30

…on react component

jloleysens marked this pull request as ready for review August 17, 2021 10:24

jloleysens requested a review from a team as a code owner August 17, 2021 10:24

jloleysens added release_note:enhancement impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort labels Aug 17, 2021

jloleysens added 3 commits August 17, 2021 14:53

remove unused import

06ef623

update locator test and use panel index number if panelIndex does not…

6cd42ba

… exist

ensure locator params are serializable

99b7479

botelastic bot added the Feature:Drilldowns Embeddable panel Drilldowns label Aug 17, 2021

big update to locator params: type fixes and added options key

c3a8f4f

tsullivan reviewed Aug 23, 2021

View reviewed changes

tsullivan changed the title ~~[Dashboard] Update integration to use v2 reports~~ [Reporting/Dashboard] Update integration to use v2 reports Aug 23, 2021

jloleysens mentioned this pull request Aug 24, 2021

[Dashboard/Reporting] Report generated in "Edit mode" contains non-vis UI #109785

Closed

Merge branch 'master' into dashboard/update-report-to-use-v2

4f2b82d

tsullivan reviewed Aug 24, 2021

View reviewed changes

tsullivan approved these changes Aug 24, 2021

View reviewed changes

Merge branch 'master' into dashboard/update-report-to-use-v2

2ad676a

kibanamachine requested a review from a team as a code owner August 25, 2021 10:08

jloleysens added 2 commits August 25, 2021 12:15

added comment about why we are using "type" alias instead of "interfa…

cb39ce9

…ce" declaration

simplify is v2 job param check

30978d9

jloleysens merged commit 9e04d2c into elastic:master Aug 26, 2021

jloleysens deleted the dashboard/update-report-to-use-v2 branch August 26, 2021 07:53

jloleysens mentioned this pull request Aug 26, 2021

[7.x] [Reporting/Dashboard] Update integration to use v2 reports (#108553) #110177

Merged

This was referenced Sep 9, 2021

[Dashboard/Reporting] Dashboard controls rendered in page optimized report #111696

Closed

[Reporting/Dashboard] Guide users when copying URLs for reporting #112122

Closed

jloleysens mentioned this pull request Sep 23, 2021

[Reporting] Add guidance when copying URLs that may have issues #112979

Merged

2 tasks

jloleysens mentioned this pull request Oct 13, 2021

[Reporting] Saved POST URLs for dashboard are not migration-capable #103561

Closed

ThomThomson mentioned this pull request Oct 28, 2021

[Dashboard] Create Report from Unsaved Dashboard #116526

Closed

This was referenced Nov 23, 2021

[Reporting] Save before generating report #18439

Closed

[Reporting] Migrate to v2 PNG and PDF report types #108500

Closed

sophiec20 added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Aug 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reporting/Dashboard] Update integration to use v2 reports #108553

[Reporting/Dashboard] Update integration to use v2 reports #108553

jloleysens commented Aug 13, 2021 •

edited

Loading

jloleysens commented Aug 16, 2021

elasticmachine commented Aug 17, 2021

elasticmachine commented Aug 17, 2021

elasticmachine commented Aug 17, 2021

tsullivan Aug 23, 2021

jloleysens Aug 24, 2021

tsullivan Aug 23, 2021

jloleysens Aug 24, 2021

jloleysens commented Aug 24, 2021

tsullivan Aug 24, 2021

tsullivan Aug 24, 2021

jloleysens Aug 25, 2021 •

edited

Loading

tsullivan left a comment

jloleysens commented Aug 25, 2021

kibanamachine commented Aug 25, 2021

API count

References to deprecated APIs

jloleysens commented Aug 26, 2021

[Reporting/Dashboard] Update integration to use v2 reports #108553

[Reporting/Dashboard] Update integration to use v2 reports #108553

Conversation

jloleysens commented Aug 13, 2021 • edited Loading

Summary

How to test

To reviewers

Screenshots

Release note

TODO

jloleysens commented Aug 16, 2021

elasticmachine commented Aug 17, 2021

elasticmachine commented Aug 17, 2021

elasticmachine commented Aug 17, 2021

tsullivan Aug 23, 2021

Choose a reason for hiding this comment

jloleysens Aug 24, 2021

Choose a reason for hiding this comment

tsullivan Aug 23, 2021

Choose a reason for hiding this comment

jloleysens Aug 24, 2021

Choose a reason for hiding this comment

jloleysens commented Aug 24, 2021

tsullivan Aug 24, 2021

Choose a reason for hiding this comment

tsullivan Aug 24, 2021

Choose a reason for hiding this comment

jloleysens Aug 25, 2021 • edited Loading

Choose a reason for hiding this comment

tsullivan left a comment

Choose a reason for hiding this comment

jloleysens commented Aug 25, 2021

kibanamachine commented Aug 25, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

References to deprecated APIs

History

jloleysens commented Aug 26, 2021

jloleysens commented Aug 13, 2021 •

edited

Loading

jloleysens Aug 25, 2021 •

edited

Loading