Merge pull request glitch-soc#2776 from ClearlyClaire/glitch-soc/merg…

…e-upstream

Merge upstream changes up to 00cb4a0

Gemfile.lock

@@ -756,7 +756,7 @@ GEM
       rack (>= 1.1)
       rubocop (>= 1.33.0, < 2.0)
       rubocop-ast (>= 1.31.1, < 2.0)
-    rubocop-rspec (3.0.2)
+    rubocop-rspec (3.0.3)
       rubocop (~> 1.61)
     rubocop-rspec_rails (2.30.0)
       rubocop (~> 1.61)

app/controllers/api/v1/notifications/requests_controller.rb

@@ -28,14 +28,14 @@ def accept
   end
 
   def dismiss
-    @request.update!(dismissed: true)
+    @request.destroy!
     render_empty
   end
 
   private
 
   def load_requests
-    requests = NotificationRequest.where(account: current_account).where(dismissed: truthy_param?(:dismissed) || false).includes(:last_status, from_account: [:account_stat, :user]).to_a_paginated_by_id(
+    requests = NotificationRequest.where(account: current_account).includes(:last_status, from_account: [:account_stat, :user]).to_a_paginated_by_id(
       limit_param(DEFAULT_ACCOUNTS_LIMIT),
       params_slice(:max_id, :since_id, :min_id)
     )
@@ -68,8 +68,4 @@ def pagination_max_id
   def pagination_since_id
     @requests.first.id
   end
-
-  def pagination_params(core_params)
-    params.slice(:dismissed).permit(:dismissed).merge(core_params)
-  end
 end

app/models/notification_policy.rb

@@ -31,6 +31,6 @@ def summarize!
   private
 
   def pending_notification_requests
-    @pending_notification_requests ||= notification_requests.where(dismissed: false).limit(MAX_MEANINGFUL_COUNT).pick(Arel.sql('count(*), coalesce(sum(notifications_count), 0)::bigint'))
+    @pending_notification_requests ||= notification_requests.limit(MAX_MEANINGFUL_COUNT).pick(Arel.sql('count(*), coalesce(sum(notifications_count), 0)::bigint'))
   end
 end

app/models/notification_request.rb

@@ -9,12 +9,13 @@
 #  from_account_id     :bigint(8)        not null
 #  last_status_id      :bigint(8)
 #  notifications_count :bigint(8)        default(0), not null
-#  dismissed           :boolean          default(FALSE), not null
 #  created_at          :datetime         not null
 #  updated_at          :datetime         not null
 #
 
 class NotificationRequest < ApplicationRecord
+  self.ignored_columns += %w(dismissed)
+
   include Paginable
 
   MAX_MEANINGFUL_COUNT = 100
@@ -34,8 +35,6 @@ def self.preload_cache_collection(requests)
   end
 
   def reconsider_existence!
-    return if dismissed?
-
     prepare_notifications_count
 
     if notifications_count.positive?

db/post_migrate/20240712064044_remove_dismissed_from_notification_requests.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class RemoveDismissedFromNotificationRequests < ActiveRecord::Migration[7.1]
+  def up
+    safety_assured do
+      execute 'DELETE FROM notification_requests WHERE dismissed'
+      remove_column :notification_requests, :dismissed
+    end
+  end
+
+  def down
+    add_column :notification_requests, :dismissed, :boolean, default: false, null: false
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_06_07_094856) do
+ActiveRecord::Schema[7.1].define(version: 2024_07_12_064044) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -706,11 +706,9 @@
     t.bigint "from_account_id", null: false
     t.bigint "last_status_id"
     t.bigint "notifications_count", default: 0, null: false
-    t.boolean "dismissed", default: false, null: false
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.index ["account_id", "from_account_id"], name: "index_notification_requests_on_account_id_and_from_account_id", unique: true
-    t.index ["account_id", "id"], name: "index_notification_requests_on_account_id_and_id", order: { id: :desc }, where: "(dismissed = false)"
     t.index ["from_account_id"], name: "index_notification_requests_on_from_account_id"
     t.index ["last_status_id"], name: "index_notification_requests_on_last_status_id"
   end

spec/fabricators/notification_request_fabricator.rb

@@ -4,5 +4,4 @@
   account
   from_account { Fabricate.build(:account) }
   last_status { Fabricate.build(:status) }
-  dismissed false
 end

spec/lib/vacuum/media_attachments_vacuum_spec.rb

@@ -17,32 +17,21 @@
     let!(:old_unattached_media) { Fabricate(:media_attachment, account_id: nil, created_at: 10.days.ago) }
     let!(:new_unattached_media) { Fabricate(:media_attachment, account_id: nil, created_at: 1.hour.ago) }
 
