diff --git a/app/assets/javascripts/discourse-common/addon/resolver.js b/app/assets/javascripts/discourse-common/addon/resolver.js index 7ad517b2c46..4fcc1966d15 100644 --- a/app/assets/javascripts/discourse-common/addon/resolver.js +++ b/app/assets/javascripts/discourse-common/addon/resolver.js @@ -52,6 +52,24 @@ export function buildResolver(baseName) { return "service:app-events"; } + for (const [key, value] of Object.entries({ + "controller:discovery.categoryWithID": "controller:discovery.category", + "controller:discovery.parentCategory": "controller:discovery.category", + "controller:tags-show": "controller:tag-show", + "controller:tags.show": "controller:tag.show", + "controller:tagsShow": "controller:tagShow", + "route:discovery.categoryWithID": "route:discovery.category", + "route:discovery.parentCategory": "route:discovery.category", + "route:tags-show": "route:tag-show", + "route:tags.show": "route:tag.show", + "route:tagsShow": "route:tagShow", + })) { + if (fullName === key) { + deprecated(`${key} was replaced with ${value}`, { since: "2.6.0" }); + return value; + } + } + const split = fullName.split(":"); if (split.length > 1) { const appBase = `${baseName}/${split[0]}s/`; diff --git a/app/assets/javascripts/discourse/app/controllers/rename-tag.js b/app/assets/javascripts/discourse/app/controllers/rename-tag.js index 0993aa852e1..64a17026d7a 100644 --- a/app/assets/javascripts/discourse/app/controllers/rename-tag.js +++ b/app/assets/javascripts/discourse/app/controllers/rename-tag.js @@ -26,7 +26,7 @@ export default Controller.extend(ModalFunctionality, BufferedContent, { this.send("closeModal"); if (result.responseJson.tag) { - this.transitionToRoute("tags.show", result.responseJson.tag.id); + this.transitionToRoute("tag.show", result.responseJson.tag.id); } else { this.flash(extractError(result.responseJson.errors[0]), "error"); } diff --git a/app/assets/javascripts/discourse/app/controllers/tags-show.js b/app/assets/javascripts/discourse/app/controllers/tag-show.js similarity index 100% rename from app/assets/javascripts/discourse/app/controllers/tags-show.js rename to app/assets/javascripts/discourse/app/controllers/tag-show.js diff --git a/app/assets/javascripts/discourse/app/models/nav-item.js b/app/assets/javascripts/discourse/app/models/nav-item.js index d0979d4f8ba..ae8caa882c7 100644 --- a/app/assets/javascripts/discourse/app/models/nav-item.js +++ b/app/assets/javascripts/discourse/app/models/nav-item.js @@ -122,7 +122,7 @@ NavItem.reopenClass({ if (context.tagId && Site.currentProp("filters").includes(filterType)) { includesTagContext = true; - path += "/tags"; + path += "/tag"; } if (context.category) { diff --git a/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js b/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js index 66febe7f1bf..aada8bab6b7 100644 --- a/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js +++ b/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js @@ -1,7 +1,7 @@ import buildCategoryRoute from "discourse/routes/build-category-route"; import buildTopicRoute from "discourse/routes/build-topic-route"; import DiscoverySortableController from "discourse/controllers/discovery-sortable"; -import TagsShowRoute from "discourse/routes/tags-show"; +import TagShowRoute from "discourse/routes/tag-show"; import Site from "discourse/models/site"; import User from "discourse/models/user"; @@ -11,20 +11,16 @@ export default { initialize(registry, app) { app.DiscoveryCategoryController = DiscoverySortableController.extend(); - app.DiscoveryParentCategoryController = DiscoverySortableController.extend(); app.DiscoveryCategoryNoneController = DiscoverySortableController.extend(); app.DiscoveryCategoryAllController = DiscoverySortableController.extend(); - app.DiscoveryCategoryWithIDController = DiscoverySortableController.extend(); app.DiscoveryCategoryRoute = buildCategoryRoute("default"); - app.DiscoveryParentCategoryRoute = buildCategoryRoute("default"); app.DiscoveryCategoryNoneRoute = buildCategoryRoute("default", { no_subcategories: true, }); app.DiscoveryCategoryAllRoute = buildCategoryRoute("default", { no_subcategories: false, }); - app.DiscoveryCategoryWithIDRoute = buildCategoryRoute("default"); const site = Site.current(); site.get("filters").forEach((filter) => { @@ -35,9 +31,6 @@ export default { app[ `Discovery${filterCapitalized}CategoryController` ] = DiscoverySortableController.extend(); - app[ - `Discovery${filterCapitalized}ParentCategoryController` - ] = DiscoverySortableController.extend(); app[ `Discovery${filterCapitalized}CategoryNoneController` ] = DiscoverySortableController.extend(); @@ -48,9 +41,6 @@ export default { app[`Discovery${filterCapitalized}CategoryRoute`] = buildCategoryRoute( filter ); - app[ - `Discovery${filterCapitalized}ParentCategoryRoute` - ] = buildCategoryRoute(filter); app[ `Discovery${filterCapitalized}CategoryNoneRoute` ] = buildCategoryRoute(filter, { no_subcategories: true }); @@ -74,9 +64,6 @@ export default { app[ `DiscoveryTop${periodCapitalized}CategoryController` ] = DiscoverySortableController.extend(); - app[ - `DiscoveryTop${periodCapitalized}ParentCategoryController` - ] = DiscoverySortableController.extend(); app[ `DiscoveryTop${periodCapitalized}CategoryNoneController` ] = DiscoverySortableController.extend(); @@ -86,38 +73,26 @@ export default { app[`DiscoveryTop${periodCapitalized}CategoryRoute`] = buildCategoryRoute( "top/" + period ); - app[ - `DiscoveryTop${periodCapitalized}ParentCategoryRoute` - ] = buildCategoryRoute("top/" + period); app[ `DiscoveryTop${periodCapitalized}CategoryNoneRoute` ] = buildCategoryRoute("top/" + period, { no_subcategories: true }); }); - app["TagsShowCategoryRoute"] = TagsShowRoute.extend(); - app["TagsShowCategoryNoneRoute"] = TagsShowRoute.extend({ + app["TagsShowCategoryRoute"] = TagShowRoute.extend(); + app["TagsShowCategoryNoneRoute"] = TagShowRoute.extend({ noSubcategories: true, }); - app["TagsShowParentCategoryRoute"] = TagsShowRoute.extend(); - - app["TagShowRoute"] = TagsShowRoute; site.get("filters").forEach(function (filter) { - app["TagsShow" + filter.capitalize() + "Route"] = TagsShowRoute.extend({ - navMode: filter, - }); - app["TagShow" + filter.capitalize() + "Route"] = TagsShowRoute.extend({ + app["TagShow" + filter.capitalize() + "Route"] = TagShowRoute.extend({ navMode: filter, }); app[ "TagsShowCategory" + filter.capitalize() + "Route" - ] = TagsShowRoute.extend({ navMode: filter }); + ] = TagShowRoute.extend({ navMode: filter }); app[ "TagsShowNoneCategory" + filter.capitalize() + "Route" - ] = TagsShowRoute.extend({ navMode: filter, noSubcategories: true }); - app[ - "TagsShowParentCategory" + filter.capitalize() + "Route" - ] = TagsShowRoute.extend({ navMode: filter }); + ] = TagShowRoute.extend({ navMode: filter, noSubcategories: true }); }); }, }; diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js index c3c5f17193f..5352b1ff7c9 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -34,9 +34,6 @@ export default function () { ); this.route("discovery", { path: "/", resetNamespace: true }, function () { - // legacy route - this.route("topParentCategory", { path: "/c/:slug/l/top" }); - // top this.route("top"); this.route("topCategoryNone", { @@ -48,9 +45,6 @@ export default function () { Site.currentProp("periods").forEach((period) => { const top = "top" + period.capitalize(); - // legacy route - this.route(top + "ParentCategory", { path: "/c/:slug/l/top/" + period }); - this.route(top, { path: "/top/" + period }); this.route(top + "CategoryNone", { path: "/c/*category_slug_path_with_id/none/l/top/" + period, @@ -62,9 +56,6 @@ export default function () { // filters (e.g. bookmarks, posted, read, unread, latest) Site.currentProp("filters").forEach((filter) => { - // legacy route - this.route(filter + "ParentCategory", { path: "/c/:slug/l/" + filter }); - this.route(filter, { path: "/" + filter }); this.route(filter + "CategoryNone", { path: "/c/*category_slug_path_with_id/none/l/" + filter, @@ -76,10 +67,6 @@ export default function () { this.route("categories"); - // legacy routes - this.route("parentCategory", { path: "/c/:slug" }); - this.route("categoryWithID", { path: "/c/:parentSlug/:slug/:id" }); - // default filter for a category this.route("categoryNone", { path: "/c/*category_slug_path_with_id/none" }); this.route("categoryAll", { path: "/c/*category_slug_path_with_id/all" }); @@ -259,14 +246,6 @@ export default function () { this.route("intersection", { path: "intersection/:tag_id/*additional_tags", }); - - // legacy routes - this.route("show", { path: "/:tag_id" }); - Site.currentProp("filters").forEach((filter) => { - this.route("show" + filter.capitalize(), { - path: "/:tag_id/l/" + filter, - }); - }); }); this.route( diff --git a/app/assets/javascripts/discourse/app/routes/new-topic.js b/app/assets/javascripts/discourse/app/routes/new-topic.js index c82cc0681d3..538254dff0c 100644 --- a/app/assets/javascripts/discourse/app/routes/new-topic.js +++ b/app/assets/javascripts/discourse/app/routes/new-topic.js @@ -33,17 +33,9 @@ export default DiscourseRoute.extend({ } } - if (Boolean(category)) { - let route = "discovery.parentCategory"; - let params = { category, slug: category.slug }; - if (category.parentCategory) { - route = "discovery.category"; - params = { - category, - parentSlug: category.parentCategory.slug, - slug: category.slug, - }; - } + if (category) { + let route = "discovery.category"; + let params = { category, id: category.id }; this.replaceWith(route, params).then((e) => { if (this.controllerFor("navigation/category").canCreateTopic) { diff --git a/app/assets/javascripts/discourse/app/routes/tags-show.js b/app/assets/javascripts/discourse/app/routes/tag-show.js similarity index 95% rename from app/assets/javascripts/discourse/app/routes/tags-show.js rename to app/assets/javascripts/discourse/app/routes/tag-show.js index c6d2aa7a294..8fec14ea398 100644 --- a/app/assets/javascripts/discourse/app/routes/tags-show.js +++ b/app/assets/javascripts/discourse/app/routes/tag-show.js @@ -23,7 +23,7 @@ export default DiscourseRoute.extend(FilterModeMixin, { queryParams, renderTemplate() { - const controller = this.controllerFor("tags.show"); + const controller = this.controllerFor("tag.show"); this.render("tags.show", { controller }); }, @@ -62,7 +62,7 @@ export default DiscourseRoute.extend(FilterModeMixin, { }, afterModel(tag, transition) { - const controller = this.controllerFor("tags.show"); + const controller = this.controllerFor("tag.show"); controller.setProperties({ loading: true, showInfo: false, @@ -122,7 +122,7 @@ export default DiscourseRoute.extend(FilterModeMixin, { const filterText = I18n.t( `filters.${this.navMode.replace("/", ".")}.title` ); - const controller = this.controllerFor("tags.show"); + const controller = this.controllerFor("tag.show"); if (controller.get("model.id")) { if (this.category) { @@ -152,7 +152,7 @@ export default DiscourseRoute.extend(FilterModeMixin, { }, setupController(controller, model) { - this.controllerFor("tags.show").setProperties({ + this.controllerFor("tag.show").setProperties({ model, tag: model, additionalTags: this.additionalTags, @@ -180,7 +180,7 @@ export default DiscourseRoute.extend(FilterModeMixin, { }, createTopic() { - const controller = this.controllerFor("tags.show"); + const controller = this.controllerFor("tag.show"); if (controller.get("list.draft")) { this.openTopicDraft(controller.get("list")); @@ -233,7 +233,7 @@ export default DiscourseRoute.extend(FilterModeMixin, { resetParams, didTransition() { - this.controllerFor("tags.show")._showFooter(); + this.controllerFor("tag.show")._showFooter(); return true; }, }, diff --git a/app/assets/javascripts/discourse/app/routes/tags-intersection.js b/app/assets/javascripts/discourse/app/routes/tags-intersection.js index e8b784f0eed..7dd65e72b1c 100644 --- a/app/assets/javascripts/discourse/app/routes/tags-intersection.js +++ b/app/assets/javascripts/discourse/app/routes/tags-intersection.js @@ -1,6 +1,6 @@ -import TagsShowRoute from "discourse/routes/tags-show"; +import TagShowRoute from "discourse/routes/tag-show"; -export default TagsShowRoute.extend({}); +export default TagShowRoute.extend({}); // The tags-intersection route is exactly the same as the tags-show route, but the wildcard at the // end of the route (*additional_tags) will cause a match when query parameters are present, diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index 24797d6b8f2..910e2ed5d84 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -336,25 +336,14 @@ class ListController < ApplicationController end def set_category - parts = params.require(:category_slug_path_with_id).split('/') + category_slug_path_with_id = params.require(:category_slug_path_with_id) - if !parts.empty? && parts.last =~ /\A\d+\Z/ - id = parts.pop.to_i - end - slug_path = parts unless parts.empty? - - if id.present? - @category = Category.find_by_id(id) - elsif slug_path.present? - @category = Category.find_by_slug_path(slug_path) - - # Legacy paths - if @category.nil? && parts.last =~ /\A\d+-category/ - @category = Category.find_by_id(parts.last.to_i) - end + @category = Category.find_by_slug_path_with_id(category_slug_path_with_id) + if @category.nil? + raise Discourse::NotFound.new("category not found", check_permalinks: true) end - raise Discourse::NotFound.new("category not found", check_permalinks: true) if @category.nil? + params[:category] = @category.id.to_s if !guardian.can_see?(@category) if SiteSetting.detailed_404 @@ -364,13 +353,13 @@ class ListController < ApplicationController end end - current_slug = params.require(:category_slug_path_with_id) - real_slug = @category.full_slug("/") - + # Check if the category slug is incorrect and redirect to a link containing + # the correct one. + current_slug = category_slug_path_with_id if SiteSetting.slug_generation_method == "encoded" current_slug = current_slug.split("/").map { |slug| CGI.escape(slug) }.join("/") end - + real_slug = @category.full_slug("/") if current_slug != real_slug url = request.fullpath.gsub(current_slug, real_slug) if ActionController::Base.config.relative_url_root @@ -380,14 +369,13 @@ class ListController < ApplicationController return redirect_to path(url), status: 301 end - params[:category] = @category.id.to_s - @description_meta = if @category.uncategorized? - I18n.t('category.uncategorized_description', locale: SiteSetting.default_locale) - else + I18n.t("category.uncategorized_description", locale: SiteSetting.default_locale) + elsif @category.description_text.present? @category.description_text + else + SiteSetting.site_description end - @description_meta = SiteSetting.site_description if @description_meta.blank? if use_crawler_layout? @subcategories = @category.subcategories.select { |c| guardian.can_see?(c) } diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index c6e5810a4e5..bc2e5c106b2 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -18,7 +18,7 @@ class TagsController < ::ApplicationController skip_before_action :check_xhr, only: [:tag_feed, :show, :index] - before_action :set_category_from_params, except: [:index, :update, :destroy, + before_action :set_category, except: [:index, :update, :destroy, :tag_feed, :search, :notifications, :update_notifications, :personal_messages, :info] before_action :fetch_tag, only: [:info, :create_synonyms, :destroy_synonym] @@ -92,8 +92,9 @@ class TagsController < ::ApplicationController @description_meta = I18n.t("rss_by_tag", tag: tag_params.join(' & ')) @title = @description_meta - path_name = url_method(params.slice(:category, :parent_category)) - canonical_url "#{Discourse.base_url_no_prefix}#{public_send(path_name, *(params.slice(:parent_category, :category, :tag_id).values.map { |t| t.force_encoding("UTF-8") }))}" + canonical_params = params.slice(:category_slug_path_with_id, :tag_id) + canonical_method = url_method(canonical_params) + canonical_url "#{Discourse.base_url_no_prefix}#{public_send(canonical_method, *(canonical_params.values.map { |t| t.force_encoding("UTF-8") }))}" if @list.topics.size == 0 && params[:tag_id] != 'none' && !Tag.where_name(@tag_id).exists? raise Discourse::NotFound.new("tag not found", check_permalinks: true) @@ -347,25 +348,9 @@ class TagsController < ::ApplicationController end.compact end - def set_category_from_params + def set_category if request.path_parameters.include?(:category_slug_path_with_id) - parts = params[:category_slug_path_with_id].split('/') - - if !parts.empty? && parts.last =~ /\A\d+\Z/ - id = parts.pop.to_i - end - slug_path = parts unless parts.empty? - - if id.present? - @filter_on_category = Category.find_by_id(id) - elsif slug_path.present? - @filter_on_category = Category.find_by_slug_path(slug_path) - - # Legacy paths - if @filter_on_category.nil? && parts.last =~ /\A\d+-category/ - @filter_on_category = Category.find_by_id(parts.last.to_i) - end - end + @filter_on_category = Category.find_by_slug_path_with_id(params[:category_slug_path_with_id]) else slug_or_id = params[:category] return true if slug_or_id.nil? @@ -373,9 +358,16 @@ class TagsController < ::ApplicationController @filter_on_category = Category.query_category(slug_or_id, nil) end - category_redirect_or_not_found && (return) if !@filter_on_category - guardian.ensure_can_see!(@filter_on_category) + + if !@filter_on_category + permalink = Permalink.find_by_url("c/#{params[:category_slug_path_with_id]}") + if permalink.present? && permalink.category_id + return redirect_to "#{Discourse::base_path}/tags#{permalink.target_url}/#{params[:tag_id]}", status: :moved_permanently + end + + raise Discourse::NotFound + end end def page_params @@ -409,9 +401,7 @@ class TagsController < ::ApplicationController end def url_method(opts = {}) - if opts[:parent_category] && opts[:category] - "tag_parent_category_category_#{action_name}_path" - elsif opts[:category] + if opts[:category_slug_path_with_id] "tag_category_#{action_name}_path" else "tag_#{action_name}_path" @@ -472,19 +462,6 @@ class TagsController < ::ApplicationController options end - def category_redirect_or_not_found - # automatic redirects for renamed categories - url = params[:parent_category] ? "c/#{params[:parent_category]}/#{params[:category]}" : "c/#{params[:category]}" - permalink = Permalink.find_by_url(url) - - if permalink.present? && permalink.category_id - redirect_to "#{Discourse.base_path}/tags#{permalink.target_url}/#{params[:tag_id]}", status: :moved_permanently - else - # redirect to 404 - raise Discourse::NotFound - end - end - def tag_params [@tag_id].concat(Array(@additional_tags)) end diff --git a/app/models/category.rb b/app/models/category.rb index 13202e4b521..2623b153de5 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -810,6 +810,17 @@ class Category < ActiveRecord::Base Category.find_by_id(query) end + def self.find_by_slug_path_with_id(slug_path_with_id) + slug_path = slug_path_with_id.split("/") + + if slug_path.last =~ /\A\d+\Z/ + id = slug_path.pop.to_i + Category.find_by_id(id) + else + Category.find_by_slug_path(slug_path) + end + end + def self.find_by_slug(category_slug, parent_category_slug = nil) return nil if category_slug.nil? diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb index 8ab04925807..f191af9e5a0 100644 --- a/app/models/topic_list.rb +++ b/app/models/topic_list.rb @@ -59,7 +59,7 @@ class TopicList < DraftableList def top_tags opts = @category ? { category: @category } : {} opts[:guardian] = Guardian.new(@current_user) - Tag.top_tags(opts) + Tag.top_tags(**opts) end def preload_key diff --git a/config/routes.rb b/config/routes.rb index 791621e5f4e..c06c1cd3e33 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -921,23 +921,6 @@ Discourse::Application.routes.draw do get '/intersection/:tag_id/*additional_tag_ids' => 'tags#show', as: 'tag_intersection' end - - # legacy routes - constraints(tag_id: /[^\/]+?/, format: /json|rss/) do - get '/:tag_id.rss' => 'tags#tag_feed' - get '/:tag_id' => 'tags#show' - get '/:tag_id/info' => 'tags#info' - get '/:tag_id/notifications' => 'tags#notifications' - put '/:tag_id/notifications' => 'tags#update_notifications' - put '/:tag_id' => 'tags#update' - delete '/:tag_id' => 'tags#destroy' - post '/:tag_id/synonyms' => 'tags#create_synonyms' - delete '/:tag_id/synonyms/:synonym_id' => 'tags#destroy_synonym' - - Discourse.filters.each do |filter| - get "/:tag_id/l/#{filter}" => "tags#show_#{filter}" - end - end end resources :tag_groups, constraints: StaffConstraint.new, except: [:edit] do diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index 7ef2a23bd4f..b5c53cf4fb7 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -235,7 +235,7 @@ describe 'tags' do end end - path '/tags/{name}.json' do + path '/tag/{name}.json' do get 'Get a specific tag' do tags 'Tags'