From 898af591ade8fa54edccbb067b8f48692e6ee0ce Mon Sep 17 00:00:00 2001
From: ingoKrOffice <158591735+ingoKrOffice@users.noreply.github.com>
Date: Thu, 27 Jun 2024 11:17:32 +0200
Subject: [PATCH] 811 redirection error if the url contains parameters (#812)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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>
---
 plugin.php                            |  4 ----
 plugin/Controller/EstateDetailUrl.php |  8 ++------
 plugin/Utility/Redirector.php         |  2 +-
 tests/TestClassEstateDetailUrl.php    | 18 +++++++++---------
 tests/TestClassRedirectIfOldUrl.php   |  2 +-
 5 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/plugin.php b/plugin.php
index 442f75776..ae6fae609 100644
--- 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
index e41ef6769..648223e62 100644
--- 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;
 	}
-}
\ No newline at end of file
+}
diff --git a/plugin/Utility/Redirector.php b/plugin/Utility/Redirector.php
index 580955486..9cde7ec07 100644
--- 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
index f5d652310..75943deba 100644
--- 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));
 	}
-}
\ No newline at end of file
+}
diff --git a/tests/TestClassRedirectIfOldUrl.php b/tests/TestClassRedirectIfOldUrl.php
index 7d8e39021..c49d90108 100644
--- 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() );
 	}
 
 	/**