From 51f3e4e414a636d51a9a7bd5348ad964c56a80cd Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Tue, 16 Mar 2021 16:50:34 -0300
Subject: [PATCH] Bug 20854: (QA follow-up) Improve readability

The logout_cas method does too little tasks. In this case, I consider we
should not split it into a separate method. As the CAS implementation
lacked tests, it made sense to do it like this, so it is testable. But
now we have higher-level tests for logout_cas, we can bake the behavior
inside of it, and it is testable.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 C4/Auth_with_cas.pm | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/C4/Auth_with_cas.pm b/C4/Auth_with_cas.pm
index 5fe0640a718..4a6d0557d00 100644
--- a/C4/Auth_with_cas.pm
+++ b/C4/Auth_with_cas.pm
@@ -68,14 +68,21 @@ sub getMultipleAuth {
 
 # Logout from CAS
 sub logout_cas {
-    my ($query, $type) = @_;
-    my ( $cas, $uri ) = _get_cas_and_service($query, undef, $type);
-    $uri =~ s/\?logout\.x=1//; # We don't want to keep triggering a logout, if we got here, the borrower is already logged out of Koha
-    my $logout_url = $cas->logout_url(url => $uri);
-    $logout_url = _fix_logout_url($logout_url);
-    print $query->redirect( $logout_url );
+    my ( $query, $type ) = @_;
+    my ( $cas,   $uri )  = _get_cas_and_service( $query, undef, $type );
+
+    # We don't want to keep triggering a logout, if we got here,
+    # the borrower is already logged out of Koha
+    $uri =~ s/\?logout\.x=1//;
+
+    my $logout_url = $cas->logout_url( url => $uri );
+    $logout_url =~ s/url=/service=/
+      if C4::Context->preference('casServerVersion') eq '3';
+
+    print $query->redirect($logout_url);
 }
 
+
 # Login to CAS
 sub login_cas {
     my ($query, $type) = @_;
@@ -206,15 +213,6 @@ sub _get_cas_and_service {
     return ( $cas, $uri );
 }
 
-# Fix the logout URL when the cas server is 3.0 or superior
-sub _fix_logout_url {
-    my $url = shift;
-    if (C4::Context->preference('casServerVersion') eq '3') {
-        $url =~ s/url=/service=/;
-    }
-    return $url;
-}
-
 # Get the current URL with parameters contained directly into URL (GET params)
 # This method replaces $query->url() which will give both GET and POST params
 sub _url_with_get_params {
-- 
2.31.0