Page MenuHomePhabricator

PhabricatorObjectHandle: add icon to generated links based on differential status
Needs RevisionPublic

Authored by couture on May 8 2015, 5:14 PM.

Details

Summary

Currently Tasks that list related Differentials only change the text if
a differential is open or closed. It is helpful to know whether a
related Differential has been accepted or rejected instead of clicking
into any given Differential.

Teach PhabricatorObjectHandle to add an appropriate icon beside the
Differential to display the Differential's current state.

Test Plan

Create Differentials of various states and relate them to a Maniphest
task. View the Maniphest task and observe the different state icons.

Diff Detail

Repository
rP Phabricator
Branch
T7076
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 5896
Build 5916: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

couture updated this revision to Diff 30707.May 8 2015, 5:14 PM
couture retitled this revision from to PhabricatorObjectHandle: add icon to generated links based on differential status.
couture updated this object.
couture edited the test plan for this revision. (Show Details)
couture added a reviewer: epriestley.
couture edited edge metadata.

Make sure revision is not null.

epriestley requested changes to this revision.May 14 2015, 1:23 PM
epriestley edited edge metadata.

I added some discussion to T7076 to explain why this is harder than it seems. The specific issues this runs into are:

  • It violates policies, by using the omnipotent user instead of the viewer.
  • It performs an expensive load in an inefficient way (see https://secure.phabricator.com/book/phabcontrib/article/n_plus_one/).
  • It adds Differential-specific code to an application-agnostic part of the core infrastructure. Handles do not have any application-specific code; application concerns are modularized into ...PHIDType classes in applications.
  • It affects all places where revision handles are rendered, not just the attachment list.

Navigating these issues and the concerns discussed in T7076 is very complicated.

This revision now requires changes to proceed.May 14 2015, 1:23 PM