See title. Fixes T1809.
Details
- Reviewers
epriestley - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- Restricted Diffusion Commit
rPda845460586c: Add filter by object ability to flag query
verified each type that has flaggable interface still can be flagged
verified that new custom query filter works
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
This looks great to me, see inline for discussion.
src/applications/flag/interface/PhabricatorFlaggableInterface.php | ||
---|---|---|
5 | I think we can safely put getPHID() here now. Flags requires it and LiskDAO should always provide it. | |
src/applications/flag/query/PhabricatorFlagSearchEngine.php | ||
104 | This is kind of funky. The idea is that we want to render a dropdown with human-readable names, like "Task" instead of "ManiphestTask". PHIDType already has these strings, so we can figure this out like: $all_types = PhabricatorPHIDType::getAllTypes(); foreach ($objects as $object) { $phid = $object->generatePHID(); $phid_type = phid_get_type($phid); $type_object = idx($all_types, $phid_type); if ($type_object) { $human_readable_name = $type_object->getTypeName(); } } This will give us nice names -- "Task", "Revision", etc. The generatePHID() part is a little funky. Possibly we should do this instead, in LiskDAO or wherever the base for generatePHID() lives: public function getPHIDType() { return null; } public function generatePHID() { // return null if no phid type, or: return PhabricatorPHID::generatePHID($this->getPHIDType()); } ...or whatever (I don't have a copy open right now and might have goofed some method names). Basically, have subclasses like ManiphestTask just return their type instead of actually doing the generation. Only ApplicationTransaction and ApplicationTransactionComment have nontrivial logic in generatePHID(). But we could also skip this, there's no real harm in generating the PHIDs (they aren't written to the DB or anything). |
using the generate_phid technique - I think we can think about something a bit cleaner if / when this comes up again.
updated...
- use withTypes - no need to add the "withObjectType"
- doing so moves the filtering to the DB and makes default null
- make the list sorted and slap the default on the front at the end