Page MenuHomePhabricator
Create Task
Maniphest T13019

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

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.

Revisions and Commits

rP Phabricator
D18813rP42034e6739eb Property list view on Diffusion commits should show build status but not…

Related Objects

Event Timeline

avivey created this task.Nov 21 2017, 12:46 PM
epriestley added a subscriber: epriestley.Nov 22 2017, 5:05 PM

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 triaged this task as Low priority.Nov 22 2017, 5:05 PM
lpriestley added a revision: D18813: Property list view on Diffusion commits should show build status but not Subscriptions, Projects, or Tokens.Nov 30 2017, 9:56 PM
epriestley mentioned this in D18813: Property list view on Diffusion commits should show build status but not Subscriptions, Projects, or Tokens.Nov 30 2017, 10:04 PM
lpriestley added a commit: rP42034e6739eb: Property list view on Diffusion commits should show build status but not….Dec 1 2017, 6:16 PM
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)
altendky added a subscriber: altendky.Jun 14 2019, 1:48 AM
Herald added a subscriber: amckinley. · View Herald TranscriptJun 14 2019, 1:48 AM
• pasik added a subscriber: • pasik.Aug 29 2019, 6:09 PM
Phabricator · Built by Phacility