From bd0a4c0d0416fae46d12737cc34fd5d8c2e8aab9 Mon Sep 17 00:00:00 2001 From: elynde Date: Wed, 27 Apr 2011 01:27:06 -0700 Subject: [PATCH] Differential Updates View Summary: This adds a new view to differential called Updates. The high-level goal of Updates is to enabled differential to be effectively used without email notifications. I've tried doing things like automatically deleting differential emails where I'm in the 'to' line since they show up on the main diffential page but then there's always the chance an important diff flies by without me seeing it. Also, sometimes someone comments on a diff post-commit but differential doesn't surface those diffs. I re-created a test db on my devserver using mysqldump to get data on revs > 230000 so I would have some test data. We need to add a simple viewtime table but I didn't want to do that in production. Here's the table: CREATE TABLE differential_viewtime ( viewerPHID varchar(64) not null, objectPHID varchar(64) not null, viewTime int unsigned not null, PRIMARY KEY (viewerPHID, objectPHID) ); Issues: -Once we turn this on, all diffs will be 'unviewed'. What do you think about a 'Clear All' button or something? -Maybe we should add a pager This feature would be insanely useful, let me know what you think. Test Plan: Loaded Updates in my sandbox http://phabricator.dev1577.snc6.facebook.com/differential/filter/updates/ Clicked a diff, then went back, made sure diff disappeared from Updates list Reviewed By: tuomaspelkonen Reviewers: epriestley, jungejason, tuomaspelkonen Commenters: epriestley CC: epriestley, elynde, tuomaspelkonen Differential Revision: 169 --- resources/sql/patches/032.viewtime.sql | 6 +++ src/__phutil_library_map__.php | 2 + .../DifferentialRevisionListController.php | 10 +++++ .../DifferentialRevisionViewController.php | 10 +++++ .../controller/revisionview/__init__.php | 1 + .../DifferentialRevisionListData.php | 29 +++++++++++++ .../data/revisionlist/__init__.php | 1 + .../storage/revision/DifferentialRevision.php | 1 + .../storage/viewtime/DifferentialViewTime.php | 43 +++++++++++++++++++ .../storage/viewtime/__init__.php | 13 ++++++ 10 files changed, 116 insertions(+) create mode 100644 resources/sql/patches/032.viewtime.sql create mode 100644 src/applications/differential/storage/viewtime/DifferentialViewTime.php create mode 100644 src/applications/differential/storage/viewtime/__init__.php diff --git a/resources/sql/patches/032.viewtime.sql b/resources/sql/patches/032.viewtime.sql new file mode 100644 index 0000000000..455e3d7809 --- /dev/null +++ b/resources/sql/patches/032.viewtime.sql @@ -0,0 +1,6 @@ +CREATE TABLE phabricator_differential.differential_viewtime ( + viewerPHID varchar(64) not null, + objectPHID varchar(64) not null, + viewTime int unsigned not null, + PRIMARY KEY (viewerPHID, objectPHID) +); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6527f5a0fd..1aded6c0a3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -158,6 +158,7 @@ phutil_register_library_map(array( 'DifferentialSubscribeController' => 'applications/differential/controller/subscribe', 'DifferentialTasksAttacher' => 'applications/differential/tasks', 'DifferentialUnitStatus' => 'applications/differential/constants/unitstatus', + 'DifferentialViewTime' => 'applications/differential/storage/viewtime', 'DiffusionBranchInformation' => 'applications/diffusion/data/branch', 'DiffusionBranchQuery' => 'applications/diffusion/query/branch/base', 'DiffusionBranchTableView' => 'applications/diffusion/view/branchtable', @@ -585,6 +586,7 @@ phutil_register_library_map(array( 'DifferentialRevisionUpdateHistoryView' => 'AphrontView', 'DifferentialRevisionViewController' => 'DifferentialController', 'DifferentialSubscribeController' => 'DifferentialController', + 'DifferentialViewTime' => 'DifferentialDAO', 'DiffusionBranchTableView' => 'DiffusionView', 'DiffusionBrowseController' => 'DiffusionController', 'DiffusionBrowseFileController' => 'DiffusionController', diff --git a/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php b/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php index af365b56ad..177a7e0c4f 100644 --- a/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php +++ b/src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php @@ -90,6 +90,16 @@ class DifferentialRevisionListController extends DifferentialController { ), ), ), + 'updates' => array( + 'name' => 'Updates', + 'queries' => array( + array( + 'query' => DifferentialRevisionListData::QUERY_UPDATED_SINCE, + 'header' => + 'Diffs that have been updated since you\'ve last viewed them', + ), + ), + ), '
', 'All Revisions', 'allopen' => array( diff --git a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php index 5a03483b2e..835250f9f8 100644 --- a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php @@ -205,6 +205,8 @@ class DifferentialRevisionViewController extends DifferentialController { $comment_form->setUser($user); $comment_form->setDraft($draft); + $this->updateViewTime($user->getPHID(), $revision->getPHID()); + return $this->buildStandardPageResponse( '
'. $revision_detail->render(). @@ -593,6 +595,14 @@ class DifferentialRevisionViewController extends DifferentialController { return array($changesets, $vs_map); } + private function updateViewTime($user_phid, $revision_phid) { + $view_time = + id(new DifferentialViewTime()) + ->setViewerPHID($user_phid) + ->setObjectPHID($revision_phid) + ->setViewTime(time()) + ->replace(); + } } /* diff --git a/src/applications/differential/controller/revisionview/__init__.php b/src/applications/differential/controller/revisionview/__init__.php index 41a5ec6d22..3308871f23 100644 --- a/src/applications/differential/controller/revisionview/__init__.php +++ b/src/applications/differential/controller/revisionview/__init__.php @@ -15,6 +15,7 @@ phutil_require_module('phabricator', 'applications/differential/storage/comment' phutil_require_module('phabricator', 'applications/differential/storage/diffproperty'); phutil_require_module('phabricator', 'applications/differential/storage/inlinecomment'); phutil_require_module('phabricator', 'applications/differential/storage/revision'); +phutil_require_module('phabricator', 'applications/differential/storage/viewtime'); phutil_require_module('phabricator', 'applications/differential/view/addcomment'); phutil_require_module('phabricator', 'applications/differential/view/changesetlistview'); phutil_require_module('phabricator', 'applications/differential/view/difftableofcontents'); diff --git a/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php b/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php index c6c8f80540..afa81d7c2f 100644 --- a/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php +++ b/src/applications/differential/data/revisionlist/DifferentialRevisionListData.php @@ -29,6 +29,7 @@ class DifferentialRevisionListData { const QUERY_PHIDS = 'phids'; const QUERY_CC = 'cc'; const QUERY_ALL_OPEN = 'all-open'; + const QUERY_UPDATED_SINCE = 'updated-since'; private $ids; private $filter; @@ -170,6 +171,9 @@ class DifferentialRevisionListData { 'revision.phid in (%Ls)', $this->ids); break; + case self::QUERY_UPDATED_SINCE: + $this->revisions = $this->loadAllUpdated(); + break; } return $this->revisions; @@ -183,6 +187,31 @@ class DifferentialRevisionListData { ); } + private function loadAllUpdated() { + $revision = new DifferentialRevision(); + $min_view_time = (int)PhabricatorEnv::getEnvConfig('updates.min-view-time'); + + $data = queryfx_all( + $revision->establishConnection('r'), + 'SELECT DISTINCT revision.* FROM %T revision + JOIN %T rel ON rel.revisionID = revision.id + AND (rel.objectPHID in (%Ls) OR revision.authorPHID in (%Ls)) + LEFT JOIN %T viewtime ON viewtime.viewerPHID in (%Ls) + AND viewtime.objectPHID = revision.phid + WHERE GREATEST(%d, IFNULL(viewtime.viewTime, 0)) < revision.dateModified + %Q', + $revision->getTableName(), + DifferentialRevision::RELATIONSHIP_TABLE, + $this->ids, + $this->ids, + DifferentialRevision::TABLE_VIEW_TIME, + $this->ids, + $min_view_time, + $this->getOrderClause()); + return $revision->loadAllFromArray($data); + } + + private function loadAllOpen() { return $this->loadAllWhere('status in (%Ld)', $this->getOpenStatuses()); } diff --git a/src/applications/differential/data/revisionlist/__init__.php b/src/applications/differential/data/revisionlist/__init__.php index 166a7864cf..846363efa2 100644 --- a/src/applications/differential/data/revisionlist/__init__.php +++ b/src/applications/differential/data/revisionlist/__init__.php @@ -8,6 +8,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/storage/revision'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'storage/queryfx'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/differential/storage/revision/DifferentialRevision.php b/src/applications/differential/storage/revision/DifferentialRevision.php index ed72902df1..910e4bd290 100644 --- a/src/applications/differential/storage/revision/DifferentialRevision.php +++ b/src/applications/differential/storage/revision/DifferentialRevision.php @@ -39,6 +39,7 @@ class DifferentialRevision extends DifferentialDAO { private $commits; const RELATIONSHIP_TABLE = 'differential_relationship'; + const TABLE_VIEW_TIME = 'differential_viewtime'; const TABLE_COMMIT = 'differential_commit'; const RELATION_REVIEWER = 'revw'; diff --git a/src/applications/differential/storage/viewtime/DifferentialViewTime.php b/src/applications/differential/storage/viewtime/DifferentialViewTime.php new file mode 100644 index 0000000000..2df8075781 --- /dev/null +++ b/src/applications/differential/storage/viewtime/DifferentialViewTime.php @@ -0,0 +1,43 @@ + false, + self::CONFIG_IDS => self::IDS_MANUAL, + self::CONFIG_TIMESTAMPS => false, + ); + } + + // Primary key is (viewerPHID, objectPHID) + public function getIDKey() { + return null; + } + + protected function shouldInsertWhenSaved() { + return true; + } +} + diff --git a/src/applications/differential/storage/viewtime/__init__.php b/src/applications/differential/storage/viewtime/__init__.php new file mode 100644 index 0000000000..905e57f5c9 --- /dev/null +++ b/src/applications/differential/storage/viewtime/__init__.php @@ -0,0 +1,13 @@ +