Add support for private pinned posts
* Add support for private pinned toots * Allow local user to pin private toots * Change wording to avoid "direct message"
This commit is contained in:
parent
49dd63d6dd
commit
e4bf21df6a
19 changed files with 165 additions and 28 deletions
|
@ -28,7 +28,7 @@ class AccountsController < ApplicationController
|
|||
return
|
||||
end
|
||||
|
||||
@pinned_statuses = cache_collection(@account.pinned_statuses, Status) if show_pinned_statuses?
|
||||
@pinned_statuses = cached_filtered_status_pins if show_pinned_statuses?
|
||||
@statuses = cached_filtered_status_page
|
||||
@rss_url = rss_url
|
||||
|
||||
|
@ -64,6 +64,10 @@ class AccountsController < ApplicationController
|
|||
[replies_requested?, media_requested?, tag_requested?, params[:max_id].present?, params[:min_id].present?].none?
|
||||
end
|
||||
|
||||
def filtered_pinned_statuses
|
||||
@account.pinned_statuses.where(visibility: [:public, :unlisted])
|
||||
end
|
||||
|
||||
def filtered_statuses
|
||||
default_statuses.tap do |statuses|
|
||||
statuses.merge!(hashtag_scope) if tag_requested?
|
||||
|
@ -142,6 +146,13 @@ class AccountsController < ApplicationController
|
|||
request.path.split('.').first.end_with?(Addressable::URI.parse("/tagged/#{params[:tag]}").normalize)
|
||||
end
|
||||
|
||||
def cached_filtered_status_pins
|
||||
cache_collection(
|
||||
filtered_pinned_statuses,
|
||||
Status
|
||||
)
|
||||
end
|
||||
|
||||
def cached_filtered_status_page
|
||||
cache_collection_paginated_by_id(
|
||||
filtered_statuses,
|
||||
|
|
|
@ -21,6 +21,7 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController
|
|||
case params[:id]
|
||||
when 'featured'
|
||||
@items = for_signed_account { cache_collection(@account.pinned_statuses, Status) }
|
||||
@items = @items.map { |item| item.distributable? ? item : ActivityPub::TagManager.instance.uri_for(item) }
|
||||
when 'tags'
|
||||
@items = for_signed_account { @account.featured_tags }
|
||||
when 'devices'
|
||||
|
|
|
@ -57,9 +57,7 @@ class Api::V1::Accounts::StatusesController < Api::BaseController
|
|||
end
|
||||
|
||||
def pinned_scope
|
||||
return Status.none if @account.blocking?(current_account)
|
||||
|
||||
@account.pinned_statuses
|
||||
@account.pinned_statuses.permitted_for(@account, current_account)
|
||||
end
|
||||
|
||||
def no_replies_scope
|
||||
|
|
|
@ -125,7 +125,7 @@ class StatusActionBar extends ImmutablePureComponent {
|
|||
'withDismiss',
|
||||
'referenced',
|
||||
'contextReferenced',
|
||||
'referenceCountLimit'
|
||||
'referenceCountLimit',
|
||||
]
|
||||
|
||||
handleReplyClick = () => {
|
||||
|
@ -331,6 +331,7 @@ class StatusActionBar extends ImmutablePureComponent {
|
|||
|
||||
const anonymousAccess = !me;
|
||||
const publicStatus = ['public', 'unlisted'].includes(status.get('visibility'));
|
||||
const pinnableStatus = ['public', 'unlisted', 'private'].includes(status.get('visibility'));
|
||||
const mutingConversation = status.get('muted');
|
||||
const account = status.get('account');
|
||||
const writtenByMe = status.getIn(['account', 'id']) === me;
|
||||
|
@ -384,7 +385,7 @@ class StatusActionBar extends ImmutablePureComponent {
|
|||
menu.push({ text: intl.formatMessage(bookmarked ? messages.removeBookmark : messages.bookmark), action: this.handleBookmarkClick });
|
||||
}
|
||||
|
||||
if (writtenByMe && publicStatus && !expired) {
|
||||
if (writtenByMe && pinnableStatus && !expired) {
|
||||
menu.push({ text: intl.formatMessage(status.get('pinned') ? messages.unpin : messages.pin), action: this.handlePinClick });
|
||||
}
|
||||
|
||||
|
|
|
@ -284,6 +284,7 @@ class ActionBar extends React.PureComponent {
|
|||
const { status, relationship, intl, referenced, contextReferenced, referenceCountLimit } = this.props;
|
||||
|
||||
const publicStatus = ['public', 'unlisted'].includes(status.get('visibility'));
|
||||
const pinnableStatus = ['public', 'unlisted', 'private'].includes(status.get('visibility'));
|
||||
const mutingConversation = status.get('muted');
|
||||
const account = status.get('account');
|
||||
const writtenByMe = status.getIn(['account', 'id']) === me;
|
||||
|
@ -336,7 +337,7 @@ class ActionBar extends React.PureComponent {
|
|||
menu.push(null);
|
||||
|
||||
if (writtenByMe) {
|
||||
if (publicStatus && !expired) {
|
||||
if (pinnableStatus && !expired) {
|
||||
menu.push({ text: intl.formatMessage(status.get('pinned') ? messages.unpin : messages.pin), action: this.handlePinClick });
|
||||
menu.push(null);
|
||||
}
|
||||
|
|
|
@ -3,7 +3,7 @@
|
|||
class ActivityPub::Activity::Accept < ActivityPub::Activity
|
||||
def perform
|
||||
return accept_follow_for_relay if relay_follow?
|
||||
return follow_request_from_object.authorize! unless follow_request_from_object.nil?
|
||||
return accept_follow!(follow_request_from_object) unless follow_request_from_object.nil?
|
||||
|
||||
case @object['type']
|
||||
when 'Follow'
|
||||
|
@ -19,7 +19,16 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity
|
|||
return if target_account.nil? || !target_account.local?
|
||||
|
||||
follow_request = FollowRequest.find_by(account: target_account, target_account: @account)
|
||||
follow_request&.authorize!
|
||||
accept_follow!(follow_request)
|
||||
end
|
||||
|
||||
def accept_follow!(request)
|
||||
return if request.nil?
|
||||
|
||||
is_first_follow = !request.target_account.followers.local.exists?
|
||||
request.authorize!
|
||||
|
||||
RemoteAccountRefreshWorker.perform_async(request.target_account_id) if is_first_follow
|
||||
end
|
||||
|
||||
def accept_follow_for_relay
|
||||
|
|
|
@ -17,7 +17,7 @@ class StatusRelationshipsPresenter
|
|||
statuses = statuses.compact
|
||||
status_ids = statuses.flat_map { |s| [s.id, s.reblog_of_id] }.uniq.compact
|
||||
conversation_ids = statuses.filter_map(&:conversation_id).uniq
|
||||
pinnable_status_ids = statuses.map(&:proper).filter_map { |s| s.id if s.account_id == current_account_id && %w(public unlisted).include?(s.visibility) }
|
||||
pinnable_status_ids = statuses.map(&:proper).filter_map { |s| s.id if s.account_id == current_account_id && %w(public unlisted private).include?(s.visibility) }
|
||||
|
||||
result = ActiveRecord::Base.connection.select_all(ActiveRecord::Base.sanitize_sql_array([<<-SQL.squish, account_id: current_account_id, status_ids: status_ids, conversation_ids: conversation_ids, pinnable_status_ids: pinnable_status_ids])).to_a.first
|
||||
select
|
||||
|
|
|
@ -215,7 +215,7 @@ class REST::StatusSerializer < ActiveModel::Serializer
|
|||
def pinnable?
|
||||
owned_status? &&
|
||||
!object.reblog? &&
|
||||
%w(public unlisted).include?(object.visibility)
|
||||
%w(public unlisted private).include?(object.visibility)
|
||||
end
|
||||
|
||||
def source_requested?
|
||||
|
|
|
@ -100,4 +100,8 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService
|
|||
|
||||
@local_follower = @account.followers.local.without_suspended.first
|
||||
end
|
||||
|
||||
def local_follower
|
||||
@local_follower ||= @account.followers.local.without_suspended.first
|
||||
end
|
||||
end
|
||||
|
|
|
@ -4,7 +4,7 @@ class StatusPinValidator < ActiveModel::Validator
|
|||
def validate(pin)
|
||||
pin.errors.add(:base, I18n.t('statuses.pin_errors.reblog')) if pin.status.reblog?
|
||||
pin.errors.add(:base, I18n.t('statuses.pin_errors.ownership')) if pin.account_id != pin.status.account_id
|
||||
pin.errors.add(:base, I18n.t('statuses.pin_errors.private')) unless %w(public unlisted).include?(pin.status.visibility)
|
||||
pin.errors.add(:base, I18n.t('statuses.pin_errors.direct')) if pin.status.direct_visibility?
|
||||
pin.errors.add(:base, I18n.t('statuses.pin_errors.limit')) if pin.account.status_pins.count > 4 && pin.account.local?
|
||||
end
|
||||
end
|
||||
|
|
24
app/workers/remote_account_refresh_worker.rb
Normal file
24
app/workers/remote_account_refresh_worker.rb
Normal file
|
@ -0,0 +1,24 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class RemoteAccountRefreshWorker
|
||||
include Sidekiq::Worker
|
||||
include ExponentialBackoff
|
||||
include JsonLdHelper
|
||||
|
||||
sidekiq_options queue: 'pull', retry: 3
|
||||
|
||||
def perform(id)
|
||||
account = Account.find_by(id: id)
|
||||
return if account.nil? || account.local?
|
||||
|
||||
ActivityPub::FetchRemoteAccountService.new.call(account.uri)
|
||||
rescue Mastodon::UnexpectedResponseError => e
|
||||
response = e.response
|
||||
|
||||
if response_error_unsalvageable?(response)
|
||||
# Give up
|
||||
else
|
||||
raise e
|
||||
end
|
||||
end
|
||||
end
|
|
@ -1406,9 +1406,9 @@ en:
|
|||
open_in_web: Open in web
|
||||
over_character_limit: character limit of %{max} exceeded
|
||||
pin_errors:
|
||||
direct: Posts that are only visible to mentioned users cannot be pinned
|
||||
limit: You have already pinned the maximum number of posts
|
||||
ownership: Someone else's post cannot be pinned
|
||||
private: Non-public posts cannot be pinned
|
||||
reblog: A boost cannot be pinned
|
||||
poll:
|
||||
total_people:
|
||||
|
|
|
@ -35,6 +35,7 @@ RSpec.describe AccountsController, type: :controller do
|
|||
before do
|
||||
status_media.media_attachments << Fabricate(:media_attachment, account: account, type: :image)
|
||||
account.pinned_statuses << status_pinned
|
||||
account.pinned_statuses << status_private
|
||||
end
|
||||
|
||||
shared_examples 'preliminary checks' do
|
||||
|
|
|
@ -4,6 +4,7 @@ require 'rails_helper'
|
|||
|
||||
RSpec.describe ActivityPub::CollectionsController, type: :controller do
|
||||
let!(:account) { Fabricate(:account) }
|
||||
let!(:private_pinned) { Fabricate(:status, account: account, text: 'secret private stuff', visibility: :private) }
|
||||
let(:remote_account) { nil }
|
||||
|
||||
shared_examples 'cachable response' do
|
||||
|
@ -27,6 +28,7 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do
|
|||
|
||||
Fabricate(:status_pin, account: account)
|
||||
Fabricate(:status_pin, account: account)
|
||||
Fabricate(:status_pin, account: account, status: private_pinned)
|
||||
Fabricate(:status, account: account, visibility: :private)
|
||||
end
|
||||
|
||||
|
@ -50,7 +52,15 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do
|
|||
|
||||
it 'returns orderedItems with pinned statuses' do
|
||||
expect(body[:orderedItems]).to be_an Array
|
||||
expect(body[:orderedItems].size).to eq 2
|
||||
expect(body[:orderedItems].size).to eq 3
|
||||
end
|
||||
|
||||
it 'includes URI of private pinned status' do
|
||||
expect(body[:orderedItems]).to include(ActivityPub::TagManager.instance.uri_for(private_pinned))
|
||||
end
|
||||
|
||||
it 'does not include contents of private pinned status' do
|
||||
expect(response.body).not_to include(private_pinned.text)
|
||||
end
|
||||
|
||||
context 'when account is permanently suspended' do
|
||||
|
@ -96,7 +106,16 @@ RSpec.describe ActivityPub::CollectionsController, type: :controller do
|
|||
it 'returns orderedItems with pinned statuses' do
|
||||
json = body_as_json
|
||||
expect(json[:orderedItems]).to be_an Array
|
||||
expect(json[:orderedItems].size).to eq 2
|
||||
expect(json[:orderedItems].size).to eq 3
|
||||
end
|
||||
|
||||
it 'includes URI of private pinned status' do
|
||||
json = body_as_json
|
||||
expect(json[:orderedItems]).to include(ActivityPub::TagManager.instance.uri_for(private_pinned))
|
||||
end
|
||||
|
||||
it 'does not include contents of private pinned status' do
|
||||
expect(response.body).not_to include(private_pinned.text)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -39,7 +39,7 @@ describe Api::V1::Accounts::StatusesController do
|
|||
end
|
||||
end
|
||||
|
||||
context 'with only pinned' do
|
||||
context 'with only own pinned' do
|
||||
before do
|
||||
Fabricate(:status_pin, account: user.account, status: Fabricate(:status, account: user.account))
|
||||
end
|
||||
|
@ -50,5 +50,38 @@ describe Api::V1::Accounts::StatusesController do
|
|||
expect(response).to have_http_status(200)
|
||||
end
|
||||
end
|
||||
|
||||
context "with someone else's pinned statuses" do
|
||||
let(:account) { Fabricate(:account, username: 'bob', domain: 'example.com') }
|
||||
let(:status) { Fabricate(:status, account: account) }
|
||||
let(:private_status) { Fabricate(:status, account: account, visibility: :private) }
|
||||
let!(:pin) { Fabricate(:status_pin, account: account, status: status) }
|
||||
let!(:private_pin) { Fabricate(:status_pin, account: account, status: private_status) }
|
||||
|
||||
it 'returns http success' do
|
||||
get :index, params: { account_id: account.id, pinned: true }
|
||||
expect(response).to have_http_status(200)
|
||||
end
|
||||
|
||||
context 'when user does not follow account' do
|
||||
it 'lists the public status only' do
|
||||
get :index, params: { account_id: account.id, pinned: true }
|
||||
json = body_as_json
|
||||
expect(json.map { |item| item[:id].to_i }).to eq [status.id]
|
||||
end
|
||||
end
|
||||
|
||||
context 'when user follows account' do
|
||||
before do
|
||||
user.account.follow!(account)
|
||||
end
|
||||
|
||||
it 'lists both the public and the private statuses' do
|
||||
get :index, params: { account_id: account.id, pinned: true }
|
||||
json = body_as_json
|
||||
expect(json.map { |item| item[:id].to_i }.sort).to eq [status.id, private_status.id].sort
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -23,6 +23,7 @@ RSpec.describe ActivityPub::Activity::Accept do
|
|||
subject { described_class.new(json, sender) }
|
||||
|
||||
before do
|
||||
allow(RemoteAccountRefreshWorker).to receive(:perform_async)
|
||||
Fabricate(:follow_request, account: recipient, target_account: sender)
|
||||
subject.perform
|
||||
end
|
||||
|
@ -34,6 +35,10 @@ RSpec.describe ActivityPub::Activity::Accept do
|
|||
it 'removes the follow request' do
|
||||
expect(recipient.requested?(sender)).to be false
|
||||
end
|
||||
|
||||
it 'queues a refresh' do
|
||||
expect(RemoteAccountRefreshWorker).to have_received(:perform_async).with(sender.id)
|
||||
end
|
||||
end
|
||||
|
||||
context 'given a relay' do
|
||||
|
|
|
@ -1,8 +1,8 @@
|
|||
require 'rails_helper'
|
||||
|
||||
RSpec.describe ActivityPub::Activity::Add do
|
||||
let(:sender) { Fabricate(:account, featured_collection_url: 'https://example.com/featured') }
|
||||
let(:status) { Fabricate(:status, account: sender) }
|
||||
let(:sender) { Fabricate(:account, featured_collection_url: 'https://example.com/featured', domain: 'example.com') }
|
||||
let(:status) { Fabricate(:status, account: sender, visibility: :private) }
|
||||
|
||||
let(:json) do
|
||||
{
|
||||
|
@ -24,6 +24,8 @@ RSpec.describe ActivityPub::Activity::Add do
|
|||
end
|
||||
|
||||
context 'when status was not known before' do
|
||||
let(:service_stub) { double }
|
||||
|
||||
let(:json) do
|
||||
{
|
||||
'@context': 'https://www.w3.org/ns/activitystreams',
|
||||
|
@ -36,12 +38,40 @@ RSpec.describe ActivityPub::Activity::Add do
|
|||
end
|
||||
|
||||
before do
|
||||
stub_request(:get, 'https://example.com/unknown').to_return(status: 410)
|
||||
allow(ActivityPub::FetchRemoteStatusService).to receive(:new).and_return(service_stub)
|
||||
end
|
||||
|
||||
it 'fetches the status' do
|
||||
subject.perform
|
||||
expect(a_request(:get, 'https://example.com/unknown')).to have_been_made.at_least_once
|
||||
context 'when there is a local follower' do
|
||||
before do
|
||||
account = Fabricate(:account)
|
||||
account.follow!(sender)
|
||||
end
|
||||
|
||||
it 'fetches the status and pins it' do
|
||||
allow(service_stub).to receive(:call) do |uri, id: true, on_behalf_of: nil|
|
||||
expect(uri).to eq 'https://example.com/unknown'
|
||||
expect(id).to eq true
|
||||
expect(on_behalf_of&.following?(sender)).to eq true
|
||||
status
|
||||
end
|
||||
subject.perform
|
||||
expect(service_stub).to have_received(:call)
|
||||
expect(sender.pinned?(status)).to be true
|
||||
end
|
||||
end
|
||||
|
||||
context 'when there is no local follower' do
|
||||
it 'tries to fetch the status' do
|
||||
allow(service_stub).to receive(:call) do |uri, id: true, on_behalf_of: nil|
|
||||
expect(uri).to eq 'https://example.com/unknown'
|
||||
expect(id).to eq true
|
||||
expect(on_behalf_of).to eq nil
|
||||
nil
|
||||
end
|
||||
subject.perform
|
||||
expect(service_stub).to have_received(:call)
|
||||
expect(sender.pinned?(status)).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -24,11 +24,11 @@ RSpec.describe StatusPin, type: :model do
|
|||
expect(StatusPin.new(account: account, status: reblog).save).to be false
|
||||
end
|
||||
|
||||
it 'does not allow pins of private statuses' do
|
||||
it 'does allow pins of direct statuses' do
|
||||
account = Fabricate(:account)
|
||||
status = Fabricate(:status, account: account, visibility: :private)
|
||||
|
||||
expect(StatusPin.new(account: account, status: status).save).to be false
|
||||
expect(StatusPin.new(account: account, status: status).save).to be true
|
||||
end
|
||||
|
||||
it 'does not allow pins of direct statuses' do
|
||||
|
|
|
@ -9,7 +9,7 @@ RSpec.describe StatusPinValidator, type: :validator do
|
|||
end
|
||||
|
||||
let(:pin) { double(account: account, errors: errors, status: status, account_id: pin_account_id) }
|
||||
let(:status) { double(reblog?: reblog, account_id: status_account_id, visibility: visibility) }
|
||||
let(:status) { double(reblog?: reblog, account_id: status_account_id, visibility: visibility, direct_visibility?: visibility == 'direct') }
|
||||
let(:account) { double(status_pins: status_pins, local?: local) }
|
||||
let(:status_pins) { double(count: count) }
|
||||
let(:errors) { double(add: nil) }
|
||||
|
@ -37,11 +37,11 @@ RSpec.describe StatusPinValidator, type: :validator do
|
|||
end
|
||||
end
|
||||
|
||||
context 'unless %w(public unlisted).include?(pin.status.visibility)' do
|
||||
let(:visibility) { '' }
|
||||
context 'if pin.status.direct_visibility?' do
|
||||
let(:visibility) { 'direct' }
|
||||
|
||||
it 'calls errors.add' do
|
||||
expect(errors).to have_received(:add).with(:base, I18n.t('statuses.pin_errors.private'))
|
||||
expect(errors).to have_received(:add).with(:base, I18n.t('statuses.pin_errors.direct'))
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in a new issue