Merge branch 'fix-federation' into 'develop'

Fix federated threads

See merge request gab/social/gab-social!44

app/controllers/accounts_controller.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class AccountsController < ApplicationController
+class AccountsController < ReactController
   PAGE_SIZE = 20
 
   include AccountControllerConcern
@@ -11,24 +11,7 @@ class AccountsController < ApplicationController
     respond_to do |format|
       format.html do
         mark_cacheable! unless user_signed_in?
-
+        return process(:react)
-        @body_classes      = 'with-modals'
-        @pinned_statuses   = []
-        @endorsed_accounts = @account.endorsed_accounts.to_a.sample(4)
-
-        if current_account && @account.blocking?(current_account)
-          @statuses = []
-          return
-        end
-
-        @pinned_statuses = cache_collection(@account.pinned_statuses, Status) if show_pinned_statuses?
-        @statuses        = filtered_status_page(params)
-        @statuses        = cache_collection(@statuses, Status)
-
-        unless @statuses.empty?
-          @older_url = older_url if @statuses.last.id > filtered_statuses.last.id
-          @newer_url = newer_url if @statuses.first.id < filtered_statuses.first.id
-        end
       end
 
       format.atom do

app/controllers/api/v1/account_by_username_controller.rb

@@ -11,12 +11,7 @@ class Api::V1::AccountByUsernameController < Api::BaseController
   end
 
   def set_account
-    username, domain = params[:username].split("@")
+    @account = Account.find_acct!(params[:username])
-    if domain
-      @account = Account.find_remote!(username, domain)
-    else
-      @account = Account.find_local!(username)
-    end
   end
 
   def check_account_suspension

app/controllers/concerns/account_controller_concern.rb

@@ -6,8 +6,6 @@ module AccountControllerConcern
   FOLLOW_PER_PAGE = 12
 
   included do
-    layout 'public'
-
     before_action :set_account
     before_action :check_account_approval
     before_action :check_account_suspension
@@ -18,7 +16,7 @@ module AccountControllerConcern
   private
 
   def set_account
-    @account = Account.find_local!(username_param)
+    @account = Account.find_acct!(username_param)
   end
 
   def set_instance_presenter
@@ -26,6 +24,8 @@ module AccountControllerConcern
   end
 
   def set_link_headers
