Replace "URI->setQueryParams()" after initialization with a constructor argument
ClosedPublic
Actions

Authored by epriestley on Feb 12 2019, 7:02 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T13250: Some typeaheads use nonscalar HTTP parameters, which fatal under new "phutil_build_http_query()" rules

Commits

rP4c124201623a: Replace "URI->setQueryParams()" after initialization with a constructor argument

Summary

Ref T13250. See D20149. In a number of cases, we use setQueryParams() immediately after URI construction. To simplify this slightly, let the constructor take parameters, similar to HTTPSFuture.

Test Plan

See inlines.

Diff Detail

Repository

rP Phabricator

Branch

uri3

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 21957
Build 29985: Run Core Tests
Build 29984: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	5 ms	DiffusionURITestCase::Unknown Unit Message ("") Assertion failed, expected values to be equal (at DiffusionURITestCase.php:143). Expected: '/diffusion/A/browse/path/to/file.ext?view=things' Actual: '/diffusion/A/browse/path/to/file.ext'
	1 ms	AlmanacNamesTestCase::Unknown Unit Message ("") 30 assertions passed.
	0 ms	AlmanacServiceTypeTestCase::Unknown Unit Message ("") 1 assertion passed.
	0 ms	AphrontHTTPSinkTestCase::Unknown Unit Message ("") 4 assertions passed.
	0 ms	AphrontHTTPSinkTestCase::Unknown Unit Message ("") 3 assertions passed.
		View Full Test Results (1 Failed · 378 Passed)

Event Timeline

epriestley created this revision.Feb 12 2019, 7:02 PM

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 12 2019, 7:05 PM

Harbormaster failed remote builds in B21957: Diff 48114!

Replace more callsites.

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 12 2019, 7:11 PM

Harbormaster failed remote builds in B21961: Diff 48117!

Tests won't pass until D20149 lands.

src/applications/almanac/controller/AlmanacController.php
70–77	Edited and deleted an Almanac property.
src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php
207–212	With password auth enabled, used a one-time login link to reset my password.
src/applications/auth/future/PhabricatorDuoFuture.php
106–119 ↗	(On Diff #48117)	Added a new Duo credential, survived the Duo MFA gate.
src/applications/diffusion/view/DiffusionView.php
84–90	Did not test, since it requires a repository with a submodule that points at another tracked repository and I don't have any of those on hand. Looked at it real carefully?
src/applications/diviner/markup/DivinerSymbolRemarkupRule.php
113–123	Typed `@{class:quack}` in remarkup and clicked it.
src/applications/herald/controller/HeraldNewController.php
135	Here and below, created a new Herald rule.
src/applications/owners/controller/PhabricatorOwnersDetailController.php
68–73	Clicked "View All" under "Recent Commits" on a package detail page.
src/applications/phortune/controller/cart/PhortuneCartCheckoutController.php
137–143	Used Fund to create a payment, clicked "Add new payment method".
src/applications/phortune/provider/PhortunePayPalPaymentProvider.php
351–359	Did not test, Paypal "works" but is hard-coded to sandbox only and not in use in production today.
src/applications/phortune/provider/PhortunePaymentProvider.php
276	Edited, disabled, enabled payment providers in a Phortune merchant.
src/applications/repository/storage/PhabricatorRepository.php
831	Poked around Diffusion, pretty much everything uses this.
src/applications/transactions/editengine/PhabricatorEditEngine.php
1544 ↗	(On Diff #48117)	Browsed to a Calendar day view, created a new event, saw it select the correct day as a default.
src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php
112–113 ↗	(On Diff #48117)	Typed ahead.