Page MenuHomePhabricator

Add project tags to workboard cards

Authored by chad on Jan 3 2016, 11:12 AM.
"Like" token, awarded by avivey."Like" token, awarded by Luke081515.2."Like" token, awarded by joshuaspence."Like" token, awarded by johnny-bit.



Ref T4863. Add project tags to workboard cards.

Test Plan

Diff Detail

rP Phabricator
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

joshuaspence retitled this revision from to Add project tags to workboard cards.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added reviewers: epriestley, chad.
epriestley requested changes to this revision.Jan 9 2016, 7:52 PM
epriestley edited edge metadata.

This is inefficient because it does fetches per-card. If you load a board with a large number of cards that have projects, I expect you'll see roughly one query per unique project.

The simplest fix is probably to let PHUIHandleTagListView take a PhabricatorHandleList instead of only array $handles (it's generally sufficient to just remove the array typehint).

You can't unset() a handle list, so the new logic would look like:

$project_phids = array_fuse($task->getProjectPHIDs());

$handle_list = $viewer->loadHandles($project_phids);

That should bulk-fetch all the handles at render time in a single query.


Specifically, this will issue a query.

This revision now requires changes to proceed.Jan 9 2016, 7:52 PM
chad commandeered this revision.Jan 29 2016, 9:21 PM
chad edited reviewers, added: joshuaspence; removed: chad.
chad updated this revision to Diff 36560.Jan 29 2016, 9:50 PM
chad edited edge metadata.
  • Updates per comments.
chad added a comment.Jan 29 2016, 9:50 PM

I have this roughly working, but get a 500 whenever I edit a task. I think I need to attach Projects somewhere, but can't trace the query.

chad added a comment.Jan 29 2016, 10:00 PM
6	phabricator	applications/maniphest/storage/ManiphestTask.php : 157	PhabricatorLiskDAO::assertAttached()
5	phabricator	applications/project/view/ProjectBoardTaskCard.php : 86	ManiphestTask::getProjectPHIDs()
4	phabricator	applications/project/controller/PhabricatorProjectMoveController.php : 158	ProjectBoardTaskCard::getItem()
3	phabricator	aphront/configuration/AphrontApplicationConfiguration.php : 237	PhabricatorProjectMoveController::handleRequest()
2	phabricator	aphront/configuration/AphrontApplicationConfiguration.php : 149	AphrontApplicationConfiguration::processRequest()
1		/Users/familyroom/Sites/phabricator/webroot/index.php : 17	AphrontApplicationConfiguration::runHTTPRequest()
chad updated this revision to Diff 36561.Jan 29 2016, 10:43 PM
chad edited edge metadata.
  • fix reorder column fatal
  • Editing still fatals
chad updated this revision to Diff 36562.Jan 29 2016, 11:08 PM
  • halps here pls
chad added a comment.Jan 29 2016, 11:08 PM

Ok, I think this requires slight modification to EditEngine and not positive how to proceed. But I annotated the issue.


could this go up a level to AphrontTagView?

epriestley added inline comments.Jan 30 2016, 12:53 AM

I think the fix is just:


Basically, we can sort of cheat here since we have the $column, and each $column is associated with a project, so we can figure out which project the user must be looking at by figuring out which project the column belongs to.


Yeah, I think that's reasonable.

chad updated this revision to Diff 36563.Jan 30 2016, 2:46 AM
chad marked 2 inline comments as done.
  • ccchanges
chad updated this revision to Diff 36578.Jan 31 2016, 8:41 PM
  • rebase
epriestley accepted this revision.Jan 31 2016, 9:33 PM
epriestley edited edge metadata.
This revision is now accepted and ready to land.Jan 31 2016, 9:33 PM
chad added inline comments.Jan 31 2016, 9:39 PM

This change seemed correct to me, but unclear why ObjectQuery was used to begin with?

epriestley added inline comments.Jan 31 2016, 9:41 PM

The code currently tries to be generic when possible to make possibly putting other types of objects (mocks, events, whatever) on workboards some day easier, but it's fine to just hard-code this to tasks for now.

This revision was automatically updated to reflect the committed changes.
chad added a comment.EditedJan 31 2016, 9:44 PM

ah I see. welp, want to roll the server and see how bad this looks?

chad added a comment.Jan 31 2016, 9:49 PM

Surprisingly not terrible and somewhat useful with bugs and features. "Personal" projects (companies) might get old though.

avivey awarded a token.Feb 1 2016, 2:20 AM