+    return if !@account.local? # TODO: Handle remote users
+
     response.headers['Link'] = LinkHeader.new(
       [
         webfinger_account_link,

app/controllers/react_controller.rb

@@ -1,12 +1,12 @@
 # frozen_string_literal: true
 
-class HomeController < ApplicationController
+class ReactController < ApplicationController
-  before_action :authenticate_user!
+  before_action :authenticate_user!, only: :react
-  before_action :set_referrer_policy_header
+  before_action :set_referrer_policy_header, only: :react
-  before_action :set_initial_state_json
+  before_action :set_initial_state_json, only: :react
-  before_action :set_data_for_meta
+  before_action :set_data_for_meta, only: :react
 
-  def index
+  def react
     @body_classes = 'app-body'
   end
 
@@ -15,18 +15,6 @@ class HomeController < ApplicationController
   def set_data_for_meta
     return if find_route_matches
 
-    if params[:username].present?
-      @account = Account.find_local(params[:username])
-    elsif params[:account_username].present?
-      @account = Account.find_local(params[:account_username])
-
-      if params[:id].present? && !@account.nil?
-        @status = @account.statuses.find(params[:id])
-        @stream_entry = @status.stream_entry
-        @type = @stream_entry.activity_type.downcase
-      end
-    end
-
     if request.path.starts_with?('/tags') && params[:tag].present?
       @tag = Tag.find_normalized(params[:tag])
     end

app/controllers/statuses_controller.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class StatusesController < ApplicationController
+class StatusesController < ReactController
   include SignatureAuthentication
   include Authorization
 
@@ -8,8 +8,6 @@ class StatusesController < ApplicationController
   DESCENDANTS_LIMIT       = 60
   DESCENDANTS_DEPTH_LIMIT = 20
 
-  layout 'public'
-
   before_action :set_account
   before_action :set_status
   before_action :set_instance_presenter
@@ -32,12 +30,7 @@ class StatusesController < ApplicationController
           expires_in 10.seconds, public: true
         end
 
-        @body_classes = 'with-modals'
+        return process(:react)
-
-        set_ancestors
-        set_descendants
-
-        render 'stream_entries/show'
       end
 
       format.json do
@@ -111,7 +104,7 @@ class StatusesController < ApplicationController
   end
 
   def set_account
-    @account = Account.find_local!(params[:account_username])
+    @account = Account.find_acct!(params[:account_username])
   end
 
   def set_ancestors
@@ -174,6 +167,8 @@ class StatusesController < ApplicationController
   end
 
   def set_link_headers
+    return if !@account.local? # TODO: Handle remote accounts
+
     response.headers['Link'] = LinkHeader.new(
       [
         [account_stream_entry_url(@account, @status.stream_entry, format: 'atom'), [%w(rel alternate), %w(type application/atom+xml)]],
@@ -185,7 +180,7 @@ class StatusesController < ApplicationController
   def set_status
     @status       = @account.statuses.find(params[:id])
     @stream_entry = @status.stream_entry
-    @type         = @stream_entry.activity_type.downcase
+    @type         = @stream_entry&.activity_type&.downcase
 
     authorize @status, :show?
   rescue GabSocial::NotPermittedError

app/lib/status_finder.rb

@@ -15,7 +15,7 @@ class StatusFinder
     case recognized_params[:controller]
     when 'stream_entries'
       StreamEntry.find(recognized_params[:id]).status
-    when 'home'
+    when 'statuses'
       Status.find(recognized_params[:id])
     else
       raise ActiveRecord::RecordNotFound
@@ -29,7 +29,7 @@ class StatusFinder
   end
 
   def verify_action!
-    unless recognized_params[:action] == 'show' || recognized_params[:action] == 'index'
+    unless recognized_params[:action] == 'show'
       raise ActiveRecord::RecordNotFound
     end
   end

app/models/concerns/account_finder_concern.rb

@@ -12,6 +12,10 @@ module AccountFinderConcern
       find_remote(username, domain) || raise(ActiveRecord::RecordNotFound)
     end
 
+    def find_acct!(acct)
+      find_acct(acct) || raise(ActiveRecord::RecordNotFound)
+    end
+
     def representative
       find_local(Setting.site_contact_username.strip.gsub(/\A@/, '')) || Account.local.without_suspended.first
     end
@@ -23,6 +27,11 @@ module AccountFinderConcern
     def find_remote(username, domain)
       AccountFinder.new(username, domain).account
     end
+
+    def find_acct(acct)
+      username, domain = acct.split("@")
+      find_remote(username, domain)
+    end
   end
 
   class AccountFinder

app/views/layouts/application.html.haml

@@ -22,7 +22,7 @@
       = render 'tags/meta', tag: @tag, initial_state_json: @initial_state_json
     - elsif @stream_entry && @account
       = render 'stream_entries/meta', stream_entry: @stream_entry, account: @account
-    - elsif @account
+    - elsif @account && @account.local?
       = render 'accounts/meta', account: @account, older_url: nil, newer_url: nil
 
     %title= content_for?(:page_title) ? safe_join([yield(:page_title).chomp.html_safe, title], ' - ') : title

config/routes.rb

@@ -5,6 +5,9 @@ require 'sidekiq-scheduler/web'
 
 Sidekiq::Web.set :session_secret, Rails.application.secrets[:secret_key_base]
 
+username_regex = /([^\/]*)/
+html_only = lambda { |req| req.format.nil? || req.format.html? }
+
 Rails.application.routes.draw do
   mount LetterOpenerWeb::Engine, at: 'letter_opener' if Rails.env.development?
 
@@ -41,7 +44,11 @@ Rails.application.routes.draw do
     confirmations:      'auth/confirmations',
   }
 
-  get '/users/:username', to: redirect('/%{username}'), constraints: lambda { |req| req.format.nil? || req.format.html? }
+  get '/users/:username', to: redirect('/%{username}'), constraints: html_only, username: username_regex
+  get '/users/:username/followers', to: redirect('/%{username}/followers'), constraints: html_only, username: username_regex
+  get '/users/:username/following', to: redirect('/%{username}/following'), constraints: html_only, username: username_regex
+  get '/users/:username/statuses/:id', to: redirect('/%{username}/posts/%{id}'), constraints: html_only, username: username_regex
+
   get '/authorize_follow', to: redirect { |_, request| "/authorize_interaction?#{request.params.to_query}" }
 
   resources :accounts, path: 'users', only: [:show], param: :username do
@@ -240,7 +247,7 @@ Rails.application.routes.draw do
     resources :users, only: [] do
       resource :two_factor_authentication, only: [:destroy]
     end
-    
+
     resources :custom_emojis, only: [:index, :new, :create, :update, :destroy] do
       member do
         post :copy
@@ -335,7 +342,7 @@ Rails.application.routes.draw do
 
       get '/search', to: 'search#index', as: :search
 
-      get '/account_by_username/:username', to: 'account_by_username#show', username: /(.*)/
+      get '/account_by_username/:username', to: 'account_by_username#show', username: username_regex
 
       resources :follows,      only: [:create]
       resources :media,        only: [:create, :update]
@@ -452,17 +459,17 @@ Rails.application.routes.draw do
   get '/about/dmca',         to: 'about#dmca'
   get '/about/sales',        to: 'about#sales'
 
-  get '/tags/:tag', to: 'home#index'
+  get '/tags/:tag', to: 'react#react'
-  get '/:username', to: 'home#index', as: :short_account
+  get '/:username/with_replies', to: 'accounts#show', username: username_regex, as: :short_account_with_replies
-  get '/:username/with_replies', to: 'home#index', as: :short_account_with_replies
+  get '/:username/media', to: 'accounts#show', username: username_regex, as: :short_account_media
-  get '/:username/media', to: 'home#index', as: :short_account_media
+  get '/:username/tagged/:tag', to: 'accounts#show', username: username_regex, as: :short_account_tag
-  get '/:username/tagged/:tag', to: 'home#index', as: :short_account_tag
+  get '/:username/posts/:statusId/reblogs', to: 'statuses#show', username: username_regex
-  get '/:username/posts/:statusId/reblogs', to: 'home#index'
+  get '/:account_username/posts/:id', to: 'statuses#show', account_username: username_regex, as: :short_account_status
-  get '/:account_username/posts/:id', to: 'home#index', as: :short_account_status
+  get '/:account_username/posts/:id/embed', to: 'statuses#embed', account_username: username_regex, as: :embed_short_account_status
-  get '/:account_username/posts/:id/embed', to: 'statuses#embed', as: :embed_short_account_status
 
-  get '/(*any)', to: 'home#index', as: :web
+  get '/(*any)', to: 'react#react', as: :web
-  root 'home#index'
+  get '/:username', to: 'accounts#show', username: username_regex, as: :short_account
+  root 'react#react'
 
   # Routes that are now to be used within webapp, but still referenced within application
   # TODO : Consolidate

spec/controllers/accounts_controller_spec.rb

@@ -63,20 +63,6 @@ RSpec.describe AccountsController, type: :controller do
       end
 
       include_examples 'responses'
-
-      context 'without max_id nor since_id' do
-        let(:expected_statuses) { [status7, status6, status5, status4, status3, status2, status1] }
-
-        include_examples 'responsed streams'
-      end
-
-      context 'with max_id and since_id' do
-        let(:max_id) { status4.stream_entry.id }
-        let(:since_id) { status1.stream_entry.id }
-        let(:expected_statuses) { [status3, status2] }
-
-        include_examples 'responsed streams'
-      end
     end
 
     context 'activitystreams2' do
@@ -90,54 +76,8 @@ RSpec.describe AccountsController, type: :controller do
       let(:format) { nil }
       let(:content_type) { 'text/html' }
 
-      shared_examples 'responsed statuses' do
-        it 'assigns @pinned_statuses' do
-          pinned_statuses = assigns(:pinned_statuses).to_a
-          expect(pinned_statuses.size).to eq expected_pinned_statuses.size
-          pinned_statuses.each.zip(expected_pinned_statuses.each) do |pinned_status, expected_pinned_status|
-            expect(pinned_status).to eq expected_pinned_status
-          end
-        end
-
-        it 'assigns @statuses' do
-          statuses = assigns(:statuses).to_a
-          expect(statuses.size).to eq expected_statuses.size
-          statuses.each.zip(expected_statuses.each) do |status, expected_status|
-            expect(status).to eq expected_status
-          end
-        end
-      end
 
       include_examples 'responses'
-
-      context 'with anonymous visitor' do
-        context 'without since_id nor max_id' do
-          let(:expected_statuses) { [status7, status6, status5, status4, status3, status2, status1] }
-          let(:expected_pinned_statuses) { [status7, status5, status6] }
-
-          include_examples 'responsed statuses'
-        end
-
-        context 'with since_id nor max_id' do
-          let(:max_id) { status4.id }
-          let(:since_id) { status1.id }
-          let(:expected_statuses) { [status3, status2] }
-          let(:expected_pinned_statuses) { [] }
-
-          include_examples 'responsed statuses'
-        end
-      end
-
-      context 'with blocked visitor' do
-        let(:current_user) { eve }
-
-        context 'without since_id nor max_id' do
-          let(:expected_statuses) { [] }
-          let(:expected_pinned_statuses) { [] }
-
-          include_examples 'responsed statuses'
-        end
-      end
     end
   end
 end

spec/controllers/react_controller_spec.rb

@@ -1,10 +1,10 @@
 require 'rails_helper'
 
-RSpec.describe HomeController, type: :controller do
+RSpec.describe ReactController, type: :controller do
   render_views
 
-  describe 'GET #index' do
+  describe 'GET #react' do
-    subject { get :index }
+    subject { get :react }
 
     context 'when not signed in' do
       context 'when requested path is tag timeline' do

spec/controllers/statuses_controller_spec.rb

@@ -67,78 +67,16 @@ describe StatusesController do
         expect(assigns(:type)).to eq 'status'
       end
 
-      it 'assigns @ancestors for ancestors of the status if it is a reply' do
-        ancestor = Fabricate(:status)
-        status = Fabricate(:status, in_reply_to_id: ancestor.id)
-
-        get :show, params: { account_username: status.account.username, id: status.id }
-
-        expect(assigns(:ancestors)).to eq [ancestor]
-      end
-
-      it 'assigns @ancestors for [] if it is not a reply' do
-        status = Fabricate(:status)
-        get :show, params: { account_username: status.account.username, id: status.id }
-        expect(assigns(:ancestors)).to eq []
-      end
-
-      it 'assigns @descendant_threads for a thread with several statuses' do
-        status = Fabricate(:status)
-        child = Fabricate(:status, in_reply_to_id: status.id)
-        grandchild = Fabricate(:status, in_reply_to_id: child.id)
-
-        get :show, params: { account_username: status.account.username, id: status.id }
-
-        expect(assigns(:descendant_threads)[0][:statuses].pluck(:id)).to eq [child.id, grandchild.id]
-      end
-
-      it 'assigns @descendant_threads for several threads sharing the same descendant' do
-        status = Fabricate(:status)
-        child = Fabricate(:status, in_reply_to_id: status.id)
-        grandchildren = 2.times.map { Fabricate(:status, in_reply_to_id: child.id) }
-
-        get :show, params: { account_username: status.account.username, id: status.id }
-
-        expect(assigns(:descendant_threads)[0][:statuses].pluck(:id)).to eq [child.id, grandchildren[0].id]
-        expect(assigns(:descendant_threads)[1][:statuses].pluck(:id)).to eq [grandchildren[1].id]
-      end
-
-      it 'assigns @max_descendant_thread_id for the last thread if it is hitting the status limit' do
-        stub_const 'StatusesController::DESCENDANTS_LIMIT', 1
-        status = Fabricate(:status)
-        child = Fabricate(:status, in_reply_to_id: status.id)
-
-        get :show, params: { account_username: status.account.username, id: status.id }
-
-        expect(assigns(:descendant_threads)).to eq []
-        expect(assigns(:max_descendant_thread_id)).to eq child.id
-      end
-
-      it 'assigns @descendant_threads for threads with :next_status key if they are hitting the depth limit' do
-        stub_const 'StatusesController::DESCENDANTS_DEPTH_LIMIT', 2
-        status = Fabricate(:status)
-        child0 = Fabricate(:status, in_reply_to_id: status.id)
-        child1 = Fabricate(:status, in_reply_to_id: child0.id)
-        child2 = Fabricate(:status, in_reply_to_id: child0.id)
-
-        get :show, params: { account_username: status.account.username, id: status.id }
-
-        expect(assigns(:descendant_threads)[0][:statuses].pluck(:id)).not_to include child1.id
-        expect(assigns(:descendant_threads)[1][:statuses].pluck(:id)).not_to include child2.id
-        expect(assigns(:descendant_threads)[0][:next_status].id).to eq child1.id
-        expect(assigns(:descendant_threads)[1][:next_status].id).to eq child2.id
-      end
-
       it 'returns a success' do
         status = Fabricate(:status)
         get :show, params: { account_username: status.account.username, id: status.id }
         expect(response).to have_http_status(200)
       end
 
-      it 'renders stream_entries/show' do
+      it 'renders the React front-end' do
         status = Fabricate(:status)
         get :show, params: { account_username: status.account.username, id: status.id }
-        expect(response).to render_template 'stream_entries/show'
+        expect(response).to render_template 'react/react'
       end
     end
   end