From d89c5aedbefd2846f514e5560acdee5354e1df0e Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 16 Feb 2021 17:54:24 +0200 Subject: [PATCH] FIX: Fix subcategory, tag drops and none values (#11934) * FIX: Generate correct URLs for category and tag drops * DEV: Remove unused properties * FIX: No subcategory and tag filter did not work --- .../javascripts/discourse/app/lib/url.js | 19 ++++++ .../app/templates/components/bread-crumbs.hbs | 2 +- .../addon/components/category-drop.js | 52 ++++------------ .../select-kit/addon/components/tag-drop.js | 62 ++++--------------- app/controllers/tags_controller.rb | 2 +- 5 files changed, 44 insertions(+), 93 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/url.js b/app/assets/javascripts/discourse/app/lib/url.js index 42a87f420a7..8cc2b57d760 100644 --- a/app/assets/javascripts/discourse/app/lib/url.js +++ b/app/assets/javascripts/discourse/app/lib/url.js @@ -494,4 +494,23 @@ export function prefixProtocol(url) { : url; } +export function getCategoryAndTagUrl(category, subcategories, tag) { + let url; + + if (category) { + url = category.url; + if (!subcategories) { + url += "/none"; + } + } + + if (tag) { + url = url + ? "/tags" + url + "/" + tag.toLowerCase() + : "/tag/" + tag.toLowerCase(); + } + + return url || "/"; +} + export default _urlInstance; diff --git a/app/assets/javascripts/discourse/app/templates/components/bread-crumbs.hbs b/app/assets/javascripts/discourse/app/templates/components/bread-crumbs.hbs index 374f9a3d717..7647cc6ca7f 100644 --- a/app/assets/javascripts/discourse/app/templates/components/bread-crumbs.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/bread-crumbs.hbs @@ -18,7 +18,7 @@ {{#unless additionalTags}} {{!-- the tag filter doesn't work with tag intersections, so hide it --}} {{#if siteSettings.show_filter_by_tag}} - {{tag-drop firstCategory=category tagId=tag.id}} + {{tag-drop currentCategory=category noSubcategories=noSubcategories tagId=tag.id}} {{/if}} {{/unless}} {{/if}} diff --git a/app/assets/javascripts/select-kit/addon/components/category-drop.js b/app/assets/javascripts/select-kit/addon/components/category-drop.js index 9007a452199..bb687477f10 100644 --- a/app/assets/javascripts/select-kit/addon/components/category-drop.js +++ b/app/assets/javascripts/select-kit/addon/components/category-drop.js @@ -1,10 +1,9 @@ import Category from "discourse/models/category"; import ComboBoxComponent from "select-kit/components/combo-box"; -import DiscourseURL from "discourse/lib/url"; +import DiscourseURL, { getCategoryAndTagUrl } from "discourse/lib/url"; import I18n from "I18n"; import { categoryBadgeHTML } from "discourse/helpers/category-link"; import { computed } from "@ember/object"; -import getURL from "discourse-common/lib/get-url"; import { readOnly } from "@ember/object/computed"; export const NO_CATEGORIES_ID = "no-categories"; @@ -107,8 +106,6 @@ export default ComboBoxComponent.extend({ parentCategoryName: readOnly("selectKit.options.parentCategory.name"), - parentCategoryUrl: readOnly("selectKit.options.parentCategory.url"), - allCategoriesLabel: computed( "parentCategoryName", "selectKit.options.subCategory", @@ -123,22 +120,6 @@ export default ComboBoxComponent.extend({ } ), - allCategoriesUrl: computed( - "parentCategoryUrl", - "selectKit.options.subCategory", - function () { - return getURL( - this.selectKit.options.subCategory - ? `${this.parentCategoryUrl}/all` || "/" - : "/" - ); - } - ), - - noCategoriesUrl: computed("parentCategoryUrl", function () { - return getURL(`${this.parentCategoryUrl}/none`); - }), - search(filter) { if (filter) { let results = Category.search(filter); @@ -159,27 +140,18 @@ export default ComboBoxComponent.extend({ actions: { onChange(categoryId) { - let categoryURL; + const category = + categoryId === ALL_CATEGORIES_ID || categoryId === NO_CATEGORIES_ID + ? this.selectKit.options.parentCategory + : Category.findById(parseInt(categoryId, 10)); - if (this.tagId && !this.category) { - const category = Category.findById(parseInt(categoryId, 10)); - categoryURL = getURL( - `/tags/c/${Category.slugFor(category)}/${ - category.id - }/${this.tagId.toLowerCase()}` - ); - } else if (categoryId === ALL_CATEGORIES_ID) { - categoryURL = this.allCategoriesUrl; - } else if (categoryId === NO_CATEGORIES_ID) { - categoryURL = this.noCategoriesUrl; - } else { - const category = Category.findById(parseInt(categoryId, 10)); - categoryURL = category.url; - } - - DiscourseURL.routeToUrl(categoryURL); - - return false; + DiscourseURL.routeToUrl( + getCategoryAndTagUrl( + category, + categoryId !== NO_CATEGORIES_ID, + this.tagId + ) + ); }, }, diff --git a/app/assets/javascripts/select-kit/addon/components/tag-drop.js b/app/assets/javascripts/select-kit/addon/components/tag-drop.js index 8773d5a8d2e..13e746d6af6 100644 --- a/app/assets/javascripts/select-kit/addon/components/tag-drop.js +++ b/app/assets/javascripts/select-kit/addon/components/tag-drop.js @@ -1,11 +1,9 @@ -import { equal, gte, or, readOnly } from "@ember/object/computed"; +import { equal, gte, readOnly } from "@ember/object/computed"; import { i18n, setting } from "discourse/lib/computed"; -import Category from "discourse/models/category"; import ComboBoxComponent from "select-kit/components/combo-box"; -import DiscourseURL from "discourse/lib/url"; +import DiscourseURL, { getCategoryAndTagUrl } from "discourse/lib/url"; import TagsMixin from "select-kit/mixins/tags"; import { computed } from "@ember/object"; -import getURL from "discourse-common/lib/get-url"; import { isEmpty } from "@ember/utils"; import { makeArray } from "discourse-common/lib/helpers"; @@ -19,7 +17,6 @@ export default ComboBoxComponent.extend(TagsMixin, { classNames: ["tag-drop"], value: readOnly("tagId"), tagName: "li", - currentCategory: or("secondCategory", "firstCategory"), showFilterByTag: setting("show_filter_by_tag"), categoryStyle: setting("category_style"), maxTagSearchResults: setting("max_tag_search_results"), @@ -70,28 +67,6 @@ export default ComboBoxComponent.extend(TagsMixin, { return this.tagId ? `tag-${this.tagId}` : "tag_all"; }), - currentCategoryUrl: readOnly("currentCategory.url"), - - allTagsUrl: computed("firstCategory", "secondCategory", function () { - if (this.currentCategory) { - return getURL(`${this.currentCategoryUrl}?allTags=1`); - } else { - return getURL("/"); - } - }), - - noTagsUrl: computed("firstCategory", "secondCategory", function () { - let url; - if (this.currentCategory) { - url = `/tags/c/${Category.slugFor(this.currentCategory)}/${ - this.currentCategory.id - }`; - } else { - url = "/tag"; - } - return getURL(`${url}/${NONE_TAG_ID}`); - }), - allTagsLabel: i18n("tagging.selector_all_tags"), noTagsLabel: i18n("tagging.selector_no_tags"), @@ -171,32 +146,17 @@ export default ComboBoxComponent.extend(TagsMixin, { actions: { onChange(tagId, tag) { - let url; - - switch (tagId) { - case ALL_TAGS_ID: - url = this.allTagsUrl; - break; - case NO_TAG_ID: - url = this.noTagsUrl; - break; - default: - if (this.currentCategory) { - url = `/tags/c/${Category.slugFor(this.currentCategory)}/${ - this.currentCategory.id - }`; - } else { - url = "/tag"; - } - - if (tag && tag.targetTagId) { - url += `/${tag.targetTagId.toLowerCase()}`; - } else { - url += `/${tagId.toLowerCase()}`; - } + if (tagId === NO_TAG_ID) { + tagId = NONE_TAG_ID; + } else if (tagId === ALL_TAGS_ID) { + tagId = null; + } else if (tag && tag.targetTagId) { + tagId = tag.targetTagId; } - DiscourseURL.routeTo(getURL(url)); + DiscourseURL.routeToUrl( + getCategoryAndTagUrl(this.currentCategory, !this.noSubcategories, tagId) + ); }, }, }); diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 03b24a50b87..8f6a6083518 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -452,7 +452,7 @@ class TagsController < ::ApplicationController search: params[:search], q: params[:q] ) - options[:no_subcategories] = true if params[:no_subcategories] == 'true' + options[:no_subcategories] = true if params[:no_subcategories] == true || params[:no_subcategories] == 'true' options[:per_page] = params[:per_page].to_i.clamp(1, 30) if params[:per_page].present? if params[:tag_id] == 'none'