From 13ec744803576c97f2f9bddda40b0c0dd8b6938a Mon Sep 17 00:00:00 2001
From: Jeff An <jeff.an@robinhood.com>
Date: Mon, 27 Sep 2021 22:12:59 -0700
Subject: [PATCH 01/24] [phab] Add retries to HTTP POST harbormaster builds as
 well as grafana metrics

Summary:
this diff adds exponential backoff retries for harbormaster builds that return an error status code (for our use case, this will usually be 28 for timeouts).

in addition, it adds grafana metrics for HTTP post requests and their status codes (filterable by build plan).

Test Plan:
i copy pasted my local code onto the build-core phabricator code base and then used the bin/harbormaster binary to manually trigger runs of my build plan. you can see it in action here:
https://phabricator.robinhood.com/harbormaster/build/2385585/

i intentionally changed the TOKEN for one of the jobs to something different, and as you can see the HTTP403 was retried 3 times at increasing intervals. the other job which i didnt modify was triggered succesfully and ran to completion.

you can also see associated metrics here: http://metrics.dev.rhinternal.net/explore?orgId=1&left=%5B%22now-1h%22,%22now%22,%22cortex-multitenant%22,%7B%22exemplar%22:true,%22expr%22:%22phabricator_build_result_total%22%7D%5D

Reviewers: paul.tarjan, stephan.zharkov, harry.li

Reviewed By: paul.tarjan

Task Link: https://robinhood.atlassian.net/browse/TI-1144

Differential Revision: https://phabricator.robinhood.com/D292841
---
 .arcconfig                                    |  2 +-
 ...sterHTTPRequestBuildStepImplementation.php | 76 +++++++++++++------
 2 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/.arcconfig b/.arcconfig
index 88f04f72c5..3c1bbad847 100644
--- a/.arcconfig
+++ b/.arcconfig
@@ -1,5 +1,5 @@
 {
-  "phabricator.uri": "https://secure.phabricator.com/",
+  "phabricator.uri": "https://phabricator.robinhood.com/",
   "load": ["src/"],
   "history.immutable": false
 }
diff --git a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
index 59866d3b73..9dc270b52a 100644
--- a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
@@ -56,33 +56,62 @@ public function execute(
       'vurisprintf',
       $settings['uri'],
       $variables);
-
     $method = nonempty(idx($settings, 'method'), 'POST');
 
-    $future = id(new HTTPSFuture($uri))
-      ->setMethod($method)
-      ->setTimeout(60);
-
-    $credential_phid = $this->getSetting('credential');
-    if ($credential_phid) {
-      $key = PassphrasePasswordKey::loadFromPHID(
-        $credential_phid,
-        $viewer);
-      $future->setHTTPBasicAuthCredentials(
-        $key->getUsernameEnvelope()->openEnvelope(),
-        $key->getPasswordEnvelope());
+    $sleepDuration = 30;
+    $retry = 0;
+    while (true) {
+      $future = id(new HTTPSFuture($uri))
+        ->setMethod($method)
+        ->setTimeout(60);
+
+      $credential_phid = $this->getSetting('credential');
+      if ($credential_phid) {
+        $key = PassphrasePasswordKey::loadFromPHID(
+          $credential_phid,
+          $viewer);
+        $future->setHTTPBasicAuthCredentials(
+          $key->getUsernameEnvelope()->openEnvelope(),
+          $key->getPasswordEnvelope());
+      }
+
+      $this->resolveFutures(
+        $build,
+        $build_target,
+        array($future));
+
+      $this->logHTTPResponse($build, $build_target, $future, $uri);
+
+      list($status) = $future->resolve();
+      try {
+        $this->emitBuildResultMetric($build->getBuildPlan()->getID(), $status->getStatusCode());
+      } catch (Exception $e) {
+        echo 'Caught exception while sending Jenkins metrics to statsd: ',  $e->getMessage(), "\n";
+      }
+      if (!$status->isError()) {
+        return;
+      }
+      if ($retry == 2) {
+        throw new HarbormasterBuildFailureException();
+      }
+      sleep($sleepDuration);
+      $sleepDuration *= 2;
+      $retry++;
     }
+  }
 
-    $this->resolveFutures(
-      $build,
-      $build_target,
-      array($future));
-
-    $this->logHTTPResponse($build, $build_target, $future, $uri);
-
-    list($status) = $future->resolve();
-    if ($status->isError()) {
-      throw new HarbormasterBuildFailureException();
+  public function emitBuildResultMetric($plan, $status) {
+    $template = 'echo "phabricator_build_result_total:1|c|#plan:$plan,status:$status" | nc -w 1 -u apollo-statsd.integration-tests.svc.cluster.local 8125';
+    $vars = array(
+      '$plan' => $plan,
+      '$status' => $status,
+    );
+    $command = strtr($template, $vars);
+    $output = null;
+    $retval = null;
+    exec($command, $output, $retval);
+    if ($retval != 0) {
+      echo 'Error while emitting metrics to statsd: ', $output, "\n";
     }
   }
 
@@ -112,5 +141,4 @@ public function getFieldSpecifications() {
   public function supportsWaitForMessage() {
     return true;
   }
-
 }

From b5270332c8452de59eb8b62db39791ca76257e40 Mon Sep 17 00:00:00 2001
From: Gabriel Silk <gabriel.silk@robinhood.com>
Date: Tue, 24 Aug 2021 10:34:36 -0700
Subject: [PATCH 02/24] Return artifactData in harbormaster.artifact.search API
 call

---
 .../harbormaster/storage/build/HarbormasterBuildArtifact.php     | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php b/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php
index 8b4972c154..b03e432ccc 100644
--- a/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php
+++ b/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php
@@ -196,6 +196,7 @@ public function getFieldValuesForConduit() {
       'buildTargetPHID' => $this->getBuildTargetPHID(),
       'artifactType' => $this->getArtifactType(),
       'artifactKey' => $this->getArtifactKey(),
+      'artifactData' => $this->artifactData,
       'isReleased' => (bool)$this->getIsReleased(),
     );
   }

From 386590fcc3c09b495867c7a8f154a7d9df0c5548 Mon Sep 17 00:00:00 2001
From: Tianxiang Chen <tianxiang.chen@robinhood.com>
Date: Mon, 25 Oct 2021 13:08:33 -0700
Subject: [PATCH 03/24] [DEVX-2064] disable foist upon and commandeer

Summary:
To handle the security task, we've decided to disable 'foist upon' and
'commandeer' feature in our fork.

Test Plan:
tested in staging.
* Before:
revision editor: {F5071084}
author's UI: {F5071086}
reviewer's UI: {F5071087}

* After:
revision editor: {F5071088}
author's UI: {F5071154}
reviewer's UI: {F5071089}

Reviewers: greg.magolan, paul.tarjan

Reviewed By: paul.tarjan

Task Link: https://robinhood.atlassian.net/browse/DEVX-2064

