1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 14:52:41 +01:00

Treat Owners paths like "/src/backend" and "/src/backend/" identically

Summary:
Depends on D19183. Ref T11015. Currently, adding a trailing slash works great and omitting it mysteriously doesn't work.

Store a normalized version with an unconditional trailing slash for the lookup logic to operate on, and a separate display version which tracks what the user actually typed.

Test Plan:
  - Entered "/src/main.c", "/src/main.c/", saw them de-duplicate.
  - Entered "/src/main.c", saw it stay that way in the UI but appear as "/src/main.c/" internally.
  - Added a rule for "/src/applications/owners" (no slash), created a revision touching paths in that directory, saw Owners fire for it.
  - Changed the display value of a path only ("/src/main.c" to "/src/main.c/"), saw the update reflected in the UI without any beahvioral change.

Maniphest Tasks: T11015

Differential Revision: https://secure.phabricator.com/D19184
This commit is contained in:
epriestley 2018-03-06 20:07:32 -08:00
parent adde4089b4
commit df1e9ce646
6 changed files with 47 additions and 14 deletions

View file

@ -424,7 +424,7 @@ return array(
'rsrc/js/application/maniphest/behavior-line-chart.js' => 'e4232876', 'rsrc/js/application/maniphest/behavior-line-chart.js' => 'e4232876',
'rsrc/js/application/maniphest/behavior-list-edit.js' => 'a9f88de2', 'rsrc/js/application/maniphest/behavior-list-edit.js' => 'a9f88de2',
'rsrc/js/application/maniphest/behavior-subpriorityeditor.js' => '71237763', 'rsrc/js/application/maniphest/behavior-subpriorityeditor.js' => '71237763',
'rsrc/js/application/owners/OwnersPathEditor.js' => 'aa1733d0', 'rsrc/js/application/owners/OwnersPathEditor.js' => '996d62b9',
'rsrc/js/application/owners/owners-path-editor.js' => '7a68dda3', 'rsrc/js/application/owners/owners-path-editor.js' => '7a68dda3',
'rsrc/js/application/passphrase/passphrase-credential-control.js' => '3cb0b2fc', 'rsrc/js/application/passphrase/passphrase-credential-control.js' => '3cb0b2fc',
'rsrc/js/application/pholio/behavior-pholio-mock-edit.js' => 'bee502c8', 'rsrc/js/application/pholio/behavior-pholio-mock-edit.js' => 'bee502c8',
@ -764,7 +764,7 @@ return array(
'maniphest-task-edit-css' => 'fda62a9b', 'maniphest-task-edit-css' => 'fda62a9b',
'maniphest-task-summary-css' => '11cc5344', 'maniphest-task-summary-css' => '11cc5344',
'multirow-row-manager' => 'b5d57730', 'multirow-row-manager' => 'b5d57730',
'owners-path-editor' => 'aa1733d0', 'owners-path-editor' => '996d62b9',
'owners-path-editor-css' => '2f00933b', 'owners-path-editor-css' => '2f00933b',
'paste-css' => '9fcc9773', 'paste-css' => '9fcc9773',
'path-typeahead' => 'f7fc67ec', 'path-typeahead' => 'f7fc67ec',
@ -1676,6 +1676,14 @@ return array(
'javelin-mask', 'javelin-mask',
'phabricator-drag-and-drop-file-upload', 'phabricator-drag-and-drop-file-upload',
), ),
'996d62b9' => array(
'multirow-row-manager',
'javelin-install',
'path-typeahead',
'javelin-dom',
'javelin-util',
'phabricator-prefab',
),
'9a6dd75c' => array( '9a6dd75c' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-stratcom', 'javelin-stratcom',
@ -1766,14 +1774,6 @@ return array(
'javelin-fx', 'javelin-fx',
'javelin-util', 'javelin-util',
), ),
'aa1733d0' => array(
'multirow-row-manager',
'javelin-install',
'path-typeahead',
'javelin-dom',
'javelin-util',
'phabricator-prefab',
),
'ab2f381b' => array( 'ab2f381b' => array(
'javelin-request', 'javelin-request',
'javelin-behavior', 'javelin-behavior',

View file

@ -279,7 +279,7 @@ final class PhabricatorOwnersDetailController
$href = $repo->generateURI( $href = $repo->generateURI(
array( array(
'branch' => $repo->getDefaultBranch(), 'branch' => $repo->getDefaultBranch(),
'path' => $path->getPath(), 'path' => $path->getPathDisplay(),
'action' => 'browse', 'action' => 'browse',
)); ));
@ -288,7 +288,7 @@ final class PhabricatorOwnersDetailController
array( array(
'href' => (string)$href, 'href' => (string)$href,
), ),
$path->getPath()); $path->getPathDisplay());
$rows[] = array( $rows[] = array(
($path->getExcluded() ? '-' : '+'), ($path->getExcluded() ? '-' : '+'),

View file

@ -22,7 +22,7 @@ final class PhabricatorOwnersPathsSearchEngineAttachment
foreach ($paths as $path) { foreach ($paths as $path) {
$list[] = array( $list[] = array(
'repositoryPHID' => $path->getRepositoryPHID(), 'repositoryPHID' => $path->getRepositoryPHID(),
'path' => $path->getPath(), 'path' => $path->getPathDisplay(),
'excluded' => (bool)$path->getExcluded(), 'excluded' => (bool)$path->getExcluded(),
); );
} }

View file

@ -49,6 +49,7 @@ final class PhabricatorOwnersPath extends PhabricatorOwnersDAO {
return array( return array(
'repositoryPHID' => $this->getRepositoryPHID(), 'repositoryPHID' => $this->getRepositoryPHID(),
'path' => $this->getPath(), 'path' => $this->getPath(),
'display' => $this->getPathDisplay(),
'excluded' => (int)$this->getExcluded(), 'excluded' => (int)$this->getExcluded(),
); );
} }

View file

@ -103,6 +103,26 @@ final class PhabricatorOwnersPackagePathsTransaction
$paths = $object->getPaths(); $paths = $object->getPaths();
// We store paths in a normalized format with a trailing slash, regardless
// of whether the user enters "path/to/file.c" or "src/backend/". Normalize
// paths now.
$display_map = array();
foreach ($new as $key => $spec) {
$display_path = $spec['path'];
$raw_path = rtrim($display_path, '/').'/';
// If the user entered two paths which normalize to the same value
// (like "src/main.c" and "src/main.c/"), discard the duplicates.
if (isset($display_map[$raw_path])) {
unset($new[$key]);
continue;
}
$new[$key]['path'] = $raw_path;
$display_map[$raw_path] = $display_path;
}
$diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new); $diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new);
list($rem, $add) = $diffs; list($rem, $add) = $diffs;
@ -111,12 +131,24 @@ final class PhabricatorOwnersPackagePathsTransaction
$ref = $path->getRef(); $ref = $path->getRef();
if (PhabricatorOwnersPath::isRefInSet($ref, $set)) { if (PhabricatorOwnersPath::isRefInSet($ref, $set)) {
$path->delete(); $path->delete();
continue;
}
// If the user has changed the display value for a path but the raw
// storage value hasn't changed, update the display value.
if (isset($display_map[$path->getPath()])) {
$path
->setPathDisplay($display_map[$path->getPath()])
->save();
continue;
} }
} }
foreach ($add as $ref) { foreach ($add as $ref) {
$path = PhabricatorOwnersPath::newFromRef($ref) $path = PhabricatorOwnersPath::newFromRef($ref)
->setPackageID($object->getID()) ->setPackageID($object->getID())
->setPathDisplay($display_map[$ref['path']])
->save(); ->save();
} }
} }

View file

@ -115,7 +115,7 @@ JX.install('OwnersPathEditor', {
JX.copy( JX.copy(
path_input, path_input,
{ {
value : path_ref.path || '', value : path_ref.display || '',
name : 'path[' + this._count + ']' name : 'path[' + this._count + ']'
}); });