Page MenuHomePhabricator

Add project handles in workboards
Needs RevisionPublic

Authored by benweissmann on Nov 7 2015, 11:10 PM.

Details

Reviewers
epriestley
chad
Group Reviewers
Blessed Reviewers
Summary

Adds project handles to task cards on workboards, as discussed in T4863.

This also adds a new option, projects.projects-in-workboards, which enables/disables
this feature. As discussed in T4863, showing projects is the default.

The existing behavior for showing multiple attributes on cards (e.g. both the owner
and the projects) was to show them on the same line with a middot between. I
though this looked bad with the new project handles, so I also modified the CSS
for workboards to break them into separate lines. It looks like this:
https://i.imgur.com/UZFEOaX.png

Test Plan

I've dont manual testing on a local install. I've tested:

  • A workboard that contains some tasks that belong to other projects, some tasks that do not, and some that contain multiple other projects.
  • Viewing this workboard with project handles enabled and disable (in config)
  • Moving tasks between columns
  • Creating new tasks from the workboard
  • Editing tasks on the workboard
  • Clicking through from the project handles to the projects they link to

Diff Detail

Repository
rP Phabricator
Branch
bsw.projects-on-workboard
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 8691
Build 10081: arc lint + arc unit

Event Timeline

benweissmann retitled this revision from to Add project handles in workboards.
benweissmann updated this object.
benweissmann edited the test plan for this revision. (Show Details)
benweissmann added a reviewer: epriestley.
chad added inline comments.Nov 7 2015, 11:12 PM
src/applications/project/config/PhabricatorProjectConfigOptions.php
48 ↗(On Diff #34880)

No new options, please.

chad edited edge metadata.Nov 7 2015, 11:17 PM

Always show should just be the default.

benweissmann edited edge metadata.
  • Remove option projects.projects-in-workboards
epriestley requested changes to this revision.Nov 8 2015, 2:09 PM
epriestley edited edge metadata.

I don't really want to upstream this, since we'll have to undo the work within the next two months if things go well. Some particular generality issues:

  • This code assumes workboards belong to a single project, but projects are likely to have subprojects (T3670) soon.
  • This code assumes a workboard belongs to a project whatsoever, but this may not be true soon (T5793).
  • This code assumes all things on a workboard are tasks, which may not be true in the future (although probably not soon). The existing code partially encodes this assumption, but this change strengthens it.
  • This code generally makes display less modular, when we plan to make display fully modular soon (T4863).
  • This code does not use modern HandlePool methods (T7689 -- but using them may require nontrivial work).

This code is fine to run locally. There's nothing materially wrong with it, it's just in conflict with our plans and will make it more difficult to make the above changes.

src/applications/project/controller/PhabricatorProjectMoveController.php
164

We target PHP 5.2.3 and newer, so you can not use short [...] array syntax in this codebase.

This revision now requires changes to proceed.Nov 8 2015, 2:09 PM

Thanks for the feedback, epriestley.

Is this a change you'd be willing to upstream if I make some modifications to address your concerns? In particular:

  • change the setProject method in ProjectBoardTaskCard to setWorkboardColumn, and add a hasProject() method to PhabricatorProjectColumn. Then, we'd only filter the current board's project out of the handles if the card's column has a project, which would address your first two concerns.
  • Use the HandlePool methods
  • add a setProjectPHIDs method to ProjectBoardTaskCard that operates similarly to setOwner -- it can optionally be called to show project handles. That should make it easier to modify ProjectBoardTaskCard in the future to operate on things other than tasks, because we'll no longer need to call getProjectPHIDs on the task. The logic of calling this method could be moved to the controller, close to where the other assumptions about project board items being tasks are (and, in the case of PhabricatorProjectMoveController, conditionally call it only if the object we're operating on is a task).

I like it! Looking forward to the upstream implementation.