Wrong count in response when removing favourite/reblog (#24365)

Co-authored-by: Claire <claire.github-309c@sitedethib.com>
This commit is contained in:
Christian Schmidt 2023-07-19 09:02:30 +02:00 committed by GitHub
parent 6edd404482
commit 4c18928a93
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 73 additions and 19 deletions

View file

@ -17,13 +17,16 @@ class Api::V1::Statuses::FavouritesController < Api::BaseController
if fav if fav
@status = fav.status @status = fav.status
count = [@status.favourites_count - 1, 0].max
UnfavouriteWorker.perform_async(current_account.id, @status.id) UnfavouriteWorker.perform_async(current_account.id, @status.id)
else else
@status = Status.find(params[:status_id]) @status = Status.find(params[:status_id])
count = @status.favourites_count
authorize @status, :show? authorize @status, :show?
end end
render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, favourites_map: { @status.id => false }) relationships = StatusRelationshipsPresenter.new([@status], current_account.id, favourites_map: { @status.id => false }, attributes_map: { @status.id => { favourites_count: count } })
render json: @status, serializer: REST::StatusSerializer, relationships: relationships
rescue Mastodon::NotPermittedError rescue Mastodon::NotPermittedError
not_found not_found
end end

View file

@ -24,15 +24,18 @@ class Api::V1::Statuses::ReblogsController < Api::BaseController
if @status if @status
authorize @status, :unreblog? authorize @status, :unreblog?
@reblog = @status.reblog
count = [@reblog.reblogs_count - 1, 0].max
@status.discard @status.discard
RemovalWorker.perform_async(@status.id) RemovalWorker.perform_async(@status.id)
@reblog = @status.reblog
else else
@reblog = Status.find(params[:status_id]) @reblog = Status.find(params[:status_id])
count = @reblog.reblogs_count
authorize @reblog, :show? authorize @reblog, :show?
end end
render json: @reblog, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, reblogs_map: { @reblog.id => false }) relationships = StatusRelationshipsPresenter.new([@status], current_account.id, reblogs_map: { @reblog.id => false }, attributes_map: { @reblog.id => { reblogs_count: count } })
render json: @reblog, serializer: REST::StatusSerializer, relationships: relationships
rescue Mastodon::NotPermittedError rescue Mastodon::NotPermittedError
not_found not_found
end end

View file

@ -5,11 +5,16 @@ import { normalizeStatusTranslation } from '../actions/importer/normalizer';
import { import {
REBLOG_REQUEST, REBLOG_REQUEST,
REBLOG_FAIL, REBLOG_FAIL,
UNREBLOG_REQUEST,
UNREBLOG_FAIL,
FAVOURITE_REQUEST, FAVOURITE_REQUEST,
FAVOURITE_FAIL, FAVOURITE_FAIL,
UNFAVOURITE_SUCCESS, UNFAVOURITE_REQUEST,
UNFAVOURITE_FAIL,
BOOKMARK_REQUEST, BOOKMARK_REQUEST,
BOOKMARK_FAIL, BOOKMARK_FAIL,
UNBOOKMARK_REQUEST,
UNBOOKMARK_FAIL,
} from '../actions/interactions'; } from '../actions/interactions';
import { import {
STATUS_MUTE_SUCCESS, STATUS_MUTE_SUCCESS,
@ -72,18 +77,28 @@ export default function statuses(state = initialState, action) {
return importStatuses(state, action.statuses); return importStatuses(state, action.statuses);
case FAVOURITE_REQUEST: case FAVOURITE_REQUEST:
return state.setIn([action.status.get('id'), 'favourited'], true); return state.setIn([action.status.get('id'), 'favourited'], true);
case UNFAVOURITE_SUCCESS:
return state.updateIn([action.status.get('id'), 'favourites_count'], x => Math.max(0, x - 1));
case FAVOURITE_FAIL: case FAVOURITE_FAIL:
return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'favourited'], false); return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'favourited'], false);
case UNFAVOURITE_REQUEST:
return state.setIn([action.status.get('id'), 'favourited'], false);
case UNFAVOURITE_FAIL:
return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'favourited'], true);
case BOOKMARK_REQUEST: case BOOKMARK_REQUEST:
return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], true); return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], true);
case BOOKMARK_FAIL: case BOOKMARK_FAIL:
return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], false); return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], false);
case UNBOOKMARK_REQUEST:
return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], false);
case UNBOOKMARK_FAIL:
return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], true);
case REBLOG_REQUEST: case REBLOG_REQUEST:
return state.setIn([action.status.get('id'), 'reblogged'], true); return state.setIn([action.status.get('id'), 'reblogged'], true);
case REBLOG_FAIL: case REBLOG_FAIL:
return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'reblogged'], false); return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'reblogged'], false);
case UNREBLOG_REQUEST:
return state.setIn([action.status.get('id'), 'reblogged'], false);
case UNREBLOG_FAIL:
return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'reblogged'], true);
case STATUS_MUTE_SUCCESS: case STATUS_MUTE_SUCCESS:
return state.setIn([action.id, 'muted'], true); return state.setIn([action.id, 'muted'], true);
case STATUS_UNMUTE_SUCCESS: case STATUS_UNMUTE_SUCCESS:

View file

