Add missing_attribution boolean to preview cards (#38043)
This commit is contained in:
parent
5472ab251a
commit
8a0261c51c
@ -75,6 +75,8 @@ class StatusCacheHydrator
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
payload[:card][:missing_attribution] = status.preview_card.unverified_author_account_id == account_id if payload[:card]
|
||||||
|
|
||||||
# Nested statuses are more likely to have a stale cache
|
# Nested statuses are more likely to have a stale cache
|
||||||
fill_status_stats(payload, status) if nested
|
fill_status_stats(payload, status) if nested
|
||||||
end
|
end
|
||||||
|
|||||||
@ -33,6 +33,7 @@
|
|||||||
# published_at :datetime
|
# published_at :datetime
|
||||||
# image_description :string default(""), not null
|
# image_description :string default(""), not null
|
||||||
# author_account_id :bigint(8)
|
# author_account_id :bigint(8)
|
||||||
|
# unverified_author_account_id :bigint(8)
|
||||||
#
|
#
|
||||||
|
|
||||||
class PreviewCard < ApplicationRecord
|
class PreviewCard < ApplicationRecord
|
||||||
@ -61,6 +62,7 @@ class PreviewCard < ApplicationRecord
|
|||||||
|
|
||||||
has_one :trend, class_name: 'PreviewCardTrend', inverse_of: :preview_card, dependent: :destroy
|
has_one :trend, class_name: 'PreviewCardTrend', inverse_of: :preview_card, dependent: :destroy
|
||||||
belongs_to :author_account, class_name: 'Account', optional: true
|
belongs_to :author_account, class_name: 'Account', optional: true
|
||||||
|
belongs_to :unverified_author_account, class_name: 'Account', optional: true
|
||||||
|
|
||||||
has_attached_file :image,
|
has_attached_file :image,
|
||||||
processors: [:lazy_thumbnail, :blurhash_transcoder],
|
processors: [:lazy_thumbnail, :blurhash_transcoder],
|
||||||
|
|||||||
@ -15,6 +15,8 @@ class REST::PreviewCardSerializer < ActiveModel::Serializer
|
|||||||
|
|
||||||
has_many :authors, serializer: AuthorSerializer
|
has_many :authors, serializer: AuthorSerializer
|
||||||
|
|
||||||
|
attribute :missing_attribution, if: :current_user?
|
||||||
|
|
||||||
def url
|
def url
|
||||||
object.original_url.presence || object.url
|
object.original_url.presence || object.url
|
||||||
end
|
end
|
||||||
@ -26,4 +28,12 @@ class REST::PreviewCardSerializer < ActiveModel::Serializer
|
|||||||
def html
|
def html
|
||||||
Sanitize.fragment(object.html, Sanitize::Config::MASTODON_OEMBED)
|
Sanitize.fragment(object.html, Sanitize::Config::MASTODON_OEMBED)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def missing_attribution
|
||||||
|
object.unverified_author_account_id.present? && object.unverified_author_account_id == current_user.account_id
|
||||||
|
end
|
||||||
|
|
||||||
|
def current_user?
|
||||||
|
!current_user.nil?
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@ -159,7 +159,14 @@ class FetchLinkCardService < BaseService
|
|||||||
|
|
||||||
@card = PreviewCard.find_or_initialize_by(url: link_details_extractor.canonical_url) if link_details_extractor.canonical_url != @card.url
|
@card = PreviewCard.find_or_initialize_by(url: link_details_extractor.canonical_url) if link_details_extractor.canonical_url != @card.url
|
||||||
@card.assign_attributes(link_details_extractor.to_preview_card_attributes)
|
@card.assign_attributes(link_details_extractor.to_preview_card_attributes)
|
||||||
@card.author_account = linked_account if linked_account&.can_be_attributed_from?(domain) || provider&.trendable?
|
|
||||||
|
if linked_account.present?
|
||||||
|
# There is an overlap in the two conditions when `provider` is trendable. This is on purpose to give users
|
||||||
|
# a heads-up before we remove the `provider&.trendable?` condition.
|
||||||
|
@card.author_account = linked_account if linked_account.can_be_attributed_from?(domain) || provider&.trendable?
|
||||||
|
@card.unverified_author_account = linked_account if linked_account.local? && !linked_account.can_be_attributed_from?(domain)
|
||||||
|
end
|
||||||
|
|
||||||
@card.save_with_optional_image! unless @card.title.blank? && @card.html.blank?
|
@card.save_with_optional_image! unless @card.title.blank? && @card.html.blank?
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@ -1,6 +1,8 @@
|
|||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
class UpdateAccountService < BaseService
|
class UpdateAccountService < BaseService
|
||||||
|
PREVIEW_CARD_REATTRIBUTION_LIMIT = 1_000
|
||||||
|
|
||||||
def call(account, params, raise_error: false)
|
def call(account, params, raise_error: false)
|
||||||
was_locked = account.locked
|
was_locked = account.locked
|
||||||
update_method = raise_error ? :update! : :update
|
update_method = raise_error ? :update! : :update
|
||||||
@ -11,6 +13,7 @@ class UpdateAccountService < BaseService
|
|||||||
authorize_all_follow_requests(account) if was_locked && !account.locked
|
authorize_all_follow_requests(account) if was_locked && !account.locked
|
||||||
check_links(account)
|
check_links(account)
|
||||||
process_hashtags(account)
|
process_hashtags(account)
|
||||||
|
process_attribution_domains(account)
|
||||||
end
|
end
|
||||||
rescue Mastodon::DimensionsValidationError, Mastodon::StreamValidationError => e
|
rescue Mastodon::DimensionsValidationError, Mastodon::StreamValidationError => e
|
||||||
account.errors.add(:avatar, e.message)
|
account.errors.add(:avatar, e.message)
|
||||||
@ -36,4 +39,22 @@ class UpdateAccountService < BaseService
|
|||||||
def process_hashtags(account)
|
def process_hashtags(account)
|
||||||
account.tags_as_strings = Extractor.extract_hashtags(account.note)
|
account.tags_as_strings = Extractor.extract_hashtags(account.note)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def process_attribution_domains(account)
|
||||||
|
return unless account.attribute_previously_changed?(:attribution_domains)
|
||||||
|
|
||||||
|
# Go through the most recent cards, and do the rest in a background job
|
||||||
|
preview_cards = PreviewCard.where(unverified_author_account: account).reorder(id: :desc).limit(PREVIEW_CARD_REATTRIBUTION_LIMIT).to_a
|
||||||
|
should_queue_worker = preview_cards.size == PREVIEW_CARD_REATTRIBUTION_LIMIT
|
||||||
|
|
||||||
|
preview_cards = preview_cards.filter do |preview_card|
|
||||||
|
account.can_be_attributed_from?(preview_card.domain)
|
||||||
|
rescue Addressable::URI::InvalidURIError
|
||||||
|
false
|
||||||
|
end
|
||||||
|
|
||||||
|
PreviewCard.where(id: preview_cards.pluck(:id), unverified_author_account: account).update_all(author_account_id: account.id, unverified_author_account_id: nil)
|
||||||
|
|
||||||
|
UpdateLinkCardAttributionWorker.perform_async(account.id) if should_queue_worker
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
23
app/workers/update_link_card_attribution_worker.rb
Normal file
23
app/workers/update_link_card_attribution_worker.rb
Normal file
@ -0,0 +1,23 @@
|
|||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
class UpdateLinkCardAttributionWorker
|
||||||
|
include Sidekiq::IterableJob
|
||||||
|
|
||||||
|
def build_enumerator(account_id, cursor:)
|
||||||
|
@account = Account.find_by(id: account_id)
|
||||||
|
return if @account.blank?
|
||||||
|
|
||||||
|
scope = PreviewCard.where(unverified_author_account: @account)
|
||||||
|
active_record_batches_enumerator(scope, cursor:)
|
||||||
|
end
|
||||||
|
|
||||||
|
def each_iteration(preview_cards, account_id)
|
||||||
|
preview_cards = preview_cards.filter do |preview_card|
|
||||||
|
@account.can_be_attributed_from?(preview_card.domain)
|
||||||
|
rescue Addressable::URI::InvalidURIError
|
||||||
|
false
|
||||||
|
end
|
||||||
|
|
||||||
|
PreviewCard.where(id: preview_cards.pluck(:id), unverified_author_account: @account).update_all(author_account_id: account_id, unverified_author_account_id: nil)
|
||||||
|
end
|
||||||
|
end
|
||||||
@ -0,0 +1,10 @@
|
|||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
class AddUnverifiedAuthorAccountIdToPreviewCards < ActiveRecord::Migration[8.1]
|
||||||
|
disable_ddl_transaction!
|
||||||
|
|
||||||
|
def change
|
||||||
|
safety_assured { add_reference :preview_cards, :unverified_author_account, null: true, foreign_key: { to_table: 'accounts', on_delete: :nullify }, index: false }
|
||||||
|
add_index :preview_cards, [:unverified_author_account_id, :id], algorithm: :concurrently, where: 'unverified_author_account_id IS NOT NULL'
|
||||||
|
end
|
||||||
|
end
|
||||||
@ -10,7 +10,7 @@
|
|||||||
#
|
#
|
||||||
# It's strongly recommended that you check this file into your version control system.
|
# It's strongly recommended that you check this file into your version control system.
|
||||||
|
|
||||||
ActiveRecord::Schema[8.0].define(version: 2026_02_17_154542) do
|
ActiveRecord::Schema[8.0].define(version: 2026_03_03_144409) do
|
||||||
# These are extensions that must be enabled in order to support this database
|
# These are extensions that must be enabled in order to support this database
|
||||||
enable_extension "pg_catalog.plpgsql"
|
enable_extension "pg_catalog.plpgsql"
|
||||||
|
|
||||||
@ -964,7 +964,9 @@ ActiveRecord::Schema[8.0].define(version: 2026_02_17_154542) do
|
|||||||
t.datetime "published_at"
|
t.datetime "published_at"
|
||||||
t.string "image_description", default: "", null: false
|
t.string "image_description", default: "", null: false
|
||||||
t.bigint "author_account_id"
|
t.bigint "author_account_id"
|
||||||
|
t.bigint "unverified_author_account_id"
|
||||||
t.index ["author_account_id"], name: "index_preview_cards_on_author_account_id", where: "(author_account_id IS NOT NULL)"
|
t.index ["author_account_id"], name: "index_preview_cards_on_author_account_id", where: "(author_account_id IS NOT NULL)"
|
||||||
|
t.index ["unverified_author_account_id", "id"], name: "index_preview_cards_on_unverified_author_account_id_and_id", where: "(unverified_author_account_id IS NOT NULL)"
|
||||||
t.index ["url"], name: "index_preview_cards_on_url", unique: true
|
t.index ["url"], name: "index_preview_cards_on_url", unique: true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@ -19,6 +19,18 @@ RSpec.describe StatusCacheHydrator do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when handling a new status with a preview card with unverified account attribution' do
|
||||||
|
let(:preview_card) { Fabricate(:preview_card, unverified_author_account: account) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
PreviewCardsStatus.create(status: status, preview_card: preview_card)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'renders the same attributes as a full render' do
|
||||||
|
expect(subject).to eql(compare_to_hash)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'when handling a new status with own poll' do
|
context 'when handling a new status with own poll' do
|
||||||
let(:poll) { Fabricate(:poll, account: account) }
|
let(:poll) { Fabricate(:poll, account: account) }
|
||||||
let(:status) { Fabricate(:status, poll: poll, account: account) }
|
let(:status) { Fabricate(:status, poll: poll, account: account) }
|
||||||
|
|||||||
@ -6,10 +6,16 @@ RSpec.describe REST::PreviewCardSerializer do
|
|||||||
subject do
|
subject do
|
||||||
serialized_record_json(
|
serialized_record_json(
|
||||||
preview_card,
|
preview_card,
|
||||||
described_class
|
described_class,
|
||||||
|
options: {
|
||||||
|
scope: current_user,
|
||||||
|
scope_name: :current_user,
|
||||||
|
}
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
let(:current_user) { nil }
|
||||||
|
|
||||||
context 'when preview card does not have author data' do
|
context 'when preview card does not have author data' do
|
||||||
let(:preview_card) { Fabricate.build :preview_card }
|
let(:preview_card) { Fabricate.build :preview_card }
|
||||||
|
|
||||||
|
|||||||
@ -33,4 +33,18 @@ RSpec.describe UpdateAccountService do
|
|||||||
expect(eve).to_not be_requested(account)
|
expect(eve).to_not be_requested(account)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'adding domains to attribution_domains' do
|
||||||
|
let(:account) { Fabricate(:account) }
|
||||||
|
let!(:preview_card) { Fabricate(:preview_card, url: 'https://writer.example.com/article', unverified_author_account: account, author_account: nil) }
|
||||||
|
let!(:unattributable_preview_card) { Fabricate(:preview_card, url: 'https://otherwriter.example.com/article', unverified_author_account: account, author_account: nil) }
|
||||||
|
let!(:unrelated_preview_card) { Fabricate(:preview_card) }
|
||||||
|
|
||||||
|
it 'reattributes expected preview cards' do
|
||||||
|
expect { subject.call(account, { attribution_domains: ['writer.example.com'] }) }
|
||||||
|
.to change { preview_card.reload.author_account }.from(nil).to(account)
|
||||||
|
.and not_change { unattributable_preview_card.reload.author_account }
|
||||||
|
.and(not_change { unrelated_preview_card.reload.author_account })
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
22
spec/workers/update_link_card_attribution_worker_spec.rb
Normal file
22
spec/workers/update_link_card_attribution_worker_spec.rb
Normal file
@ -0,0 +1,22 @@
|
|||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
require 'rails_helper'
|
||||||
|
|
||||||
|
RSpec.describe UpdateLinkCardAttributionWorker do
|
||||||
|
let(:worker) { described_class.new }
|
||||||
|
|
||||||
|
let(:account) { Fabricate(:account, attribution_domains: ['writer.example.com']) }
|
||||||
|
|
||||||
|
describe '#perform' do
|
||||||
|
let!(:preview_card) { Fabricate(:preview_card, url: 'https://writer.example.com/article', unverified_author_account: account, author_account: nil) }
|
||||||
|
let!(:unattributable_preview_card) { Fabricate(:preview_card, url: 'https://otherwriter.example.com/article', unverified_author_account: account, author_account: nil) }
|
||||||
|
let!(:unrelated_preview_card) { Fabricate(:preview_card) }
|
||||||
|
|
||||||
|
it 'reattributes expected preview cards' do
|
||||||
|
expect { worker.perform(account.id) }
|
||||||
|
.to change { preview_card.reload.author_account }.from(nil).to(account)
|
||||||
|
.and not_change { unattributable_preview_card.reload.author_account }
|
||||||
|
.and(not_change { unrelated_preview_card.reload.author_account })
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Loading…
x
Reference in New Issue
Block a user