MetaMTA - add support for "Default Author"
ClosedPublic
Actions

Authored by btrahan on Jan 20 2015, 8:45 PM.

Details

Reviewers

epriestley

Maniphest Tasks

T5952: Move object creation email address configuration to applications

Commits

Restricted Diffusion Commit
rP799dada3ad09: MetaMTA - add support for "Default Author"

Summary

Ref T5952. This adds support for a "default author" and deploys it on Maniphest.

Test Plan

used augmented (by this diff) bin/mail receive-test to test creation via an application email with a default author configured and no author specified. a task was created with the author as the default author i configured.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

btrahan updated this revision to Diff 27514.Jan 20 2015, 8:45 PM

btrahan added a task: T5952: Move object creation email address configuration to applications.

btrahan retitled this revision from to MetaMTA - add support for "Default Author".

btrahan updated this object.

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

btrahan added a reviewer: epriestley.

Herald added subscribers: epriestley, Korvin. · View Herald TranscriptJan 20 2015, 8:45 PM

btrahan added inline comments.Jan 20 2015, 8:47 PM

src/applications/maniphest/mail/ManiphestCreateMailReceiver.php
21	I couldn't figure out an easy way to stick this logic in the base class without rewriting a bunch. I figure this side effect thing is pretty bad but maybe okay. Maybe the best path forward will suss itself out later as I add more applications to this new way of doing things? Open for suggestions now of course.
src/applications/metamta/storage/PhabricatorMetaMTAApplicationEmail.php
13	I will change this constant to CONFIG_DEFAULT_AUTHOR

btrahan added inline comments.Jan 20 2015, 8:48 PM

src/applications/meta/controller/PhabricatorApplicationEditEmailController.php
285–286	whoops, should use $title here

Okay, done dissecting this thing myself now. :D

resources/sql/autopatches/20150120.maniphestdefaultauthor.php
2–22	Forgot to deprecate this option in the UI...

fixed stuff from self review, except didn't come up with a better way to do the setApplicationEmail thing

The setApplicationEmail() stuff seems reasonable to me.

I think we can probably remove this entirely (it should be completely obsoleted by phabricator.allow-email-users already?) but that would (a) break installs and (b) I'm not confident the phabricator.allow-email-users stuff actually works 100%, so retaining it for now make sense. We should be able to throw this all away eventually.