From 2dd9ac62771281710e315c213e4c33593cb16cc2 Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Fri, 23 Jun 2023 08:39:37 +0800
Subject: [PATCH] DEV: Improve
 PageObjects::Components::Sidebar#has_tag_section_links? (#22250)

Why this change?

Predicate matchers are poor at providing good error messages when it
fails if all the predicate matcher does is to return a boolean. Prior to
this change, we were using `has_css? && all?` to assert for the tag
section links. There are two problems here. Firstly, when one of the matchers
fail, the error message does not provide any indication of which matcher
failed making it hard to debug failures. Secondly, the matchers were not
able to assert for the ordering of the tag section links which is an
important behaviour to assert for.

This commit changes `PageObjects::Components::Sidebar#has_tag_section_links?`
such that we make use of assertions to ensure ordering. The usage of
`all` will also provide a clear error message when things go wrong.
---
 spec/system/page_objects/components/sidebar.rb  | 17 ++++++++---------
 .../viewing_sidebar_as_anonymous_user_spec.rb   |  6 +++---
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/spec/system/page_objects/components/sidebar.rb b/spec/system/page_objects/components/sidebar.rb
index 9536656b279..7dbea8f9c07 100644
--- a/spec/system/page_objects/components/sidebar.rb
+++ b/spec/system/page_objects/components/sidebar.rb
@@ -91,17 +91,16 @@ module PageObjects
         has_section_link?(I18n.t("js.sidebar.all_tags"))
       end
 
-      def has_tags_section_links?(tags)
-        section_selector = ".sidebar-section[data-section-name='tags']"
+      def has_tag_section_links?(tags)
         tag_names = tags.map(&:name)
 
-        has_css?(
-          "#{section_selector} .sidebar-section-link-wrapper[data-tag-name]",
-          count: tag_names.length,
-        ) &&
-          all("#{section_selector} .sidebar-section-link-wrapper[data-tag-name]").all? do |row|
-            tag_names.include?(row["data-tag-name"].to_s)
-          end
+        tag_section_links =
+          all(
+            ".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name]",
+            count: tag_names.length,
+          )
+
+        expect(tag_section_links.map(&:text)).to eq(tag_names)
       end
 
       def has_no_section?(name)
diff --git a/spec/system/viewing_sidebar_as_anonymous_user_spec.rb b/spec/system/viewing_sidebar_as_anonymous_user_spec.rb
index 26df95bee23..5c7853dd294 100644
--- a/spec/system/viewing_sidebar_as_anonymous_user_spec.rb
+++ b/spec/system/viewing_sidebar_as_anonymous_user_spec.rb
@@ -32,7 +32,7 @@ describe "Viewing sidebar as anonymous user", type: :system do
 
       expect(sidebar).to have_tags_section
       expect(sidebar).to have_all_tags_section_link
-      expect(sidebar).to have_tags_section_links([tag1, tag2, tag3, tag4, tag5])
+      expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag5, tag1])
     end
 
     it "should display the site's top tags when `default_navigation_menu_tags` site setting has been set but the tags configured are hidden to the user" do
@@ -43,7 +43,7 @@ describe "Viewing sidebar as anonymous user", type: :system do
 
       expect(sidebar).to have_tags_section
       expect(sidebar).to have_all_tags_section_link
-      expect(sidebar).to have_tags_section_links([tag1, tag2, tag3, tag4, tag6])
+      expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag1, tag6])
     end
 
     it "should display the tags configured in `default_navigation_menu_tags` site setting when it has been set" do
@@ -53,7 +53,7 @@ describe "Viewing sidebar as anonymous user", type: :system do
 
       expect(sidebar).to have_tags_section
       expect(sidebar).to have_all_tags_section_link
-      expect(sidebar).to have_tags_section_links([tag3, tag4])
+      expect(sidebar).to have_tag_section_links([tag3, tag4])
     end
   end
 end