From f0364eef8af2c3ce8a527bba677d343ed9e704bf Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 6 Feb 2019 10:28:56 -0800
Subject: [PATCH] Remove weird integration between Legalpad and the
 ExternalAccount table

Summary:
Depends on D20107. Ref T6703. Legalpad currently inserts "email" records into the external account table, but they're never used for anything and nothing else references them.

They also aren't necessary for anything important to work, and the only effect they have is making the UI say "External Account" instead of "None" under the "Account" column. In particular, the signatures still record the actual email address.

Stop doing this, remove all the references, and destroy all the rows.

(Long ago, Maniphest may also have done this, but no longer does. Nuance/Gatekeeper use a more modern and more suitable "ExternalObject" thing that I initially started adapting here before realizing that Legalpad doesn't actually care about this data.)

Test Plan: Signed documents with an email address, saw signature reflected properly in UI. Grepped for other callsites.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T6703

Differential Revision: https://secure.phabricator.com/D20108
---
 .../20190206.external.01.legalpad.sql         |  2 ++
 .../20190206.external.02.email.sql            |  2 ++
 .../query/PhabricatorExternalAccountQuery.php | 31 -------------------
 .../LegalpadDocumentSignController.php        | 10 ------
 .../LegalpadDocumentSignatureSearchEngine.php |  2 +-
 5 files changed, 5 insertions(+), 42 deletions(-)
 create mode 100644 resources/sql/autopatches/20190206.external.01.legalpad.sql
 create mode 100644 resources/sql/autopatches/20190206.external.02.email.sql

diff --git a/resources/sql/autopatches/20190206.external.01.legalpad.sql b/resources/sql/autopatches/20190206.external.01.legalpad.sql
new file mode 100644
index 0000000000..8afa9dd9ff
--- /dev/null
+++ b/resources/sql/autopatches/20190206.external.01.legalpad.sql
@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_legalpad.legalpad_documentsignature
+  SET signerPHID = NULL WHERE signerPHID LIKE 'PHID-XUSR-%';
diff --git a/resources/sql/autopatches/20190206.external.02.email.sql b/resources/sql/autopatches/20190206.external.02.email.sql
new file mode 100644
index 0000000000..14f5f4791f
--- /dev/null
+++ b/resources/sql/autopatches/20190206.external.02.email.sql
@@ -0,0 +1,2 @@
+DELETE FROM {$NAMESPACE}_user.user_externalaccount
+  WHERE accountType = 'email';
diff --git a/src/applications/auth/query/PhabricatorExternalAccountQuery.php b/src/applications/auth/query/PhabricatorExternalAccountQuery.php
index b34199ce60..c4a53c12f8 100644
--- a/src/applications/auth/query/PhabricatorExternalAccountQuery.php
+++ b/src/applications/auth/query/PhabricatorExternalAccountQuery.php
@@ -168,35 +168,4 @@ final class PhabricatorExternalAccountQuery
     return 'PhabricatorPeopleApplication';
   }
 
-  /**
-   * Attempts to find an external account and if none exists creates a new
-   * external account with a shiny new ID and PHID.
-   *
-   * NOTE: This function assumes the first item in various query parameters is
-   * the correct value to use in creating a new external account.
-   */
-  public function loadOneOrCreate() {
-    $account = $this->executeOne();
-    if (!$account) {
-      $account = new PhabricatorExternalAccount();
-      if ($this->accountIDs) {
-        $account->setAccountID(reset($this->accountIDs));
-      }
-      if ($this->accountTypes) {
-        $account->setAccountType(reset($this->accountTypes));
-      }
-      if ($this->accountDomains) {
-        $account->setAccountDomain(reset($this->accountDomains));
-      }
-      if ($this->accountSecrets) {
-        $account->setAccountSecret(reset($this->accountSecrets));
-      }
-      if ($this->userPHIDs) {
-        $account->setUserPHID(reset($this->userPHIDs));
-      }
-      $account->save();
-    }
-    return $account;
-  }
-
 }
diff --git a/src/applications/legalpad/controller/LegalpadDocumentSignController.php b/src/applications/legalpad/controller/LegalpadDocumentSignController.php
index ab98c0bb78..f09d95af29 100644
--- a/src/applications/legalpad/controller/LegalpadDocumentSignController.php
+++ b/src/applications/legalpad/controller/LegalpadDocumentSignController.php
@@ -364,16 +364,6 @@ final class LegalpadDocumentSignController extends LegalpadController {
             if ($email_obj) {
               return $this->signInResponse();
             }
-            $external_account = id(new PhabricatorExternalAccountQuery())
-              ->setViewer($viewer)
-              ->withAccountTypes(array('email'))
-              ->withAccountDomains(array($email->getDomainName()))
-              ->withAccountIDs(array($email->getAddress()))
-              ->loadOneOrCreate();
-            if ($external_account->getUserPHID()) {
-              return $this->signInResponse();
-            }
-            $signer_phid = $external_account->getPHID();
           }
         }
         break;
diff --git a/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php b/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php
index 9df8d2478d..ea14fd4a2f 100644
--- a/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php
+++ b/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php
@@ -226,7 +226,7 @@ final class LegalpadDocumentSignatureSearchEngine
         $handles[$document->getPHID()]->renderLink(),
         $signer_phid
           ? $handles[$signer_phid]->renderLink()
-          : null,
+          : phutil_tag('em', array(), pht('None')),
         $name,
         phutil_tag(
           'a',