Not sure this is the best fix, but seems to work? Uses PHID if present, engine/key if not.
Details
Details
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- form-typeahead (branched from master)
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 15003 Build 19675: Run Core Tests Build 19674: arc lint + arc unit
Event Timeline
Comment Actions
I think this is the best approach, yeah -- some tweaks inline.
| src/applications/transactions/typeahead/PhabricatorEditEngineDatasource.php | ||
|---|---|---|
| 28 | On the other side, I think you'll need to split this apart into the engine key and form key again. If you separate them with a ., this will be ambiguous: you can not tell which engine a.b.c is:
Since engine keys and form keys can both contain periods, it could go either way. Instead, you could use some character which does not appear in any keys yet (maybe /) and then update PhabricatorEditEngine and PhabricatorEditEngineConfiguration to forbid this character in keys. Specifically, in PhabricatorEditEngine->getEngineKey(), throw if the key contains a / before returning it. Then probably implement PhabricatorEditEngineConfiguration->setBuiltinKey() and have it throw if the argument contains /, then call parent::setBuiltinKey(...) if things look good. | |
Comment Actions
@epriestley I don't see any great way to solve N + 1 with this query, since each query (phid, engine, key) seems unique. Should I just do one query for all forms and only attach the ones needed? Or will that cause a problem if people have 900 forms (which, at least 1 install does).
Comment Actions
ah from PhabricatorEditEngineCreateQuickActions looks like you grab them all and pop out what u need