From da60c71fb8bc26a191d3ddfc1967f07ea6676964 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 26 May 2013 14:15:29 -0700
Subject: [PATCH] Fix ApplicationTransaction "no effect" error for mentions of
 already-mentioned users

Summary:
Fixes T3139. See that task for discussion.

If all mentions are removed because they're already subscribed, we currently generate an empty transaction, which later gets picked up as having no effect and the user gets yelled at.

Instead, don't generate a transaction if no PHIDs remain after filtering already-subscribed PHIDs.

Test Plan: Followed plan in T3139.

Reviewers: garoevans, btrahan

Reviewed By: garoevans

CC: aran

Maniphest Tasks: T3139

Differential Revision: https://secure.phabricator.com/D6048
---
 .../PhabricatorApplicationTransactionEditor.php    | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index ff28167194..ae39382c77 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -269,11 +269,7 @@ abstract class PhabricatorApplicationTransactionEditor
 
   public function setContentSourceFromRequest(AphrontRequest $request) {
     return $this->setContentSource(
-      PhabricatorContentSource::newForSource(
-        PhabricatorContentSource::SOURCE_WEB,
-        array(
-          'ip' => $request->getRemoteAddr(),
-        )));
+      PhabricatorContentSource::newFromRequest($request));
   }
 
   public function getContentSource() {
@@ -571,10 +567,6 @@ abstract class PhabricatorApplicationTransactionEditor
 
     $this->mentionedPHIDs = $phids;
 
-    if (!$phids) {
-      return null;
-    }
-
     if ($object->getPHID()) {
       // Don't try to subscribe already-subscribed mentions: we want to generate
       // a dialog about an action having no effect if the user explicitly adds
@@ -589,6 +581,10 @@ abstract class PhabricatorApplicationTransactionEditor
     }
     $phids = array_values($phids);
 
+    if (!$phids) {
+      return null;
+    }
+
     $xaction = newv(get_class(head($xactions)), array());
     $xaction->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS);
     $xaction->setNewValue(array('+' => $phids));