Allow "Change Subtype" to be selected from the comment action stack
ClosedPublic
Actions

Authored by epriestley on Nov 27 2018, 12:27 AM.

Details

Reviewers

amckinley

Maniphest Tasks

T13222: 2018 Week 48-51 Bonus Content

Commits

rP2f11001f6e3c: Allow "Change Subtype" to be selected from the comment action stack

Summary

Ref T13222. See PHI683. Currently, you can "Change subtype..." via Conduit and the bulk editor, but not via the comment action stack or edit forms.

In PHI683 an install is doing this often enough that they'd like it to become a first-class action. I've generally been cautious about pushing this action to become a first-class action (there are some inevitable rough edges and I don't want to add too much complexity if there isn't a use case for it) but since we have evidence that users would find it useful and nothing has exploded yet, I'm comfortable taking another step forward.

Currently, EditEngine has this sort of weird setIsConduitOnly() method. This actually means more like "this doesn't show up on forms". Make it better align with that. In particular, a "conduit only" field can already show up in the bulk editor, which is goofy. Change this to setIsFormField() and convert/simplify existing callsites.

Test Plan

There are a lot of ways to reach EditEngine so this probably isn't entirely exhaustive, but I think I got pretty much anything which is likely to break:

Searched for setIsConduitOnly() and getIsConduitOnly(), converted all callsites to setIsFormField().
Searched for setIsLockable(), setIsReorderable() and setIsDefaultable() and aligned these calls to intent where applicable.
Created an Almanac binding.
Edited an Almanac binding.
Created an Almanac service.
Edited an Almanac service.
Edited a binding property.
Deleted a binding property.
Created and edited a badge.
Awarded and revoked a badge.
Created and edited an event.
Made an event recurring.
Created and edited a Conpherence thread.
Edited and updated the diff for a revision.
Created and edited a repository.
Created and disabled repository URIs.
Created and edited a blueprint.
Created and edited tasks.
Created a paste, edited/archived a paste.
Created/edited/archived a package.
Created/edited a project.
Made comments.
Moved tasks on workboards via comment action stack.
Changed task subtype via comment action stack.

Diff Detail

Repository

rP Phabricator

Branch

comment1

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 21229
Build 28875: Run Core Tests
Build 28874: arc lint + arc unit

Event Timeline

epriestley created this revision.Nov 27 2018, 12:27 AM

Owners added a subscriber: Restricted Owners Package.Nov 27 2018, 12:27 AM

Harbormaster completed remote builds in B21227: Diff 47378.Nov 27 2018, 12:28 AM

epriestley requested review of this revision.Nov 27 2018, 12:28 AM

Needs more thorough testing.

Actually tested, with one behavioral fix for "Initial Members" in Projects.

Harbormaster completed remote builds in B21229: Diff 47380.Nov 27 2018, 12:59 PM

amckinley accepted this revision.Nov 28 2018, 6:56 PM

This revision is now accepted and ready to land.Nov 28 2018, 6:56 PM

Closed by commit rP2f11001f6e3c: Allow "Change Subtype" to be selected from the comment action stack (authored by epriestley). · Explain WhyNov 28 2018, 9:40 PM

This revision was automatically updated to reflect the committed changes.

epriestley mentioned this in D19845: Address a transaction issue with some audit actions not applying correctly.Dec 5 2018, 1:57 PM

epriestley marked an inline comment as done.Dec 5 2018, 2:01 PM

epriestley added inline comments.

src/applications/transactions/editfield/PhabricatorEditField.php
888	This is the change which exposed the bug identified and fixed in D19845. I believe the old behavior was not sufficiently strict (it should have checked both this AND "conduit only").