Fix sharing status with local users when importing archive

* Local contacts also start sharing again with imported user if they
  were sharing with the old account
* Don't create empty contact entities for contacts which the imported
  user doesn't share with and also maybe the contact doesn't share with
  the importer
* Ensure people which were a contact in the archive still receive the
  migration, even when the importer doesn't share with them, so they can
  resend their contact message

fixes #8106 for real this time
This commit is contained in:
Benjamin Neff 2021-09-29 04:23:03 +02:00
parent d9116efb85
commit 0e6caf61ff
No known key found for this signature in database
GPG key ID: 971464C3F1A90194
5 changed files with 49 additions and 9 deletions

View file

@ -14,6 +14,8 @@ class AccountMigration < ApplicationRecord
attr_accessor :old_private_key
attr_writer :old_person_diaspora_id
attr_accessor :archive_contacts
def receive(*)
perform!
end
@ -29,10 +31,11 @@ class AccountMigration < ApplicationRecord
# executes a migration plan according to this AccountMigration object
def perform!
raise "already performed" if performed?
validate_sender if locally_initiated?
tombstone_old_user_and_update_all_references if old_person
dispatch if locally_initiated?
dispatch_contacts if remotely_initiated?
dispatch_contacts
update(completed_at: Time.zone.now)
end
@ -40,13 +43,20 @@ class AccountMigration < ApplicationRecord
!completed_at.nil?
end
# We assume that migration message subscribers are people that are subscribed to a new user profile updates.
# Since during the migration we update contact references, this includes all the contacts of the old person.
# Send migration to all imported contacts, but also send it to all contacts from the archive which weren't imported,
# but maybe share with the old account, so they can update contact information and resend the contact message.
# In case when a user migrated to our pod from a remote one, we include remote person to subscribers so that
# the new pod is informed about the migration as well.
def subscribers
new_user.profile.subscribers.remote.to_a.tap do |subscribers|
subscribers.push(old_person) if old_person&.remote?
archive_contacts&.each do |contact|
diaspora_id = contact.fetch("account_id")
next if subscribers.any? {|s| s.diaspora_handle == diaspora_id }
person = Person.by_account_identifier(diaspora_id)
subscribers.push(person) if person&.remote?
end
end
end

View file

@ -49,7 +49,8 @@ class MigrationService
old_person: old_person,
new_person: archive_importer.user.person,
old_private_key: archive_importer.serialized_private_key,
old_person_diaspora_id: archive_importer.archive_author_diaspora_id
old_person_diaspora_id: archive_importer.archive_author_diaspora_id,
archive_contacts: archive_importer.contacts
)
end

View file

@ -13,6 +13,8 @@ class ArchiveImporter
attr_reader :user
def import
return unless json.fetch("receiving")
@imported_contact = create_contact
add_to_aspects
rescue ActiveRecord::RecordInvalid => e
@ -34,7 +36,8 @@ class ArchiveImporter
def create_contact
person = Person.by_account_identifier(json.fetch("account_id"))
user.contacts.create!(person_id: person.id, sharing: false, receiving: json.fetch("receiving"))
# see AccountMigration#dispatch_contacts for the other half of this when the contact is sharing with the user
user.contacts.create!(person_id: person.id, sharing: false, receiving: true)
end
end
end

View file

@ -128,7 +128,7 @@ describe MigrationService do
"following": true,
"followed": false,
"account_id": "#{contact1_diaspora_id}",
"contact_groups_membership": ["Family"]
"contact_groups_membership": []
},
{
"sharing": true,
@ -286,9 +286,7 @@ describe MigrationService do
expect(like.author).not_to eq(user.person)
contact = user.contacts.find_by(person: Person.by_account_identifier(contact1_diaspora_id))
expect(contact).not_to be_nil
expect(contact.sharing).to be_falsey
expect(contact.receiving).to be_falsey
expect(contact).to be_nil
contact = user.contacts.find_by(person: Person.by_account_identifier(contact2_diaspora_id))
expect(contact).not_to be_nil

View file

@ -93,6 +93,34 @@ describe AccountMigration, type: :model do
expect(account_migration.subscribers).to be_empty
end
end
context "with contacts from the archive" do
it "includes contacts from the archive" do
archive_person = FactoryBot.create(:person)
remote_contact = DataGenerator.create(new_user, :remote_mutual_friend)
contacts = [
{
"sharing" => true,
"receiving" => false,
"following" => true,
"followed" => false,
"account_id" => archive_person.diaspora_handle,
"contact_groups_membership" => []
},
{
"sharing" => true,
"receiving" => true,
"following" => true,
"followed" => true,
"account_id" => remote_contact.person.diaspora_handle,
"contact_groups_membership" => []
}
]
account_migration =
AccountMigration.create!(old_person: old_person, new_person: new_person, archive_contacts: contacts)
expect(account_migration.subscribers).to match_array([remote_contact.person, archive_person, old_person])
end
end
end
end