From b856a231fa4a8ecbfe83630827bb4b83eca9a0c6 Mon Sep 17 00:00:00 2001
From: Richard Vsiansky <r.vsia@seznam.cz>
Date: Tue, 30 Jul 2019 13:33:00 +0200
Subject: [PATCH] Do not append title to breadcrumbs on show_list pages

---
 app/controllers/mixins/breadcrumbs_mixin.rb   | 17 ++++++-
 .../mixins/breadcrumbs_mixin_spec.rb          | 46 +++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/app/controllers/mixins/breadcrumbs_mixin.rb b/app/controllers/mixins/breadcrumbs_mixin.rb
index a3710b0509b3..f7388fe57369 100644
--- a/app/controllers/mixins/breadcrumbs_mixin.rb
+++ b/app/controllers/mixins/breadcrumbs_mixin.rb
@@ -22,7 +22,8 @@ def data_for_breadcrumbs
         breadcrumbs.push(special_page_breadcrumb(@tagitems || @politems || @ownershipitems || @retireitems))
 
         # Append title breadcrumb if they exist and not same as previous breadcrumb (eg "Editing name")
-        if @title && @title != breadcrumbs.compact.last.try(:[], :title)
+        # Also do not append when user is on show_list page, the menu title is used instead of it
+        if @title && !same_as_last_breadcrumb?(breadcrumbs, @title) && !show_list_page?
           breadcrumbs.push(:title => @title)
         end
       else
@@ -150,6 +151,20 @@ def not_show_page?
       (action_name == "show" && params["display"] && !%w[dashboard main].include?(params["display"])) || (action_name != "show")
     end
 
+    # User is on show_list page
+    # A lot of pages has different header than last item in menu
+    # The last item in menu is an abbreviation of the header
+    # So, breadcrumbs should contain only the abbreviation
+    def show_list_page?
+      action_name == "show_list"
+    end
+
+    # Checks if the title is same as the last item in breadcrumbs
+    # If so, then there is no reason to append the title to breadcrumbs
+    def same_as_last_breadcrumb?(breadcrumbs, title)
+      title == breadcrumbs.compact.last.try(:[], :title)
+    end
+
     # Controls on tagging screen if the tagged item is floating_ip
     def floating_ip_address?(variable)
       (variable&.respond_to?(:has_attribute?) && variable&.has_attribute?(:address)) || controller_name == 'floating_ips'
diff --git a/spec/controllers/mixins/breadcrumbs_mixin_spec.rb b/spec/controllers/mixins/breadcrumbs_mixin_spec.rb
index dcd14de14251..1eb95c513727 100644
--- a/spec/controllers/mixins/breadcrumbs_mixin_spec.rb
+++ b/spec/controllers/mixins/breadcrumbs_mixin_spec.rb
@@ -193,6 +193,52 @@ class BreadcrumbsTestController < ActionController::Base
           )
         end
       end
+
+      it "not contain header on show_list page" do
+        allow(subject).to receive(:action_name).and_return("show_list")
+        subject.instance_variable_set(:@title, "Do not show me")
+
+        expect(subject.data_for_breadcrumbs).to eq(
+          [
+            {:title=>"First Layer"},
+            {:title=>"Second Layer"}
+          ]
+        )
+      end
+
+      it "contain header on not show_list page" do
+        allow(subject).to receive(:action_name).and_return("edot")
+        subject.instance_variable_set(:@title, "Do show me")
+
+        expect(subject.data_for_breadcrumbs).to eq(
+          [
+            {:title=>"First Layer"},
+            {:title=>"Second Layer"},
+            {:title=>"Do show me"}
+          ]
+        )
+      end
+    end
+
+    describe "#same_as_last_breadcrumb?" do
+      let(:breadcrumbs) do
+        [
+          {:title=>"controller"},
+          {:title=>"header"}
+        ]
+      end
+
+      it "returns true if header is same as last breadcrumb" do
+        expect(subject.send(:same_as_last_breadcrumb?, breadcrumbs, "header")).to eq(true)
+      end
+
+      it "returns false if header is not same as last breadcrumb" do
+        expect(subject.send(:same_as_last_breadcrumb?, breadcrumbs, "different header")).to eq(false)
+      end
+
+      it "returns false if header is same as last breadcrumb but it in the breadcrumbs on different place" do
+        expect(subject.send(:same_as_last_breadcrumb?, breadcrumbs, "controller")).to eq(false)
+      end
     end
 
     describe "#special_page_breadcrumb" do