811 redirection error if the url contains parameters (#812)

* remove add_filter, to make the filter useful * fix redirect with params * fix tests --------- Co-authored-by: Anja Möller <22166320+andernath@users.noreply.github.com>
onOffice-Web-Org · Jun 27, 2024 · 898af59 · 898af59
1 parent ae56458
commit 898af59
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 21 deletions.
diff --git a/plugin.php b/plugin.php
@@ -329,10 +329,6 @@ function customFieldCallback( $pDI, $format, $limitEllipsis, $meta_key ) {
 	}
 });
 
-add_filter('oo_is_detailpage_redirection', function($value) {
-	return $value;
-});
-
 $pEstateRedirection = apply_filters('oo_is_detailpage_redirection', true);
 
 add_action('parse_request', function(WP $pWP) use ($pDI, $pEstateRedirection) {

diff --git a/plugin/Controller/EstateDetailUrl.php b/plugin/Controller/EstateDetailUrl.php
@@ -146,13 +146,9 @@ public function getUrlWithEstateTitle(int $estateId, string $title = null, strin
 		$urlLsSwitcher = $urlElements['scheme'] . '://' . $urlElements['host'] . $newPath . '/' . $urlTemp;
 
 		if (!empty($getParameters)) {
-			if ($pEstateRedirection) {
-				$urlLsSwitcher .= '';
-			} elseif(!$tickerUrlHasTitleFlag) {
-				$urlLsSwitcher .= '?' . http_build_query($getParameters);
-			}
+			$urlLsSwitcher = add_query_arg($getParameters,$urlLsSwitcher);
 		}
 
 		return $urlLsSwitcher;
 	}
-}
+}
diff --git a/plugin/Utility/Redirector.php b/plugin/Utility/Redirector.php
@@ -117,6 +117,6 @@ public function getUri()
 	public function getCurrentLink(): string
 	{
 		global $wp;
-		return home_url(add_query_arg($_GET, trailingslashit($wp->request)));
+		return home_url(add_query_arg($_GET, $wp->request));
 	}
 }
diff --git a/tests/TestClassEstateDetailUrl.php b/tests/TestClassEstateDetailUrl.php
@@ -153,7 +153,7 @@ public function testGetUrlWithFilterTrueAndEnableOptionShowTitleWithParameter()
 		$title = 'test-title';
 		$pInstance = $this->_pContainer->get(EstateDetailUrl::class);
 		$url = 'https://www.onoffice.de/detail/123?lang=en';
-		$expectedUrl = 'https://www.onoffice.de/detail/123-test-title';
+		$expectedUrl = 'https://www.onoffice.de/detail/123-test-title?lang=en';
 		$this->assertEquals($expectedUrl, $pInstance->getUrlWithEstateTitle($estateId, $title, $url, false, $pEstateRedirection));
 	}
 
@@ -169,7 +169,7 @@ public function testGetUrlWithFilterTrueAndDisableOptionShowTitleWithParameter()
 		$title = 'test-title';
 		$pInstance = $this->_pContainer->get(EstateDetailUrl::class);
 		$url = 'https://www.onoffice.de/detail/123?lang=en';
-		$expectedUrl = 'https://www.onoffice.de/detail/123';
+		$expectedUrl = 'https://www.onoffice.de/detail/123?lang=en';
 		$this->assertEquals($expectedUrl, $pInstance->getUrlWithEstateTitle($estateId, $title, $url, false, $pEstateRedirection));
 	}
 
@@ -210,16 +210,16 @@ public function testGetUrlWithFilterFalseAndEnableOptionShowTitleWithParameter()
 		$expectedUrlHasTitleWithoutParamster = 'https://www.onoffice.de/detail/123-test-title';
 		$this->assertEquals($expectedUrlHasTitleWithoutParamster, $pInstance->getUrlWithEstateTitle($estateId, $title, $urlHasTitleWithoutParamster, false, $pEstateRedirection));
 
-		$urlWithoutTitle = 'https://www.onoffice.de/detail/123/?lang=en';
+		$urlWithoutTitle = 'https://www.onoffice.de/detail/123?lang=en';
 		$expectedUrlWithoutTitle = 'https://www.onoffice.de/detail/123?lang=en';
 		$this->assertEquals($expectedUrlWithoutTitle, $pInstance->getUrlWithEstateTitle($estateId, $title, $urlWithoutTitle, false, $pEstateRedirection));
 
-		$urlHasTitleHasParamster = 'https://www.onoffice.de/detail/123-test-title';
-		$expectedUrlHasTitleHasParamster = 'https://www.onoffice.de/detail/123-test-title';
-		$this->assertEquals($expectedUrlHasTitleHasParamster, $pInstance->getUrlWithEstateTitle($estateId, $title, $urlHasTitleHasParamster, true, $pEstateRedirection));
+		$urlHasTitleHasParameter = 'https://www.onoffice.de/detail/123-test-title?lang=en';
+		$expectedUrlHasTitleHasParameter = 'https://www.onoffice.de/detail/123-test-title?lang=en';
+		$this->assertEquals($expectedUrlHasTitleHasParameter, $pInstance->getUrlWithEstateTitle($estateId, $title, $urlHasTitleHasParameter, true, $pEstateRedirection));
 
-		$urlHasTitle = 'https://www.onoffice.de/detail/123-test-title/?lang=en';
-		$expectedUrlHasTitle = 'https://www.onoffice.de/detail/123-test-title';
+		$urlHasTitle = 'https://www.onoffice.de/detail/123-test-title?lang=en';
+		$expectedUrlHasTitle = 'https://www.onoffice.de/detail/123-test-title?lang=en';
 		$this->assertEquals($expectedUrlHasTitle, $pInstance->getUrlWithEstateTitle($estateId, $title, $urlHasTitle, true, $pEstateRedirection));
 	}
-}
+}
diff --git a/tests/TestClassRedirectIfOldUrl.php b/tests/TestClassRedirectIfOldUrl.php
@@ -67,7 +67,7 @@ public function stestInstance()
 	 */
 	public function testGetCurrentLink()
 	{
-		$this->assertEquals( 'http://example.org/detail-view/123/', $this->_pRedirectIfOldUrl->getCurrentLink() );
+		$this->assertEquals( 'http://example.org/detail-view/123', $this->_pRedirectIfOldUrl->getCurrentLink() );
 	}
 
 	/**