From 3fd6ab99e69e736e10c7022f07dfa34b60a4e54d Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 5 Jul 2019 02:14:56 +0200 Subject: [PATCH] Remove deprecated REST API `GET /api/v1/timelines/direct` (#11212) --- .../api/v1/timelines/direct_controller.rb | 63 ------------------- app/models/status.rb | 41 ------------ config/routes.rb | 1 - .../v1/timelines/direct_controller_spec.rb | 17 ----- spec/models/status_spec.rb | 50 --------------- 5 files changed, 172 deletions(-) delete mode 100644 app/controllers/api/v1/timelines/direct_controller.rb delete mode 100644 spec/controllers/api/v1/timelines/direct_controller_spec.rb diff --git a/app/controllers/api/v1/timelines/direct_controller.rb b/app/controllers/api/v1/timelines/direct_controller.rb deleted file mode 100644 index d8a76d153..000000000 --- a/app/controllers/api/v1/timelines/direct_controller.rb +++ /dev/null @@ -1,63 +0,0 @@ -# frozen_string_literal: true - -class Api::V1::Timelines::DirectController < Api::BaseController - before_action -> { doorkeeper_authorize! :read, :'read:statuses' }, only: [:show] - before_action :require_user!, only: [:show] - after_action :insert_pagination_headers, unless: -> { @statuses.empty? } - - respond_to :json - - def show - @statuses = load_statuses - render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) - end - - private - - def load_statuses - cached_direct_statuses - end - - def cached_direct_statuses - cache_collection direct_statuses, Status - end - - def direct_statuses - direct_timeline_statuses - end - - def direct_timeline_statuses - # this query requires built in pagination. - Status.as_direct_timeline( - current_account, - limit_param(DEFAULT_STATUSES_LIMIT), - params[:max_id], - params[:since_id], - true # returns array of cache_ids object - ) - end - - def insert_pagination_headers - set_pagination_headers(next_path, prev_path) - end - - def pagination_params(core_params) - params.permit(:local, :limit).merge(core_params) - end - - def next_path - api_v1_timelines_direct_url pagination_params(max_id: pagination_max_id) - end - - def prev_path - api_v1_timelines_direct_url pagination_params(since_id: pagination_since_id) - end - - def pagination_max_id - @statuses.last.id - end - - def pagination_since_id - @statuses.first.id - end -end diff --git a/app/models/status.rb b/app/models/status.rb index fb9bbc9a9..2258e2d07 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -280,47 +280,6 @@ class Status < ApplicationRecord where(account: [account] + account.following).where(visibility: [:public, :unlisted, :private]) end - def as_direct_timeline(account, limit = 20, max_id = nil, since_id = nil, cache_ids = false) - # direct timeline is mix of direct message from_me and to_me. - # 2 queries are executed with pagination. - # constant expression using arel_table is required for partial index - - # _from_me part does not require any timeline filters - query_from_me = where(account_id: account.id) - .where(Status.arel_table[:visibility].eq(3)) - .limit(limit) - .order('statuses.id DESC') - - # _to_me part requires mute and block filter. - # FIXME: may we check mutes.hide_notifications? - query_to_me = Status - .joins(:mentions) - .merge(Mention.where(account_id: account.id)) - .where(Status.arel_table[:visibility].eq(3)) - .limit(limit) - .order('mentions.status_id DESC') - .not_excluded_by_account(account) - - if max_id.present? - query_from_me = query_from_me.where('statuses.id < ?', max_id) - query_to_me = query_to_me.where('mentions.status_id < ?', max_id) - end - - if since_id.present? - query_from_me = query_from_me.where('statuses.id > ?', since_id) - query_to_me = query_to_me.where('mentions.status_id > ?', since_id) - end - - if cache_ids - # returns array of cache_ids object that have id and updated_at - (query_from_me.cache_ids.to_a + query_to_me.cache_ids.to_a).uniq(&:id).sort_by(&:id).reverse.take(limit) - else - # returns ActiveRecord.Relation - items = (query_from_me.select(:id).to_a + query_to_me.select(:id).to_a).uniq(&:id).sort_by(&:id).reverse.take(limit) - Status.where(id: items.map(&:id)) - end - end - def as_public_timeline(account = nil, local_only = false) query = timeline_scope(local_only).without_replies diff --git a/config/routes.rb b/config/routes.rb index 764db8db2..29db54a32 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -299,7 +299,6 @@ Rails.application.routes.draw do end namespace :timelines do - resource :direct, only: :show, controller: :direct resource :home, only: :show, controller: :home resource :public, only: :show, controller: :public resources :tag, only: :show diff --git a/spec/controllers/api/v1/timelines/direct_controller_spec.rb b/spec/controllers/api/v1/timelines/direct_controller_spec.rb deleted file mode 100644 index a22c2cbea..000000000 --- a/spec/controllers/api/v1/timelines/direct_controller_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Api::V1::Timelines::DirectController, type: :controller do - let(:user) { Fabricate(:user) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:statuses') } - - describe 'GET #show' do - it 'returns 200' do - allow(controller).to receive(:doorkeeper_token) { token } - get :show - - expect(response).to have_http_status(200) - end - end -end diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index e8cf18af9..a8c567414 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -339,56 +339,6 @@ RSpec.describe Status, type: :model do end end - describe '.as_direct_timeline' do - let(:account) { Fabricate(:account) } - let(:followed) { Fabricate(:account) } - let(:not_followed) { Fabricate(:account) } - - before do - Fabricate(:follow, account: account, target_account: followed) - - @self_public_status = Fabricate(:status, account: account, visibility: :public) - @self_direct_status = Fabricate(:status, account: account, visibility: :direct) - @followed_public_status = Fabricate(:status, account: followed, visibility: :public) - @followed_direct_status = Fabricate(:status, account: followed, visibility: :direct) - @not_followed_direct_status = Fabricate(:status, account: not_followed, visibility: :direct) - - @results = Status.as_direct_timeline(account) - end - - it 'does not include public statuses from self' do - expect(@results).to_not include(@self_public_status) - end - - it 'includes direct statuses from self' do - expect(@results).to include(@self_direct_status) - end - - it 'does not include public statuses from followed' do - expect(@results).to_not include(@followed_public_status) - end - - it 'does not include direct statuses not mentioning recipient from followed' do - expect(@results).to_not include(@followed_direct_status) - end - - it 'does not include direct statuses not mentioning recipient from non-followed' do - expect(@results).to_not include(@not_followed_direct_status) - end - - it 'includes direct statuses mentioning recipient from followed' do - Fabricate(:mention, account: account, status: @followed_direct_status) - results2 = Status.as_direct_timeline(account) - expect(results2).to include(@followed_direct_status) - end - - it 'includes direct statuses mentioning recipient from non-followed' do - Fabricate(:mention, account: account, status: @not_followed_direct_status) - results2 = Status.as_direct_timeline(account) - expect(results2).to include(@not_followed_direct_status) - end - end - describe '.as_public_timeline' do it 'only includes statuses with public visibility' do public_status = Fabricate(:status, visibility: :public)