@ -4,7 +4,7 @@ class StatusRelationshipsPresenter
PINNABLE_VISIBILITIES = %w(public unlisted private).freeze PINNABLE_VISIBILITIES = %w(public unlisted private).freeze
attr_reader :reblogs_map, :favourites_map, :mutes_map, :pins_map, attr_reader :reblogs_map, :favourites_map, :mutes_map, :pins_map,
:bookmarks_map, :filters_map :bookmarks_map, :filters_map, :attributes_map
def initialize(statuses, current_account_id = nil, **options) def initialize(statuses, current_account_id = nil, **options)
if current_account_id.nil? if current_account_id.nil?
@ -26,6 +26,7 @@ class StatusRelationshipsPresenter
@bookmarks_map = Status.bookmarks_map(status_ids, current_account_id).merge(options[:bookmarks_map] || {}) @bookmarks_map = Status.bookmarks_map(status_ids, current_account_id).merge(options[:bookmarks_map] || {})
@mutes_map = Status.mutes_map(conversation_ids, current_account_id).merge(options[:mutes_map] || {}) @mutes_map = Status.mutes_map(conversation_ids, current_account_id).merge(options[:mutes_map] || {})
@pins_map = Status.pins_map(pinnable_status_ids, current_account_id).merge(options[:pins_map] || {}) @pins_map = Status.pins_map(pinnable_status_ids, current_account_id).merge(options[:pins_map] || {})
@attributes_map = options[:attributes_map] || {}
end end
end end

View file

@ -81,49 +81,57 @@ class REST::StatusSerializer < ActiveModel::Serializer
ActivityPub::TagManager.instance.url_for(object) ActivityPub::TagManager.instance.url_for(object)
end end
def reblogs_count
relationships&.attributes_map&.dig(object.id, :reblogs_count) || object.reblogs_count
end
def favourites_count
relationships&.attributes_map&.dig(object.id, :favourites_count) || object.favourites_count
end
def favourited def favourited
if instance_options && instance_options[:relationships] if relationships
instance_options[:relationships].favourites_map[object.id] || false relationships.favourites_map[object.id] || false
else else
current_user.account.favourited?(object) current_user.account.favourited?(object)
end end
end end
def reblogged def reblogged
if instance_options && instance_options[:relationships] if relationships
instance_options[:relationships].reblogs_map[object.id] || false relationships.reblogs_map[object.id] || false
else else
current_user.account.reblogged?(object) current_user.account.reblogged?(object)
end end
end end
def muted def muted
if instance_options && instance_options[:relationships] if relationships
instance_options[:relationships].mutes_map[object.conversation_id] || false relationships.mutes_map[object.conversation_id] || false
else else
current_user.account.muting_conversation?(object.conversation) current_user.account.muting_conversation?(object.conversation)
end end
end end
def bookmarked def bookmarked
if instance_options && instance_options[:relationships] if relationships
instance_options[:relationships].bookmarks_map[object.id] || false relationships.bookmarks_map[object.id] || false
else else
current_user.account.bookmarked?(object) current_user.account.bookmarked?(object)
end end
end end
def pinned def pinned
if instance_options && instance_options[:relationships] if relationships
instance_options[:relationships].pins_map[object.id] || false relationships.pins_map[object.id] || false
else else
current_user.account.pinned?(object) current_user.account.pinned?(object)
end end
end end
def filtered def filtered
if instance_options && instance_options[:relationships] if relationships
instance_options[:relationships].filters_map[object.id] || [] relationships.filters_map[object.id] || []
else else
current_user.account.status_matches_filters(object) current_user.account.status_matches_filters(object)
end end
@ -144,6 +152,12 @@ class REST::StatusSerializer < ActiveModel::Serializer
object.active_mentions.to_a.sort_by(&:id) object.active_mentions.to_a.sort_by(&:id)
end end
private
def relationships
instance_options && instance_options[:relationships]
end
class ApplicationSerializer < ActiveModel::Serializer class ApplicationSerializer < ActiveModel::Serializer
attributes :name, :website attributes :name, :website

View file

@ -10,6 +10,12 @@ describe Api::V1::Statuses::ReblogsController do
let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'write:statuses', application: app) } let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'write:statuses', application: app) }
context 'with an oauth token' do context 'with an oauth token' do
around do |example|
Sidekiq::Testing.fake! do
example.run
end
end
before do before do
allow(controller).to receive(:doorkeeper_token) { token } allow(controller).to receive(:doorkeeper_token) { token }
end end

View file

@ -77,6 +77,12 @@ RSpec.describe 'Favourites' do
let(:status) { Fabricate(:status) } let(:status) { Fabricate(:status) }
around do |example|
Sidekiq::Testing.fake! do
example.run
end
end
it_behaves_like 'forbidden for wrong scope', 'read read:favourites' it_behaves_like 'forbidden for wrong scope', 'read read:favourites'
context 'with public status' do context 'with public status' do
@ -88,6 +94,9 @@ RSpec.describe 'Favourites' do
subject subject
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(user.account.favourited?(status)).to be true
UnfavouriteWorker.drain
expect(user.account.favourited?(status)).to be false expect(user.account.favourited?(status)).to be false
end end
@ -110,6 +119,9 @@ RSpec.describe 'Favourites' do
subject subject
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(user.account.favourited?(status)).to be true
UnfavouriteWorker.drain
expect(user.account.favourited?(status)).to be false expect(user.account.favourited?(status)).to be false
end end