From d9116efb858984c050ed0cf121c1191baf50ed34 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 29 Sep 2021 01:58:12 +0200 Subject: [PATCH 1/2] Revert #8254 This reverts commit 123b8b906cce08a1020c7d960e569cc56cabd349. This reverts commit 60f9dbcdbd2c8d2217c9496926fd17cd53e2906b. --- lib/archive_importer/contact_importer.rb | 2 +- spec/integration/migration_service_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/archive_importer/contact_importer.rb b/lib/archive_importer/contact_importer.rb index b64dfe7b7..492375691 100644 --- a/lib/archive_importer/contact_importer.rb +++ b/lib/archive_importer/contact_importer.rb @@ -34,7 +34,7 @@ class ArchiveImporter def create_contact person = Person.by_account_identifier(json.fetch("account_id")) - user.contacts.create!(person_id: person.id, sharing: json.fetch("sharing"), receiving: json.fetch("receiving")) + user.contacts.create!(person_id: person.id, sharing: false, receiving: json.fetch("receiving")) end end end diff --git a/spec/integration/migration_service_spec.rb b/spec/integration/migration_service_spec.rb index c4c306afa..f8cb79f98 100644 --- a/spec/integration/migration_service_spec.rb +++ b/spec/integration/migration_service_spec.rb @@ -287,17 +287,17 @@ describe MigrationService do contact = user.contacts.find_by(person: Person.by_account_identifier(contact1_diaspora_id)) expect(contact).not_to be_nil - expect(contact.sharing).to be_truthy + expect(contact.sharing).to be_falsey expect(contact.receiving).to be_falsey contact = user.contacts.find_by(person: Person.by_account_identifier(contact2_diaspora_id)) expect(contact).not_to be_nil - expect(contact.sharing).to be_truthy + expect(contact.sharing).to be_falsey expect(contact.receiving).to be_truthy contact = user.contacts.find_by(person: Person.by_account_identifier(migrated_contact_new_diaspora_id)) expect(contact).not_to be_nil - expect(contact.sharing).to be_truthy + expect(contact.sharing).to be_falsey expect(contact.receiving).to be_truthy aspect = user.aspects.find_by(name: "Friends") From 0e6caf61ff6d63f8cc185cba6637c73143cfee94 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 29 Sep 2021 04:23:03 +0200 Subject: [PATCH 2/2] 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 --- app/models/account_migration.rb | 16 ++++++++++--- app/services/migration_service.rb | 3 ++- lib/archive_importer/contact_importer.rb | 5 +++- spec/integration/migration_service_spec.rb | 6 ++--- spec/models/account_migration_spec.rb | 28 ++++++++++++++++++++++ 5 files changed, 49 insertions(+), 9 deletions(-) diff --git a/app/models/account_migration.rb b/app/models/account_migration.rb index e8590bc2b..8f45fd1d8 100644 --- a/app/models/account_migration.rb +++ b/app/models/account_migration.rb @@ -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 diff --git a/app/services/migration_service.rb b/app/services/migration_service.rb index ff418b5e8..68f891b1f 100644 --- a/app/services/migration_service.rb +++ b/app/services/migration_service.rb @@ -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 diff --git a/lib/archive_importer/contact_importer.rb b/lib/archive_importer/contact_importer.rb index 492375691..f392d7a21 100644 --- a/lib/archive_importer/contact_importer.rb +++ b/lib/archive_importer/contact_importer.rb @@ -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 diff --git a/spec/integration/migration_service_spec.rb b/spec/integration/migration_service_spec.rb index f8cb79f98..67f8513ee 100644 --- a/spec/integration/migration_service_spec.rb +++ b/spec/integration/migration_service_spec.rb @@ -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 diff --git a/spec/models/account_migration_spec.rb b/spec/models/account_migration_spec.rb index 68fe01436..c4da638ac 100644 --- a/spec/models/account_migration_spec.rb +++ b/spec/models/account_migration_spec.rb @@ -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