From be21dd3b52b8b83839d225e39ed9bd37e0a2658a Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 13 Feb 2019 09:13:38 -0800 Subject: [PATCH] Fix some "URI->alter(X, null)" callsites Summary: Ref T13250. This internally calls `replaceQueryParam(X, null)` now, which fatals if the second parameter is `null`. I hit these legitimately, but I'll look for more callsites and follow up by either allowing this, removing `alter()`, fixing the callsites, or some combination. (I'm not much of a fan of `alter()`.) Test Plan: Browsing a paginated list no longer complains about URI construction. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13250 Differential Revision: https://secure.phabricator.com/D20162 --- src/view/control/AphrontCursorPagerView.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/view/control/AphrontCursorPagerView.php b/src/view/control/AphrontCursorPagerView.php index 4608158481..cdb9562624 100644 --- a/src/view/control/AphrontCursorPagerView.php +++ b/src/view/control/AphrontCursorPagerView.php @@ -99,9 +99,9 @@ final class AphrontCursorPagerView extends AphrontView { return null; } - return $this->uri - ->alter('before', null) - ->alter('after', null); + return id(clone $this->uri) + ->removeQueryParam('after') + ->removeQueryParam('before'); } public function getPrevPageURI() { @@ -113,9 +113,9 @@ final class AphrontCursorPagerView extends AphrontView { return null; } - return $this->uri - ->alter('after', null) - ->alter('before', $this->prevPageID); + return id(clone $this->uri) + ->removeQueryParam('after') + ->replaceQueryParam('before', $this->prevPageID); } public function getNextPageURI() { @@ -127,9 +127,9 @@ final class AphrontCursorPagerView extends AphrontView { return null; } - return $this->uri - ->alter('after', $this->nextPageID) - ->alter('before', null); + return id(clone $this->uri) + ->replaceQueryParam('after', $this->nextPageID) + ->removeQueryParam('before'); } public function render() { -- 2.51.2