Paths

Table of Contentst

Differential D7394

Enable prefilling of projects and priority fields in Maniphest task creation
ClosedPublic
Actions

Authored by erik.fercak on Oct 25 2013, 9:47 AM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Commits

Restricted Diffusion Commit
rP1fb2f39fc0f6: Enable prefilling of projects and priority fields in Maniphest task creation

Summary

Projects and priority inputs can be prefilled similar to how title
and description fields work.

Prefilling of projects already worked but used PHIDs instead of
more user friendly name so I changed that too.

Test Plan

Visit example
and see prefilled form fields.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

This looks good. See inline note. Pseudocode for the full-strength solution:

$tokens = explode(...);
$slug_map = id(new PhabricatorProjectQuery())->...->execute();
$name_map = id(new PhabricatorProjectQuery())->...->execute();
$phid_map = id(new PhabricatorProjectQuery())->...->execute();
$all_map = mpull($slug_map, null, 'getPhrictionSlug') +
           mpull($name_map, null, 'getName') +
           mpull($phid_map, null, 'getPHID');
foreach ($tokens as $token) {
  if (isset($all_map[$token])) {
    // use $all_map[$token]->getPHID()
  } else {
    // discard token
  }
}

src/applications/maniphest/controller/ManiphestTaskEditController.php
54–56	This should use `PhabricatorProjectQuery`. If you use loadAllWhere(), it bypasses policy controls. Ideally, I think this algorithm should be: Split the string; try to load projects using the tokens as slugs, with withPhrictionSlugs() (see T4021), this will let "qa" work; try to load projects using the tokens as names, with withNames(), this will let "Quality Assurance" work; try to load projects using the tokens as PHIDs, with withPHIDs(), this will let PHID-PROJ-asdfa98fd work; for each token, check for a matching slug, or a matching name, or a matching PHID, in order. If none match, discard it. This is a bit more involved than the proposed implementation. We don't need to go that far if you aren't feeling ambitious, but I do think we should avoid breaking existing uses of PHIDs. Allowing PHIDs is sometimes useful for scripts and external systems, too, since they may be able to construct a URI with PHIDs but not quickly or easily look up the correct project name.

Make projects prefilling policy aware.

One minor inline, I'll just clean that up in the pull. Thanks!

src/applications/maniphest/controller/ManiphestTaskEditController.php
58–59	You can safely omit these, CAN_VIEW is the default.

Phriction slugs are stored in DB with trailing slash so you must use "qa/" instead of "qa".
Also I could add PHID support to other inputs, notably assignee, while I'm at it.

Yeah -- I think we'll clean up that "/" thing when T4021 happens, I imagine we'll just replace the idea of "PhrictionSlug" with "Slug" and drop the "/". I think it's cleaner to leave it like this for now and clean it up globally later than introduce a bunch of rtrim() / "/" gymnastics that might get overlooked.

Allowing users to be addressed by PHID seems reasonable if you want to do a followup.

Closed by commit rP1fb2f39fc0f6 (authored by @erik.fercak, committed by @epriestley).

Revision Contents
Changeset List

Path

Size

src/

applications/

maniphest/

controller/

ManiphestTaskEditController.php

46 lines

project/

query/

PhabricatorProjectQuery.php

13 lines

Diff 16667

View Options

src/applications/maniphest/controller/ManiphestTaskEditController.php