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

Dashboard drilldowns are not using saved object references #71409

Closed
timroes opened this issue Jul 13, 2020 · 11 comments · Fixed by #82602
Closed

Dashboard drilldowns are not using saved object references #71409

timroes opened this issue Jul 13, 2020 · 11 comments · Fixed by #82602
Assignees
Labels
Feature:Drilldowns Embeddable panel Drilldowns technical debt Improvement of the software architecture and operational architecture

Comments

@timroes
Copy link
Contributor

timroes commented Jul 13, 2020

When currently storing a dashboard drilldown on a panel, the config inside the panels object in the dashboard saved object will look like:

dynamicActions": {
  "events": [
    {
      "eventId": "c2363b35-045c-4d25-979e-cb9703f002db",
      "triggers": [
        "VALUE_CLICK_TRIGGER",
        "SELECT_RANGE_TRIGGER"
      ],
      "action": {
        "name": "iuyhgft",
        "config": {
          "dashboardId": "edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b",
          "useCurrentFilters": true,
          "useCurrentDateRange": true
        },
        "factoryId": "DASHBOARD_TO_DASHBOARD_DRILLDOWN"
      }
    }
  ]
}

The drilldown contains a dashboardId directly instead of using the saved object reference system for it and properly stored the link to the saved object in the reference part of the saved object. This means those drilldowns will all break when the saved objects would change their ids (currently planned for the sharing between spaces feature).

We need to change this behavior and migrate all existing drilldown saved objects to use references correctly.

cc @ppisljar @alexh97 and @legrego (sorry this seems to be another (new) blocker).

Part of #42845


@Dosant:

[2020-10-19] An update on this:

@timroes timroes added bug Fixes for quality problems that affect the customer experience Team:AppArch Feature:Drilldowns Embeddable panel Drilldowns labels Jul 13, 2020
@elasticmachine
Copy link
Contributor

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

@legrego
Copy link
Member

legrego commented Jul 13, 2020

Thanks for the ping @timroes. I think this is blocking phase 3 of sharing saved objects, since drilldowns are always(?) referencing dashboards, and not say, index patterns.

Phase 2 (our next phase) is concerned with sharing saved objects which themselves don't have references (such as index patterns).

So, this is something we absolutely need fixed, but it's not as high of a priority as #59960, which is blocking Phase 2 of our work.

cc @jportner

@streamich
Copy link
Contributor

streamich commented Jul 13, 2020

To fix this we will need the dashboard plugin to provide a way to update references and then we will need to add APIs to embeddable which will allow to update references when input changes.

We should also discuss if we/how we want to store the references for drilldown saved objects; as storing the references would mean that when object is copied to another space all other saved objects it references are also copied to that space, AFAIK. Meaning if a dashboard has drilldowns to few other dashboard and we store references for those, those other dashboards will also get copied to the new space together with the source dashboard.

@streamich streamich mentioned this issue Jul 13, 2020
47 tasks
@ppisljar
Copy link
Member

this is blocked by #67931

@streamich
Copy link
Contributor

also blocked by #71431

@Dosant Dosant added enhancement New value added to drive a business result and removed blocked bug Fixes for quality problems that affect the customer experience labels Sep 24, 2020
@Dosant Dosant added technical debt Improvement of the software architecture and operational architecture and removed enhancement New value added to drive a business result labels Sep 24, 2020
@Dosant
Copy link
Contributor

Dosant commented Oct 16, 2020

Started working on this on top of #71431.

Q: Are cyclic references allowed? Seems like no?
It is a completely valid use case to link dashboard with drilldowns in the following way:
DashboardA -> DashboardB -> DashboardA

What would be a recommended approach for handling such references?

@Dosant Dosant self-assigned this Oct 16, 2020
@mshustov
Copy link
Contributor

@Dosant Could you elaborate how do you model data storage? Does reference chain look like
DashboardA ---> DrilldownA ---> Dashboard B ---> DrilldownB ---> DashboardA ?

@Dosant
Copy link
Contributor

Dosant commented Oct 16, 2020

@restrry,
Drilldown is not an SO. In the end it is just serialized into dashboard SO inside the dashboard panels. panelsJSON
So from SO perspective data storage is literally: DashboardA -> DashboardB -> DashboardA

@ppisljar
Copy link
Member

@Dosant from dashboards perspective, its just extracting references, where one of them might point to another dashboard. it doesn't care if that other dashboard points back to it. But you are right we'll need to make sure any logic using this references is good enough so it doesn't break under cyclic reference. But from user experience perspective i don't see an issue. if i have Dashboard A->Dashboard B->Dashboard A ... when i copy dashboard A to new space dashboard B will be copied with it. And if i copy dashboard B, dashboard A will be copied with it. all good.

@Dosant
Copy link
Contributor

Dosant commented Oct 19, 2020

An update on this:

@Dosant Dosant added the blocked label Oct 19, 2020
@Dosant
Copy link
Contributor

Dosant commented Oct 30, 2020

Checked that with the #80966 the export and copy to space part works as expected 🙌
To continue, need to address #80973 (cc @ppisljar) Actually we are good. Don't need migrate function on embeddables for this.

@Dosant Dosant removed the blocked label Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Drilldowns Embeddable panel Drilldowns technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants