Notes to myself:

• Tested task creation as average user. Works as expected: Priority field not shown in the UI and task successfully created wth default priority.
• This code change does not affect bulk editing: The "Set Priority" option is still available as a bulk action even though the performing user is not a member of one of the four projects. That's fine IMO as bulk edits are limited to acl*Batch-Editors members anyway.

aklapper opened https://gitlab.wikimedia.org/repos/phabricator/phabricator/-/merge_requests/50

Draft: Expose the Edit Task Priority dropdown field only to trusted users

Would this mean that people like me couldn’t put Unbreak Now! priority to the tasks that require it?

It would mean that only members of Trusted-Contributors, WMF-NDA acl*sre-team or acl*security can change the Priority field value.

In T363337#9811366, @Aklapper wrote:

It would mean that only members of Trusted-Contributors, WMF-NDA acl*sre-team or acl*security can change the Priority field value.

I hope this would mean that addition to Trusted-Contributors would be more active then because this would effectively hinder ability of many less active Phabricator contributors to report the urgent tasks (I was added to the group but I assume many others, even admins from different wikis, will be forgotten).

In T363337#9811366, @Aklapper wrote:

It would mean that only members of Trusted-Contributors, WMF-NDA acl*sre-team or acl*security can change the Priority field value.

my guess is onboarding varies a bit so we make sure wmf+affiliate staff are all in at least one of those groups.

To make this change in behavior less disruptive for PMs etc, should check first which non-deactivated Phab user accounts have been active setting the Priority field in the last six months and are neither in Trusted-Contributors nor WMF-NDA, and add them:
SELECT usr.username, COUNT(usr.username) AS cnt FROM phabricator_maniphest.maniphest_transaction trs JOIN phabricator_user.user usr WHERE trs.transactionType = "priority" AND FROM_UNIXTIME(trs.dateModified) >= (NOW() - INTERVAL 6 MONTH) AND trs.authorPHID = usr.phid AND usr.isDisabled != 1 AND usr.phid NOT IN (SELECT e.src FROM phabricator_user.edge e WHERE e.dst = "PHID-PROJ-rte2hubdntrhiwyoau6n" AND e.type = 14) AND usr.phid NOT IN (SELECT e.src FROM phabricator_user.edge e WHERE e.dst = "PHID-PROJ-ibxm3v6ithf3jpqpqhl7" AND e.type = 14) GROUP BY usr.username ORDER BY cnt DESC LIMIT 80;

brennen merged https://gitlab.wikimedia.org/repos/phabricator/phabricator/-/merge_requests/50

Expose the Edit Task Priority dropdown field only to trusted users

In T363337#10124562, @Aklapper wrote:

To make this change in behavior less disruptive for PMs etc, should check first which non-deactivated Phab user accounts have been active setting the Priority field in the last six months and are neither in Trusted-Contributors nor WMF-NDA, and add them:

Was this done? And is there any plan to add active members of Phabricator community to Trusted-Contributors more quickly after this change got merged?

Was this done?

Yes.

And is there any plan to add active members of Phabricator community to Trusted-Contributors more quickly after this change got merged?

No (as I cannot predict how quick nearly 800 people will do things in the future). :)

In T363337#10234235, @Aklapper wrote:

as I cannot predict how quick nearly 800 people will do things in the future :)

I mean, you spearheaded this effort to tie an important permission to the group. I think there should be some (organisational) responsibility on your part to give the permissions out to people who are definitely trusted enough after this change got merged (IAdmins or admins on Wikimedia wikis, for instance). At least one of the members of the new Product and Technology Advisory Council wasn’t in that group until I added them yesterday, I wasn’t until I mentioned it in this very task, and how many others are like that?

to give the permissions out to people who are definitely trusted enough after this change got merged

I did for those who have already been active setting priorities in T363337#10124562. I cannot foresee the future though. :)

IAdmins or admins on Wikimedia wikis, for instance

I don't plan to collect data from all 900 wikis and sync it against Phabricator accounts.
If you or others know and trust folks who should have priority setting permissions, feel free to add them to Trusted-Contributors, just like before.

until I added them yesterday

Thanks for doing that!

how many others are like that?

If you have an idea how to find out, please share it. Thanks!

Basically: If you think you need permissions, ask for permissions. That's a common concept in Wikimedia (and Phabricator, for example to create projects, mass-edit tasks, create forms, etc etc). I don't plan to invert the concept by starting to actively search for folks who should have permissions in this very case. Sure, for a few folks this change might be initially disruptive but that's the thing with changes - we had the same situation when we introduced Trusted-Contributors ("why can't I not do xyz in Phab anymore?"), it's hard to always identify everyone who may benefit from being included in those additional permissions.

My patch merged on 2024-10-22 never worked as expected, for reasons I still cannot figure out.
I'll try again with https://gitlab.wikimedia.org/repos/phabricator/phabricator/-/commit/4b00a968e116d235b6c9f7c73916d758f5293e9f which seems to work.

aklapper opened https://gitlab.wikimedia.org/repos/phabricator/phabricator/-/merge_requests/84

Revert "Expose the Edit Task Priority dropdown field only to trusted users"

aklapper merged https://gitlab.wikimedia.org/repos/phabricator/phabricator/-/merge_requests/84

Revert "Expose the Edit Task Priority dropdown field only to trusted users"

Pasting the latest code I played with, in case someone ever wants to pick this up again:

[acko@fedora phabricator (wmf/stable *$|u=)]$ git diff src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php
diff --git a/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php b/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php
index e3448c676b..e3f1a41c61 100644
--- a/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php
+++ b/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php
@@ -18,6 +18,8 @@ final class PhabricatorEditEngineConfiguration
   protected $editOrder = 0;
   protected $subtype;
 
+  private $actor; // WMF T363337
+
   private $engine = self::ATTACHABLE;
 
   const LOCK_VISIBLE = 'visible';
@@ -32,6 +34,7 @@ final class PhabricatorEditEngineConfiguration
     PhabricatorUser $actor,
     PhabricatorEditEngine $engine) {
 
+    $this->$actor = $actor; // WMF T363337; no other way to get actor. But does not work because static function.
     return id(new PhabricatorEditEngineConfiguration())
       ->setSubtype(PhabricatorEditEngine::SUBTYPE_DEFAULT)
       ->setEngineKey($engine->getEngineKey())
@@ -175,6 +178,23 @@ final class PhabricatorEditEngineConfiguration
           if ($field->getIsLockable()) {
             $field->setIsLocked(false);
           }
+          // WMF hack BEGIN: Only show Edit Priority when user is trusted - T363337
+          if ($key == 'priority') {
+            $actor_phid = $this->$actor->getPHID();
+            $trusted_project_names = ["Trusted-Contributors", "WMF-NDA", "acl*sre-team",
+                                      "acl*security", "acl*Batch-Editors", "acl*phabricator"];
+            $projects = WMFSecurityPolicy::getProjectByName($trusted_project_names, $actor_phid, true);
+            if ($projects) { // allow to edit priority if no $projects found AT ALL
+              foreach ($projects as $proj) {
+                if ($proj instanceof PhabricatorProject && $proj->isUserMember($actor_phid)) {
+                  break;
+                }
+              }
+            } else {
+              $field->setIsLocked(true);
+            }
+          }
+          // WMF hack END: Only show Edit Priority when user is trusted - T363337
           break;
         default:
           // If we don't have an explicit value, hide the field. This way,