Page MenuHomePhabricator

Convert dashboard read/display pathways to the new panel storage format
ClosedPublic

Authored by epriestley on Apr 12 2019, 4:08 PM.

Details

Summary

Depends on D20406. Ref T13272. This gets about half of Dashboards working with the new "duplicate panel friendly" storage format. Followups will fix the write pathways.

Collateral damage here includes:

  • Remove the old Dashboard/Panel edge type. We have a new, more general edge type for "container X uses panel Y", and we don't need this edge type for anything else.
  • Remove "attachPanels()" from Dashboard. Only rendering actually needs this, and it can just load the panels.
  • Remove "attachPanelPHIDs()" from Dashboard. We can look at the panel refs to figure this out.
  • Remove "attachProjects()" from Dashboard. Nothing uses this and it's not a very modern approach.
  • getPanelPHIDs() just looks at the config now.
  • Deleted some LayoutConfig-related code which is broken/obsolete.
Test Plan
  • Viewed various dashboards which were created before the changes, saw them render correctly.
  • Viewed a dashboard with two of the same panel! AMAZING!

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Apr 12 2019, 4:08 PM
epriestley requested review of this revision.Apr 12 2019, 4:10 PM
amckinley accepted this revision.Apr 12 2019, 7:45 PM
amckinley added inline comments.
src/applications/dashboard/controller/PhabricatorDashboardRemovePanelController.php
54

Do we need to migrate these edges to the new type?

src/applications/dashboard/storage/PhabricatorDashboard.php
122

Intentionally throwing away our new duplicate panels?

This revision is now accepted and ready to land.Apr 12 2019, 7:45 PM

The edge no longer does anything, and was always redundant with the actual panel layout information.

Once, it powered "Used By: Dashboard 123" on Panel view pages, but it didn't really work for that because panels may be used by other containers (today: tab panels).

I replaced the "track usage sites" with a dedicated search index edge, and replaced "get a list of PHIDs this dashboard uses" with "just look at the panel config", so now the edge has no writers and no readers.

We should probably go delete it (and maybe go delete the transactions about it), and I noted that on T13272, but this is low priority because it's just taking up a few bytes of storage and adding a few extra lines of text to timelines on older dashboards.

src/applications/dashboard/storage/PhabricatorDashboard.php
122

getPanelPHIDs() is only used for "I want to load all the handles used by this dashboard" or "I want to index all the panels used by this dashboard", so callers don't care about order/layout/sequence. If they care about that stuff, they should use the getPanelRefList() API instead. This one means get[FlatListOfUnique]PanelPHIDs[UsedByThisDashboardInArbitraryOrder]().

This revision was automatically updated to reflect the committed changes.