Page MenuHomePhorge

Add (Advanced) Custom Fields to Item List
AcceptedPublic

Authored by avivey on Mar 1 2024, 11:03.
Tags
None
Referenced Files
Unknown Object (File)
Sat, May 4, 18:30
Unknown Object (File)
Sat, May 4, 14:22
Unknown Object (File)
Sat, May 4, 14:22
Unknown Object (File)
Sat, May 4, 14:22
Unknown Object (File)
Sat, May 4, 13:24
Unknown Object (File)
Sat, May 4, 12:30
Unknown Object (File)
Wed, May 1, 18:47
Unknown Object (File)
Wed, May 1, 16:12

Details

Summary

Allow PHP-coded Custom Fields to show things in Lists.

Also add Repository to Revision List and Flags to Maniphest lists.

Closes T15133. Ref T15512, T15750

Test Plan

Look at Repository List and Task lists that have flags.

Diff Detail

Repository
rP Phorge
Branch
arcpatch-D25548
Lint
Lint Passed
SeverityLocationCodeMessage
Advicesrc/applications/search/engine/PhabricatorApplicationSearchEngine.php:1647XHP16TODO Comment
Unit
Test Failures
Build Status
Buildable 1198
Build 1198: arc lint + arc unit

Unit TestsFailed

TimeTest
391 msPhabricatorLibraryTestCase::testEverythingImplemented
EXCEPTION (Exception): Source file "/home/avivey/devtools/phorge/src/applications/maniphest/capability/ManiphestDefaultPriorityEditCapability.php" failed to load. #0 /home/avivey/devtools/arcanist/src/init/lib/PhutilBootloader.php(211): PhutilBootloader->executeInclude('...') #1 /home/avivey/devtools/arcanist/src/symbols/PhutilSymbolLoader.php(429): PhutilBootloader->loadLibrarySource('...', '...')
289 msPhabricatorLibraryTestCase::testLibraryMap
Assertion failed, expected 'true' (at PhutilLibraryTestCase.php:45): The library map is out of date. Rebuild it with `arc liberate`. These entries differ: class.ManiphestDefaultPriorityEditCapability, xmap.ManiphestDefaultPriorityEditCapability.
223 msPhabricatorLibraryTestCase::testMethodVisibility
EXCEPTION (Exception): Source file "/home/avivey/devtools/phorge/src/applications/maniphest/capability/ManiphestDefaultPriorityEditCapability.php" failed to load. #0 /home/avivey/devtools/arcanist/src/init/lib/PhutilBootloader.php(211): PhutilBootloader->executeInclude('...') #1 /home/avivey/devtools/arcanist/src/symbols/PhutilSymbolLoader.php(429): PhutilBootloader->loadLibrarySource('...', '...')
2 msDifferentialBranchFieldTestCase::testRenderDiffPropertyViewValue
1 assertion passed.
93 msDifferentialParseRenderTestCase::testParseRender
18 assertions passed.
View Full Test Results (3 Failed · 7 Passed)

Event Timeline

avivey held this revision as a draft.
src/applications/maniphest/query/ManiphestTaskSearchEngine.php
396–412

Moving this here is a small improvement, but in reality the "$handles" array used
in this flow is inferior to just using $viewer->renderHandle(), and
should be replace with that.

Before this change, we'd actually load all the labels twice using the old, uncached mechanism.

avivey published this revision for review.Mar 1 2024, 11:39
aklapper subscribed.

IIUC this adds Flags in Differential, Maniphest, etc list views, and adds the repository in Differential list view?

Functionality wise this looks great and works as expected for me:

Screenshot from 2024-04-01 19-27-51.png (970×948 px, 98 KB)

Screenshot from 2024-04-01 19-25-22.png (526×948 px, 107 KB)

Use Results > Export Data > CSV in both Differential and Maniphest query result lists does not include a repository column (that is fine) and is still valid CSV.

Wondering if rendered flags should be links to http://phorge.localhost/flag/ but that would be a separate request anyway.

Giving a +1 on personal capacity; I don't really feel knowledgeable enough to give a +1 as O1 (yet).

(Flags only show up in Maniphest on this one; I'll add them to more apps in a layer diff)

I'm shocked to see the flag on the revisions list. Turns out it's already a thing (on master!). Did not know about it, I'll have to look for it in the code.

Thanks!

Traced the Flags in Revision back to its origin in https://secure.phabricator.com/T1557. I knew about this one - but I was 100% sure that the feature was lost when the SearchEngine classes were introduced.

I see "Unknown object" if the Diff repository is unset. Is that known?

List with no repo.png (191×478 px, 17 KB)

To reproduce, go to a Diff, and manually unset the repo.

src/applications/differential/customfield/DifferentialRepositoryField.php
72

Probably this should be wrapped in if ($this->getValue()) {

  • check for non-null repository in Revision
  • rebase

I see "Unknown object" if the Diff repository is unset. Is that known?

Did not check that one; Updated now.

whatcouldgowrong

P.S.

I cannot reproduce the above arc unit issues, locally.

This revision is now accepted and ready to land.Mon, Apr 29, 06:31