From 0df7743d78cf3fadb1540fd2a4cf47bc57e3517d Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 10 Mar 2023 11:37:55 +1100 Subject: [PATCH] FIX: tag dropdown not working with default_list_filter (#20608) If you set a category to `default_list_filter` none. Information was not passed to the tag route and routing was incorrect. This patch fails, cause on reload route does not point to the right place. ``` -- a/app/assets/javascripts/discourse/app/routes/tag-show.js +++ b/app/assets/javascripts/discourse/app/routes/tag-show.js @@ -89,6 +89,8 @@ export default DiscourseRoute.extend(FilterModeMixin, { filter = `tag/${tagId}/l/${topicFilter}`; } const list = await findTopicList( this.store, this.topicTrackingState, @@ -123,7 +125,7 @@ export default DiscourseRoute.extend(FilterModeMixin, { }, setupController(controller, model) { - const noSubcategories = + this.noSubcategories = this.noSubcategories === undefined ? model.category?.default_list_filter === NONE : this.noSubcategories; @@ -133,7 +135,7 @@ export default DiscourseRoute.extend(FilterModeMixin, { ...model, period: model.list.for_period, navMode: this.navMode, - noSubcategories, + noSubcategories: this.noSubcategories, loading: false, }); ``` Long term we don't want to hide this logic from the routing (even in the category case) it just cause unneeded confusion and fragility. --- .../javascripts/discourse/app/lib/url.js | 7 +++-- .../discourse/tests/unit/lib/url-test.js | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/url.js b/app/assets/javascripts/discourse/app/lib/url.js index 996a51f94dd..53b2f3e70c3 100644 --- a/app/assets/javascripts/discourse/app/lib/url.js +++ b/app/assets/javascripts/discourse/app/lib/url.js @@ -506,9 +506,12 @@ export function getCategoryAndTagUrl(category, subcategories, tag) { if (category) { url = category.path; - if (subcategories && category.default_list_filter === "none") { + if (subcategories && (category.default_list_filter === "none" || tag)) { url += "/all"; - } else if (!subcategories && category.default_list_filter === "all") { + } else if ( + !subcategories && + (category.default_list_filter === "all" || tag) + ) { url += "/none"; } } diff --git a/app/assets/javascripts/discourse/tests/unit/lib/url-test.js b/app/assets/javascripts/discourse/tests/unit/lib/url-test.js index 1d1cc04dc89..9f8380c47e2 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/url-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/url-test.js @@ -141,6 +141,34 @@ module("Unit | Utility | url", function () { ), "/c/foo/1" ); + + assert.strictEqual( + getCategoryAndTagUrl( + { path: "/c/foo/1", default_list_filter: "none" }, + false, + "bar" + ), + "/tags/c/foo/1/none/bar" + ); + + assert.strictEqual( + getCategoryAndTagUrl({ path: "/c/foo/1" }, false, "bar"), + "/tags/c/foo/1/none/bar" + ); + + assert.strictEqual( + getCategoryAndTagUrl( + { path: "/c/foo/1", default_list_filter: "all" }, + true, + "bar" + ), + "/tags/c/foo/1/all/bar" + ); + + assert.strictEqual( + getCategoryAndTagUrl({ path: "/c/foo/1" }, true, "bar"), + "/tags/c/foo/1/all/bar" + ); }); test("routeTo redirects secure uploads URLS because they are server side only", async function (assert) {