Page MenuHomePhabricator

Support Custom Fields in List View
Needs ReviewPublic

Authored by avivey on Dec 7 2016, 12:54 AM.

Details

Summary

Ref T418.
This adds values list, list.icon, list.label as described in T418#55775, and supports them in
Maniphest and in Differential.

Also modernized a bit the code that I actually went through wrt handles.

P2022 is an example of a custom Differential field that shows on the list.

Test Plan

Camera icon comes from "f1" field (Type text), and the orange USB is a boolean field.

Diff Detail

Repository
rP Phabricator
Branch
arcpatch-D17004
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 15279
Build 20101: Run Core Tests
Build 20100: arc lint + arc unit

Event Timeline

avivey updated this revision to Diff 40908.Dec 7 2016, 12:54 AM
avivey retitled this revision from to Support Custom Fields in List View.
avivey updated this object.
avivey edited the test plan for this revision. (Show Details)
avivey added reviewers: epriestley, chad.
chad edited edge metadata.Dec 7 2016, 12:55 AM

test plan seems dubious

avivey edited the test plan for this revision. (Show Details)Dec 7 2016, 1:00 AM
avivey edited edge metadata.
avivey updated this revision to Diff 40909.Dec 7 2016, 1:05 AM
  • Remove a blank line
  • Allow users to shoot themselves in foot
avivey added inline comments.Dec 9 2016, 7:58 PM
src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldRemarkup.php
37–39

Oh, using this on the list page might have a few more queries...

avivey planned changes to this revision.Dec 20 2016, 1:52 AM
avivey added inline comments.
src/infrastructure/customfield/field/PhabricatorCustomField.php
1313

getRequiredHandlePHIDsForListView

src/infrastructure/customfield/field/PhabricatorCustomFieldList.php
211–215

this can be one loop

src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php
321

should be called getValueForListItem.

avivey updated this revision to Diff 41125.Dec 20 2016, 7:14 PM
avivey marked 3 inline comments as done.
  • some minor updates
epriestley requested changes to this revision.Jan 11 2017, 3:27 PM

I believe enabling this will add ~100-200 queries to the page and it's basically a nonstarter until we have a fix for that.

This revision now requires changes to proceed.Jan 11 2017, 3:27 PM
kfa added a subscriber: kfa.Jan 14 2017, 6:37 AM
avivey updated this revision to Diff 41390.Jan 16 2017, 7:13 PM
avivey edited edge metadata.
  • use batch-loading system from D16351 to load fields.
  • update documentation.
  • hard-code fa- prefix for icons.
avivey added a comment.May 4 2017, 8:30 PM

@epriestley: I think this one should actually be in Request Review state?

This revision now requires changes to proceed.May 4 2017, 8:30 PM
epriestley requested changes to this revision.May 4 2017, 9:29 PM

Do "Request Review" again? I think this got stuck in a questionable state during a brief bug with T11114, but kicking it back and forth should clear things.

avivey requested review of this revision.May 4 2017, 9:30 PM
avivey edited edge metadata.