Page MenuHomePhabricator
Files F15343934

D10838.id26015.diff
No OneTemporary
Actions

D10838.id26015.diff
View Options

diff --git a/src/applications/phriction/application/PhabricatorPhrictionApplication.php b/src/applications/phriction/application/PhabricatorPhrictionApplication.php
--- a/src/applications/phriction/application/PhabricatorPhrictionApplication.php
+++ b/src/applications/phriction/application/PhabricatorPhrictionApplication.php
@@ -52,7 +52,7 @@
'edit/(?:(?P<id>[1-9]\d*)/)?' => 'PhrictionEditController',
'delete/(?P<id>[1-9]\d*)/' => 'PhrictionDeleteController',
'new/' => 'PhrictionNewController',
- 'move/(?:(?P<id>[1-9]\d*)/)?' => 'PhrictionMoveController',
+ 'move/(?P<id>[1-9]\d*)/' => 'PhrictionMoveController',
'preview/' => 'PhabricatorMarkupPreviewController',
'diff/(?P<id>[1-9]\d*)/' => 'PhrictionDiffController',
diff --git a/src/applications/phriction/controller/PhrictionMoveController.php b/src/applications/phriction/controller/PhrictionMoveController.php
--- a/src/applications/phriction/controller/PhrictionMoveController.php
+++ b/src/applications/phriction/controller/PhrictionMoveController.php
@@ -2,79 +2,72 @@
final class PhrictionMoveController extends PhrictionController {
- private $id;
-
- public function willProcessRequest(array $data) {
- $this->id = idx($data, 'id');
- }
-
- public function processRequest() {
- $request = $this->getRequest();
- $user = $request->getUser();
-
- if ($this->id) {
- $document = id(new PhrictionDocumentQuery())
- ->setViewer($user)
- ->withIDs(array($this->id))
- ->needContent(true)
- ->requireCapabilities(
- array(
- PhabricatorPolicyCapability::CAN_VIEW,
- PhabricatorPolicyCapability::CAN_EDIT,
- ))
- ->executeOne();
- } else {
- $slug = PhabricatorSlug::normalize(
- $request->getStr('slug'));
- if (!$slug) {
- return new Aphront404Response();
- }
-
- $document = id(new PhrictionDocumentQuery())
- ->setViewer($user)
- ->withSlugs(array($slug))
- ->needContent(true)
- ->requireCapabilities(
- array(
- PhabricatorPolicyCapability::CAN_VIEW,
- PhabricatorPolicyCapability::CAN_EDIT,
- ))
- ->executeOne();
- }
-
+ public function handleRequest(AphrontRequest $request) {
+ $viewer = $this->getViewer();
+
+ $document = id(new PhrictionDocumentQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($request->getURIData('id')))
+ ->needContent(true)
+ ->requireCapabilities(
+ array(
+ PhabricatorPolicyCapability::CAN_VIEW,
+ PhabricatorPolicyCapability::CAN_EDIT,
+ ))
+ ->executeOne();
if (!$document) {
return new Aphront404Response();
}
- if (!isset($slug)) {
- $slug = $document->getSlug();
- }
+ $slug = $document->getSlug();
+ $cancel_uri = PhrictionDocument::getSlugURI($slug);
- $target_slug = PhabricatorSlug::normalize(
- $request->getStr('new-slug', $slug));
+ $v_slug = $slug;
+ $e_slug = null;
- $submit_uri = $request->getRequestURI()->getPath();
- $cancel_uri = PhrictionDocument::getSlugURI($slug);
+ $v_note = '';
- $e_url = true;
$validation_exception = null;
- $content = $document->getContent();
-
if ($request->isFormPost()) {
+ $v_note = $request->getStr('description');
+ $v_slug = $request->getStr('slug');
+
+ // If what the user typed isn't what we're actually using, warn them
+ // about it.
+ if (strlen($v_slug)) {
+ $normal_slug = PhabricatorSlug::normalize($v_slug);
+ if ($normal_slug !== $v_slug) {
+ return $this->newDialog()
+ ->setTitle(pht('Adjust Path'))
+ ->appendParagraph(
+ pht(
+ 'The path you entered (%s) is not a valid wiki document '.
+ 'path. Paths may not contain special characters.',
+ phutil_tag('strong', array(), $v_slug)))
+ ->appendParagraph(
+ pht(
+ 'Would you like to use the path %s instead?',
+ phutil_tag('strong', array(), $normal_slug)))
+ ->addHiddenInput('slug', $normal_slug)
+ ->addHiddenInput('description', $v_note)
+ ->addCancelButton($cancel_uri)
+ ->addSubmitButton(pht('Accept Path'));
+ }
+ }
$editor = id(new PhrictionTransactionEditor())
- ->setActor($user)
+ ->setActor($viewer)
->setContentSourceFromRequest($request)
->setContinueOnNoEffect(true)
- ->setDescription($request->getStr('description'));
+ ->setDescription($v_note);
$xactions = array();
$xactions[] = id(new PhrictionTransaction())
->setTransactionType(PhrictionTransaction::TYPE_MOVE_TO)
->setNewValue($document);
$target_document = PhrictionDocument::initializeNewDocument(
- $user,
- $target_slug);
+ $viewer,
+ $v_slug);
try {
$editor->applyTransactions($target_document, $xactions);
$redir_uri = PhrictionDocument::getSlugURI(
@@ -82,40 +75,40 @@
return id(new AphrontRedirectResponse())->setURI($redir_uri);
} catch (PhabricatorApplicationTransactionValidationException $ex) {
$validation_exception = $ex;
- $e_url = $ex->getShortMessage(PhrictionTransaction::TYPE_MOVE_TO);
+ $e_slug = $ex->getShortMessage(PhrictionTransaction::TYPE_MOVE_TO);
}
}
- $form = id(new PHUIFormLayoutView())
- ->setUser($user)
+
+ $form = id(new AphrontFormView())
+ ->setUser($viewer)
->appendChild(
id(new AphrontFormStaticControl())
- ->setLabel(pht('Title'))
- ->setValue($content->getTitle()))
+ ->setLabel(pht('Title'))
+ ->setValue($document->getContent()->getTitle()))
->appendChild(
id(new AphrontFormTextControl())
- ->setLabel(pht('New URI'))
- ->setValue($target_slug)
- ->setError($e_url)
- ->setName('new-slug')
- ->setCaption(pht('The new location of the document.')))
+ ->setLabel(pht('Current Path'))
+ ->setDisabled(true)
+ ->setValue($slug))
->appendChild(
id(new AphrontFormTextControl())
- ->setLabel(pht('Edit Notes'))
- ->setValue(pht('Moving document to a new location.'))
- ->setError(null)
- ->setName('description'));
+ ->setLabel(pht('New Path'))
+ ->setValue($v_slug)
+ ->setError($e_slug)
+ ->setName('slug'))
+ ->appendChild(
+ id(new AphrontFormTextControl())
+ ->setLabel(pht('Edit Notes'))
+ ->setValue($v_note)
+ ->setName('description'));
- $dialog = id(new AphrontDialogView())
- ->setUser($user)
- ->setValidationException($validation_exception)
+ return $this->newDialog()
->setTitle(pht('Move Document'))
- ->appendChild($form)
- ->setSubmitURI($submit_uri)
+ ->setValidationException($validation_exception)
+ ->appendForm($form)
->addSubmitButton(pht('Move Document'))
->addCancelButton($cancel_uri);
-
- return id(new AphrontDialogResponse())->setDialog($dialog);
}
}
diff --git a/src/applications/phriction/editor/PhrictionTransactionEditor.php b/src/applications/phriction/editor/PhrictionTransactionEditor.php
--- a/src/applications/phriction/editor/PhrictionTransactionEditor.php
+++ b/src/applications/phriction/editor/PhrictionTransactionEditor.php
@@ -537,13 +537,24 @@
->needContent(true)
->executeOne();
- // Considering to overwrite existing docs? Nuke this!
+ // Prevent overwrites and no-op moves.
$exists = PhrictionDocumentStatus::STATUS_EXISTS;
- if ($target_document && $target_document->getStatus() == $exists) {
+ if ($target_document) {
+ if ($target_document->getSlug() == $source_document->getSlug()) {
+ $message = pht(
+ 'You can not move a document to its existing location. '.
+ 'Choose a different location to move the document to.');
+ } else if ($target_document->getStatus() == $exists) {
+ $message = pht(
+ 'You can not move this document there, because it would '.
+ 'overwrite an existing document which is already at that '.
+ 'location. Move or delete the existing document first.');
+ }
+
$error = new PhabricatorApplicationTransactionValidationError(
$type,
- pht('Can not move document.'),
- pht('Can not overwrite existing target document.'),
+ pht('Invalid'),
+ $message,
$xaction);
$errors[] = $error;
}
diff --git a/src/infrastructure/util/PhabricatorSlug.php b/src/infrastructure/util/PhabricatorSlug.php
--- a/src/infrastructure/util/PhabricatorSlug.php
+++ b/src/infrastructure/util/PhabricatorSlug.php
@@ -8,7 +8,17 @@
$slug = phutil_utf8_strtolower($slug);
$slug = preg_replace("@[\\x00-\\x19#%&+=\\\\?<> ]+@", '_', $slug);
$slug = preg_replace('@_+@', '_', $slug);
- $slug = trim($slug, '_');
+
+ // Remove leading and trailing underscores from each component, if the
+ // component has not been reduced to a single underscore. For example, "a?"
+ // converts to "a", but "??" converts to "_".
+ $parts = explode('/', $slug);
+ foreach ($parts as $key => $part) {
+ if ($part != '_') {
+ $parts[$key] = trim($part, '_');
+ }
+ }
+ $slug = implode('/', $parts);
// Specifically rewrite these slugs. It's OK to have a slug like "a..b",
// but not a slug which is only "..".
diff --git a/src/infrastructure/util/__tests__/PhabricatorSlugTestCase.php b/src/infrastructure/util/__tests__/PhabricatorSlugTestCase.php
--- a/src/infrastructure/util/__tests__/PhabricatorSlugTestCase.php
+++ b/src/infrastructure/util/__tests__/PhabricatorSlugTestCase.php
@@ -7,7 +7,7 @@
'' => '/',
'/' => '/',
'//' => '/',
- '&&&' => '/',
+ '&&&' => '_/',
'/derp/' => 'derp/',
'derp' => 'derp/',
'derp//derp' => 'derp/derp/',
@@ -27,6 +27,13 @@
'../a' => 'dotdot/a/',
'a/..' => 'a/dotdot/',
'a/../' => 'a/dotdot/',
+ 'a?' => 'a/',
+ '??' => '_/',
+ 'a/?' => 'a/_/',
+ '??/a/??' => '_/a/_/',
+ 'a/??/c' => 'a/_/c/',
+ 'a/?b/c' => 'a/b/c/',
+ 'a/b?/c' => 'a/b/c/',
);
foreach ($slugs as $slug => $normal) {

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 11, 2:03 AM (1 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7490442
Default Alt Text
D10838.id26015.diff (10 KB)

Event Timeline

Phabricator · Built by Phacility