Page MenuHomePhabricator

Add Projects to Dashboards and Panels
ClosedPublic

Authored by chad on Jul 19 2015, 5:06 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 6:36 AM
Unknown Object (File)
Thu, Dec 12, 6:29 AM
Unknown Object (File)
Thu, Dec 12, 5:31 AM
Unknown Object (File)
Thu, Dec 12, 5:07 AM
Unknown Object (File)
Tue, Dec 3, 11:50 AM
Unknown Object (File)
Tue, Dec 3, 11:50 AM
Unknown Object (File)
Tue, Dec 3, 11:50 AM
Unknown Object (File)
Tue, Dec 3, 11:50 AM
Subscribers

Details

Reviewers
epriestley
btrahan
Commits
Restricted Diffusion Commit
rP6be53bd91608: Add Projects to Dashboards and Panels
Summary

Making an attempt here. This adds the ability to set Projects on Dashboards and Dashboard Panels. Most of this went smooth, but I can't figure out why the queries don't automatically show searching by Projects. I'm stumped. Rest seems fine.

Test Plan

Assign a Project to a Dashboard and a Panel, see Project show up, edit it, see transactions.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad retitled this revision from to Add Projects to Dashboards and Panels.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added reviewers: epriestley, btrahan.

I'm making some headway

  • project show as searchable in dashboards

I made progress here, but I'm stumped. Even though it lets me put in a project, it doesn't really return it as sorted results. I think I borked something in the conversion. In Panels I have other issues.

src/applications/dashboard/query/PhabricatorDashboardPanelQuery.php
35–49

I get this error here:

Attempting to use a panel in a way that requires an implementation, but the panel implementation ("") is unknown to Phabricator.

And I can't find any (easy) means of resolving it. Am I in over my head? I think I have to some fancy initializing like in Alamac's query, but not sure what the magic sauce is.

  • Fix searching by Project
epriestley edited edge metadata.

The error is because DashboardPanel is a bit unusual.

The SearchEngine sees that DashboardPanel implements PhabricatorCustomFieldInterface, so it says "okay, I'm going to get a list of the custom fields on this object so I can let the user search for them". In normal applications like Maniphest, this lets us automatically expose custom fields in ApplicationSearch without requiring extra code.

However, for Dashboard panels, the custom fields depend on the type of the panel, and the panel returned from newResultObject() doesn't have a type.

I think the easiest fix is to just give it a basic type for now:

diff --git a/src/applications/dashboard/query/PhabricatorDashboardPanelQuery.php b/src/applications/dashboard/query/PhabricatorDashboardPanelQuery.php
index cbfe163..3a6589a 100644
--- a/src/applications/dashboard/query/PhabricatorDashboardPanelQuery.php
+++ b/src/applications/dashboard/query/PhabricatorDashboardPanelQuery.php
@@ -33,7 +33,15 @@ final class PhabricatorDashboardPanelQuery
   }
 
   public function newResultObject() {
-    return new PhabricatorDashboardPanel();
+    // TODO: If we don't do this, SearchEngine explodes when trying to
+    // enumerate custom fields. For now, just give the panel a default panel
+    // type so custom fields work. In the long run, we may want to find a
+    // cleaner or more general approach for this.
+    $text_type = id(new PhabricatorDashboardTextPanelType())
+      ->getPanelTypeKey();
+
+    return id(new PhabricatorDashboardPanel())
+      ->setPanelType($text_type);
   }
 
   protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {

This isn't the cleanest thing in the world, but should fix the issue without causing any fallout, and we can maybe find a better fix eventually (some other applications, like Harbormaster, might run into similar issues).

src/applications/dashboard/query/PhabricatorDashboardPanelSearchEngine.php
13–38

Since you now implement buildCustomSearchFields(), you should remove these and implement buildQueryFromParameters() instead (basically, convert buildQueryFromSavedQuery() -- the old way of doing things -- into buildQueryFromParameters() -- the new hotness).

50

Although paneltypes is probably a better key for this, you should retain the old key so we don't break anyone's saved searches (that is, using a one-character-better internal key probably isn't worth potentially breaking / annoying some users).

This revision now requires changes to proceed.Jul 20 2015, 8:20 PM
chad edited edge metadata.
chad marked 3 inline comments as done.
  • updates per comments, all searchs working
epriestley edited edge metadata.
This revision is now accepted and ready to land.Jul 21 2015, 6:59 PM
This revision was automatically updated to reflect the committed changes.