From bb856dbd149fe9298e52e535fd32a37e3061cb31 Mon Sep 17 00:00:00 2001
From: kavya-gf <kavya-gf@thoughtmachine.net>
Date: Mon, 1 Dec 2025 14:46:57 +0000
Subject: [PATCH 1/7] stop notifying reviewers if revision is in  or  state

---
 .../differential/editor/DifferentialTransactionEditor.php   | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index b935b09b2d..dcc0beaddd 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -536,7 +536,11 @@ protected function shouldPublishFeedStory(
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
-    return true;
+      // Don't notify reviewers for revisions in "changes planned" / "request changes" state
+      if ($object->isChangePlanned() || $object->isNeedsRevision()) {
+        return false;
+      }
+      return true;
   }
 
   protected function getMailTo(PhabricatorLiskDAO $object) {

From 52c6bf023dd055943fc98c22e7281b3bd2b8f4f8 Mon Sep 17 00:00:00 2001
From: kavya-gf <kavya-gf@thoughtmachine.net>
Date: Mon, 1 Dec 2025 14:47:31 +0000
Subject: [PATCH 2/7] format fix

---
 .../editor/DifferentialTransactionEditor.php           | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index dcc0beaddd..fc32b636d6 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -536,11 +536,11 @@ protected function shouldPublishFeedStory(
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
-      // Don't notify reviewers for revisions in "changes planned" / "request changes" state
-      if ($object->isChangePlanned() || $object->isNeedsRevision()) {
-        return false;
-      }
-      return true;
+    // Don't notify reviewers for revisions in "changes planned" / "request changes" state
+    if ($object->isChangePlanned() || $object->isNeedsRevision()) {
+      return false;
+    }
+    return true;
   }
 
   protected function getMailTo(PhabricatorLiskDAO $object) {

From ff585efa1bf0ad2975696d7eb8d9fe1f53cf6341 Mon Sep 17 00:00:00 2001
From: kavya-gf <kavya-gf@thoughtmachine.net>
Date: Mon, 1 Dec 2025 15:00:43 +0000
Subject: [PATCH 3/7] transfer  state to  when there are no more rejecting
 reviewers

---
 .../differential/editor/DifferentialTransactionEditor.php       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index fc32b636d6..afe17316b4 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -474,7 +474,7 @@ private function updateReviewStatus(
       // This revision was "Needs Revision", but no longer has any rejecting
       // reviewers. This usually happens after the last rejecting reviewer
       // resigns or is removed. Put the revision back in "Needs Review".
-      $new_status = DifferentialRevisionStatus::NEEDS_REVIEW;
+      $new_status = DifferentialRevisionStatus::CHANGES_PLANNED;
     }
 
     if ($new_status === null) {

From afe5b2f990a4ef0e51783c0f5ce3cb53fd30dc65 Mon Sep 17 00:00:00 2001
From: kavya-gf <kavya-gf@thoughtmachine.net>
Date: Mon, 1 Dec 2025 15:28:58 +0000
Subject: [PATCH 4/7] update comment

---
 .../differential/editor/DifferentialTransactionEditor.php     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index afe17316b4..946f131a5a 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -471,9 +471,9 @@ private function updateReviewStatus(
       // reviewer resigns or is removed.
       $new_status = DifferentialRevisionStatus::NEEDS_REVIEW;
     } else if ($was_revision) {
-      // This revision was "Needs Revision", but no longer has any rejecting
+      // This revision no longer has any rejecting
       // reviewers. This usually happens after the last rejecting reviewer
-      // resigns or is removed. Put the revision back in "Needs Review".
+      // resigns or is removed. Put the revision in "Changes Planned".
       $new_status = DifferentialRevisionStatus::CHANGES_PLANNED;
     }
 

From 15ae388365892b2b511a4fa5a5d471c282466aff Mon Sep 17 00:00:00 2001
From: kavya-gf <kavya-gf@thoughtmachine.net>
Date: Tue, 10 Feb 2026 10:23:56 +0000
Subject: [PATCH 5/7] stop changing state from  or  and add arc flag

---
 ...erentialUpdateRevisionConduitAPIMethod.php |  7 +++++
 .../editor/DifferentialTransactionEditor.php  | 31 ++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php
index d9b0779211..ee19a101b5 100644
--- a/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php
@@ -27,6 +27,7 @@ protected function defineParamTypes() {
       'diffid'    => 'required diffid',
       'fields'    => 'required dict',
       'message'   => 'required string',
+      'needs-review' => 'optional bool',
     );
   }
 
@@ -45,6 +46,7 @@ protected function defineErrorTypes() {
 
   protected function execute(ConduitAPIRequest $request) {
     $viewer = $request->getUser();
+    $needs_review = $request->getValue('needs-review', false);
 
     $diff = id(new DifferentialDiffQuery())
       ->setViewer($viewer)
@@ -54,6 +56,11 @@ protected function execute(ConduitAPIRequest $request) {
       throw new ConduitException('ERR_BAD_DIFF');
     }
 
+    $xactions[] = id(new DifferentialTransaction())
+      ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+      ->setMetadataValue('needs-review', $needs_review)
+      ->setNewValue($diff);
+
     $revision = id(new DifferentialRevisionQuery())
       ->setViewer($request->getUser())
       ->withIDs(array($request->getValue('id')))
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index 946f131a5a..cbd1665fe3 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -404,9 +404,29 @@ private function updateReviewStatus(
     DifferentialRevision $revision,
     array $xactions) {
 
+    $was_change_planned = $revision->isChangePlanned();
     $was_accepted = $revision->isAccepted();
     $was_revision = $revision->isNeedsRevision();
     $was_review = $revision->isNeedsReview();
+
+    // Revisions should not transition out of "Request Changes"
+    // or "Changes Planned" state unless the "needs-review" flag
+    // has been passed from arcanist
+    if ($was_change_planned || $was_revision) {
+      $needs_review = false;
+      // Check if any transaction has needs-review flag
+      foreach ($xactions as $xaction) {
+        if ($xaction->getMetadataValue('needs-review')) {
+          $needs_review = true;
+          break;
+        }
+      }
+
+      if (!$needs_review) {
+        return $xactions;
+      }
+    }
+
     if (!$was_accepted && !$was_revision && !$was_review) {
       // Revisions can't transition out of other statuses (like closed or
       // abandoned) as a side effect of reviewer status changes.
@@ -471,7 +491,7 @@ private function updateReviewStatus(
       // reviewer resigns or is removed.
       $new_status = DifferentialRevisionStatus::NEEDS_REVIEW;
     } else if ($was_revision) {
-      // This revision no longer has any rejecting
+      // This revision was "Needs Revision", and no longer has any rejecting
       // reviewers. This usually happens after the last rejecting reviewer
       // resigns or is removed. Put the revision in "Changes Planned".
       $new_status = DifferentialRevisionStatus::CHANGES_PLANNED;
@@ -536,10 +556,19 @@ protected function shouldPublishFeedStory(
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
+
+    // Check if any transaction has needs-review flag
+    foreach ($xactions as $xaction) {
+      if ($xaction->getMetadataValue('needs-review')) {
+        return true;
+      }
+    }
+
     // Don't notify reviewers for revisions in "changes planned" / "request changes" state
     if ($object->isChangePlanned() || $object->isNeedsRevision()) {
       return false;
     }
+
     return true;
   }
 

From 1b511485fdc6e47e07675ab62d1a8535e1f0fab5 Mon Sep 17 00:00:00 2001
From: kavya-gf <kavya-gf@thoughtmachine.net>
Date: Tue, 10 Feb 2026 15:27:20 +0000
Subject: [PATCH 6/7] add needs review param to revision class

---
 ...erentialUpdateRevisionConduitAPIMethod.php |  6 +-----
 .../query/DifferentialRevisionQuery.php       | 19 ++++++++++++++++++-
 .../storage/DifferentialRevision.php          | 11 +++++++++++
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php
index ee19a101b5..d171e0bcef 100644
--- a/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php
@@ -56,15 +56,11 @@ protected function execute(ConduitAPIRequest $request) {
       throw new ConduitException('ERR_BAD_DIFF');
     }
 
-    $xactions[] = id(new DifferentialTransaction())
-      ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
-      ->setMetadataValue('needs-review', $needs_review)
-      ->setNewValue($diff);
-
     $revision = id(new DifferentialRevisionQuery())
       ->setViewer($request->getUser())
       ->withIDs(array($request->getValue('id')))
       ->needReviewers(true)
+      ->needReview($request->getValue('needs-review'))
       ->needActiveDiffs(true)
       ->requireCapabilities(
         array(
diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php
index 75d3e052e4..341aca66bd 100644
--- a/src/applications/differential/query/DifferentialRevisionQuery.php
+++ b/src/applications/differential/query/DifferentialRevisionQuery.php
@@ -34,7 +34,8 @@ final class DifferentialRevisionQuery
   private $needDiffIDs        = false;
   private $needCommitPHIDs    = false;
   private $needHashes         = false;
-  private $needReviewers = false;
+  private $needReviewers      = false;
+  private $needReview         = false;
   private $needReviewerAuthority;
   private $needDrafts;
   private $needFlags;
@@ -286,6 +287,22 @@ public function needReviewers($need_reviewers) {
   }
 
 
+  /**
+   * Set whether or not the query is ready for review and
+   * should send notifications to associated reviewers.
+   *
+   * @param bool True to mark revision as "Needs Review" and
+   * send notifications to reviewers.
+   *
+   * @return this
+   * @task config
+   */
+  public function needReview($need_review) {
+    $this->needReview = $need_review;
+    return $this;
+  }
+
+
   /**
    * Request information about the viewer's authority to act on behalf of each
    * reviewer. In particular, they have authority to act on behalf of projects
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
index 92b303d0ba..3c6bd539af 100644
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -36,6 +36,7 @@ final class DifferentialRevision extends DifferentialDAO
   protected $branchName;
   protected $repositoryPHID;
   protected $activeDiffPHID;
+  protected $needs_review;
 
   protected $viewPolicy = PhabricatorPolicies::POLICY_USER;
   protected $editPolicy = PhabricatorPolicies::POLICY_USER;
@@ -700,6 +701,16 @@ public function setShouldBroadcast($should_broadcast) {
       $should_broadcast);
   }
 
+  public function getNeedsReview() {
+    return $this->getProperty(self::PROPERTY_NEEDS_REVIEW, false);
+  }
+
+  public function setNeedsReview($needs_review) {
+    return $this->setProperty(
+      self::PROPERTY_NEEDS_REVIEW,
+      $needs_review);
+  }
+
   public function setAddedLineCount($count) {
     return $this->setProperty(self::PROPERTY_LINES_ADDED, $count);
   }

From 4b676d7bdb4c60393e9acb569adee2a85a47d396 Mon Sep 17 00:00:00 2001
From: kavya-gf <kavya-gf@thoughtmachine.net>
Date: Fri, 13 Feb 2026 15:01:10 +0000
Subject: [PATCH 7/7] add requestReview param to diffs instead of revisions

---
 ...erentialUpdateRevisionConduitAPIMethod.php |  2 +-
 .../editor/DifferentialTransactionEditor.php  | 34 +++++++++----------
 .../query/DifferentialDiffQuery.php           | 17 ++++++++++
 .../query/DifferentialRevisionQuery.php       | 17 ----------
 .../differential/storage/DifferentialDiff.php | 13 +++++++
 .../storage/DifferentialRevision.php          | 11 ------
 6 files changed, 48 insertions(+), 46 deletions(-)

diff --git a/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php
index d171e0bcef..25d30dc0b1 100644
--- a/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialUpdateRevisionConduitAPIMethod.php
@@ -50,6 +50,7 @@ protected function execute(ConduitAPIRequest $request) {
 
     $diff = id(new DifferentialDiffQuery())
       ->setViewer($viewer)
+      ->needReview($needs_review)
       ->withIDs(array($request->getValue('diffid')))
       ->executeOne();
     if (!$diff) {
@@ -60,7 +61,6 @@ protected function execute(ConduitAPIRequest $request) {
       ->setViewer($request->getUser())
       ->withIDs(array($request->getValue('id')))
       ->needReviewers(true)
-      ->needReview($request->getValue('needs-review'))
       ->needActiveDiffs(true)
       ->requireCapabilities(
         array(
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index cbd1665fe3..f9b9af9e58 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -409,23 +409,23 @@ private function updateReviewStatus(
     $was_revision = $revision->isNeedsRevision();
     $was_review = $revision->isNeedsReview();
 
-    // Revisions should not transition out of "Request Changes"
-    // or "Changes Planned" state unless the "needs-review" flag
-    // has been passed from arcanist
-    if ($was_change_planned || $was_revision) {
-      $needs_review = false;
-      // Check if any transaction has needs-review flag
-      foreach ($xactions as $xaction) {
-        if ($xaction->getMetadataValue('needs-review')) {
-          $needs_review = true;
-          break;
-        }
-      }
-
-      if (!$needs_review) {
-        return $xactions;
-      }
-    }
+    // // Revisions should not transition out of "Request Changes"
+    // // or "Changes Planned" state unless the "needs-review" flag
+    // // has been passed from arcanist
+    // if ($was_change_planned || $was_revision) {
+    //   $needs_review = false;
+    //   // Check if any transaction has needs-review flag
+    //   foreach ($xactions as $xaction) {
+    //     if ($xaction->getMetadataValue('needs-review')) {
+    //       $needs_review = true;
+    //       break;
+    //     }
+    //   }
+
+    //   if (!$needs_review) {
+    //     return $xactions;
+    //   }
+    // }
 
     if (!$was_accepted && !$was_revision && !$was_review) {
       // Revisions can't transition out of other statuses (like closed or
diff --git a/src/applications/differential/query/DifferentialDiffQuery.php b/src/applications/differential/query/DifferentialDiffQuery.php
index 04019df1e0..32f80f69b3 100644
--- a/src/applications/differential/query/DifferentialDiffQuery.php
+++ b/src/applications/differential/query/DifferentialDiffQuery.php
@@ -12,6 +12,7 @@ final class DifferentialDiffQuery
 
   private $needChangesets = false;
   private $needProperties;
+  private $requestReview = false;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -53,6 +54,22 @@ public function needProperties($need_properties) {
     return $this;
   }
 
+  /**
+   * Set whether or not the user has passed --needs-review flag,
+   * indicating query is ready for review and
+   * should send notifications to associated reviewers.
+   *
+   * @param bool True to update revision to "Needs Review" and
+   * send notifications to reviewers.
+   *
+   * @return this
+   * @task config
+   */
+  public function requestReview($requestReview) {
+    $this->requestReview = $requestReview;
+    return $this;
+  }
+
   public function newResultObject() {
     return new DifferentialDiff();
   }
diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php
index 341aca66bd..11af4af4b8 100644
--- a/src/applications/differential/query/DifferentialRevisionQuery.php
+++ b/src/applications/differential/query/DifferentialRevisionQuery.php
@@ -35,7 +35,6 @@ final class DifferentialRevisionQuery
   private $needCommitPHIDs    = false;
   private $needHashes         = false;
   private $needReviewers      = false;
-  private $needReview         = false;
   private $needReviewerAuthority;
   private $needDrafts;
   private $needFlags;
@@ -287,22 +286,6 @@ public function needReviewers($need_reviewers) {
   }
 
 
-  /**
-   * Set whether or not the query is ready for review and
-   * should send notifications to associated reviewers.
-   *
-   * @param bool True to mark revision as "Needs Review" and
-   * send notifications to reviewers.
-   *
-   * @return this
-   * @task config
-   */
-  public function needReview($need_review) {
-    $this->needReview = $need_review;
-    return $this;
-  }
-
-
   /**
    * Request information about the viewer's authority to act on behalf of each
    * reviewer. In particular, they have authority to act on behalf of projects
diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php
index a33a13f0b6..f0686a6eff 100644
--- a/src/applications/differential/storage/DifferentialDiff.php
+++ b/src/applications/differential/storage/DifferentialDiff.php
@@ -39,6 +39,8 @@ final class DifferentialDiff
 
   protected $viewPolicy;
 
+  protected $requestReview;
+
   private $unsavedChangesets = array();
   private $changesets = self::ATTACHABLE;
   private $revision = self::ATTACHABLE;
@@ -66,6 +68,7 @@ protected function getConfiguration() {
         'bookmark' => 'text255?',
         'repositoryUUID' => 'text64?',
         'commitPHID' => 'phid?',
+        'requestReview' => 'bool?',
 
         // T6203/NULLABILITY
         // These should be non-null; all diffs should have a creation method
@@ -363,6 +366,16 @@ public function getDiffProperties() {
     return $this->assertAttached($this->properties);
   }
 
+    public function getRequestReview() {
+    return $this->getProperty(self::PROPERTY_REQUEST_REVIEW, false);
+  }
+
+  public function setRequestReview($requestReview) {
+    return $this->setProperty(
+      self::PROPERTY_REQUEST_REVIEW,
+      $requestReview);
+  }
+
   public function attachBuildable(HarbormasterBuildable $buildable = null) {
     $this->buildable = $buildable;
     return $this;
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
index 3c6bd539af..92b303d0ba 100644
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -36,7 +36,6 @@ final class DifferentialRevision extends DifferentialDAO
   protected $branchName;
   protected $repositoryPHID;
   protected $activeDiffPHID;
-  protected $needs_review;
 
   protected $viewPolicy = PhabricatorPolicies::POLICY_USER;
   protected $editPolicy = PhabricatorPolicies::POLICY_USER;
@@ -701,16 +700,6 @@ public function setShouldBroadcast($should_broadcast) {
       $should_broadcast);
   }
 
-  public function getNeedsReview() {
-    return $this->getProperty(self::PROPERTY_NEEDS_REVIEW, false);
-  }
-
-  public function setNeedsReview($needs_review) {
-    return $this->setProperty(
-      self::PROPERTY_NEEDS_REVIEW,
-      $needs_review);
-  }
-
   public function setAddedLineCount($count) {
     return $this->setProperty(self::PROPERTY_LINES_ADDED, $count);
   }