This adds support for custom fields in diffusion property views.
Use case: Used by Wikimedia to parse the gerrit change-id field from
commits and display a link to gerrit in the commit property view.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers
I tested as follows
- Installed https://secure.phabricator.com/P1819 (dropped the file into phabricator/extensions/)
- viewed a commit with a Change-Id: field in the commit message.
- Saw the Change-Id: in the property view, complete with a link to gerrit.
Note: P1819 is pretty messy but the code in this change should be
pretty solid. Some of it was lifted from the differential conduit api.
Diff Detail
Diff Detail
- Branch
- redesign-2015-wmf
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 7184 Build 7413: [Placeholder Plan] Wait for 30 Seconds Build 7412: arc lint + arc unit
Event Timeline
Comment Actions
Details inline, but I think the shortest path to getting what you want here is:
- Do vanilla field rendering in Diffusion (appendFieldsToPropertyList(...)).
- Have your field reach into the commit object when reading from storage (or just when rendering) and just parse out Change-Id. e.g., your renderValueForPropertyDetailView() or whatever method can just do $commit->getData()->getCommitMessage(), parse that, then throw most if away and just use your field.
- Some day, we'll have a real parse phase in MessageParserWorker and you can parse + store there, which will enable search/sort, etc (these aren't too useful for this field, anyway).
| src/applications/diffusion/controller/DiffusionCommitController.php | ||
|---|---|---|
| 1087 | Can we typehint $commit and $data? They're PhabricatorRepositoryCommit and PhabricatorRepositoryCommitData, I think? | |
| 1092 | This needs to use the actual viewer (to respect policies), not the omnipotent viewer. You can get it with $this->getViewer(). | |
| 1095 | I'd expect that the rest of this method is basically just: $field_list->appendFieldsToPropertyList(...); (Although the caller would need to be rearranged a bit.) That won't work in your use case (where you want to read directly from the commit message), but I think your use case should be supported by a new "read commit message into fields" phase during commit parsing. For example, it would be desirable for Diffusion to let you search for commits with a given "Change-Id", but this won't work (and can never work) if we re-parse the message at display time, since we won't have any database value that we can run a WHERE customfield_123_value = 'some-change-id' query against. Instead, we'd need to parse the message at import time and write the fields to database storage (so they become searchable), then read them from storage here. I think adding this phase is conceptually fine, but likely quite complicated to develop/test. An alternative would be to make your custom field fake its read-from-storage and have it go fish around in the commit message instead. This isn't "right" -- and won't work with the "search" use case -- but should work fine until we have an import phase. | |