From 5a58d168ed5d5c790ddaaadf3fbe8976edd2c3a5 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 16 Nov 2012 11:48:17 -0800 Subject: [PATCH] Differential - add unit and lint information to diff view Summary: see title. Note that fields that use customs storage don't work because I didn't think it made sense to load a revision object to get that data. Further, we don't have a revision id at some points, so its not clear what does / does not work...? Also added a link to upsell this diff view as I had trouble finding it. Test Plan: viewed some diffs Reviewers: epriestley, vrana Reviewed By: epriestley CC: chad, aran, Korvin Maniphest Tasks: T2026 Differential Revision: https://secure.phabricator.com/D3962 --- .../DifferentialDiffViewController.php | 37 ++++++++++-- .../DifferentialFieldSpecification.php | 57 +++++++++++++++++++ .../DifferentialLintFieldSpecification.php | 11 ++++ .../DifferentialUnitFieldSpecification.php | 12 ++++ .../DifferentialRevisionUpdateHistoryView.php | 6 +- 5 files changed, 118 insertions(+), 5 deletions(-) diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php index fcc699eb63..6da0c4d600 100644 --- a/src/applications/differential/controller/DifferentialDiffViewController.php +++ b/src/applications/differential/controller/DifferentialDiffViewController.php @@ -83,10 +83,38 @@ final class DifferentialDiffViewController extends DifferentialController { $top_panel = $action_panel; } - $arc_unit = id(new DifferentialDiffProperty())->loadOneWhere( - 'diffID = %d and name = %s', - $this->id, - 'arc:unit'); + $props = id(new DifferentialDiffProperty())->loadAllWhere( + 'diffID = %d', + $diff->getID()); + $props = mpull($props, 'getData', 'getName'); + + $aux_fields = DifferentialFieldSelector::newSelector() + ->getFieldSpecifications(); + foreach ($aux_fields as $key => $aux_field) { + if (!$aux_field->shouldAppearOnDiffView()) { + unset($aux_fields[$key]); + } else { + $aux_field->setUser($this->getRequest()->getUser()); + } + } + + $dict = array(); + foreach ($aux_fields as $key => $aux_field) { + $aux_field->setDiff($diff); + $aux_field->setManualDiff($diff); + $aux_field->setDiffProperties($props); + $value = $aux_field->renderValueForDiffView(); + if (strlen($value)) { + $label = rtrim($aux_field->renderLabelForDiffView(), ':'); + $dict[$label] = $value; + } + } + + $action_panel = new AphrontHeadsupView(); + $action_panel->setProperties($dict); + $action_panel->setHeader(pht('Diff Properties')); + + $arc_unit = idx($props, 'arc:unit'); if ($arc_unit) { $test_data = array($arc_unit->getName() => $arc_unit->getData()); } else { @@ -120,6 +148,7 @@ final class DifferentialDiffViewController extends DifferentialController { ->appendChild( array( $top_panel->render(), + $action_panel->render(), $table_of_contents->render(), $details->render(), )), diff --git a/src/applications/differential/field/specification/DifferentialFieldSpecification.php b/src/applications/differential/field/specification/DifferentialFieldSpecification.php index eb64558a94..cce1c250f9 100644 --- a/src/applications/differential/field/specification/DifferentialFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialFieldSpecification.php @@ -338,6 +338,63 @@ abstract class DifferentialFieldSpecification { } +/* -( Extending the Diff View Interface )------------------------------ */ + + + /** + * Determine if this field should appear on the diff detail view + * interface. One use of this interface is to add purely informational + * fields to the diff view, without any sort of backing storage. + * + * NOTE: These diffs are not necessarily attached yet to a revision. + * As such, a field on the diff view can not rely on the existence of a + * revision or use storage attached to the revision. + * + * If you return true from this method, you must implement the methods + * @{method:renderLabelForDiffView} and + * @{method:renderValueForDiffView}. + * + * @return bool True if this field should appear when viewing a diff. + * @task view + */ + public function shouldAppearOnDiffView() { + return false; + } + + + /** + * Return a string field label which will appear in the diff detail + * table. + * + * You must implement this method if you return true from + * @{method:shouldAppearOnDiffView}. + * + * @return string Label for field in revision detail view. + * @task view + */ + public function renderLabelForDiffView() { + throw new DifferentialFieldSpecificationIncompleteException($this); + } + + + /** + * Return a markup block representing the field for the diff detail + * view. Note that you can return null to suppress display (for instance, + * if the field shows related objects of some type and the revision doesn't + * have any related objects). + * + * You must implement this method if you return true from + * @{method:shouldAppearOnDiffView}. + * + * @return string|null Display markup for field value, or null to suppress + * field rendering. + * @task view + */ + public function renderValueForDiffView() { + throw new DifferentialFieldSpecificationIncompleteException($this); + } + + /* -( Extending the E-mail Interface )------------------------------------- */ diff --git a/src/applications/differential/field/specification/DifferentialLintFieldSpecification.php b/src/applications/differential/field/specification/DifferentialLintFieldSpecification.php index 200d7ef0db..c6a4c72607 100644 --- a/src/applications/differential/field/specification/DifferentialLintFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialLintFieldSpecification.php @@ -3,6 +3,17 @@ final class DifferentialLintFieldSpecification extends DifferentialFieldSpecification { + public function shouldAppearOnDiffView() { + return true; + } + + public function renderLabelForDiffView() { + return $this->renderLabelForRevisionView(); + } + + public function renderValueForDiffView() { + return $this->renderValueForRevisionView(); + } public function shouldAppearOnRevisionView() { return true; } diff --git a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php b/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php index b235fe78df..8900aef7f8 100644 --- a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php @@ -3,6 +3,18 @@ final class DifferentialUnitFieldSpecification extends DifferentialFieldSpecification { + public function shouldAppearOnDiffView() { + return true; + } + + public function renderLabelForDiffView() { + return $this->renderLabelForRevisionView(); + } + + public function renderValueForDiffView() { + return $this->renderValueForRevisionView(); + } + public function shouldAppearOnRevisionView() { return true; } diff --git a/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php b/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php index ae96c1ad29..f0127feb53 100644 --- a/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php +++ b/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php @@ -155,10 +155,14 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView { } $last_base = $base; + $id_link = phutil_render_tag( + 'a', + array('href' => '/differential/diff/'.$id.'/'), + phutil_escape_html($id)); $rows[] = ''. ''.phutil_escape_html($name).''. - ''.phutil_escape_html($id).''. + ''.$id_link.''. ''.phutil_escape_html($base).''. ''.phutil_escape_html($desc).''. ''.$age.''.