Differential Revision: https://phabricator.robinhood.com/D304649
---
 src/__phutil_library_map__.php                                 | 3 ++-
 .../differential/editor/DifferentialRevisionEditEngine.php     | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 421ee76400..87a5166046 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -6769,7 +6769,8 @@
     'DifferentialRevisionCloseDetailsController' => 'DifferentialController',
     'DifferentialRevisionCloseTransaction' => 'DifferentialRevisionActionTransaction',
     'DifferentialRevisionClosedStatusDatasource' => 'PhabricatorTypeaheadDatasource',
-    'DifferentialRevisionCommandeerTransaction' => 'DifferentialRevisionActionTransaction',
+    // DEVX-2064
+    // 'DifferentialRevisionCommandeerTransaction' => 'DifferentialRevisionActionTransaction',
     'DifferentialRevisionContentAddedHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionContentHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionContentRemovedHeraldField' => 'DifferentialRevisionHeraldField',
diff --git a/src/applications/differential/editor/DifferentialRevisionEditEngine.php b/src/applications/differential/editor/DifferentialRevisionEditEngine.php
index 74fee82219..690fb0c5c4 100644
--- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php
+++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php
@@ -200,7 +200,8 @@ protected function buildCustomEditFields($object) {
       $author_field->setCommentActionLabel(pht('Foist Upon'));
     }
 
-    $fields[] = $author_field;
+    // DEVX-2064
+    // $fields[] = $author_field;
 
     $fields[] = id(new PhabricatorRemarkupEditField())
       ->setKey(DifferentialRevisionSummaryTransaction::EDITKEY)

From b31feee39aebda4c50e19c7c027454db6bfe805d Mon Sep 17 00:00:00 2001
From: Tianxiang Chen <tianxiang.chen@robinhood.com>
Date: Fri, 29 Oct 2021 13:56:00 -0700
Subject: [PATCH 04/24] [phabricator] only save build_unit_message for failed
 case or coverage

Summary:
Try to improve the revision page load latency.
Jenkins test will report back to harbormaster for every single test case run, and phabricator will save all of them to database.
Currently the table is having 1.8 billion rows, and for the revision that has a lot of test cases in CI, the page load logic will read
huge amount of data, and often we see timeouts.

After this change, we will only save for failed test case (so that dev can still view the test log), and if coverage data is provided in the unit_message, we will save as well.

Test Plan: manual apply change to staging and test

Reviewers: boyang.tian

Reviewed By: boyang.tian

Task Link: DEVX-2087

Differential Revision: https://phabricator.robinhood.com/D306789
---
 .../conduit/HarbormasterSendMessageConduitAPIMethod.php    | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php b/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php
