Fixes T10673. Set up Phurl to use Edit Engine. There's no way this is all I needed to do to get it working, so I'll be making another pass at it and testing more thoroughly...
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T10673: Convert Phurl to use EditEngine
- Commits
- rP0817eb14a94b: Update Phurl to use EditEngine
Ran through the Phurl URL creation/edit/deletion process.
Diff Detail
- Repository
- rP Phabricator
- Branch
- PhurlEditEngine (branched from master)
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 13772 Build 17790: Run Core Tests Build 17789: arc lint + arc unit
Event Timeline
Edit engine is deceitfully magical, and you may have indeed done everything you needed to here.
This looks correct to me.
In this diff, you should change PhabricatorPhurlController->buildApplicationCrumbs() to use EditEngine to generate the "Create Thing" link. You can follow ManiphestController->buildApplicationCrumbs(). Without this change, the "Create URL" button won't automatically update to reflect the availability of multiple forms if users later define them.
In this diff, you should also change PhabricatorPhurlApplication->getRoutes() to use $this->getEditRoutePattern('edit/') to generate the edit route, instead of hard-coding edit/. You can look at PhabricatorPasteApplication for an example. You should be able to remove the create/ route after making this change. (It looks like Paste also has a defunct create/ route which could be safely removed). Without this change, stuff lik Edit URL → Configure Form → HTTP Parameters won't work properly.
In this or a future diff, you should change PhabricatorPhurlURLViewController to use EditEngine for commenting. D16222 has some extra stuff, but the changes to PhamePostViewController->buildCommentForm() in that change are generally reasonable as a model. Or just look at PhabricatorPasteViewController for a possibly-simpler example. After making this change, delete PhabricatorPhurlURLCommentController and the route for it (these will be fully handled by EditEngine, and no longer need a separate route). This change will enable the "Change Subscribers" and "Change Project Tags" actions when adding a comment, improve the draft code, and reduce the total amount of code in the codebase.
src/applications/phurl/editor/PhabricatorPhurlURLEditEngine.php | ||
---|---|---|
57 | I changed this from getApplicationURI('edit/') to getApplicationURI('url/edit/') to get the edit+create links all working. Is my thinking correct here or should it be using a similar mechanism that the routes use rather than hard-coding this? |
src/applications/phurl/editor/PhabricatorPhurlURLEditEngine.php | ||
---|---|---|
57 | This is fine, some applications have /app/edit/ and others have /app/thing/edit/. Mostly depends on app complexity / which app got copy-pasted to create a new app. In theory we should probably always have it for consistency, but some apps like Paste are very unlikely to have multiple/secondary objects. |