HomePhabricator

make repo callsigns optional

Authored by fabe.

Description

make repo callsigns optional

Summary:
Ref T4245 Make repo callsigns optional
This is far from done and still very ugly. I'm just submitting it to check if i'm solving this in the right places.
Right now there's three places with duplicate code and building the identifierMap in the CommitQuery is very ugly.
If we only want to support this in the user frontend then i could hack it into the Markup rule itself and not touch the CommitQuery. Even uglier but more limited in scope...

Generally this approach will need a lot of "check this first and then try the other" in a few places.
I could move the Repository queries into a specialised PhabricatorRepositoryQuery method (withCallsignOrID) but i'm not sure about that.

Test Plan:

  • phid.lookup works with R1 and rTEST (which is the same repo)
  • R1 and rTEST euqally work in remarkup (tested in comments).
  • Reviewed the following syntax also all works:

rTEST
rTESTd773137a7cb9
rTEST:d773137a7cb9
R1
R1:d773137a7cb9
d773137a7cb9
{rTEST}
{rTESTd773137a7cb9}
{rTEST:d773137a7cb9}
rJX Javelin
{R1:d773137a7cb9}
{d773137a7cb9}

Reviewers: epriestley, Blessed Reviewers

Reviewed By: epriestley, Blessed Reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D11050

Details

Auditors
richardvanvelzen
Committed
epriestleyJan 1 2015, 4:07 PM
Pushed
epriestleyJan 1 2015, 4:07 PM
Reviewer
Blessed Reviewers
Differential Revision
D11050: make repo callsigns optional
Parents
rPcd677161e16a: Do not CC users without permissions to view an object
Branches
Unknown
Tags
Unknown
Tasks
T4245: Make repository URIs slightly prettier

Event Timeline

richardvanvelzen raised a concern with this commit.Jan 6 2015, 9:06 AM
richardvanvelzen added a subscriber: richardvanvelzen.

I'm getting an error via Conduit when adding a Conpherence message. See inline for where it goes wrong.

Trace:

#0 PhabricatorLiskDAO::assertAttached(string) called at [<phabricator>/src/applications/repository/storage/PhabricatorRepositoryCommit.php:46]
#1 PhabricatorRepositoryCommit::getRepository() called at [<phabricator>/src/applications/diffusion/query/DiffusionCommitQuery.php:201]
#2 DiffusionCommitQuery::willFilterPage(array) called at [<phabricator>/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php:236]
#3 PhabricatorPolicyAwareQuery::execute() called at [<phabricator>/src/applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php:24]
#4 DiffusionCommitRemarkupRule::loadObjects(array) called at [<phabricator>/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php:309]
#5 PhabricatorObjectRemarkupRule::didMarkupText() called at [<phutil>/src/markup/engine/PhutilRemarkupEngine.php:292]
#6 PhutilRemarkupEngine::postprocessText(array) called at [<phutil>/src/markup/engine/PhutilRemarkupEngine.php:93]
#7 PhutilRemarkupEngine::markupText(string) called at [<phabricator>/src/infrastructure/markup/PhabricatorMarkupEngine.php:550]
#8 PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles(PhabricatorUser, array) called at [<phabricator>/src/applications/conpherence/editor/ConpherenceEditor.php:93]
#9 ConpherenceEditor::generateTransactionsFromText(PhabricatorUser, ConpherenceThread, string) called at [<phabricator>/src/applications/conpherence/conduit/ConpherenceUpdateThreadConduitAPIMethod.php:97]
#10 ConpherenceUpdateThreadConduitAPIMethod::execute(ConduitAPIRequest) called at [<phabricator>/src/applications/conduit/method/ConduitAPIMethod.php:65]
#11 ConduitAPIMethod::executeMethod(ConduitAPIRequest) called at [<phabricator>/src/applications/conduit/call/ConduitCall.php:130]
#12 ConduitCall::executeMethod() called at [<phabricator>/src/applications/conduit/call/ConduitCall.php:80]
#13 ConduitCall::execute() called at [<phabricator>/src/applications/conduit/controller/PhabricatorConduitAPIController.php:85]
#14 PhabricatorConduitAPIController::processRequest() called at [<phabricator>/src/aphront/AphrontController.php:33]
#15 AphrontController::handleRequest(AphrontRequest) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:196]
#16 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:121]
#17 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:19]
/src/applications/diffusion/query/DiffusionCommitQuery.php
194

The loop used to end after this statement, which meant that $commit was gone from the list if it had no repo.

However, the loop now continues below, breaking a conduit call.