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

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

Details

Reviewers

amckinley

Maniphest Tasks

T13272: Remove the hard-coded homepage, and other Dashboards improvements

Commits

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

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

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Apr 12 2019, 4:08 PM

Harbormaster completed remote builds in B22567: Diff 48696.Apr 12 2019, 4:10 PM

epriestley requested review of this revision.Apr 12 2019, 4:10 PM

epriestley added a child revision: D20408: Update "add panel" and "remove panel" Dashboard flows to the new panel storage format.Apr 12 2019, 6:29 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]()`.