Page MenuHomePhabricator

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

Authored by epriestley on Apr 12 2019, 4:08 PM.
Tags
None
Referenced Files
F12848261: D20407.id.diff
Fri, Mar 29, 3:27 AM
Unknown Object (File)
Wed, Mar 27, 1:56 AM
Unknown Object (File)
Wed, Mar 27, 1:55 AM
Unknown Object (File)
Sun, Mar 10, 10:01 AM
Unknown Object (File)
Sun, Mar 10, 9:59 AM
Unknown Object (File)
Feb 2 2024, 4:16 AM
Unknown Object (File)
Jan 28 2024, 11:12 PM
Unknown Object (File)
Jan 25 2024, 2:01 AM
Subscribers
None

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
Branch
panel9
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 22567
Build 30908: Run Core Tests
Build 30907: arc lint + arc unit

Event Timeline

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.