Add support for private pinned posts (#16954)

* Add support for private pinned toots

* Allow local user to pin private toots

* Change wording to avoid "direct message"

spec/controllers/accounts_controller_spec.rb

@@ -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

spec/controllers/activitypub/collections_controller_spec.rb

@@ -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
 

spec/controllers/api/v1/accounts/statuses_controller_spec.rb

@@ -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

spec/lib/activitypub/activity/accept_spec.rb

@@ -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

spec/lib/activitypub/activity/add_spec.rb

@@ -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

spec/models/status_pin_spec.rb

@@ -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

spec/validators/status_pin_validator_spec.rb

@@ -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