-    before do
-      subject.perform
-    end
-
-    it 'deletes cache of remote media attachments past the retention period' do
-      expect(old_remote_media.reload.file).to be_blank
-    end
-
-    it 'does not touch local media attachments past the retention period' do
-      expect(old_local_media.reload.file).to_not be_blank
-    end
-
-    it 'does not delete cache of remote media attachments within the retention period' do
-      expect(new_remote_media.reload.file).to_not be_blank
-    end
-
-    it 'does not touch local media attachments within the retention period' do
-      expect(new_local_media.reload.file).to_not be_blank
-    end
-
-    it 'deletes unattached media attachments past TTL' do
-      expect { old_unattached_media.reload }.to raise_error(ActiveRecord::RecordNotFound)
-    end
-
-    it 'does not delete unattached media attachments within TTL' do
-      expect(new_unattached_media.reload).to be_persisted
+    before { subject.perform }
+
+    it 'handles attachments based on metadata details' do
+      expect(old_remote_media.reload.file) # Remote and past retention period
+        .to be_blank
+      expect(old_local_media.reload.file) # Local and past retention
+        .to_not be_blank
+      expect(new_remote_media.reload.file) # Remote and within retention
+        .to_not be_blank
+      expect(new_local_media.reload.file) # Local and within retention
+        .to_not be_blank
+      expect { old_unattached_media.reload } # Unattached and past TTL
+        .to raise_error(ActiveRecord::RecordNotFound)
+      expect(new_unattached_media.reload) # Unattached and within TTL
+        .to be_persisted
     end
   end
 end

spec/models/notification_request_spec.rb

@@ -4,9 +4,7 @@
 
 RSpec.describe NotificationRequest do
   describe '#reconsider_existence!' do
-    subject { Fabricate(:notification_request, dismissed: dismissed) }
-
-    let(:dismissed) { false }
+    subject { Fabricate(:notification_request) }
 
     context 'when there are remaining notifications' do
       before do
@@ -28,14 +26,6 @@
         subject.reconsider_existence!
       end
 
-      context 'when dismissed' do
-        let(:dismissed) { true }
-
-        it 'leaves request intact' do
-          expect(subject.destroyed?).to be false
-        end
-      end
-
       it 'removes the request' do
         expect(subject.destroyed?).to be true
       end

spec/requests/api/v1/notifications/requests_spec.rb

@@ -17,7 +17,6 @@
 
     before do
       Fabricate(:notification_request, account: user.account)
-      Fabricate(:notification_request, account: user.account, dismissed: true)
     end
 
     it_behaves_like 'forbidden for wrong scope', 'write write:notifications'
@@ -29,16 +28,6 @@
         expect(response).to have_http_status(200)
       end
     end
-
-    context 'with dismissed' do
-      let(:params) { { dismissed: '1' } }
-
-      it 'returns http success', :aggregate_failures do
-        subject
-
-        expect(response).to have_http_status(200)
-      end
-    end
   end
 
   describe 'POST /api/v1/notifications/requests/:id/accept' do
@@ -78,15 +67,14 @@
       post "/api/v1/notifications/requests/#{notification_request.id}/dismiss", headers: headers
     end
 
-    let(:notification_request) { Fabricate(:notification_request, account: user.account) }
+    let!(:notification_request) { Fabricate(:notification_request, account: user.account) }
 
     it_behaves_like 'forbidden for wrong scope', 'read read:notifications'
 
-    it 'returns http success and dismisses the notification request', :aggregate_failures do
-      subject
+    it 'returns http success and destroys the notification request', :aggregate_failures do
+      expect { subject }.to change(NotificationRequest, :count).by(-1)
 
       expect(response).to have_http_status(200)
-      expect(notification_request.reload.dismissed?).to be true
     end
 
     context 'when notification request belongs to someone else' do

spec/services/notify_service_spec.rb

@@ -129,6 +129,39 @@
     end
   end
 
+  context 'with filtered notifications' do
+    let(:unknown)  { Fabricate(:account, username: 'unknown') }
+    let(:status)   { Fabricate(:status, account: unknown) }
+    let(:activity) { Fabricate(:mention, account: recipient, status: status) }
+    let(:type)     { :mention }
+
+    before do
+      Fabricate(:notification_policy, account: recipient, filter_not_following: true)
+    end
+
+    it 'creates a filtered notification' do
+      expect { subject }.to change(Notification, :count)
+      expect(Notification.last).to be_filtered
+    end
+
+    context 'when no notification request exists' do
+      it 'creates a notification request' do
+        expect { subject }.to change(NotificationRequest, :count)
+      end
+    end
+
+    context 'when a notification request exists' do
+      let!(:notification_request) do
+        Fabricate(:notification_request, account: recipient, from_account: unknown, last_status: Fabricate(:status, account: unknown))
+      end
+
+      it 'updates the existing notification request' do
+        expect { subject }.to_not change(NotificationRequest, :count)
+        expect(notification_request.reload.last_status).to eq status
+      end
+    end
+  end
+
   describe NotifyService::DismissCondition do
     subject { described_class.new(notification) }
 

