Page MenuHomePhabricator

Build Status is integrated into Diffusion in a hacky way
Open, LowPublic

Description

A bug caused "Build Status" to be dropped from Diffusion commit views.

It has been restored, but the current implementation is pretty hacky (lots of instanceof stuff). This should eventually be cleaned up.


Originally

From https://discourse.phabricator-community.org/t/749:

b744e898-bea5-11e5-8d1d-185528ed6627.png (443×685 px, 54 KB)

Best guess is the event stopped firing at some point? Or maybe the layout changes made something break.

Event Timeline

This can be "fixed" like this:

diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
index d7734856f0..cdce8702ca 100644
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -438,7 +438,8 @@ final class DiffusionCommitController extends DiffusionController {
     $repository = $drequest->getRepository();
 
     $view = id(new PHUIPropertyListView())
-      ->setUser($this->getRequest()->getUser());
+      ->setUser($this->getRequest()->getUser())
+      ->setObject($commit);
 
     $edge_query = id(new PhabricatorEdgeQuery())
       ->withSourcePHIDs(array($commit_phid))

However, this restores the redundant "Projects" and "Subscribers" (and, I think, "Tokens') items, which are now in the right-hand curtain element.

I'd accept a patch to do this, then bail early from those listeners (ala the early bailout for $object instanceof DifferentialRevision in HarbormasterUIEventListener) as a temporary fix to get this working again. The real fix is probably to finally get rid of TYPE_UI_WILLRENDERPROPERTIES and replace it with a more modern Engine / EngineExtension sort of setup that's more flexible, but I'm not exactly sure what that should look like. I think TYPE_UI_WILLRENDERPROPERTIES is still used in Phriction, at a minimum, and maybe elsewhere.

epriestley renamed this task from Build Status got dropped from Commit Details View to Build Status is integrated into Diffusion in a hacky way.Dec 18 2017, 5:21 PM
epriestley updated the task description. (Show Details)