From edf09ec747ebba5a170e27eb13663462a116ec6c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 7 Mar 2022 09:36:47 +0100 Subject: [PATCH] Add `/api/v1/accounts/familiar_followers` to REST API (#17700) * Add `/api/v1/accounts/familiar_followers` to REST API * Change hide network preference to be stored consistently for local and remote accounts * Add dummy classes to migration * Apply suggestions from code review Co-authored-by: Claire Co-authored-by: Claire --- .../accounts/familiar_followers_controller.rb | 25 ++++++++ .../follower_accounts_controller.rb | 6 +- .../following_accounts_controller.rb | 6 +- .../settings/preferences_controller.rb | 1 - .../settings/profiles_controller.rb | 2 +- app/lib/user_settings_decorator.rb | 5 -- app/models/account.rb | 4 +- app/models/user.rb | 6 +- .../familiar_followers_presenter.rb | 17 ++++++ .../rest/familiar_followers_serializer.rb | 11 ++++ app/views/follower_accounts/index.html.haml | 2 +- app/views/following_accounts/index.html.haml | 2 +- .../settings/preferences/other/show.html.haml | 3 - app/views/settings/profiles/show.html.haml | 5 +- config/locales/simple_form.en.yml | 3 +- config/routes.rb | 1 + config/settings.yml | 1 - ...4195405_migrate_hide_network_preference.rb | 37 ++++++++++++ db/schema.rb | 2 +- .../follower_accounts_controller_spec.rb | 2 +- .../following_accounts_controller_spec.rb | 2 +- .../familiar_followers_presenter_spec.rb | 58 +++++++++++++++++++ 22 files changed, 169 insertions(+), 32 deletions(-) create mode 100644 app/controllers/api/v1/accounts/familiar_followers_controller.rb create mode 100644 app/presenters/familiar_followers_presenter.rb create mode 100644 app/serializers/rest/familiar_followers_serializer.rb create mode 100644 db/migrate/20220304195405_migrate_hide_network_preference.rb create mode 100644 spec/presenters/familiar_followers_presenter_spec.rb diff --git a/app/controllers/api/v1/accounts/familiar_followers_controller.rb b/app/controllers/api/v1/accounts/familiar_followers_controller.rb new file mode 100644 index 0000000000..b0bd8018a2 --- /dev/null +++ b/app/controllers/api/v1/accounts/familiar_followers_controller.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class Api::V1::Accounts::FamiliarFollowersController < Api::BaseController + before_action -> { doorkeeper_authorize! :read, :'read:follows' } + before_action :require_user! + before_action :set_accounts + + def index + render json: familiar_followers.accounts, each_serializer: REST::FamiliarFollowersSerializer + end + + private + + def set_accounts + @accounts = Account.without_suspended.where(id: account_ids).select('id, hide_collections').index_by(&:id).values_at(*account_ids).compact + end + + def familiar_followers + FamiliarFollowersPresenter.new(@accounts, current_user.account_id) + end + + def account_ids + Array(params[:id]).map(&:to_i) + end +end diff --git a/app/controllers/follower_accounts_controller.rb b/app/controllers/follower_accounts_controller.rb index b3589a39f8..f3f8336c95 100644 --- a/app/controllers/follower_accounts_controller.rb +++ b/app/controllers/follower_accounts_controller.rb @@ -15,13 +15,13 @@ class FollowerAccountsController < ApplicationController format.html do expires_in 0, public: true unless user_signed_in? - next if @account.user_hides_network? + next if @account.hide_collections? follows end format.json do - raise Mastodon::NotPermittedError if page_requested? && @account.user_hides_network? + raise Mastodon::NotPermittedError if page_requested? && @account.hide_collections? expires_in(page_requested? ? 0 : 3.minutes, public: public_fetch_mode?) @@ -82,7 +82,7 @@ class FollowerAccountsController < ApplicationController end def restrict_fields_to - if page_requested? || !@account.user_hides_network? + if page_requested? || !@account.hide_collections? # Return all fields else %i(id type total_items) diff --git a/app/controllers/following_accounts_controller.rb b/app/controllers/following_accounts_controller.rb index 8a72dc475b..9d7f4c9bf3 100644 --- a/app/controllers/following_accounts_controller.rb +++ b/app/controllers/following_accounts_controller.rb @@ -15,13 +15,13 @@ class FollowingAccountsController < ApplicationController format.html do expires_in 0, public: true unless user_signed_in? - next if @account.user_hides_network? + next if @account.hide_collections? follows end format.json do - raise Mastodon::NotPermittedError if page_requested? && @account.user_hides_network? + raise Mastodon::NotPermittedError if page_requested? && @account.hide_collections? expires_in(page_requested? ? 0 : 3.minutes, public: public_fetch_mode?) @@ -82,7 +82,7 @@ class FollowingAccountsController < ApplicationController end def restrict_fields_to - if page_requested? || !@account.user_hides_network? + if page_requested? || !@account.hide_collections? # Return all fields else %i(id type total_items) diff --git a/app/controllers/settings/preferences_controller.rb b/app/controllers/settings/preferences_controller.rb index 32b5d79487..c7492700cd 100644 --- a/app/controllers/settings/preferences_controller.rb +++ b/app/controllers/settings/preferences_controller.rb @@ -47,7 +47,6 @@ class Settings::PreferencesController < Settings::BaseController :setting_system_font_ui, :setting_noindex, :setting_theme, - :setting_hide_network, :setting_aggregate_reblogs, :setting_show_application, :setting_advanced_layout, diff --git a/app/controllers/settings/profiles_controller.rb b/app/controllers/settings/profiles_controller.rb index 0c15447a6c..be5b4f3029 100644 --- a/app/controllers/settings/profiles_controller.rb +++ b/app/controllers/settings/profiles_controller.rb @@ -20,7 +20,7 @@ class Settings::ProfilesController < Settings::BaseController private def account_params - params.require(:account).permit(:display_name, :note, :avatar, :header, :locked, :bot, :discoverable, fields_attributes: [:name, :value]) + params.require(:account).permit(:display_name, :note, :avatar, :header, :locked, :bot, :discoverable, :hide_collections, fields_attributes: [:name, :value]) end def set_account diff --git a/app/lib/user_settings_decorator.rb b/app/lib/user_settings_decorator.rb index e37bc6d9f1..de054e403e 100644 --- a/app/lib/user_settings_decorator.rb +++ b/app/lib/user_settings_decorator.rb @@ -31,7 +31,6 @@ class UserSettingsDecorator user.settings['system_font_ui'] = system_font_ui_preference if change?('setting_system_font_ui') user.settings['noindex'] = noindex_preference if change?('setting_noindex') user.settings['theme'] = theme_preference if change?('setting_theme') - user.settings['hide_network'] = hide_network_preference if change?('setting_hide_network') user.settings['aggregate_reblogs'] = aggregate_reblogs_preference if change?('setting_aggregate_reblogs') user.settings['show_application'] = show_application_preference if change?('setting_show_application') user.settings['advanced_layout'] = advanced_layout_preference if change?('setting_advanced_layout') @@ -97,10 +96,6 @@ class UserSettingsDecorator boolean_cast_setting 'setting_noindex' end - def hide_network_preference - boolean_cast_setting 'setting_hide_network' - end - def show_application_preference boolean_cast_setting 'setting_show_application' end diff --git a/app/models/account.rb b/app/models/account.rb index dfdf9045ff..1717f16055 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -349,11 +349,11 @@ class Account < ApplicationRecord end def hides_followers? - hide_collections? || user_hides_network? + hide_collections? end def hides_following? - hide_collections? || user_hides_network? + hide_collections? end def object_type diff --git a/app/models/user.rb b/app/models/user.rb index bbf850d84d..146bdcd2a9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -126,7 +126,7 @@ class User < ApplicationRecord has_many :session_activations, dependent: :destroy delegate :auto_play_gif, :default_sensitive, :unfollow_modal, :boost_modal, :delete_modal, - :reduce_motion, :system_font_ui, :noindex, :theme, :display_media, :hide_network, + :reduce_motion, :system_font_ui, :noindex, :theme, :display_media, :expand_spoilers, :default_language, :aggregate_reblogs, :show_application, :advanced_layout, :use_blurhash, :use_pending_items, :trends, :crop_images, :disable_swiping, @@ -273,10 +273,6 @@ class User < ApplicationRecord settings.notification_emails['trending_tag'] end - def hides_network? - @hides_network ||= settings.hide_network - end - def aggregates_reblogs? @aggregates_reblogs ||= settings.aggregate_reblogs end diff --git a/app/presenters/familiar_followers_presenter.rb b/app/presenters/familiar_followers_presenter.rb new file mode 100644 index 0000000000..c1d944b80b --- /dev/null +++ b/app/presenters/familiar_followers_presenter.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class FamiliarFollowersPresenter + class Result < ActiveModelSerializers::Model + attributes :id, :accounts + end + + def initialize(accounts, current_account_id) + @accounts = accounts + @current_account_id = current_account_id + end + + def accounts + map = Follow.includes(account: :account_stat).where(target_account_id: @accounts.map(&:id)).where(account_id: Follow.where(account_id: @current_account_id).joins(:target_account).merge(Account.where(hide_collections: [nil, false])).select(:target_account_id)).group_by(&:target_account_id) + @accounts.map { |account| Result.new(id: account.id, accounts: (account.hide_collections? ? [] : (map[account.id] || [])).map(&:account)) } + end +end diff --git a/app/serializers/rest/familiar_followers_serializer.rb b/app/serializers/rest/familiar_followers_serializer.rb new file mode 100644 index 0000000000..0a7e923f80 --- /dev/null +++ b/app/serializers/rest/familiar_followers_serializer.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class REST::FamiliarFollowersSerializer < ActiveModel::Serializer + attribute :id + + has_many :accounts, serializer: REST::AccountSerializer + + def id + object.id.to_s + end +end diff --git a/app/views/follower_accounts/index.html.haml b/app/views/follower_accounts/index.html.haml index 645dd2de17..92de35a9fc 100644 --- a/app/views/follower_accounts/index.html.haml +++ b/app/views/follower_accounts/index.html.haml @@ -7,7 +7,7 @@ = render 'accounts/header', account: @account -- if @account.user_hides_network? +- if @account.hide_collections? .nothing-here= t('accounts.network_hidden') - elsif user_signed_in? && @account.blocking?(current_account) .nothing-here= t('accounts.unavailable') diff --git a/app/views/following_accounts/index.html.haml b/app/views/following_accounts/index.html.haml index 17fe790188..9bb1a9eddf 100644 --- a/app/views/following_accounts/index.html.haml +++ b/app/views/following_accounts/index.html.haml @@ -7,7 +7,7 @@ = render 'accounts/header', account: @account -- if @account.user_hides_network? +- if @account.hide_collections? .nothing-here= t('accounts.network_hidden') - elsif user_signed_in? && @account.blocking?(current_account) .nothing-here= t('accounts.unavailable') diff --git a/app/views/settings/preferences/other/show.html.haml b/app/views/settings/preferences/other/show.html.haml index b7ae3d2ef7..44f4af2eba 100644 --- a/app/views/settings/preferences/other/show.html.haml +++ b/app/views/settings/preferences/other/show.html.haml @@ -10,9 +10,6 @@ .fields-group = f.input :setting_noindex, as: :boolean, wrapper: :with_label - .fields-group - = f.input :setting_hide_network, as: :boolean, wrapper: :with_label - .fields-group = f.input :setting_aggregate_reblogs, as: :boolean, wrapper: :with_label, recommended: true diff --git a/app/views/settings/profiles/show.html.haml b/app/views/settings/profiles/show.html.haml index d325a9ea5c..fe9666d841 100644 --- a/app/views/settings/profiles/show.html.haml +++ b/app/views/settings/profiles/show.html.haml @@ -30,7 +30,10 @@ = f.input :bot, as: :boolean, wrapper: :with_label, hint: t('simple_form.hints.defaults.bot') .fields-group - = f.input :discoverable, as: :boolean, wrapper: :with_label, hint: t(Setting.profile_directory ? 'simple_form.hints.defaults.discoverable' : 'simple_form.hints.defaults.discoverable_no_directory'), recommended: true + = f.input :discoverable, as: :boolean, wrapper: :with_label, hint: t('simple_form.hints.defaults.discoverable'), recommended: true + + .fields-group + = f.input :hide_collections, as: :boolean, wrapper: :with_label, label: t('simple_form.labels.defaults.setting_hide_network'), hint: t('simple_form.hints.defaults.setting_hide_network') %hr.spacer/ diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index c5e75b408c..b19b7891fd 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -37,8 +37,7 @@ en: current_password: For security purposes please enter the password of the current account current_username: To confirm, please enter the username of the current account digest: Only sent after a long period of inactivity and only if you have received any personal messages in your absence - discoverable: Allow your account to be discovered by strangers through recommendations, profile directory and other features - discoverable_no_directory: Allow your account to be discovered by strangers through recommendations and other features + discoverable: Allow your account to be discovered by strangers through recommendations, trends and other features email: You will be sent a confirmation e-mail fields: You can have up to 4 items displayed as a table on your profile header: PNG, GIF or JPG. At most %{size}. Will be downscaled to %{dimensions}px diff --git a/config/routes.rb b/config/routes.rb index 25eb1558fc..9e2f7a6482 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -493,6 +493,7 @@ Rails.application.routes.draw do resource :search, only: :show, controller: :search resource :lookup, only: :show, controller: :lookup resources :relationships, only: :index + resources :familiar_followers, only: :index end resources :accounts, only: [:create, :show] do diff --git a/config/settings.yml b/config/settings.yml index e63788ba2f..06dd2b3f3e 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -17,7 +17,6 @@ defaults: &defaults timeline_preview: true show_staff_badge: true default_sensitive: false - hide_network: false unfollow_modal: false boost_modal: false delete_modal: true diff --git a/db/migrate/20220304195405_migrate_hide_network_preference.rb b/db/migrate/20220304195405_migrate_hide_network_preference.rb new file mode 100644 index 0000000000..102ee46d6f --- /dev/null +++ b/db/migrate/20220304195405_migrate_hide_network_preference.rb @@ -0,0 +1,37 @@ +class MigrateHideNetworkPreference < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + # Dummy classes, to make migration possible across version changes + class Account < ApplicationRecord + has_one :user, inverse_of: :account + scope :local, -> { where(domain: nil) } + end + + class User < ApplicationRecord + belongs_to :account + end + + def up + Account.reset_column_information + + Setting.unscoped.where(thing_type: 'User', var: 'hide_network').find_each do |setting| + account = User.find(setting.thing_id).account + + ApplicationRecord.transaction do + account.update(hide_collections: setting.value) + setting.delete + end + rescue ActiveRecord::RecordNotFound + next + end + end + + def down + Account.local.where(hide_collections: true).includes(:user).find_each do |account| + ApplicationRecord.transaction do + Setting.create(thing_type: 'User', thing_id: account.user.id, var: 'hide_network', value: account.hide_collections?) + account.update(hide_collections: nil) + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 756e5e9aba..3666804eec 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_02_27_041951) do +ActiveRecord::Schema.define(version: 2022_03_04_195405) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/controllers/follower_accounts_controller_spec.rb b/spec/controllers/follower_accounts_controller_spec.rb index eb095cf30c..4d2a6e01a9 100644 --- a/spec/controllers/follower_accounts_controller_spec.rb +++ b/spec/controllers/follower_accounts_controller_spec.rb @@ -103,7 +103,7 @@ describe FollowerAccountsController do context 'when account hides their network' do before do - alice.user.settings.hide_network = true + alice.update(hide_collections: true) end it 'returns followers count' do diff --git a/spec/controllers/following_accounts_controller_spec.rb b/spec/controllers/following_accounts_controller_spec.rb index af5ce07870..bb6d221cac 100644 --- a/spec/controllers/following_accounts_controller_spec.rb +++ b/spec/controllers/following_accounts_controller_spec.rb @@ -103,7 +103,7 @@ describe FollowingAccountsController do context 'when account hides their network' do before do - alice.user.settings.hide_network = true + alice.update(hide_collections: true) end it 'returns followers count' do diff --git a/spec/presenters/familiar_followers_presenter_spec.rb b/spec/presenters/familiar_followers_presenter_spec.rb new file mode 100644 index 0000000000..17be4b9715 --- /dev/null +++ b/spec/presenters/familiar_followers_presenter_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe FamiliarFollowersPresenter do + describe '#accounts' do + let(:account) { Fabricate(:account) } + let(:familiar_follower) { Fabricate(:account) } + let(:requested_accounts) { Fabricate.times(2, :account) } + + subject { described_class.new(requested_accounts, account.id) } + + before do + familiar_follower.follow!(requested_accounts.first) + account.follow!(familiar_follower) + end + + it 'returns a result for each requested account' do + expect(subject.accounts.map(&:id)).to eq requested_accounts.map(&:id) + end + + it 'returns followers you follow' do + result = subject.accounts.first + + expect(result).to_not be_nil + expect(result.id).to eq requested_accounts.first.id + expect(result.accounts).to match_array([familiar_follower]) + end + + context 'when requested account hides followers' do + before do + requested_accounts.first.update(hide_collections: true) + end + + it 'does not return followers you follow' do + result = subject.accounts.first + + expect(result).to_not be_nil + expect(result.id).to eq requested_accounts.first.id + expect(result.accounts).to be_empty + end + end + + context 'when familiar follower hides follows' do + before do + familiar_follower.update(hide_collections: true) + end + + it 'does not return followers you follow' do + result = subject.accounts.first + + expect(result).to_not be_nil + expect(result.id).to eq requested_accounts.first.id + expect(result.accounts).to be_empty + end + end + end +end