streaming/index.js

@@ -524,43 +524,27 @@ const startServer = async () => {
    * @param {any} req
    * @returns {Promise<ResolvedAccount>}
    */
-  const accountFromToken = (token, req) => new Promise((resolve, reject) => {
-    pgPool.connect((err, client, done) => {
-      if (err) {
-        reject(err);
-        return;
-      }
-
-      // @ts-ignore
-      client.query('SELECT oauth_access_tokens.id, oauth_access_tokens.resource_owner_id, users.account_id, users.chosen_languages, oauth_access_tokens.scopes, devices.device_id FROM oauth_access_tokens INNER JOIN users ON oauth_access_tokens.resource_owner_id = users.id LEFT OUTER JOIN devices ON oauth_access_tokens.id = devices.access_token_id WHERE oauth_access_tokens.token = $1 AND oauth_access_tokens.revoked_at IS NULL LIMIT 1', [token], (err, result) => {
-        done();
-
-        if (err) {
-          reject(err);
-          return;
-        }
+  const accountFromToken = async (token, req) => {
+    const result = await pgPool.query('SELECT oauth_access_tokens.id, oauth_access_tokens.resource_owner_id, users.account_id, users.chosen_languages, oauth_access_tokens.scopes, devices.device_id FROM oauth_access_tokens INNER JOIN users ON oauth_access_tokens.resource_owner_id = users.id LEFT OUTER JOIN devices ON oauth_access_tokens.id = devices.access_token_id WHERE oauth_access_tokens.token = $1 AND oauth_access_tokens.revoked_at IS NULL LIMIT 1', [token]);
 
-        if (result.rows.length === 0) {
-          reject(new AuthenticationError('Invalid access token'));
-          return;
-        }
-
-        req.accessTokenId = result.rows[0].id;
-        req.scopes = result.rows[0].scopes.split(' ');
-        req.accountId = result.rows[0].account_id;
-        req.chosenLanguages = result.rows[0].chosen_languages;
-        req.deviceId = result.rows[0].device_id;
+    if (result.rows.length === 0) {
+      throw new AuthenticationError('Invalid access token');
+    }
 
-        resolve({
-          accessTokenId: result.rows[0].id,
-          scopes: result.rows[0].scopes.split(' '),
-          accountId: result.rows[0].account_id,
-          chosenLanguages: result.rows[0].chosen_languages,
-          deviceId: result.rows[0].device_id
-        });
-      });
-    });
-  });
+    req.accessTokenId = result.rows[0].id;
+    req.scopes = result.rows[0].scopes.split(' ');
+    req.accountId = result.rows[0].account_id;
+    req.chosenLanguages = result.rows[0].chosen_languages;
+    req.deviceId = result.rows[0].device_id;
+
+    return {
+      accessTokenId: result.rows[0].id,
+      scopes: result.rows[0].scopes.split(' '),
+      accountId: result.rows[0].account_id,
+      chosenLanguages: result.rows[0].chosen_languages,
+      deviceId: result.rows[0].device_id
+    };
+  };
 
   /**
    * @param {any} req
@@ -771,28 +755,15 @@ const startServer = async () => {
    * @param {any} req
    * @returns {Promise.<void>}
    */
-  const authorizeListAccess = (listId, req) => new Promise((resolve, reject) => {
+  const authorizeListAccess = async (listId, req) => {
     const { accountId } = req;
 
-    pgPool.connect((err, client, done) => {
-      if (err) {
-        reject();
-        return;
-      }
-
-      // @ts-ignore
-      client.query('SELECT id, account_id FROM lists WHERE id = $1 LIMIT 1', [listId], (err, result) => {
-        done();
-
-        if (err || result.rows.length === 0 || result.rows[0].account_id !== accountId) {
-          reject();
-          return;
-        }
+    const result = await pgPool.query('SELECT id, account_id FROM lists WHERE id = $1 AND account_id = $2 LIMIT 1', [listId, accountId]);
 
-        resolve();
-      });
-    });
-  });
+    if (result.rows.length === 0) {
+      throw new AuthenticationError('List not found');
+    }
+  };
 
   /**
    * @param {string[]} channelIds