FIX: check permalinks for deleted topics

- allow to specify 410 vs 404 in Discourse::NotFound exception
- remove unused `permalink_redirect_or_not_found` which
- handle JS side links to topics via Discourse-Xhr-Redirect mechanism

app/assets/javascripts/discourse/lib/ajax.js.es6

@@ -29,6 +29,17 @@ export function handleLogoff(xhr) {
   }
 }
 
+function handleRedirect(data) {
+  if (
+    data &&
+    data.getResponseHeader &&
+    data.getResponseHeader("Discourse-Xhr-Redirect")
+  ) {
+    window.location.replace(data.responseText);
+    window.location.reload();
+  }
+}
+
 /**
   Our own $.ajax method. Makes sure the .then method executes in an Ember runloop
   for performance reasons. Also automatically adjusts the URL to support installs
@@ -76,6 +87,7 @@ export function ajax() {
     }
 
     args.success = (data, textStatus, xhr) => {
+      handleRedirect(data);
       handleLogoff(xhr);
 
       Ember.run(() => {

app/controllers/application_controller.rb

@@ -173,7 +173,16 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  rescue_from Discourse::NotFound, PluginDisabled, ActionController::RoutingError  do
+  rescue_from Discourse::NotFound do |e|
+    rescue_discourse_actions(
+      :not_found,
+      e.status,
+      check_permalinks: e.check_permalinks,
+      original_path: e.original_path
+    )
+  end
+
+  rescue_from PluginDisabled, ActionController::RoutingError  do
     rescue_discourse_actions(:not_found, 404)
   end
 
@@ -194,12 +203,37 @@ class ApplicationController < ActionController::Base
     render_json_error I18n.t('read_only_mode_enabled'), type: :read_only, status: 503
   end
 
+  def redirect_with_client_support(url, options)
+    if request.xhr?
+      response.headers['Discourse-Xhr-Redirect'] = 'true'
+      render plain: url
+    else
+      redirect_to url, options
+    end
+  end
+
   def rescue_discourse_actions(type, status_code, opts = nil)
     opts ||= {}
     show_json_errors = (request.format && request.format.json?) ||
                        (request.xhr?) ||
                        ((params[:external_id] || '').ends_with? '.json')
 
+    if type == :not_found && opts[:check_permalinks]
+      url = opts[:original_path] || request.fullpath
+      permalink = Permalink.find_by_url(url)
+
+      if permalink.present?
+        # permalink present, redirect to that URL
+        if permalink.external_url
+          redirect_with_client_support permalink.external_url, status: :moved_permanently
+          return
+        elsif permalink.target_url
+          redirect_with_client_support "#{Discourse::base_uri}#{permalink.target_url}", status: :moved_permanently
+          return
+        end
+      end
+    end
+
     message = opts[:custom_message_translated] || I18n.t(opts[:custom_message] || type)
 
     if show_json_errors
@@ -444,25 +478,6 @@ class ApplicationController < ActionController::Base
     request.session_options[:skip] = true
   end
 
-  def permalink_redirect_or_not_found
-    url = request.fullpath
-    permalink = Permalink.find_by_url(url)
-
-    if permalink.present?
-      # permalink present, redirect to that URL
-      if permalink.external_url
-        redirect_to permalink.external_url, status: :moved_permanently
-      elsif permalink.target_url
-        redirect_to "#{Discourse::base_uri}#{permalink.target_url}", status: :moved_permanently
-      else
-        raise Discourse::NotFound
-      end
-    else
-      # redirect to 404
-      raise Discourse::NotFound
-    end
-  end
-
   def secure_session
     SecureSession.new(session["secure_session_id"] ||= SecureRandom.hex)
   end

app/controllers/list_controller.rb

@@ -344,7 +344,7 @@ class ListController < ApplicationController
     parent_category_id = nil
     if parent_slug_or_id.present?
       parent_category_id = Category.query_parent_category(parent_slug_or_id)
-      permalink_redirect_or_not_found && (return) if parent_category_id.blank? && !id
+      raise Discourse::NotFound.new("category not found", check_permalinks: true) if parent_category_id.blank? && !id
     end
 
     @category = Category.query_category(slug_or_id, parent_category_id)
@@ -355,7 +355,7 @@ class ListController < ApplicationController
       (redirect_to category.url, status: 301) && return if category
     end
 
-    permalink_redirect_or_not_found && (return) if !@category
+    raise Discourse::NotFound.new("category not found", check_permalinks: true) if !@category
 
     @description_meta = @category.description_text
     raise Discourse::NotFound unless guardian.can_see?(@category)

app/controllers/tags_controller.rb

@@ -90,7 +90,7 @@ class TagsController < ::ApplicationController
       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") }))}"
 
       if @list.topics.size == 0 && params[:tag_id] != 'none' && !Tag.where(name: @tag_id).exists?
-        permalink_redirect_or_not_found
+        raise Discourse::NotFound.new("tag not found", check_permalinks: true)
       else
         respond_with_list(@list)
       end

app/controllers/topics_controller.rb

@@ -135,8 +135,12 @@ class TopicsController < ApplicationController
     end
 
     if ex.obj && Topic === ex.obj && guardian.can_see_topic_if_not_deleted?(ex.obj)
-      rescue_discourse_actions(:not_found, 410)
+      raise Discourse::NotFound.new(
-      return
+        "topic was deleted",
+        status: 410,
+        check_permalinks: true,
+        original_path: ex.obj.relative_url
+      )
     end
 
     raise ex

lib/discourse.rb

@@ -77,7 +77,18 @@ module Discourse
   end
 
   # When something they want is not found
-  class NotFound < StandardError; end
+  class NotFound < StandardError
+    attr_reader :status
+    attr_reader :check_permalinks
+    attr_reader :original_path
+
+    def initialize(message = nil, status: 404, check_permalinks: false, original_path: nil)
+      @status = status
+      @check_permalinks = check_permalinks
+      @original_path = original_path
+      super(message)
+    end
+  end
 
   # When a setting is missing
   class SiteSettingMissing < StandardError; end

spec/requests/application_controller_spec.rb

@@ -17,6 +17,26 @@ RSpec.describe ApplicationController do
 
   describe 'build_not_found_page' do
     describe 'topic not found' do
+
+      it 'should return permalink for deleted topics' do
+        topic = create_post.topic
+        external_url = 'https://somewhere.over.rainbow'
+        Permalink.create!(url: topic.relative_url, external_url: external_url)
+        topic.trash!
+
+        get topic.relative_url
+        expect(response.status).to eq(301)
+        expect(response).to redirect_to(external_url)
+
+        get "/t/#{topic.id}.json"
+        expect(response.status).to eq(301)
+        expect(response).to redirect_to(external_url)
+
+        get "/t/#{topic.id}.json", xhr: true
+        expect(response.status).to eq(200)
+        expect(response.body).to eq(external_url)
+      end
+
       it 'should return 404 and show Google search' do
         get "/t/nope-nope/99999999"
         expect(response.status).to eq(404)