From 14cc0abeb37fd411aceed31db22a820537264fe5 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 30 Nov 2017 12:13:07 -0800
Subject: [PATCH] Fix several safety issues with repository URIs

Summary:
See PHI234. Several issues here:

  - The warning about observing a repository in Read/Write mode checks the raw I/O type, not the effective I/O type. That means we can fail to warn if other URIs are set to "Default", and "Default" is "Read/Write" in practice.
  - There's just an actual typo which prevents the "Observe" version of this error from triggering properly.

Additionally, add more forceful warnings that "Observe" and "Mirror" mean that you want to //replace// a repository with another one, not that we somehow merge branches selectively. It isn't necessarily obvious that "Observe" doesn't mean "merge/union", since the reasons it can't in the general case are somewhat subtle (conflicts between refs with the same names, detecting ref deletion).

Test Plan:
Read documentation. Hit the error locally by trying to "Observe" while in Read/Write mode:

{F5302655}

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18810
---
 .../diffusion/editor/DiffusionURIEditor.php            |  4 ++--
 src/docs/user/userguide/diffusion_uris.diviner         | 10 ++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/applications/diffusion/editor/DiffusionURIEditor.php b/src/applications/diffusion/editor/DiffusionURIEditor.php
index 674efbc158..90ced865f0 100644
--- a/src/applications/diffusion/editor/DiffusionURIEditor.php
+++ b/src/applications/diffusion/editor/DiffusionURIEditor.php
@@ -347,11 +347,11 @@ final class DiffusionURIEditor
                 continue;
               }
 
-              $io_type = $uri->getIoType();
+              $io_type = $uri->getEffectiveIOType();
 
               if ($io_type == PhabricatorRepositoryURI::IO_READWRITE) {
                 if ($no_readwrite) {
-                  $readwite_conflict = $uri;
+                  $readwrite_conflict = $uri;
                   break;
                 }
               }
diff --git a/src/docs/user/userguide/diffusion_uris.diviner b/src/docs/user/userguide/diffusion_uris.diviner
index 140948f55e..a19d38d3a5 100644
--- a/src/docs/user/userguide/diffusion_uris.diviner
+++ b/src/docs/user/userguide/diffusion_uris.diviner
@@ -239,6 +239,11 @@ You can not add a URI in Observe mode if an existing builtin URI is in
 authorities: the observed remote copy and the hosted local copy. Take the
 other URI out of //Read/Write// mode first.
 
+WARNING: If you observe a remote repository, the entire state of the working
+copy that Phabricator maintains will be deleted and replaced with the state of
+the remote. If some changes are present only in Phabricator's working copy,
+they will be unrecoverably destroyed.
+
 **Mirror**: Phabricator will push any changes made to this repository to the
 remote URI, keeping a read-only mirror hosted at that URI up to date.
 
@@ -251,6 +256,11 @@ It is possible to mirror a repository to another repository that is also
 hosted by Phabricator by adding that other repository's URI, although this is
 silly and probably very rarely of any use.
 
+WARNING: If you mirror to a remote repository, the entire state of that remote
+will be replaced with the state of the working copy Phabricator maintains. If
+some changes are present only in the remote, they will be unrecoverably
+destroyed.
+
 **None**: Phabricator will not fetch changes from or push changes to this URI.
 For builtin URIs, it will not let users fetch changes from or push changes to
 this URI.