FIX: More encoded slug fixes (#8191)

FIX: More encoded slug fixes (#8191)

  • FIX: Do not encode the URL twice

Now that we encode slugs in the server we don’t need this anymore.

Reverts fe5na33

  • FIX: More places do deal with encoded slugs

  • the param is a string now, not a hash

  • FIX: Handle the nil slug on /categories

  • DEV: Add seeded? method to identity default categories

  • DEV: Use SiteSetting to keep track of seeded categories

diff --git a/app/assets/javascripts/discourse/components/share-popup.js.es6 b/app/assets/javascripts/discourse/components/share-popup.js.es6
index dad5d68949..10e723a196 100644
--- a/app/assets/javascripts/discourse/components/share-popup.js.es6
+++ b/app/assets/javascripts/discourse/components/share-popup.js.es6
@@ -85,7 +85,7 @@ export default Ember.Component.extend({
     if (!this.site.mobileView) {
       $this.css({ left: "" + x + "px" });
     }
-    this.set("link", encodeURI(url));
+    this.set("link", url);
     this.set("visible", true);
 
     Ember.run.scheduleOnce("afterRender", this, this._focusUrl);
diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6
index 8f53b70f5d..11bdbf9b8a 100644
--- a/app/assets/javascripts/discourse/models/category.js.es6
+++ b/app/assets/javascripts/discourse/models/category.js.es6
@@ -243,7 +243,11 @@ Category.reopenClass({
   },
 
   findSingleBySlug(slug) {
-    return Category.list().find(c => Category.slugFor(c) === slug);
+    if (Discourse.SiteSettings.slug_generation_method !== "encoded") {
+      return Category.list().find(c => Category.slugFor(c) === slug);
+    } else {
+      return Category.list().find(c => Category.slugFor(c) === encodeURI(slug));
+    }
   },
 
   findById(id) {
diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index a548e6c815..c20c08bcac 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -18,7 +18,7 @@ class CategoriesController < ApplicationController
 
     @description = SiteSetting.site_description
 
-    parent_category = Category.find_by(slug: params[:parent_category_id]) || Category.find_by(id: params[:parent_category_id].to_i)
+    parent_category = Category.find_by_slug(params[:parent_category_id]) || Category.find_by(id: params[:parent_category_id].to_i)
 
     category_options = {
       is_homepage: current_homepage == "categories".freeze,
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index d806413fbc..1b2bc12702 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -36,7 +36,7 @@ class TopicsController < ApplicationController
   skip_before_action :check_xhr, only: [:show, :feed]
 
   def id_for_slug
-    topic = Topic.find_by(slug: params[:slug].downcase)
+    topic = Topic.find_by_slug(params[:slug])
     guardian.ensure_can_see!(topic)
     raise Discourse::NotFound unless topic
     render json: { slug: topic.slug, topic_id: topic.id, url: topic.url }
@@ -64,7 +64,7 @@ class TopicsController < ApplicationController
     # Special case: a slug with a number in front should look by slug first before looking
     # up that particular number
     if params[:id] && params[:id] =~ /^\d+[^\d\\]+$/
-      topic = Topic.find_by(slug: params[:id].downcase)
+      topic = Topic.find_by_slug(params[:id])
       return redirect_to_correct_topic(topic, opts[:post_number]) if topic
     end
 
@@ -81,7 +81,7 @@ class TopicsController < ApplicationController
       @topic_view = TopicView.new(params[:id] || params[:topic_id], current_user, opts)
     rescue Discourse::NotFound => ex
       if params[:id]
-        topic = Topic.find_by(slug: params[:id].downcase)
+        topic = Topic.find_by_slug(params[:id])
         return redirect_to_correct_topic(topic, opts[:post_number]) if topic
       end
 
diff --git a/app/jobs/onceoff/fix_encoded_category_slugs.rb b/app/jobs/onceoff/fix_encoded_category_slugs.rb
new file mode 100644
index 0000000000..28947125d9
--- /dev/null
+++ b/app/jobs/onceoff/fix_encoded_category_slugs.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module Jobs
+
+  class FixEncodedCategorySlugs < ::Jobs::Onceoff
+    def execute_onceoff(args)
+      return unless SiteSetting.slug_generation_method == 'encoded'
+
+      #Make custom categories slugs nil and let the app regenerate with proper encoded ones
+      Category.all.reject { |c| c.seeded? }.each do |c|
+        c.slug = nil
+        c.save!
+      end
+    end
+  end
+
+end
diff --git a/app/models/category.rb b/app/models/category.rb
index 2dcdd7d1e8..e76089baad 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -612,7 +612,8 @@ class Category < ActiveRecord::Base
   end
 
   def self.query_category(slug_or_id, parent_category_id)
-    self.where(slug: slug_or_id, parent_category_id: parent_category_id).first ||
+    encoded_slug_or_id = CGI.escape(slug_or_id) if SiteSetting.slug_generation_method == 'encoded'
+    self.where(slug: (encoded_slug_or_id || slug_or_id), parent_category_id: parent_category_id).first ||
     self.where(id: slug_or_id.to_i, parent_category_id: parent_category_id).first
   end
 
@@ -629,6 +630,15 @@ class Category < ActiveRecord::Base
     id == SiteSetting.uncategorized_category_id
   end
 
+  def seeded?
+    [
+      SiteSetting.lounge_category_id,
+      SiteSetting.meta_category_id,
+      SiteSetting.staff_category_id,
+      SiteSetting.uncategorized_category_id,
+    ].include? id
+  end
+
   @@url_cache = DistributedCache.new('category_url')
 
   def clear_url_cache
@@ -708,6 +718,14 @@ class Category < ActiveRecord::Base
   end
 
   def self.find_by_slug(category_slug, parent_category_slug = nil)
+
+    return nil if category_slug.nil?
+
+    if SiteSetting.slug_generation_method == "encoded"
+      parent_category_slug = CGI.escape(parent_category_slug) unless parent_category_slug.nil?
+      category_slug = CGI.escape(category_slug)
+    end
+
     if parent_category_slug
       parent_category_id = self.where(slug: parent_category_slug, parent_category_id: nil).select(:id)
 
diff --git a/app/models/topic.rb b/app/models/topic.rb
index ed98ba47f3..9061198bab 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -988,6 +988,15 @@ class Topic < ActiveRecord::Base
     slug
   end
 
+  def self.find_by_slug(slug)
+    if SiteSetting.slug_generation_method != "encoded"
+      Topic.find_by(slug: slug.downcase)
+    else
+      encoded_slug = CGI.escape(slug)
+      Topic.find_by(slug: encoded_slug)
+    end
+  end
+
   def title=(t)
     slug = Slug.for(t.to_s)
     write_attribute(:slug, slug)
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 2f3962e4c0..603ac3cc89 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1730,6 +1730,7 @@ uncategorized:
   slug_generation_method:
     default: "ascii"
     enum: "SlugSetting"
+    client: true
     locale_default:
       ja: "none"
       zh_CN: "none"

GitHub sha: 6e9c8fe8