Paths

Table of Contentst

Allow admins to edit email addresses of mailing list accounts again
AcceptedPublic
Actions

Authored by aklapper on Sat, Jun 14, 22:10.

Details

Reviewers

mainframe98
valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T16110: Cannot change email address of mailing list account

Summary

rP04b9f946020de32c513b69954933630641e8ff8f introduced isEditableByAdministrators() in src/applications/settings/panel/PhabricatorSettingsPanel.php.
In rP421bf2e548f06411a2ce86583333ec5e3e92c643, isEditableByAdministrators() was renamed to isManagementPanel().
Thus the addition of isEditableByAdministrators() to src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php in rPe0a3e6ba3b87f96c83905e7188bb6161f7480784 had no effect - that function has no callers anywhere in the codebase.
Fix that by using the correct function name.

Closes T16110

Test Plan

Go to http://phorge.localhost/settings/user/SomeMailingListUser/, see new item "Email Addresses" in the panel on the left.
Go to http://phorge.localhost/settings/user/SomeMailingListUser/page/email/ directly, don't get a 404 anymore.

Diff Detail

Repository

rP Phorge

Branch

T16110userEmailList (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 2108
Build 2108: arc lint + arc unit

Event Timeline

aklapper created this revision.Sat, Jun 14, 22:10

Owners added a reviewer: O1: Blessed Committers.Sat, Jun 14, 22:10

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptSat, Jun 14, 22:10

aklapper requested review of this revision.Sat, Jun 14, 22:10

Harbormaster completed remote builds in B2108: Diff 3234.Sat, Jun 14, 22:10

aklapper edited the summary of this revision. (Show Details)Sat, Jun 14, 22:12

aklapper edited the test plan for this revision. (Show Details)

aklapper edited the summary of this revision. (Show Details)Sat, Jun 14, 22:20

mainframe98 mentioned this in T16111: Cannot change email address of bot account.Sun, Jun 15, 08:17

Tested as part of D26090, which should land after this but I couldn't find a way to stack then without arc wanting to steal this diff.

This revision is now accepted and ready to land.Sun, Jun 15, 08:35

Maybe nice to expand the test plan to cover non-admin and admin

In D26087#28967, @valerio.bozzolan wrote:

Maybe nice to expand the test plan to cover non-admin and admin

@valerio.bozzolan: Hmm, same before and after that change as this doesn't influence behavior for non-admins (as a non-admin you cannot edit settings of other users. All related options on http://phorge.localhost/people/manage/1/ are greyed out and going to http://phorge.localhost/settings/user/SomeMailingListUser/ gives an "Access Denied"). Not sure that's worth to list?

Exactly that is worth to list 👍 knowing that non-admins cannot manage this is enough for me for my appppproval.

Revision Contents
Changeset List

Path

Size

Coverage (All)

Coverage (Touched)

Packages

src/

applications/

settings/

panel/

PhabricatorEmailAddressesSettingsPanel.php

2 lines

O1: Blessed Committers

Diff 3234

View Options

src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php

Allow admins to edit email addresses of mailing list accounts againAcceptedPublicActions