From 68a33289992050be2b68ba4e7f0f8b0ecec1bafe Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Mon, 3 Jul 2023 14:29:05 +0800 Subject: [PATCH] FIX: Tags in tag groups not shown in edit nav menu tags modal (#22382) What is the problem? Before this change, the edit navigation menu tags modal was not displaying tags that belonged to a tag_group when the tags_listed_by_group site setting was set to true. This is because we are relying on the /tags endpoint which returned tags in various keys depending on the tags_listed_by_group site setting. When the site setting is set to true, tags under belonging to tag groups were returned in the extra.tag_groups attribute. What is the fix? This commit fixes it by pushing all tags in returned under the `tag_groups` attribute into the list of tags to displayed. In a following commit, we will move away from the `/tags` endpoint to a dedicated route to handle the listing of tags in the modal. --- .../edit-navigation-modal-form/tags-form.js | 16 +++++++- .../editing_sidebar_tags_navigation_spec.rb | 39 ++++++++++++++++--- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-modal-form/tags-form.js b/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-modal-form/tags-form.js index c149d1593fe..dd42d5d506a 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-modal-form/tags-form.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-modal-form/tags-form.js @@ -29,8 +29,20 @@ export default class extends Component { // `/tags` route as well so we have decided to kick this can of worms down the road for now. await this.store .findAll("tag") - .then((tags) => { - this.tags = tags.content.sort((a, b) => { + .then((result) => { + const tags = [...result.content]; + + if (result.extras) { + if (result.extras.tag_groups) { + result.extras.tag_groups.forEach((tagGroup) => { + tagGroup.tags.forEach((tag) => { + tags.push(tag); + }); + }); + } + } + + this.tags = tags.sort((a, b) => { return a.name.localeCompare(b.name); }); diff --git a/spec/system/editing_sidebar_tags_navigation_spec.rb b/spec/system/editing_sidebar_tags_navigation_spec.rb index 2dedc345ba6..c60e7d6c387 100644 --- a/spec/system/editing_sidebar_tags_navigation_spec.rb +++ b/spec/system/editing_sidebar_tags_navigation_spec.rb @@ -12,26 +12,36 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do Fabricate(:tag, name: "tag3").tap { |tag| Fabricate.times(1, :topic, tags: [tag]) } end + fab!(:tag4) do + Fabricate(:tag, name: "tag4").tap do |tag| + Fabricate.times(1, :topic, tags: [tag]) + + # Ensures tags in tag groups are shown as well + Fabricate(:tag_group, tags: [tag]) + end + end + # This tag should not be displayed in the modal as it has not been used in a topic - fab!(:tag4) { Fabricate(:tag, name: "tag4") } + fab!(:tag5) { Fabricate(:tag, name: "tag5") } let(:sidebar) { PageObjects::Components::Sidebar.new } before { sign_in(user) } - it "allows a user to edit the sidebar categories navigation" do + it "allows a user to edit the sidebar tags navigation" do visit "/latest" expect(sidebar).to have_tags_section expect(sidebar).to have_section_link(tag1.name) expect(sidebar).to have_section_link(tag2.name) expect(sidebar).to have_section_link(tag3.name) + expect(sidebar).to have_section_link(tag4.name) modal = sidebar.click_edit_tags_button expect(modal).to have_right_title(I18n.t("js.sidebar.tags_form_modal.title")) try_until_success { expect(modal).to have_focus_on_filter_input } - expect(modal).to have_tag_checkboxes([tag1, tag2, tag3]) + expect(modal).to have_tag_checkboxes([tag1, tag2, tag3, tag4]) modal.toggle_tag_checkbox(tag1).toggle_tag_checkbox(tag2).save @@ -39,12 +49,14 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do expect(sidebar).to have_section_link(tag1.name) expect(sidebar).to have_section_link(tag2.name) expect(sidebar).to have_no_section_link(tag3.name) + expect(sidebar).to have_no_section_link(tag4.name) visit "/latest" expect(sidebar).to have_section_link(tag1.name) expect(sidebar).to have_section_link(tag2.name) expect(sidebar).to have_no_section_link(tag3.name) + expect(sidebar).to have_no_section_link(tag4.name) modal = sidebar.click_edit_tags_button modal.toggle_tag_checkbox(tag2).save @@ -54,6 +66,17 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do expect(sidebar).to have_section_link(tag1.name) expect(sidebar).to have_no_section_link(tag2.name) expect(sidebar).to have_no_section_link(tag3.name) + expect(sidebar).to have_no_section_link(tag4.name) + end + + it "displays the all tags in the modal when `tags_listed_by_group` site setting is true" do + SiteSetting.tags_listed_by_group = true + + visit "/latest" + + modal = sidebar.click_edit_tags_button + + expect(modal).to have_tag_checkboxes([tag1, tag2, tag3, tag4]) end it "allows a user to filter the tags in the modal by the tag's name" do @@ -65,7 +88,7 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do modal.filter("tag") - expect(modal).to have_tag_checkboxes([tag1, tag2, tag3]) + expect(modal).to have_tag_checkboxes([tag1, tag2, tag3, tag4]) modal.filter("tag2") @@ -85,6 +108,7 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do expect(sidebar).to have_section_link(tag1.name) expect(sidebar).to have_no_section_link(tag2.name) expect(sidebar).to have_no_section_link(tag3.name) + expect(sidebar).to have_no_section_link(tag4.name) modal = sidebar.click_edit_tags_button modal.deselect_all.save @@ -92,6 +116,7 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do expect(sidebar).to have_section_link(tag1.name) expect(sidebar).to have_section_link(tag2.name) expect(sidebar).to have_section_link(tag3.name) + expect(sidebar).to have_section_link(tag4.name) end it "allows a user to reset to the default navigation menu tags site setting" do @@ -105,6 +130,7 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do expect(sidebar).to have_section_link(tag1.name) expect(sidebar).to have_no_section_link(tag2.name) expect(sidebar).to have_no_section_link(tag3.name) + expect(sidebar).to have_no_section_link(tag4.name) modal = sidebar.click_edit_tags_button modal.click_reset_to_defaults_button.save @@ -113,6 +139,7 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do expect(sidebar).to have_no_section_link(tag1.name) expect(sidebar).to have_section_link(tag2.name) expect(sidebar).to have_section_link(tag3.name) + expect(sidebar).to have_no_section_link(tag4.name) end it "allows a user to filter the tag in the modal by selection" do @@ -134,10 +161,10 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do modal.filter("").filter_by_unselected - expect(modal).to have_tag_checkboxes([tag3]) + expect(modal).to have_tag_checkboxes([tag3, tag4]) modal.filter_by_all - expect(modal).to have_tag_checkboxes([tag1, tag2, tag3]) + expect(modal).to have_tag_checkboxes([tag1, tag2, tag3, tag4]) end end