From 44207b6af6977f44200b6fd82f3e18ba70fd7cc7 Mon Sep 17 00:00:00 2001 From: Surinna Curtis Date: Mon, 17 Jul 2017 21:10:37 -0500 Subject: Add muting_notifications? and a notifications argument to mute! --- app/models/concerns/account_interactions.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'app/models/concerns') diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index b26520f5b..976452f12 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -70,8 +70,8 @@ module AccountInteractions block_relationships.find_or_create_by!(target_account: other_account) end - def mute!(other_account) - mute_relationships.find_or_create_by!(target_account: other_account) + def mute!(other_account, notifications: true) + mute_relationships.create_with(hide_notifications: notifications).find_or_create_by!(target_account: other_account) end def mute_conversation!(conversation) @@ -127,6 +127,10 @@ module AccountInteractions conversation_mutes.where(conversation: conversation).exists? end + def muting_notifications?(other_account) + mute_relationships.where(target_account: other_account, hide_notifications: true).exists? + end + def requested?(other_account) follow_requests.where(target_account: other_account).exists? end -- cgit From 7dbcc7ed3df9ed858ebdd44607d2baa0b2939b4c Mon Sep 17 00:00:00 2001 From: Surinna Curtis Date: Sat, 9 Sep 2017 05:05:43 -0500 Subject: Expose whether a mute hides notifications in the api/v1/relationships endpoint --- app/models/concerns/account_interactions.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'app/models/concerns') diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 976452f12..c258bfbfc 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -17,7 +17,11 @@ module AccountInteractions end def muting_map(target_account_ids, account_id) - follow_mapping(Mute.where(target_account_id: target_account_ids, account_id: account_id), :target_account_id) + Mute.where(target_account_id: target_account_ids, account_id: account_id).each_with_object({}) do |mute, mapping| + mapping[mute.target_account_id] = { + notifications: mute.hide_notifications? + } + end end def requested_map(target_account_ids, account_id) -- cgit From 0284fd723b4d45ab2b381ee0c4f6550f44d3abf5 Mon Sep 17 00:00:00 2001 From: Surinna Curtis Date: Sat, 9 Sep 2017 05:16:06 -0500 Subject: Allow modifying the hide_notifications of a mute with the /api/v1/accounts/:id/mute endpoint --- app/models/concerns/account_interactions.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'app/models/concerns') diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index c258bfbfc..4a4265df2 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -75,7 +75,11 @@ module AccountInteractions end def mute!(other_account, notifications: true) - mute_relationships.create_with(hide_notifications: notifications).find_or_create_by!(target_account: other_account) + mute = mute_relationships.create_with(hide_notifications: notifications).find_or_create_by!(target_account: other_account) + if mute.hide_notifications? != notifications + mute.hide_notifications = notifications + mute.save! + end end def mute_conversation!(conversation) -- cgit From 211f0a951375a33ed4278eef030e76a6bde9c396 Mon Sep 17 00:00:00 2001 From: Surinna Curtis Date: Wed, 13 Sep 2017 17:42:52 -0500 Subject: add an explanatory comment to AccountInteractions --- app/models/concerns/account_interactions.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'app/models/concerns') diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 4a4265df2..6f69ce1d4 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -76,6 +76,7 @@ module AccountInteractions def mute!(other_account, notifications: true) mute = mute_relationships.create_with(hide_notifications: notifications).find_or_create_by!(target_account: other_account) + # When toggling a mute between hiding and allowing notifications, the mute will already exist, so the find_or_create_by! call will return the existing Mute without updating the hide_notifications attribute. Therefore, we check that hide_notifications? is what we want and set it if it isn't. if mute.hide_notifications? != notifications mute.hide_notifications = notifications mute.save! -- cgit From f9d7b8a94f4a89d76081a6265103f6d7439be250 Mon Sep 17 00:00:00 2001 From: Surinna Curtis Date: Wed, 13 Sep 2017 18:32:10 -0500 Subject: Refactor handling of default params for muting to make code cleaner --- app/controllers/api/v1/accounts_controller.rb | 2 +- app/models/concerns/account_interactions.rb | 3 ++- app/services/mute_service.rb | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) (limited to 'app/models/concerns') diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index de2cb0d97..3e9ac1025 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -33,7 +33,7 @@ class Api::V1::AccountsController < Api::BaseController end def mute - MuteService.new.call(current_user.account, @account, **params.permit(:notifications).to_hash.symbolize_keys) + MuteService.new.call(current_user.account, @account, notifications: params(:notifications)) render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships end diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 6f69ce1d4..0afdebf89 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -74,7 +74,8 @@ module AccountInteractions block_relationships.find_or_create_by!(target_account: other_account) end - def mute!(other_account, notifications: true) + def mute!(other_account, notifications: nil) + notifications = true if notifications.nil? mute = mute_relationships.create_with(hide_notifications: notifications).find_or_create_by!(target_account: other_account) # When toggling a mute between hiding and allowing notifications, the mute will already exist, so the find_or_create_by! call will return the existing Mute without updating the hide_notifications attribute. Therefore, we check that hide_notifications? is what we want and set it if it isn't. if mute.hide_notifications? != notifications diff --git a/app/services/mute_service.rb b/app/services/mute_service.rb index fc63f83e9..56cbebd5d 100644 --- a/app/services/mute_service.rb +++ b/app/services/mute_service.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class MuteService < BaseService - def call(account, target_account, **opts) + def call(account, target_account, notifications: nil) return if account.id == target_account.id FeedManager.instance.clear_from_timeline(account, target_account) - account.mute!(target_account, **opts.slice(:notifications)) + account.mute!(target_account, notifications: notifications) end end -- cgit From 49445150202f0bdaae942b9ae1ba44802a1c22e9 Mon Sep 17 00:00:00 2001 From: aschmitz Date: Thu, 9 Nov 2017 08:41:10 -0600 Subject: "Show reblogs" per-follower UI/database changes TODO: * Tests (particularly for FollowRequests). * Anything to respect the setting when putting reblogs in timelines. --- app/controllers/api/v1/accounts_controller.rb | 6 +++-- app/javascript/glitch/components/account/header.js | 2 +- app/javascript/mastodon/actions/accounts.js | 4 +-- app/javascript/mastodon/components/account.js | 2 +- .../features/account/components/action_bar.js | 12 +++++++++ .../features/account_timeline/components/header.js | 6 +++++ .../containers/header_container.js | 8 ++++++ app/models/concerns/account_interactions.rb | 23 +++++++++++++--- app/models/follow.rb | 1 + app/models/follow_request.rb | 3 ++- app/services/follow_service.rb | 31 ++++++++++++++++------ db/schema.rb | 4 ++- 12 files changed, 82 insertions(+), 20 deletions(-) (limited to 'app/models/concerns') diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 4676f60de..afdbf6e2d 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -13,9 +13,11 @@ class Api::V1::AccountsController < Api::BaseController end def follow - FollowService.new.call(current_user.account, @account.acct) + reblogs_arg = { reblogs: params[:reblogs] } + + FollowService.new.call(current_user.account, @account.acct, reblogs_arg) - options = @account.locked? ? {} : { following_map: { @account.id => true }, requested_map: { @account.id => false } } + options = @account.locked? ? {} : { following_map: reblogs_arg, requested_map: { @account.id => false } } render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options) end diff --git a/app/javascript/glitch/components/account/header.js b/app/javascript/glitch/components/account/header.js index f4a413aa3..c94fb0851 100644 --- a/app/javascript/glitch/components/account/header.js +++ b/app/javascript/glitch/components/account/header.js @@ -152,7 +152,7 @@ appropriate icon. diff --git a/app/javascript/mastodon/actions/accounts.js b/app/javascript/mastodon/actions/accounts.js index fbaebf786..cabf72bde 100644 --- a/app/javascript/mastodon/actions/accounts.js +++ b/app/javascript/mastodon/actions/accounts.js @@ -105,11 +105,11 @@ export function fetchAccountFail(id, error) { }; }; -export function followAccount(id) { +export function followAccount(id, reblogs = true) { return (dispatch, getState) => { dispatch(followAccountRequest(id)); - api(getState).post(`/api/v1/accounts/${id}/follow`).then(response => { + api(getState).post(`/api/v1/accounts/${id}/follow`, { reblogs }).then(response => { dispatch(followAccountSuccess(response.data)); }).catch(error => { dispatch(followAccountFail(error)); diff --git a/app/javascript/mastodon/components/account.js b/app/javascript/mastodon/components/account.js index 7cdb8c672..376e544fb 100644 --- a/app/javascript/mastodon/components/account.js +++ b/app/javascript/mastodon/components/account.js @@ -93,7 +93,7 @@ export default class Account extends ImmutablePureComponent { ); } else { - buttons = ; + buttons = ; } } diff --git a/app/javascript/mastodon/features/account/components/action_bar.js b/app/javascript/mastodon/features/account/components/action_bar.js index 2819ae252..718e7fbad 100644 --- a/app/javascript/mastodon/features/account/components/action_bar.js +++ b/app/javascript/mastodon/features/account/components/action_bar.js @@ -19,6 +19,8 @@ const messages = defineMessages({ media: { id: 'account.media', defaultMessage: 'Media' }, blockDomain: { id: 'account.block_domain', defaultMessage: 'Hide everything from {domain}' }, unblockDomain: { id: 'account.unblock_domain', defaultMessage: 'Unhide {domain}' }, + hideReblogs: { id: 'account.hide_reblogs', defaultMessage: 'Hide boosts from @{name}' }, + showReblogs: { id: 'account.show_reblogs', defaultMessage: 'Show boosts from @{name}' }, }); @injectIntl @@ -30,6 +32,7 @@ export default class ActionBar extends React.PureComponent { onFollow: PropTypes.func, onBlock: PropTypes.func.isRequired, onMention: PropTypes.func.isRequired, + onReblogToggle: PropTypes.func.isRequired, onReport: PropTypes.func.isRequired, onMute: PropTypes.func.isRequired, onBlockDomain: PropTypes.func.isRequired, @@ -60,6 +63,15 @@ export default class ActionBar extends React.PureComponent { if (account.get('id') === me) { menu.push({ text: intl.formatMessage(messages.edit_profile), href: '/settings/profile' }); } else { + const following = account.getIn(['relationship', 'following']); + if (following) { + if (following.get('reblogs')) { + menu.push({ text: intl.formatMessage(messages.hideReblogs, { name: account.get('username') }), action: this.props.onReblogToggle }); + } else { + menu.push({ text: intl.formatMessage(messages.showReblogs, { name: account.get('username') }), action: this.props.onReblogToggle }); + } + } + if (account.getIn(['relationship', 'muting'])) { menu.push({ text: intl.formatMessage(messages.unmute, { name: account.get('username') }), action: this.props.onMute }); } else { diff --git a/app/javascript/mastodon/features/account_timeline/components/header.js b/app/javascript/mastodon/features/account_timeline/components/header.js index c3cd4e55d..b33df282f 100644 --- a/app/javascript/mastodon/features/account_timeline/components/header.js +++ b/app/javascript/mastodon/features/account_timeline/components/header.js @@ -14,6 +14,7 @@ export default class Header extends ImmutablePureComponent { onFollow: PropTypes.func.isRequired, onBlock: PropTypes.func.isRequired, onMention: PropTypes.func.isRequired, + onReblogToggle: PropTypes.func.isRequired, onReport: PropTypes.func.isRequired, onMute: PropTypes.func.isRequired, onBlockDomain: PropTypes.func.isRequired, @@ -40,6 +41,10 @@ export default class Header extends ImmutablePureComponent { this.props.onReport(this.props.account); } + handleReblogToggle = () => { + this.props.onReblogToggle(this.props.account); + } + handleMute = () => { this.props.onMute(this.props.account); } @@ -80,6 +85,7 @@ export default class Header extends ImmutablePureComponent { me={me} onBlock={this.handleBlock} onMention={this.handleMention} + onReblogToggle={this.handleReblogToggle} onReport={this.handleReport} onMute={this.handleMute} onBlockDomain={this.handleBlockDomain} diff --git a/app/javascript/mastodon/features/account_timeline/containers/header_container.js b/app/javascript/mastodon/features/account_timeline/containers/header_container.js index 9ad13a231..68c037e9b 100644 --- a/app/javascript/mastodon/features/account_timeline/containers/header_container.js +++ b/app/javascript/mastodon/features/account_timeline/containers/header_container.js @@ -68,6 +68,14 @@ const mapDispatchToProps = (dispatch, { intl }) => ({ dispatch(mentionCompose(account, router)); }, + onReblogToggle (account) { + if (account.getIn(['relationship', 'following', 'reblogs'])) { + dispatch(followAccount(account.get('id'), false)); + } else { + dispatch(followAccount(account.get('id'), true)); + } + }, + onReport (account) { dispatch(initReport(account)); }, diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 0afdebf89..088fef4da 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -5,7 +5,11 @@ module AccountInteractions class_methods do def following_map(target_account_ids, account_id) - follow_mapping(Follow.where(target_account_id: target_account_ids, account_id: account_id), :target_account_id) + Follow.where(target_account_id: target_account_ids, account_id: account_id).each_with_object({}) do |follow, mapping| + mapping[follow.target_account_id] = { + reblogs: follow.show_reblogs? + } + end end def followed_by_map(target_account_ids, account_id) @@ -25,7 +29,11 @@ module AccountInteractions end def requested_map(target_account_ids, account_id) - follow_mapping(FollowRequest.where(target_account_id: target_account_ids, account_id: account_id), :target_account_id) + FollowRequest.where(target_account_id: target_account_ids, account_id: account_id).each_with_object({}) do |follow_request, mapping| + mapping[follow_request.target_account_id] = { + reblogs: follow_request.show_reblogs? + } + end end def domain_blocking_map(target_account_ids, account_id) @@ -66,8 +74,15 @@ module AccountInteractions has_many :domain_blocks, class_name: 'AccountDomainBlock', dependent: :destroy end - def follow!(other_account) - active_relationships.find_or_create_by!(target_account: other_account) + def follow!(other_account, reblogs: nil) + reblogs = true if reblogs.nil? + rel = active_relationships.create_with(show_reblogs: reblogs).find_or_create_by!(target_account: other_account) + if rel.show_reblogs != reblogs + rel.show_reblogs = reblogs + rel.save! + end + + rel end def block!(other_account) diff --git a/app/models/follow.rb b/app/models/follow.rb index 667720a88..a8ddcb7f0 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -8,6 +8,7 @@ # account_id :integer not null # id :integer not null, primary key # target_account_id :integer not null +# show_reblogs :boolean default(TRUE), not null # class Follow < ApplicationRecord diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index 60036d903..0608ffabc 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -8,6 +8,7 @@ # account_id :integer not null # id :integer not null, primary key # target_account_id :integer not null +# show_reblogs :boolean default(TRUE), not null # class FollowRequest < ApplicationRecord @@ -21,7 +22,7 @@ class FollowRequest < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } def authorize! - account.follow!(target_account) + account.follow!(target_account, reblogs: reblogs) MergeWorker.perform_async(target_account.id, account.id) destroy! diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 791773f25..70572110d 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -6,25 +6,40 @@ class FollowService < BaseService # Follow a remote user, notify remote user about the follow # @param [Account] source_account From which to follow # @param [String, Account] uri User URI to follow in the form of username@domain (or account record) - def call(source_account, uri) + def call(source_account, uri, reblogs: nil) + reblogs = true if reblogs.nil? target_account = uri.is_a?(Account) ? uri : ResolveRemoteAccountService.new.call(uri) raise ActiveRecord::RecordNotFound if target_account.nil? || target_account.id == source_account.id || target_account.suspended? raise Mastodon::NotPermittedError if target_account.blocking?(source_account) || source_account.blocking?(target_account) - return if source_account.following?(target_account) || source_account.requested?(target_account) + if source_account.following?(target_account) + # We're already following this account, but we'll call follow! again to + # make sure the reblogs status is set correctly. + source_account.follow!(target_account, reblogs: reblogs) + return + elsif source_account.requested?(target_account) + # This isn't managed by a method in AccountInteractions, so we modify it + # ourselves if necessary. + req = follow_requests.find_by(target_account: other_account) + if req.show_reblogs != reblogs + req.show_reblogs = reblogs + req.save! + end + return + end if target_account.locked? || target_account.activitypub? - request_follow(source_account, target_account) + request_follow(source_account, target_account, reblogs: reblogs) else - direct_follow(source_account, target_account) + direct_follow(source_account, target_account, reblogs: reblogs) end end private - def request_follow(source_account, target_account) - follow_request = FollowRequest.create!(account: source_account, target_account: target_account) + def request_follow(source_account, target_account, reblogs: true) + follow_request = FollowRequest.create!(account: source_account, target_account: target_account, reblogs: reblogs) if target_account.local? NotifyService.new.call(target_account, follow_request) @@ -38,8 +53,8 @@ class FollowService < BaseService follow_request end - def direct_follow(source_account, target_account) - follow = source_account.follow!(target_account) + def direct_follow(source_account, target_account, reblogs: true) + follow = source_account.follow!(target_account, reblogs: reblogs) if target_account.local? NotifyService.new.call(target_account, follow) diff --git a/db/schema.rb b/db/schema.rb index f96a5340f..93505f9a0 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: 20171021191900) do +ActiveRecord::Schema.define(version: 20171028221157) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -145,6 +145,7 @@ ActiveRecord::Schema.define(version: 20171021191900) do t.datetime "updated_at", null: false t.bigint "account_id", null: false t.bigint "target_account_id", null: false + t.boolean "show_reblogs", default: true, null: false t.index ["account_id", "target_account_id"], name: "index_follow_requests_on_account_id_and_target_account_id", unique: true end @@ -153,6 +154,7 @@ ActiveRecord::Schema.define(version: 20171021191900) do t.datetime "updated_at", null: false t.bigint "account_id", null: false t.bigint "target_account_id", null: false + t.boolean "show_reblogs", default: true, null: false t.index ["account_id", "target_account_id"], name: "index_follows_on_account_id_and_target_account_id", unique: true end -- cgit From b95c48748cd4e7a1181cdf3f17e23d6e526a9d95 Mon Sep 17 00:00:00 2001 From: aschmitz Date: Fri, 10 Nov 2017 20:11:10 -0600 Subject: Per-user reblog hiding implementation/fixes/tests Note that this will only hide/show *future* reblogs by a user, and does nothing to remove/add reblogs that are already in the timeline. I don't think that's a particularly confusing behavior, and it's a lot easier to implement (similar to mutes, I believe). --- app/models/concerns/account_interactions.rb | 6 ++- app/models/follow_request.rb | 2 +- app/services/follow_service.rb | 2 +- app/services/notify_service.rb | 2 +- .../20171028221157_add_reblogs_to_follows.rb | 21 ++++++++ .../v1/accounts/relationships_controller_spec.rb | 4 +- .../controllers/api/v1/accounts_controller_spec.rb | 8 +-- spec/models/concerns/account_interactions_spec.rb | 37 ++++++++++++++ spec/models/follow_request_spec.rb | 24 ++++++++- spec/services/follow_service_spec.rb | 58 ++++++++++++++++++++-- spec/services/notify_service_spec.rb | 20 ++++++++ 11 files changed, 170 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20171028221157_add_reblogs_to_follows.rb (limited to 'app/models/concerns') diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 088fef4da..60fd6ded5 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -81,7 +81,7 @@ module AccountInteractions rel.show_reblogs = reblogs rel.save! end - + rel end @@ -156,6 +156,10 @@ module AccountInteractions mute_relationships.where(target_account: other_account, hide_notifications: true).exists? end + def muting_reblogs?(other_account) + active_relationships.where(target_account: other_account, show_reblogs: false).exists? + end + def requested?(other_account) follow_requests.where(target_account: other_account).exists? end diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index 0608ffabc..1a1c52382 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -22,7 +22,7 @@ class FollowRequest < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } def authorize! - account.follow!(target_account, reblogs: reblogs) + account.follow!(target_account, reblogs: show_reblogs) MergeWorker.perform_async(target_account.id, account.id) destroy! diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 70572110d..6db591999 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -39,7 +39,7 @@ class FollowService < BaseService private def request_follow(source_account, target_account, reblogs: true) - follow_request = FollowRequest.create!(account: source_account, target_account: target_account, reblogs: reblogs) + follow_request = FollowRequest.create!(account: source_account, target_account: target_account, show_reblogs: reblogs) if target_account.local? NotifyService.new.call(target_account, follow_request) diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index fb09df983..3fa3f152c 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -29,7 +29,7 @@ class NotifyService < BaseService end def blocked_reblog? - false + @recipient.muting_reblogs?(@notification.from_account) end def blocked_follow_request? diff --git a/db/migrate/20171028221157_add_reblogs_to_follows.rb b/db/migrate/20171028221157_add_reblogs_to_follows.rb new file mode 100644 index 000000000..eb4640a20 --- /dev/null +++ b/db/migrate/20171028221157_add_reblogs_to_follows.rb @@ -0,0 +1,21 @@ +require Rails.root.join('lib', 'mastodon', 'migration_helpers') + +class AddReblogsToFollows < ActiveRecord::Migration[5.1] + include Mastodon::MigrationHelpers + + safety_assured do + disable_ddl_transaction! + end + + def up + safety_assured do + add_column_with_default :follows, :show_reblogs, :boolean, default: true, allow_null: false + add_column_with_default :follow_requests, :show_reblogs, :boolean, default: true, allow_null: false + end + end + + def down + remove_column :follows, :show_reblogs + remove_column :follow_requests, :show_reblogs + end +end diff --git a/spec/controllers/api/v1/accounts/relationships_controller_spec.rb b/spec/controllers/api/v1/accounts/relationships_controller_spec.rb index 431fc2194..f25b86ac1 100644 --- a/spec/controllers/api/v1/accounts/relationships_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/relationships_controller_spec.rb @@ -32,7 +32,7 @@ describe Api::V1::Accounts::RelationshipsController do json = body_as_json expect(json).to be_a Enumerable - expect(json.first[:following]).to be true + expect(json.first[:following]).to be_truthy expect(json.first[:followed_by]).to be false end end @@ -51,7 +51,7 @@ describe Api::V1::Accounts::RelationshipsController do expect(json).to be_a Enumerable expect(json.first[:id]).to eq simon.id.to_s - expect(json.first[:following]).to be true + expect(json.first[:following]).to be_truthy expect(json.first[:followed_by]).to be false expect(json.first[:muting]).to be false expect(json.first[:requested]).to be false diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 053c53e5a..f3b879421 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -31,10 +31,10 @@ RSpec.describe Api::V1::AccountsController, type: :controller do expect(response).to have_http_status(:success) end - it 'returns JSON with following=true and requested=false' do + it 'returns JSON with following=truthy and requested=false' do json = body_as_json - expect(json[:following]).to be true + expect(json[:following]).to be_truthy expect(json[:requested]).to be false end @@ -50,11 +50,11 @@ RSpec.describe Api::V1::AccountsController, type: :controller do expect(response).to have_http_status(:success) end - it 'returns JSON with following=false and requested=true' do + it 'returns JSON with following=false and requested=truthy' do json = body_as_json expect(json[:following]).to be false - expect(json[:requested]).to be true + expect(json[:requested]).to be_truthy end it 'creates a follow request relation between user and target user' do diff --git a/spec/models/concerns/account_interactions_spec.rb b/spec/models/concerns/account_interactions_spec.rb index ef957fc1d..f47d9d057 100644 --- a/spec/models/concerns/account_interactions_spec.rb +++ b/spec/models/concerns/account_interactions_spec.rb @@ -37,4 +37,41 @@ describe AccountInteractions do end end end + + describe 'ignoring reblogs from an account' do + before do + @me = Fabricate(:account, username: 'Me') + @you = Fabricate(:account, username: 'You') + end + + context 'with the reblogs option unspecified' do + before do + @me.follow!(@you) + end + + it 'defaults to showing reblogs' do + expect(@me.muting_reblogs?(@you)).to be(false) + end + end + + context 'with the reblogs option set to false' do + before do + @me.follow!(@you, reblogs: false) + end + + it 'does mute reblogs' do + expect(@me.muting_reblogs?(@you)).to be(true) + end + end + + context 'with the reblogs option set to true' do + before do + @me.follow!(@you, reblogs: true) + end + + it 'does not mute reblogs' do + expect(@me.muting_reblogs?(@you)).to be(false) + end + end + end end diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index cc6f8ee62..62bd724d7 100644 --- a/spec/models/follow_request_spec.rb +++ b/spec/models/follow_request_spec.rb @@ -1,7 +1,29 @@ require 'rails_helper' RSpec.describe FollowRequest, type: :model do - describe '#authorize!' + describe '#authorize!' do + it 'generates a Follow' do + follow_request = Fabricate.create(:follow_request) + follow_request.authorize! + target = follow_request.target_account + expect(follow_request.account.following?(target)).to be true + end + + it 'correctly passes show_reblogs when true' do + follow_request = Fabricate.create(:follow_request, show_reblogs: true) + follow_request.authorize! + target = follow_request.target_account + expect(follow_request.account.muting_reblogs?(target)).to be false + end + + it 'correctly passes show_reblogs when false' do + follow_request = Fabricate.create(:follow_request, show_reblogs: false) + follow_request.authorize! + target = follow_request.target_account + expect(follow_request.account.muting_reblogs?(target)).to be true + end + end + describe '#reject!' describe 'validations' do diff --git a/spec/services/follow_service_spec.rb b/spec/services/follow_service_spec.rb index ceb39e5e6..e59a2f1a6 100644 --- a/spec/services/follow_service_spec.rb +++ b/spec/services/follow_service_spec.rb @@ -13,8 +13,20 @@ RSpec.describe FollowService do subject.call(sender, bob.acct) end - it 'creates a follow request' do - expect(FollowRequest.find_by(account: sender, target_account: bob)).to_not be_nil + it 'creates a follow request with reblogs' do + expect(FollowRequest.find_by(account: sender, target_account: bob, show_reblogs: true)).to_not be_nil + end + end + + describe 'locked account, no reblogs' do + let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, locked: true, username: 'bob')).account } + + before do + subject.call(sender, bob.acct, reblogs: false) + end + + it 'creates a follow request without reblogs' do + expect(FollowRequest.find_by(account: sender, target_account: bob, show_reblogs: false)).to_not be_nil end end @@ -25,8 +37,22 @@ RSpec.describe FollowService do subject.call(sender, bob.acct) end - it 'creates a following relation' do + it 'creates a following relation with reblogs' do + expect(sender.following?(bob)).to be true + expect(sender.muting_reblogs?(bob)).to be false + end + end + + describe 'unlocked account, no reblogs' do + let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } + + before do + subject.call(sender, bob.acct, reblogs: false) + end + + it 'creates a following relation without reblogs' do expect(sender.following?(bob)).to be true + expect(sender.muting_reblogs?(bob)).to be true end end @@ -42,6 +68,32 @@ RSpec.describe FollowService do expect(sender.following?(bob)).to be true end end + + describe 'already followed account, turning reblogs off' do + let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } + + before do + sender.follow!(bob, reblogs: true) + subject.call(sender, bob.acct, reblogs: false) + end + + it 'disables reblogs' do + expect(sender.muting_reblogs?(bob)).to be true + end + end + + describe 'already followed account, turning reblogs on' do + let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } + + before do + sender.follow!(bob, reblogs: false) + subject.call(sender, bob.acct, reblogs: true) + end + + it 'disables reblogs' do + expect(sender.muting_reblogs?(bob)).to be false + end + end end context 'remote OStatus account' do diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 7088ae9d1..250a880a2 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -48,6 +48,26 @@ RSpec.describe NotifyService do is_expected.to_not change(Notification, :count) end + describe 'reblogs' do + let(:status) { Fabricate(:status, account: Fabricate(:account)) } + let(:activity) { Fabricate(:status, account: sender, reblog: status) } + + it 'shows reblogs by default' do + recipient.follow!(sender) + is_expected.to change(Notification, :count) + end + + it 'shows reblogs when explicitly enabled' do + recipient.follow!(sender, reblogs: true) + is_expected.to change(Notification, :count) + end + + it 'hides reblogs when disabled' do + recipient.follow!(sender, reblogs: false) + is_expected.to_not change(Notification, :count) + end + end + context do let(:asshole) { Fabricate(:account, username: 'asshole') } let(:reply_to) { Fabricate(:status, account: asshole) } -- cgit From 5128c4261e8c067321e70ffda560f14036f56bb0 Mon Sep 17 00:00:00 2001 From: aschmitz Date: Sat, 11 Nov 2017 14:37:23 -0600 Subject: Updates per code review Thanks, @valerauko! --- app/controllers/api/v1/accounts_controller.rb | 2 +- app/models/concerns/account_interactions.rb | 5 +---- app/services/follow_service.rb | 6 ++---- 3 files changed, 4 insertions(+), 9 deletions(-) (limited to 'app/models/concerns') diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index afdbf6e2d..85eb2d60e 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -17,7 +17,7 @@ class Api::V1::AccountsController < Api::BaseController FollowService.new.call(current_user.account, @account.acct, reblogs_arg) - options = @account.locked? ? {} : { following_map: reblogs_arg, requested_map: { @account.id => false } } + options = @account.locked? ? {} : { following_map: { @account.id => reblogs_arg }, requested_map: { @account.id => false } } render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options) end diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 60fd6ded5..a68f7c3d8 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -77,10 +77,7 @@ module AccountInteractions def follow!(other_account, reblogs: nil) reblogs = true if reblogs.nil? rel = active_relationships.create_with(show_reblogs: reblogs).find_or_create_by!(target_account: other_account) - if rel.show_reblogs != reblogs - rel.show_reblogs = reblogs - rel.save! - end + rel.update!(show_reblogs: reblogs) rel end diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 6db591999..20579ca63 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -6,6 +6,7 @@ class FollowService < BaseService # Follow a remote user, notify remote user about the follow # @param [Account] source_account From which to follow # @param [String, Account] uri User URI to follow in the form of username@domain (or account record) + # @param [true, false, nil] reblogs Whether or not to show reblogs, defaults to true def call(source_account, uri, reblogs: nil) reblogs = true if reblogs.nil? target_account = uri.is_a?(Account) ? uri : ResolveRemoteAccountService.new.call(uri) @@ -22,10 +23,7 @@ class FollowService < BaseService # This isn't managed by a method in AccountInteractions, so we modify it # ourselves if necessary. req = follow_requests.find_by(target_account: other_account) - if req.show_reblogs != reblogs - req.show_reblogs = reblogs - req.save! - end + req.update!(show_reblogs: reblogs) return end -- cgit