mirror of
https://we.phorge.it/source/phorge.git
synced 2025-02-19 18:28:39 +01:00
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
This commit is contained in:
parent
afedb711d9
commit
bd0a4c0d04
10 changed files with 116 additions and 0 deletions
6
resources/sql/patches/032.viewtime.sql
Normal file
6
resources/sql/patches/032.viewtime.sql
Normal file
|
@ -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)
|
||||
);
|
|
@ -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',
|
||||
|
|
|
@ -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',
|
||||
),
|
||||
),
|
||||
),
|
||||
'<hr />',
|
||||
'All Revisions',
|
||||
'allopen' => array(
|
||||
|
|
|
@ -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(
|
||||
'<div class="differential-primary-pane">'.
|
||||
$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();
|
||||
}
|
||||
}
|
||||
/*
|
||||
|
||||
|
|
|
@ -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');
|
||||
|
|
|
@ -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());
|
||||
}
|
||||
|
|
|
@ -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');
|
||||
|
|
|
@ -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';
|
||||
|
|
|
@ -0,0 +1,43 @@
|
|||
<?php
|
||||
|
||||
/*
|
||||
* Copyright 2011 Facebook, Inc.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
class DifferentialViewTime extends DifferentialDAO {
|
||||
|
||||
protected
|
||||
$viewerPHID,
|
||||
$objectPHID,
|
||||
$viewTime;
|
||||
|
||||
protected function getConfiguration() {
|
||||
return array(
|
||||
self::CONFIG_OPTIMISTIC_LOCKS => 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;
|
||||
}
|
||||
}
|
||||
|
13
src/applications/differential/storage/viewtime/__init__.php
Normal file
13
src/applications/differential/storage/viewtime/__init__.php
Normal file
|
@ -0,0 +1,13 @@
|
|||
<?php
|
||||
/**
|
||||
* This file is automatically generated. Lint this module to rebuild it.
|
||||
* @generated
|
||||
*/
|
||||
|
||||
|
||||
|
||||
phutil_require_module('phabricator', 'applications/differential/storage/base');
|
||||
|
||||
|
||||
phutil_require_source('DifferentialViewTime.php');
|
||||
|
Loading…
Add table
Reference in a new issue