Page MenuHomePhabricator

Correct a datasource issue when viewing repository URIs in "Manage Repository"
ClosedPublic

Authored by epriestley on Jun 30 2017, 2:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 3:12 PM
Unknown Object (File)
Thu, Apr 11, 7:20 AM
Unknown Object (File)
Fri, Apr 5, 2:33 PM
Unknown Object (File)
Sat, Mar 30, 12:47 PM
Unknown Object (File)
Wed, Mar 27, 8:14 PM
Unknown Object (File)
Wed, Mar 20, 3:20 PM
Unknown Object (File)
Feb 21 2024, 9:32 AM
Unknown Object (File)
Feb 21 2024, 5:06 AM
Subscribers

Details

Summary

Fixes T12884. In cases other than this UI, applications access URIs through the Repository they're part of. This means that applications interact with URIs which have gone through the correction/adjustment logic in PhabricatorRepository->attachURIs(), which fixes up "builtin" URIs to have the right values based on configuration.

In this case (and, as far as I can tell, only this case) we load the URI directly and act on its properties which depend on configuration and repository state.

This can mean we're using a different view of the URI than we should be.

To fix this: after loading the URI, reload it through the repository so the relevant adjustments are applied.

I think this is the most reasonable fix. We could try to make RepositoryURIQuery somehow enforce this, but the cost of this error is small (mild confusion about display state), the other things which do direct loads don't depend on this state (editing), and everything else loads via a repository and is likely to continue doing that forever.

Test Plan

Screen Shot 2017-06-30 at 6.56.03 AM.png (849×1 px, 134 KB)

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Jun 30 2017, 2:05 PM
This revision was automatically updated to reflect the committed changes.
jmeador added inline comments.
src/applications/diffusion/controller/DiffusionRepositoryURIViewController.php
30

@epriestley I believe this query is resulting in 10 second page loads for us. I haven't tested it, though so I can't be sure. It does look like it is loading every URI for every repository though. Is this intentional? would a constraint on this query break how this fix works?

Yikes! Pretty sure the query is just completely wrong, give me a second and I'll get a fix out.

D19036 should make the cost of this page more reasonable.