[Rt-commit] rt branch 5.0/menu-tiny-potential-fix created. rt-5.0.4-21-gf86b72470c

[BPS Git Server]

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".

The branch, 5.0/menu-tiny-potential-fix has been created
        at  f86b72470c493acc690f1b14c00cab17907969b4 (commit)

- Log -----------------------------------------------------------------
commit f86b72470c493acc690f1b14c00cab17907969b4
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Sun Jun 11 14:24:42 2023 +0300

    Remove trailing artefacts before adding query part
    
    Old code could add query part, eg '?foo=bar' to $base_path, so
    removing trailing 'index.html' and '/' would never work.
    
    So remove trailing parts first and only after that add query string
    if it's not empty.
    
    It only influence whether child is marked as active or not.
    
    It is hard to see if it affects production code or not, but new
    code is more correct than the old one.

diff --git a/lib/RT/Interface/Web/Menu.pm b/lib/RT/Interface/Web/Menu.pm
index 5acb9ea290..96915e900a 100644
--- a/lib/RT/Interface/Web/Menu.pm
+++ b/lib/RT/Interface/Web/Menu.pm
@@ -237,12 +237,13 @@ sub child {
         # Activate it
         if ( defined $path and length $path ) {
             my $base_path = $HTML::Mason::Commands::r->path_info;
-            my $query     = URI->new(RT::Interface::Web::RequestENV('REQUEST_URI'))->query;
             $base_path =~ s!/+!/!g;
-            $base_path .= "?$query" if defined $query and length $query;
-
             $base_path =~ s/index\.html$//;
             $base_path =~ s/\/+$//;
+
+            my $query = URI->new(RT::Interface::Web::RequestENV('REQUEST_URI'))->query;
+            $base_path .= "?$query" if defined $query and length $query;
+
             $path =~ s/index\.html$//;
             $path =~ s/\/+$//;
 
diff --git a/t/api/menu.t b/t/api/menu.t
index 9171b69211..3c6a427a5d 100644
--- a/t/api/menu.t
+++ b/t/api/menu.t
@@ -13,13 +13,15 @@ sub child_path_is($$$) {
 
 {
     package FakeRequest;
+    our $PATH_INFO = "";
     sub new { bless {}, shift }
-    sub path_info { "" }
+    sub path_info { $PATH_INFO }
 
     package FakeInterp;
+    our $URI = "";
     require CGI::PSGI;
     sub new { bless {}, shift }
-    sub cgi_object { CGI::PSGI->new({ REQUEST_URI => '' }) }
+    sub cgi_object { CGI::PSGI->new({ REQUEST_URI => $URI }) }
 }
 
 local $HTML::Mason::Commands::r = FakeRequest->new;
@@ -82,4 +84,19 @@ order_ok $menu, [qw(foo baz pre bat bar)], "sorted between (before)";
 ok $menu->child("bat")->add_after("post", title => "post"), "added child post after bat";
 order_ok $menu, [qw(foo baz pre bat post bar)], "sorted between (after)";
 
+{
+    my $menu = RT::Interface::Web::Menu->new;
+    local $FakeRequest::PATH_INFO = "/Tools/index.html";
+    $menu->child("tools", title => "Tools", path => "/Tools/");
+    ok $menu->child('tools')->active, "Tools is active";
+}
+
+{
+    local $FakeRequest::PATH_INFO = "/Tools/index.html";
+    local $FakeInterp::URI = "/Tools/index.html?foo=bar";
+    my $menu = RT::Interface::Web::Menu->new;
+    $menu->child("tools", title => "Tools", path => "/Tools?foo=bar");
+    ok $menu->child('tools')->active, "Tools is active";
+}
+
 done_testing;

-----------------------------------------------------------------------


hooks/post-receive
-- 
rt