Allow admins to edit email addresses of mailing list accounts again
ClosedPublic
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

Commits

rP0bd19d8ed977: Allow admins to edit email addresses of mailing list accounts again

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

As an admin, go to http://phorge.localhost/settings/user/SomeMailingListUser/, see new item "Email Addresses" in the panel on the left.
As an admin, go to http://phorge.localhost/settings/user/SomeMailingListUser/page/email/ directly, don't get a 404 anymore.
As a non-admin, go to http://phorge.localhost/settings/user/SomeMailingListUser/page/email/ and get a 404 as before.
As an admin, on http://phorge.localhost/settings/user/SomeMailingListUser/page/email/ directly, add a new email address, mark it as primary, be able to remove the old email address. Check the phabricator_user.user_email DB table that the account's email address has been correctly updated.

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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?