Page MenuHomePhabricator

Add project tags to workboard cards
ClosedPublic

Authored by chad on Jan 3 2016, 11:12 AM.
Tags
None
Referenced Files
F14064091: D14935.diff
Mon, Nov 18, 9:46 PM
F14047536: D14935.diff
Thu, Nov 14, 4:20 AM
F14045658: D14935.id36078.diff
Wed, Nov 13, 7:57 AM
F14009610: D14935.id36561.diff
Wed, Oct 30, 9:01 PM
F14005062: D14935.diff
Sun, Oct 27, 6:52 AM
F14002922: D14935.id.diff
Sat, Oct 26, 12:03 AM
F13997109: D14935.id36563.diff
Thu, Oct 24, 2:10 AM
F13997108: D14935.id36561.diff
Thu, Oct 24, 2:10 AM
Tokens
"Like" token, awarded by avivey."Like" token, awarded by Luke081515.2."Like" token, awarded by joshuaspence."Like" token, awarded by johnny-bit.

Details

Summary

Ref T4863. Add project tags to workboard cards.

Test Plan

Test Board.png (227×378 px, 22 KB)

Diff Detail

Repository
rP Phabricator
Branch
arcpatch-D14935
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 10399
Build 12697: Run Core Tests
Build 12696: arc lint + arc unit

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 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());
unset($project_phids[$this->project->getPHID()]);

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

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

src/applications/project/view/ProjectBoardTaskCard.php
87

Specifically, this will issue a query.

This revision now requires changes to proceed.Jan 9 2016, 7:52 PM
chad edited reviewers, added: joshuaspence; removed: chad.
chad edited edge metadata.
  • Updates per comments.

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.

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 edited edge metadata.
  • fix reorder column fatal
  • Editing still fatals

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

src/applications/phid/view/PHUIHandleTagListView.php
42

could this go up a level to AphrontTagView?

src/applications/maniphest/editor/ManiphestEditEngine.php
337–338 ↗(On Diff #36562)

I think the fix is just:

->setProject($column->getProject())

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.

src/applications/phid/view/PHUIHandleTagListView.php
42

Yeah, I think that's reasonable.

chad marked 2 inline comments as done.
  • ccchanges
epriestley edited edge metadata.
This revision is now accepted and ready to land.Jan 31 2016, 9:33 PM
src/applications/project/controller/PhabricatorProjectMoveController.php
29 ↗(On Diff #36578)

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

src/applications/project/controller/PhabricatorProjectMoveController.php
29 ↗(On Diff #36578)

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.

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

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