From a1ad184ddd8cd7d788320e871c36b4c1a144fc2c Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 13 Dec 2017 07:36:06 -0800 Subject: [PATCH] Denormalize added and removed line counts for the current diff onto revisions Summary: See PHI230. Currently, we denormalize raw line counts onto diffs and revisions, but not added/removed line counts. I'd like to try a `[---+ ]` sort of size hint element (see D16322 for more) as a general approach to conveying size information at a glance and see how it feels, since I think the raw size number isn't very scannable/useful and it may be a significant improvement to hint about how much of a change is throwing stuff out vs adding new stuff. This just makes the data available without any subquerying and doesn't actually change the UI. Test Plan: Created a revision, saw detailed change information populate in the database. ``` mysql> select * from differential_revision where id = 292\G *************************** 1. row *************************** id: 292 title: WIP originalTitle: WIP phid: PHID-DREV-ux3cxptibn3l5pxsug3z status: draft summary: asdf testPlan: asdf authorPHID: PHID-USER-cvfydnwadpdj7vdon36z lastReviewerPHID: NULL lineCount: 41 dateCreated: 1513179418 dateModified: 1513179418 attached: [] mailKey: h4mn6perdio47o4beomyvu75zezwvredx3mbrlgz branchName: NULL viewPolicy: users editPolicy: users repositoryPHID: PHID-REPO-wif5lutk5gn3y6ursk4p properties: {"lines.added":40,"lines.removed":1} activeDiffPHID: PHID-DIFF-ixjphpunpkenqgukpmce 1 row in set (0.00 sec) ``` Reviewers: amckinley Reviewed By: amckinley Differential Revision: https://secure.phabricator.com/D18832 --- .../editor/DifferentialTransactionEditor.php | 24 ++++++++++++++++++- .../storage/DifferentialRevision.php | 17 +++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index b2fc179002..e6dfed88a7 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -132,12 +132,14 @@ final class DifferentialTransactionEditor $diff = $this->requireDiff($xaction->getNewValue()); - $object->setLineCount($diff->getLineCount()); + $this->updateRevisionLineCounts($object, $diff); + if ($this->repositoryPHIDOverride !== false) { $object->setRepositoryPHID($this->repositoryPHIDOverride); } else { $object->setRepositoryPHID($diff->getRepositoryPHID()); } + $object->attachActiveDiff($diff); $object->setActiveDiffPHID($diff->getPHID()); return; @@ -1645,5 +1647,25 @@ final class DifferentialTransactionEditor return true; } + private function updateRevisionLineCounts( + DifferentialRevision $revision, + DifferentialDiff $diff) { + + $revision->setLineCount($diff->getLineCount()); + + $conn = $revision->establishConnection('r'); + + $row = queryfx_one( + $conn, + 'SELECT SUM(addLines) A, SUM(delLines) D FROM %T + WHERE diffID = %d', + id(new DifferentialChangeset())->getTableName(), + $diff->getID()); + + if ($row) { + $revision->setAddedLineCount((int)$row['A']); + $revision->setRemovedLineCount((int)$row['D']); + } + } } diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 73f22c91e4..2c82de164a 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -61,6 +61,8 @@ final class DifferentialRevision extends DifferentialDAO const PROPERTY_CLOSED_FROM_ACCEPTED = 'wasAcceptedBeforeClose'; const PROPERTY_DRAFT_HOLD = 'draft.hold'; const PROPERTY_HAS_BROADCAST = 'draft.broadcast'; + const PROPERTY_LINES_ADDED = 'lines.added'; + const PROPERTY_LINES_REMOVED = 'lines.removed'; public static function initializeNewRevision(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) @@ -728,6 +730,21 @@ final class DifferentialRevision extends DifferentialDAO return $this->setProperty(self::PROPERTY_HAS_BROADCAST, $has_broadcast); } + public function setAddedLineCount($count) { + return $this->setProperty(self::PROPERTY_LINES_ADDED, $count); + } + + public function getAddedLineCount() { + return $this->getProperty(self::PROPERTY_LINES_ADDED); + } + + public function setRemovedLineCount($count) { + return $this->setProperty(self::PROPERTY_LINES_REMOVED, $count); + } + + public function getRemovedLineCount() { + return $this->getProperty(self::PROPERTY_LINES_REMOVED); + } public function loadActiveBuilds(PhabricatorUser $viewer) { $diff = $this->getActiveDiff(); -- 2.51.2