This code is mostly lifted from the PhabricatorProjectCreateController.
Details
- Reviewers
epriestley • chasemp - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T5691: Allow creating projects via API
- Commits
- Restricted Diffusion Commit
rP5f82705e1234: First stab at a conduit method for creating projects.
currently untested
Diff Detail
- Repository
- rP Phabricator
- Branch
- D10036
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 2037 Build 2038: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
This is basically fine. Some general inlines.
src/applications/project/conduit/ConduitAPI_project_create_Method.php | ||
---|---|---|
3–5 ↗ | (On Diff #24123) | You can drop these @group comments now (I think we removed them all recently from the rest of the codebase). They're set by config instead, now. |
6 ↗ | (On Diff #24123) | We're about to rename all the Conduit stuff to be more consistent, see D9991. |
8 ↗ | (On Diff #24123) | After D9991, you'll need to explicitly specify that the method name is 'project.create'. |
9 ↗ | (On Diff #24123) | pht() |
24–26 ↗ | (On Diff #24123) | Just drop this, I want to move Conduit more toward handling exceptions in a reasonable way rather than defining a custom exception type for each exception. In practice, no one cares about or handles these special cases, and we can implement them more simply/generically at a higher level anyway. |
33–34 ↗ | (On Diff #24123) | ConduitMethod might not have this method, you can add it if that's the case. |
38–48 ↗ | (On Diff #24123) | This can be simplified a bit, just ->setNewValue($request->getValue('name')) without lines 38..44 |
54 ↗ | (On Diff #24123) | This should be unnecessary. |
56–62 ↗ | (On Diff #24123) | Just do: '+' => array_fuse($members) ...instead of looping. That is, generate one transaction which sets all of the members. |
68 ↗ | (On Diff #24123) | I think this only accepts AphrontRequest, not ConduitRequest. |
69–74 ↗ | (On Diff #24123) | Just let this exception escape. |
- Rebased on top of the changes from D9991
- Addressed @epriestley's feedback
- implemented ConduitAPIMethod::requireApplicationCapability
A little minor cleanup, one behavioral issue. This looks good otherwise.
src/applications/project/conduit/ProjectCreateConduitAPIMethod.php | ||
---|---|---|
3–5 | Nuke | |
13 | Wrap this in pht() so it can be translated. | |
41 | Unused. | |
43 | Unused. | |
51 | Adding the user is a surprising behavior to me for an API method. Do we need this for something to work? If so, maybe we should fix that. If not, let's just omit it? This is consistent with how the web workflow works, but it feels odd to me in an API context. I think it's reasonable to expect API callers to include themselves if they want to be project members, and also reasonable that bot/script project creation may not want to make projectbot a member of every project. |