index a7ee75b2a1..e9c07a5de8 100644
--- a/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php
+++ b/src/applications/harbormaster/conduit/HarbormasterSendMessageConduitAPIMethod.php
@@ -593,9 +593,14 @@ private function sendToTarget(
 
     $unit_messages = $request->getValue('unit', array());
     foreach ($unit_messages as $unit) {
-      $save[] = HarbormasterBuildUnitMessage::newFromDictionary(
+      $bu_message = HarbormasterBuildUnitMessage::newFromDictionary(
         $build_target,
         $unit);
+      // DEVX-2087:only save to db for failed test case and coverage information
+      if (($bu_message->getResult() == ArcanistUnitTestResult::RESULT_FAIL) ||
+          ($bu_message->getProperty('coverage') != null)) {
+        $save[] = $bu_message;
+      }
     }
 
     $save[] = HarbormasterBuildMessage::initializeNewMessage($viewer)

From 6ea07f7138bed08be651eb56d610b1054a0f0b0c Mon Sep 17 00:00:00 2001
From: Tianxiang Chen <tianxiang.chen@robinhood.com>
Date: Sat, 30 Oct 2021 08:30:41 -0700
Subject: [PATCH 05/24] [perf] skip file policy check if it is public or
 readable by all users

Summary:
Phabricator is checking files' policy if they are used in timeline or feed.

For commonly used files, like a meme in the test failure message, the file will be attached to
huge amount of the revisions, and make the policy check very slow and resource hogging.

This change adds a short-circuit evaluation: if file policy is public or all_users, skip doing the
expensive check.

Test Plan: Change in staging and test.

Reviewers: boyang.tian

Reviewed By: boyang.tian

Differential Revision: https://phabricator.robinhood.com/D306850
---
 src/applications/files/query/PhabricatorFileQuery.php | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php
index c19574acaa..7a648073f2 100644
--- a/src/applications/files/query/PhabricatorFileQuery.php
+++ b/src/applications/files/query/PhabricatorFileQuery.php
@@ -183,6 +183,13 @@ protected function loadPage() {
         $always_visible = true;
       }
 
+      // DEVX-2087: skip file policy check if it is PUBLIC or ALL_USERS
+      $file_view_policy = $file->getViewPolicy();
+      if ($file_view_policy == PhabricatorPolicies::POLICY_PUBLIC ||
+          $file_view_policy == PhabricatorPolicies::POLICY_USER) {
+        $always_visible = true;
+      }
+
       if ($always_visible) {
         // We just treat these files as though they aren't attached to
         // anything. This saves a query in common cases when we're loading

From aace7afdf8f9e19b3fa18fc6eea505ddf617b6c2 Mon Sep 17 00:00:00 2001
From: Tianxiang Chen <tianxiang.chen@robinhood.com>
Date: Mon, 1 Nov 2021 15:24:34 -0700
Subject: [PATCH 06/24] Re-enable commandeer

Summary:
Based on the slack discussion: https://hood.slack.com/archives/C016GGP0QDD/p1635800664006300

Partially revert https://github.com/robinhoodmarkets/phabricator/commit/386590fcc3c09b495867c7a8f154a7d9df0c5548 to restore commandeer.

Please notice that with commandeer, hacker can use single machine to land the code: grabbing an
accepted diff -> change -> land.

Test Plan: staging

Reviewers: paul.tarjan

Reviewed By: paul.tarjan

Subscribers: paul.tarjan

Differential Revision: https://phabricator.robinhood.com/D307232
---
 src/__phutil_library_map__.php | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 87a5166046..421ee76400 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -6769,8 +6769,7 @@
     'DifferentialRevisionCloseDetailsController' => 'DifferentialController',
     'DifferentialRevisionCloseTransaction' => 'DifferentialRevisionActionTransaction',
     'DifferentialRevisionClosedStatusDatasource' => 'PhabricatorTypeaheadDatasource',
-    // DEVX-2064
-    // 'DifferentialRevisionCommandeerTransaction' => 'DifferentialRevisionActionTransaction',
+    'DifferentialRevisionCommandeerTransaction' => 'DifferentialRevisionActionTransaction',
     'DifferentialRevisionContentAddedHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionContentHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionContentRemovedHeraldField' => 'DifferentialRevisionHeraldField',

From ce54a03e2436638e9c872b7b8d0e5a5a8e048142 Mon Sep 17 00:00:00 2001
From: Jeff An <jeff.an@robinhood.com>
Date: Thu, 21 Oct 2021 13:43:26 -0700
Subject: [PATCH 07/24] [phab] fix error handling for metrics emitting and
 increase nc timeout

Summary: ^

Test Plan:
i can patch in the code on canary phab and make sure harboramster is still good

tested live on prod phabricator as well

Reviewers: paul.tarjan

Reviewed By: paul.tarjan

Differential Revision: https://phabricator.robinhood.com/D303561
---
 ...sterHTTPRequestBuildStepImplementation.php | 24 +++++++++++++------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
index 9dc270b52a..75b3362733 100644
--- a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
@@ -84,7 +84,7 @@ public function execute(
 
       list($status) = $future->resolve();
       try {
-        $this->emitBuildResultMetric($build->getBuildPlan()->getID(), $status->getStatusCode());
+        $this->emitBuildResultMetric($build->getBuildPlan()->getID(), $status->getStatusCode(), $retry);
       } catch (Exception $e) {
         echo 'Caught exception while sending Jenkins metrics to statsd: ',  $e->getMessage(), "\n";
       }
@@ -100,19 +100,29 @@ public function execute(
     }
   }
 
-  public function emitBuildResultMetric($plan, $status) {
-    $template = 'echo "phabricator_build_result_total:1|c|#plan:$plan,status:$status" | nc -w 1 -u apollo-statsd.integration-tests.svc.cluster.local 8125';
+  public function emitBuildResultMetric($plan, $status, $retry) {
+    $template = 'echo "phabricator_build_result_total:1|c|#plan:$plan,status:$status,retry:$retry" | nc -w 3 -vu apollo-statsd.integration-tests.svc.cluster.local 8125';
     $vars = array(
       '$plan' => $plan,
       '$status' => $status,
+      '$retry' => $retry,
     );
     $command = strtr($template, $vars);
+    echo $command;
+
+    $metrics_retry = 0;
+    $retval = 0;
     $output = null;
-    $retval = null;
-    exec($command, $output, $retval);
-    if ($retval != 0) {
-      echo 'Error while emitting metrics to statsd: ', $output, "\n";
+    while ($metrics_retry < 3) {
+      exec($command, $output, $retval);
+      if ($retval == 0) {
+        return;
+      }
+      $metrics_retry++;
+      sleep(1);
     }
+    echo "Got error code $retval when sending metrics to statsd with output:\n";
+    print_r($output);
   }
 
   public function getFieldSpecifications() {

From 6e700e56c2cf34018da4d176ce46101349d0b18b Mon Sep 17 00:00:00 2001
From: Anna Pstrucha <anna.pstrucha@robinhood.com>
Date: Tue, 22 Feb 2022 19:16:51 -0800
Subject: [PATCH 08/24] Disable changing username during registration

Summary:
We want to ensure that Phabricator username is the same as user's email prefix and not changed during registration.

SW-ID-6553158c3b

Test Plan: Observe registration flow after deployment with an employee who is not an engineer

Reviewers: gabriel.silk, paul.tarjan

Reviewed By: gabriel.silk

Differential Revision: https://phabricator.robinhood.com/D348202
---
 .../auth/controller/PhabricatorAuthRegisterController.php      | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/applications/auth/controller/PhabricatorAuthRegisterController.php b/src/applications/auth/controller/PhabricatorAuthRegisterController.php
index 30aa770f30..65f0d5a4dd 100644
--- a/src/applications/auth/controller/PhabricatorAuthRegisterController.php
+++ b/src/applications/auth/controller/PhabricatorAuthRegisterController.php
@@ -525,7 +525,8 @@ public function handleRequest(AphrontRequest $request) {
           ->setLabel(pht('Username'))
           ->setName('username')
           ->setValue($value_username)
-          ->setError($e_username));
+          ->setError($e_username)
+          ->setDisabled(true));
     } else {
       $form->appendChild(
         id(new AphrontFormMarkupControl())

From 39a52379bd87418478ca0d3e2d8a685f5188adbd Mon Sep 17 00:00:00 2001
From: Anna Pstrucha <anna.pstrucha@robinhood.com>
Date: Thu, 24 Feb 2022 15:33:12 -0800
Subject: [PATCH 09/24] Disable changing username during registration, try 2

Summary:
The previous change did disable the box, but resulted in an error after clicking the register button
afterwards. This is attempt to do this differently - using another control type.

SW-ID-a8c6c5308e

Test Plan: Observe registration flow after deployment with an employee who is not an engineer

Reviewers: gabriel.silk, paul.tarjan

Reviewed By: gabriel.silk

Differential Revision: https://phabricator.robinhood.com/D349374
---
 .../auth/controller/PhabricatorAuthRegisterController.php    | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/applications/auth/controller/PhabricatorAuthRegisterController.php b/src/applications/auth/controller/PhabricatorAuthRegisterController.php
index 65f0d5a4dd..ab45f37ed9 100644
--- a/src/applications/auth/controller/PhabricatorAuthRegisterController.php
+++ b/src/applications/auth/controller/PhabricatorAuthRegisterController.php
@@ -519,14 +519,13 @@ public function handleRequest(AphrontRequest $request) {
               ->setAuthProvider($provider)));
     }
 
-    if ($can_edit_username) {
+    if ($can_edit_username && $is_default) {
       $form->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel(pht('Username'))
           ->setName('username')
           ->setValue($value_username)
-          ->setError($e_username)
-          ->setDisabled(true));
+          ->setError($e_username));
     } else {
       $form->appendChild(
         id(new AphrontFormMarkupControl())

From 2c9c3ba58e947baeda406ecd4c955750b1587860 Mon Sep 17 00:00:00 2001
From: Anna Pstrucha <anna.pstrucha@robinhood.com>
Date: Mon, 28 Feb 2022 16:13:58 -0800
Subject: [PATCH 10/24] Disable changing username during registration, try 3

Summary:
I understood this controller calls this page multiple times, so this change
takes this into account.

SW-ID-d68578809d

Test Plan: Observe registration flow after deployment with an employee who is not an engineer

Reviewers: gabriel.silk, paul.tarjan

Reviewed By: gabriel.silk

Differential Revision: https://phabricator.robinhood.com/D350590
---
 .../auth/controller/PhabricatorAuthRegisterController.php     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/applications/auth/controller/PhabricatorAuthRegisterController.php b/src/applications/auth/controller/PhabricatorAuthRegisterController.php
index ab45f37ed9..5cdb6871ed 100644
--- a/src/applications/auth/controller/PhabricatorAuthRegisterController.php
+++ b/src/applications/auth/controller/PhabricatorAuthRegisterController.php
@@ -198,7 +198,7 @@ public function handleRequest(AphrontRequest $request) {
       ->setDefaultUsername($default_username)
       ->setDefaultEmail($default_email)
       ->setDefaultRealName($default_realname)
-      ->setCanEditUsername(true)
+      ->setCanEditUsername(($default_username === null))
       ->setCanEditEmail(($default_email === null))
       ->setCanEditRealName(true)
       ->setShouldVerifyEmail(false);
@@ -519,7 +519,7 @@ public function handleRequest(AphrontRequest $request) {
               ->setAuthProvider($provider)));
     }
 
-    if ($can_edit_username && $is_default) {
+    if ($can_edit_username) {
       $form->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel(pht('Username'))

From c9155b90c34dd80f383e4fecf70381c3741e65c4 Mon Sep 17 00:00:00 2001
From: "anna.pstrucha" <anna.pstrucha@robinhood.com>
Date: Fri, 2 Sep 2022 15:45:16 -0700
Subject: [PATCH 11/24] Improve kubediff comments in Phab

Summary:
^

SW-ID-69834bc741

Test Plan: None

Reviewers: david.aghassi

Reviewed By: david.aghassi

Differential Revision: https://phabricator.robinhood.com/D428019
---
 src/__phutil_library_map__.php                |  2 ++
 .../PhutilRemarkupDiscolureRule.php           | 25 +++++++++++++++++++
 2 files changed, 27 insertions(+)
 create mode 100644 src/infrastructure/markup/markuprule/PhutilRemarkupDiscolureRule.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 421ee76400..3d4fa93de2 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -5766,6 +5766,7 @@
     'PhutilRemarkupCodeBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupCodeBlockRule.php',
     'PhutilRemarkupDefaultBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupDefaultBlockRule.php',
     'PhutilRemarkupDelRule' => 'infrastructure/markup/markuprule/PhutilRemarkupDelRule.php',
+    'PhutilRemarkupDiscolureRule' => 'infrastructure/markup/markuprule/PhutilRemarkupDiscolureRule.php',
     'PhutilRemarkupDocumentLinkRule' => 'infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php',
     'PhutilRemarkupEngine' => 'infrastructure/markup/remarkup/PhutilRemarkupEngine.php',
     'PhutilRemarkupEngineTestCase' => 'infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php',
@@ -12767,6 +12768,7 @@
     'PhutilRemarkupCodeBlockRule' => 'PhutilRemarkupBlockRule',
     'PhutilRemarkupDefaultBlockRule' => 'PhutilRemarkupBlockRule',
     'PhutilRemarkupDelRule' => 'PhutilRemarkupRule',
+    'PhutilRemarkupDiscolureRule' => 'PhutilRemarkupRule',
     'PhutilRemarkupDocumentLinkRule' => 'PhutilRemarkupRule',
     'PhutilRemarkupEngine' => 'PhutilMarkupEngine',
     'PhutilRemarkupEngineTestCase' => 'PhutilTestCase',
diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupDiscolureRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupDiscolureRule.php
new file mode 100644
index 0000000000..553cb91f9c
--- /dev/null
+++ b/src/infrastructure/markup/markuprule/PhutilRemarkupDiscolureRule.php
@@ -0,0 +1,25 @@
+<?php
+
+final class PhutilRemarkupDiscolureRule extends PhutilRemarkupRule {
+
+  public function apply($text) {
+    if ($this->getEngine()->isTextMode()) {
+      return $text;
+    }
+
+    /**
+     * Matches the details markdown tag
+     * with a summary element
+     * https://regex101.com/r/Pe2G41/1
+     */
+    return $this->replaceHTML(
+      '@(?<!:)<details>(.+?)<summary>(.+?)</summary>(.+?)</details>@s',
+      array($this, 'applyCallback'),
+      $text);
+  }
+
+  protected function applyCallback(array $matches) {
+    return hsprintf("<details>\n<summary>%s</summary>\n\n%s\n</details>", $matches[2], $matches[3]);
+  }
+
+}

From d2a19fe9361c22b9885750cd3547d63341dc561c Mon Sep 17 00:00:00 2001
From: Anna Pstrucha <anna.pstrucha@robinhood.com>
Date: Tue, 13 Sep 2022 14:37:58 -0700
Subject: [PATCH 12/24] Improve comments take 2

Summary:
^

SW-ID-4df721d987

Test Plan: staging

Reviewers: david.aghassi

Reviewed By: david.aghassi

Differential Revision: https://phabricator.robinhood.com/D431131
---
 src/__phutil_library_map__.php                                | 4 ++--
 src/infrastructure/markup/PhabricatorMarkupEngine.php         | 1 +
 ...rkupDiscolureRule.php => PhutilRemarkupDisclosureRule.php} | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)
 rename src/infrastructure/markup/markuprule/{PhutilRemarkupDiscolureRule.php => PhutilRemarkupDisclosureRule.php} (88%)

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 3d4fa93de2..8250aeec2f 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -5766,7 +5766,7 @@
     'PhutilRemarkupCodeBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupCodeBlockRule.php',
     'PhutilRemarkupDefaultBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupDefaultBlockRule.php',
     'PhutilRemarkupDelRule' => 'infrastructure/markup/markuprule/PhutilRemarkupDelRule.php',
-    'PhutilRemarkupDiscolureRule' => 'infrastructure/markup/markuprule/PhutilRemarkupDiscolureRule.php',
+    'PhutilRemarkupDisclosureRule' => 'infrastructure/markup/markuprule/PhutilRemarkupDisclosureRule.php',
     'PhutilRemarkupDocumentLinkRule' => 'infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php',
     'PhutilRemarkupEngine' => 'infrastructure/markup/remarkup/PhutilRemarkupEngine.php',
     'PhutilRemarkupEngineTestCase' => 'infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php',
@@ -12768,7 +12768,7 @@
     'PhutilRemarkupCodeBlockRule' => 'PhutilRemarkupBlockRule',
     'PhutilRemarkupDefaultBlockRule' => 'PhutilRemarkupBlockRule',
     'PhutilRemarkupDelRule' => 'PhutilRemarkupRule',
-    'PhutilRemarkupDiscolureRule' => 'PhutilRemarkupRule',
+    'PhutilRemarkupDisclosureRule' => 'PhutilRemarkupRule',
     'PhutilRemarkupDocumentLinkRule' => 'PhutilRemarkupRule',
     'PhutilRemarkupEngine' => 'PhutilMarkupEngine',
     'PhutilRemarkupEngineTestCase' => 'PhutilTestCase',
diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php
index 3ad5304945..13c469dacb 100644
--- a/src/infrastructure/markup/PhabricatorMarkupEngine.php
+++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php
@@ -541,6 +541,7 @@ public static function newMarkupEngine(array $options) {
     $rules[] = new PhutilRemarkupUnderlineRule();
     $rules[] = new PhutilRemarkupHighlightRule();
     $rules[] = new PhutilRemarkupAnchorRule();
+    $rules[] = new PhutilRemarkupDisclosureRule();
 
     foreach (self::loadCustomInlineRules() as $rule) {
       $rules[] = clone $rule;
diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupDiscolureRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupDisclosureRule.php
similarity index 88%
rename from src/infrastructure/markup/markuprule/PhutilRemarkupDiscolureRule.php
rename to src/infrastructure/markup/markuprule/PhutilRemarkupDisclosureRule.php
index 553cb91f9c..08b89b20da 100644
--- a/src/infrastructure/markup/markuprule/PhutilRemarkupDiscolureRule.php
+++ b/src/infrastructure/markup/markuprule/PhutilRemarkupDisclosureRule.php
@@ -1,6 +1,6 @@
 <?php
 
-final class PhutilRemarkupDiscolureRule extends PhutilRemarkupRule {
+final class PhutilRemarkupDisclosureRule extends PhutilRemarkupRule {
 
   public function apply($text) {
     if ($this->getEngine()->isTextMode()) {

From 74b6f303c8f0dc85dde37ad2fa30c529415689ca Mon Sep 17 00:00:00 2001
From: Ryan Anderson <ryan.anderson@robinhood.com>
Date: Mon, 10 Oct 2022 21:49:29 -0700
Subject: [PATCH 13/24] Require revisions to be updated in the last 30 days by
 default

Summary:
Stale revisions are unlandable and make it very hard to find things in
the UI.

SW-ID-f8f44e04f7

Test Plan: staging

Reviewers: anna.pstrucha

Reviewed By: anna.pstrucha

Differential Revision: https://phabricator.robinhood.com/D447876
---
 src/applications/home/view/PHUIHomeView.php | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/applications/home/view/PHUIHomeView.php b/src/applications/home/view/PHUIHomeView.php
index 45750f5a93..00da3b88f3 100644
--- a/src/applications/home/view/PHUIHomeView.php
+++ b/src/applications/home/view/PHUIHomeView.php
@@ -86,7 +86,8 @@ private function buildRevisionPanel() {
     $panel = $this->newQueryPanel()
       ->setName(pht('Active Revisions'))
       ->setProperty('class', 'DifferentialRevisionSearchEngine')
-      ->setProperty('key', 'active');
+      ->setProperty('key', 'active')
+      ->setProperty('modifiedStart', '30 days ago');
 
     return $this->renderPanel($panel);
   }

From f2c2599d78d3febcc7d65351d763e129e9788593 Mon Sep 17 00:00:00 2001
From: Ryan Anderson <ryan.anderson@robinhood.com>
Date: Fri, 21 Oct 2022 12:10:50 -0700
Subject: [PATCH 14/24] [differential] Enable the modified start/end fields in
 search

Summary:
This will help manage the large number of stale and abandoned diffs
left after the layoffs.

SW-ID-b9431677ef

Test Plan: staging

Reviewers: anna.pstrucha

Reviewed By: anna.pstrucha

Differential Revision: https://phabricator.robinhood.com/D447877
---
 .../differential/query/DifferentialRevisionSearchEngine.php   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php
index 1ec5265630..5aef5a1800 100644
--- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php
+++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php
@@ -113,13 +113,13 @@ protected function buildCustomSearchFields() {
       id(new PhabricatorSearchDateField())
         ->setLabel(pht('Modified After'))
         ->setKey('modifiedStart')
-        ->setIsHidden(true)
+        ->setIsHidden(false)
         ->setDescription(
           pht('Find revisions modified at or after a particular time.')),
       id(new PhabricatorSearchDateField())
         ->setLabel(pht('Modified Before'))
         ->setKey('modifiedEnd')
-        ->setIsHidden(true)
+        ->setIsHidden(false)
         ->setDescription(
           pht('Find revisions modified at or before a particular time.')),
       id(new PhabricatorSearchStringListField())

From 0e8a5ea93def69d766be1f6e1b82dfa7c98c8176 Mon Sep 17 00:00:00 2001
From: Gavin Loughridge <gavin.loughridge@robinhood.com>
Date: Thu, 5 Jan 2023 13:34:32 -0800
Subject: [PATCH 15/24] Editing the Phabricator disclosure markup rule

Summary:
Phabricator alters html symbols like < and > before rule processing which prevents the current diclosure rule from ever being applied. This changes the rule to use
the syntax of confluences expand macro to avoid using html symbols.

Test Plan: local testing

Differential Revision: https://phabricator.robinhood.com/D475229
---
 .../PhutilRemarkupDisclosureRule.php          | 31 ++++++++++++-------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupDisclosureRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupDisclosureRule.php
index 08b89b20da..4c530a14b7 100644
--- a/src/infrastructure/markup/markuprule/PhutilRemarkupDisclosureRule.php
+++ b/src/infrastructure/markup/markuprule/PhutilRemarkupDisclosureRule.php
@@ -7,19 +7,26 @@ public function apply($text) {
       return $text;
     }
 
-    /**
-     * Matches the details markdown tag
-     * with a summary element
-     * https://regex101.com/r/Pe2G41/1
-     */
-    return $this->replaceHTML(
-      '@(?<!:)<details>(.+?)<summary>(.+?)</summary>(.+?)</details>@s',
-      array($this, 'applyCallback'),
-      $text);
-  }
+    // Tags to match in text and what the tag should look like after
+    // HTML sanitization. Altering the left margin creates indentation.
+    $replacements = array(
+      '<details>' => hsprintf('<details style="margin-left: 2em">'),
+      '</details>' => hsprintf('</details>'),
+      '<summary>' => hsprintf('<summary style="margin-left: -2em">'),
+      '</summary>' => hsprintf('</summary>'),
+    );
+
+    // Sanitize text and replace each sanitized tag with it's
+    // corresponding replacement text.
+    foreach ($replacements as $match => $replacement) {
+      $text = PhutilSafeHTML::applyFunction(
+        'preg_replace',
+        hsprintf('@\s?%s\s?@s', $match),
+        $replacement,
+        $text);
+    }
 
-  protected function applyCallback(array $matches) {
-    return hsprintf("<details>\n<summary>%s</summary>\n\n%s\n</details>", $matches[2], $matches[3]);
+    return $text;
   }
 
 }

From 4a0844b30efb4c8b9f0ca2d129ca1b8c2e202fc8 Mon Sep 17 00:00:00 2001
From: Joel Jeske <joel.jeske@robinhood.com>
Date: Wed, 7 Jun 2023 13:31:05 -0500
Subject: [PATCH 16/24] [commit-rendering] include project tags in commit
 template

Summary:

This is expected to include project tags in the commit message that is fetched by PHLQ

Test Plan: Deploy to Phabricator staging and use https://secure.phabricator.com/conduit/method/differential.getcommitmessage/ to view rendering

Reviewers: #devx

Subscribers:

Task Link:
---
 .../differential/field/DifferentialTagsCommitMessageField.php   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/applications/differential/field/DifferentialTagsCommitMessageField.php b/src/applications/differential/field/DifferentialTagsCommitMessageField.php
index fc6267f1ce..704d32a5c8 100644
--- a/src/applications/differential/field/DifferentialTagsCommitMessageField.php
+++ b/src/applications/differential/field/DifferentialTagsCommitMessageField.php
@@ -22,7 +22,7 @@ public function getFieldAliases() {
   }
 
   public function isTemplateField() {
-    return false;
+    return true;
   }
 
   public function parseFieldValue($value) {

From cbbfd72bd5cb86aa03d12700d2150a6aca8c4bbb Mon Sep 17 00:00:00 2001
From: Joel Jeske <joel.jeske@robinhood.com>
Date: Wed, 28 Jun 2023 17:08:14 -0500
Subject: [PATCH 17/24] [phabricator] allow exposing current action to search
 (#3)

---
 .../DifferentialReviewersSearchEngineAttachment.php            | 3 +++
 src/applications/differential/storage/DifferentialReviewer.php | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/applications/differential/engineextension/DifferentialReviewersSearchEngineAttachment.php b/src/applications/differential/engineextension/DifferentialReviewersSearchEngineAttachment.php
index 33fb606a60..c74983b459 100644
--- a/src/applications/differential/engineextension/DifferentialReviewersSearchEngineAttachment.php
+++ b/src/applications/differential/engineextension/DifferentialReviewersSearchEngineAttachment.php
@@ -13,10 +13,12 @@ public function getAttachmentDescription() {
 
   public function willLoadAttachmentData($query, $spec) {
     $query->needReviewers(true);
+    $query->needActiveDiffs(true);
   }
 
   public function getAttachmentForObject($object, $data, $spec) {
     $reviewers = $object->getReviewers();
+    $diff_phid = $object->getActiveDiff()->getPHID();
 
     $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
 
@@ -30,6 +32,7 @@ public function getAttachmentForObject($object, $data, $spec) {
         'status' => $status,
         'isBlocking' => $is_blocking,
         'actorPHID' => $reviewer->getLastActorPHID(),
+        'isCurrentAction' => $reviewer->isCurrentAction($diff_phid),
       );
     }
 
diff --git a/src/applications/differential/storage/DifferentialReviewer.php b/src/applications/differential/storage/DifferentialReviewer.php
index 823047f218..4870e73b84 100644
--- a/src/applications/differential/storage/DifferentialReviewer.php
+++ b/src/applications/differential/storage/DifferentialReviewer.php
@@ -134,7 +134,7 @@ public function isAccepted($diff_phid) {
     return false;
   }
 
-  private function isCurrentAction($diff_phid) {
+  public function isCurrentAction($diff_phid) {
     if (!$diff_phid) {
       return true;
     }

From 56cd30ae9d2b8ea403cafcce3044b4ef3c5e2790 Mon Sep 17 00:00:00 2001
From: Joel Jeske <joel.jeske@robinhood.com>
Date: Thu, 10 Aug 2023 12:50:24 -0500
Subject: [PATCH 18/24] [diffs] prevent force accept (#4)

---
 .../differential/storage/DifferentialRevision.php            | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
index 92b303d0ba..cc825fcf98 100644
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -223,6 +223,11 @@ public function canReviewerForceAccept(
     PhabricatorUser $viewer,
     DifferentialReviewer $reviewer) {
 
+    // NOTE(joel.jeske)
+    // We do not want to support force-acceptance if owning a parent package
+    // All ownership rules should be declared in OWNERS.toml files
+    return false;
+
     if (!$reviewer->isPackage()) {
       return false;
     }

From f7434d67b1f43b5b11b8c2ee644b26c3787fa025 Mon Sep 17 00:00:00 2001
From: joeljeske <joel.jeske@robinhood.com>
Date: Wed, 6 Sep 2023 18:37:04 +0000
Subject: [PATCH 19/24] Remove "Owns No Changed Paths" in Diff View

---
 .../differential/view/DifferentialReviewersView.php | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php
index ad6bf1462b..1245530ffe 100644
--- a/src/applications/differential/view/DifferentialReviewersView.php
+++ b/src/applications/differential/view/DifferentialReviewersView.php
@@ -166,11 +166,14 @@ public function render() {
           null,
           $diff->getPHID()));
 
-      if ($reviewer->isPackage()) {
-        if (!$reviewer->getChangesets()) {
-          $item->setNote(pht('(Owns No Changed Paths)'));
-        }
-      }
+      // With advanced Ownership in OWNERS.toml, this is message is
+      // no longer accurate.
+      //
+      // if ($reviewer->isPackage()) {
+      //   if (!$reviewer->getChangesets()) {
+      //     $item->setNote(pht('(Owns No Changed Paths)'));
+      //   }
+      // }
 
       if ($handle->hasCapabilities()) {
         if (!$handle->hasViewCapability($diff)) {

From 9ab3223107c3316ec9b9f85e3e5e113e9478de54 Mon Sep 17 00:00:00 2001
From: Yi Lu <yi.lu@robinhood.com>
Date: Fri, 9 Feb 2024 12:22:27 -0800
Subject: [PATCH 20/24] [phabricator] add exact responsible viewer function

Summary:
when we query revisions, there is `exact(<username>)` function that matches the exact user only, and not projects or groups the user is a member of; this is useful when we search revisions that we are directly tagged on, however this function must take in a specific username, so I can't build a query generalized for everyone

there is also a `viewer()` function, which is supposed to be a general query that can be used to build dashboards, so whoever views a dashboard used query with this function only sees their own stuff, but this includes the viewer's projects and groups and everything

this diffs adds an `exact-viewer()` function, which combines the two, so we can query things match exactly the viewer

why this change: when we discussed in our team sync how the current default phabricator "Home" dashboard is not so useful and I built my own query, @brian.myers said he wanted it to. So I figure instead of teaching people the query, I can just build a new home dashboard that's more informational. this exact-viewer() function makes the query generalized so I can just build a dashboard on top this query and make it available to everyone

Test Plan:
tested locally, and on phabricator staging

when I query "exact current viewer"
{F14482326}
I get my own stuff
{F14482342}

when I query "current viewer"
{F14482353}
I get a diff where I'm the group reviewer
{F14482375}

Reviewers: brian.myers, joel.jeske

Reviewed By: brian.myers

Subscribers: brian.myers

Differential Revision: https://phabricator.robinhood.com/D643179
---
 src/__phutil_library_map__.php                |  2 +
 ...actResponsibleViewerFunctionDatasource.php | 73 +++++++++++++++++++
 .../DifferentialResponsibleDatasource.php     |  1 +
 3 files changed, 76 insertions(+)
 create mode 100644 src/applications/differential/typeahead/DifferentialExactResponsibleViewerFunctionDatasource.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 8250aeec2f..0d0fda0820 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -542,6 +542,7 @@
     'DifferentialDiffViewController' => 'applications/differential/controller/DifferentialDiffViewController.php',
     'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php',
     'DifferentialDraftField' => 'applications/differential/customfield/DifferentialDraftField.php',
+    'DifferentialExactResponsibleViewerFunctionDatasource' => 'applications/differential/typeahead/DifferentialExactResponsibleViewerFunctionDatasource.php',
     'DifferentialExactUserFunctionDatasource' => 'applications/differential/typeahead/DifferentialExactUserFunctionDatasource.php',
     'DifferentialFieldParseException' => 'applications/differential/exception/DifferentialFieldParseException.php',
     'DifferentialFieldValidationException' => 'applications/differential/exception/DifferentialFieldValidationException.php',
@@ -6662,6 +6663,7 @@
     'DifferentialDiffViewController' => 'DifferentialController',
     'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'DoorkeeperFeedStoryPublisher',
     'DifferentialDraftField' => 'DifferentialCoreCustomField',
+    'DifferentialExactResponsibleViewerFunctionDatasource' => 'PhabricatorTypeaheadDatasource',
     'DifferentialExactUserFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'DifferentialFieldParseException' => 'Exception',
     'DifferentialFieldValidationException' => 'Exception',
diff --git a/src/applications/differential/typeahead/DifferentialExactResponsibleViewerFunctionDatasource.php b/src/applications/differential/typeahead/DifferentialExactResponsibleViewerFunctionDatasource.php
new file mode 100644
index 0000000000..c0f29669d5
--- /dev/null
+++ b/src/applications/differential/typeahead/DifferentialExactResponsibleViewerFunctionDatasource.php
@@ -0,0 +1,73 @@
+<?php
+
+final class DifferentialExactResponsibleViewerFunctionDatasource
+  extends PhabricatorTypeaheadDatasource {
+
+  public function getBrowseTitle() {
+    return pht('Browse Exact Viewer');
+  }
+
+  public function getPlaceholderText() {
+    return pht('Type exact-viewer()...');
+  }
+
+  public function getDatasourceApplicationClass() {
+    return 'PhabricatorPeopleApplication';
+  }
+
+  public function getDatasourceFunctions() {
+    return array(
+      'exact-viewer' => array(
+        'name' => pht('Exact Current Viewer'),
+        'summary' => pht('Results matching the current viewing user exactly.'),
+        'description' => pht(
+          'Find revisions the current viewer is responsible for, exactly,'.
+          'and not include those through their projects or packages. '),
+        ),
+    );
+  }
+
+  public function loadResults() {
+    if ($this->getViewer()->getPHID()) {
+      $results = array($this->renderViewerFunctionToken());
+    } else {
+      $results = array();
+    }
+
+    return $this->filterResultsAgainstTokens($results);
+  }
+
+  protected function canEvaluateFunction($function) {
+    if (!$this->getViewer()->getPHID()) {
+      return false;
+    }
+
+    return parent::canEvaluateFunction($function);
+  }
+
+  protected function evaluateFunction($function, array $argv_list) {
+    $results = array();
+    foreach ($argv_list as $argv) {
+      $results[] = $this->getViewer()->getPHID();
+    }
+    return $results;
+  }
+
+  public function renderFunctionTokens($function, array $argv_list) {
+    $tokens = array();
+    foreach ($argv_list as $argv) {
+      $tokens[] = PhabricatorTypeaheadTokenView::newFromTypeaheadResult(
+        $this->renderViewerFunctionToken());
+    }
+    return $tokens;
+  }
+
+  private function renderViewerFunctionToken() {
+    return $this->newFunctionResult()
+      ->setName(pht('Exact: Current Viewer'))
+      ->setPHID('exact-viewer()')
+      ->setIcon('fa-user')
+      ->setUnique(true);
+  }
+
+}
diff --git a/src/applications/differential/typeahead/DifferentialResponsibleDatasource.php b/src/applications/differential/typeahead/DifferentialResponsibleDatasource.php
index 5307ab1866..4e109ac183 100644
--- a/src/applications/differential/typeahead/DifferentialResponsibleDatasource.php
+++ b/src/applications/differential/typeahead/DifferentialResponsibleDatasource.php
@@ -18,6 +18,7 @@ public function getDatasourceApplicationClass() {
   public function getComponentDatasources() {
     return array(
       new DifferentialResponsibleUserDatasource(),
+      new DifferentialExactResponsibleViewerFunctionDatasource(),
       new DifferentialResponsibleViewerFunctionDatasource(),
       new DifferentialExactUserFunctionDatasource(),
       new PhabricatorProjectDatasource(),

From fc3ce9d6992d0b439dc53321e76e16b91f45b1c7 Mon Sep 17 00:00:00 2001
From: "marcelo.fabri" <marcelo.fabri@robinhood.com>
Date: Sun, 3 Mar 2024 18:22:07 -0800
Subject: [PATCH 21/24] [custom fields] Call setObject before
 shouldEnableForRole

---
 .../customfield/field/PhabricatorCustomField.php          | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php
index 9e2bf6895f..8ae2c1f3eb 100644
--- a/src/infrastructure/customfield/field/PhabricatorCustomField.php
+++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php
@@ -79,6 +79,10 @@ public static function getObjectFields(
         $role,
         $fields);
 
+      foreach ($fields as $field) {
+        $field->setObject($object);
+      }
+
       foreach ($fields as $key => $field) {
         // NOTE: We perform this filtering in "buildFieldList()", but may need
         // to filter again after subtype adjustment.
@@ -93,10 +97,6 @@ public static function getObjectFields(
         }
       }
 
-      foreach ($fields as $field) {
-        $field->setObject($object);
-      }
-
       $field_list = new PhabricatorCustomFieldList($fields);
       $attachment->addCustomFieldList($role, $field_list);
     }

From f8fbf1a40bf5a35dc7b90a0e3ec2aa3515effa99 Mon Sep 17 00:00:00 2001
From: Yi Lu <yi.lu@robinhood.com>
Date: Fri, 8 Mar 2024 09:49:44 -0800
Subject: [PATCH 22/24] Revert "Merge pull request #5 from
 robinhoodmarkets/marcelo/custom-field-set-object"

This reverts commit 436872463487f32c63ff843417993425f23a1683, reversing
changes made to 9ab3223107c3316ec9b9f85e3e5e113e9478de54.
---
 .../customfield/field/PhabricatorCustomField.php          | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php
index 8ae2c1f3eb..9e2bf6895f 100644
--- a/src/infrastructure/customfield/field/PhabricatorCustomField.php
+++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php
@@ -79,10 +79,6 @@ public static function getObjectFields(
         $role,
         $fields);
 
-      foreach ($fields as $field) {
-        $field->setObject($object);
-      }
-
       foreach ($fields as $key => $field) {
         // NOTE: We perform this filtering in "buildFieldList()", but may need
         // to filter again after subtype adjustment.
@@ -97,6 +93,10 @@ public static function getObjectFields(
         }
       }
 
+      foreach ($fields as $field) {
+        $field->setObject($object);
+      }
+
       $field_list = new PhabricatorCustomFieldList($fields);
       $attachment->addCustomFieldList($role, $field_list);
     }

From e310cbdcee3409edc3cf6a7725aa437d99427999 Mon Sep 17 00:00:00 2001
From: Joel Jeske <joel.jeske@robinhood.com>
Date: Wed, 17 Jul 2024 12:04:11 -0500
Subject: [PATCH 23/24] [phabricator] prevent sending email for project/owner
 updates

---
 .../editor/PhabricatorOwnersPackageTransactionEditor.php     | 5 ++++-
 .../project/editor/PhabricatorProjectTransactionEditor.php   | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php
index 8885872706..5de5e41a56 100644
--- a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php
+++ b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php
@@ -23,7 +23,10 @@ public function getTransactionTypes() {
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
-    return true;
+    // Don't ever send email for changes to a Phab Owner Package.
+    // We perform bulk updates in response to OWNER file changes
+    // and this can cause queuing and a lot of email spam.
+    return false;
   }
 
   protected function getMailSubjectPrefix() {
diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
index eb57c39b2c..c3e9ee4040 100644
--- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
+++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
@@ -127,7 +127,10 @@ protected function willPublish(PhabricatorLiskDAO $object, array $xactions) {
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
-    return true;
+    // Don't ever send email for changes to a Phab Projects.
+    // We perform bulk updates in response to OWNER file changes
+    // and this can cause queuing and a lot of email spam.
+    return false;
   }
 
   protected function getMailSubjectPrefix() {

From c2eab37ed4715e367801a28d3e584631038166ff Mon Sep 17 00:00:00 2001
From: Kevin Vissuet <kevin.vissuet@robinhood.com>
Date: Wed, 11 Sep 2024 15:15:17 -0700
Subject: [PATCH 24/24] [phabricator] Add slack channel to ping for review to
 owners page

Summary: When engineers know which channel to ping for review, reviews are done faster. For every 5 minutes it takes to do a code review costs 1600 engineer days are saved per year.

Test Plan: Demo

Reviewers:

Subscribers:

Task Link:

RedOak ID:

Tags:
---
 .../PhabricatorOwnersDetailController.php     | 10 +++
 .../PhabricatorOwnersPackageEditEngine.php    |  8 ++
 .../storage/PhabricatorOwnersPackage.php      |  4 +
 ...abricatorOwnersPackageSlackTransaction.php | 85 +++++++++++++++++++
 4 files changed, 107 insertions(+)
 create mode 100644 src/applications/owners/xaction/PhabricatorOwnersPackageSlackTransaction.php

diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
index 8512107e75..1ad380ef4d 100644
--- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
@@ -220,8 +220,18 @@ private function buildPackageDetailView(
       $ignored = phutil_tag('em', array(), pht('None'));
     }
 
+
+
     $view->addProperty(pht('Ignored Attributes'), $ignored);
 
+    // Add Slack Channel Field
+    $slack_channel = $package->getSlackChannel();
+    if ($slack_channel) {
+      $view->addProperty(pht('Slack Channel'), $slack_channel);
+    } else {
+      $view->addProperty(pht('Slack Channel'), phutil_tag('em', array(), pht('None')));
+    }
+
     $description = $package->getDescription();
     if (strlen($description)) {
       $description = new PHUIRemarkupView($viewer, $description);
diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
index 416f2e38f2..ab4ff97e39 100644
--- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
+++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php
@@ -182,6 +182,14 @@ protected function buildCustomEditFields($object) {
           array(
             'generated' => pht('Ignore generated files (review only).'),
           )),
+      id(new PhabricatorTextEditField())
+        ->setKey('slackChannel')
+        ->setLabel(pht('Slack Channel'))
+        ->setDescription(pht('Slack channel associated with this package.'))
+        ->setTransactionType(
+          PhabricatorOwnersPackageSlackChannelTransaction::TRANSACTIONTYPE)
+        ->setIsCopyable(true)
+        ->setValue($object->getSlackChannel()),
       id(new PhabricatorConduitEditField())
         ->setKey('paths.set')
         ->setLabel(pht('Paths'))
diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php
index 7e58d586b2..77e1d5798b 100644
--- a/src/applications/owners/storage/PhabricatorOwnersPackage.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php
@@ -727,6 +727,10 @@ public function getFieldSpecificationsForConduit() {
         ->setKey('ignored')
         ->setType('map<string, wild>')
         ->setDescription(pht('Ignored attribute information.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('slack')
+        ->setType('map<string, wild>')
+        ->setDescription(pht('Slack Channel to Ping for Review.')),
     );
   }
 
diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageSlackTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageSlackTransaction.php
new file mode 100644
index 0000000000..ff186d1f36
--- /dev/null
+++ b/src/applications/owners/xaction/PhabricatorOwnersPackageSlackTransaction.php
@@ -0,0 +1,85 @@
+<?php
+
+final class PhabricatorOwnersPackageSlackTransaction
+  extends PhabricatorOwnersPackageTransactionType {
+
+  const TRANSACTIONTYPE = 'owners.slack';
+
+  public function generateOldValue($object) {
+    return $object->getSlackPathAttributes();
+  }
+
+  public function generateNewValue($object, $value) {
+    return array_fill_keys($value, true);
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setSlackPathAttributes($value);
+  }
+
+  public function getTitle() {
+    $old = array_keys($this->getOldValue());
+    $new = array_keys($this->getNewValue());
+
+    $add = array_diff($new, $old);
+    $rem = array_diff($old, $new);
+
+    $all_n = new PhutilNumber(count($add) + count($rem));
+    $add_n = phutil_count($add);
+    $rem_n = phutil_count($rem);
+
+    if ($new && $old) {
+      return pht(
+        '%s changed %s slack attribute(s), added %s: %s; removed %s: %s.',
+        $this->renderAuthor(),
+        $all_n,
+        $add_n,
+        $this->renderValueList($add),
+        $rem_n,
+        $this->renderValueList($rem));
+    } else if ($new) {
+      return pht(
+        '%s changed %s slack attribute(s), added %s: %s.',
+        $this->renderAuthor(),
+        $all_n,
+        $add_n,
+        $this->rendervalueList($add));
+    } else {
+      return pht(
+        '%s changed %s slack attribute(s), removed %s: %s.',
+        $this->renderAuthor(),
+        $all_n,
+        $rem_n,
+        $this->rendervalueList($rem));
+    }
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $valid_attributes = array(
+      'generated' => true,
+    );
+
+    foreach ($xactions as $xaction) {
+      $new = $xaction->getNewValue();
+
+      foreach ($new as $attribute) {
+        if (isset($valid_attributes[$attribute])) {
+          continue;
+        }
+
+        $errors[] = $this->newInvalidError(
+          pht(
+            'Changeset attribute "%s" is not valid. Valid changeset '.
+            'attributes are: %s.',
+            $attribute,
+            implode(', ', array_keys($valid_attributes))),
+          $xaction);
+      }
+    }
+
+    return $errors;
+  }
+
+}