From e9f6dbdffd8b9c74238e1183cff3918a487e4a89 Mon Sep 17 00:00:00 2001 From: cmrd Senya Date: Wed, 25 Apr 2018 16:07:43 +0300 Subject: [PATCH 1/2] Add unique index to poll participations on author_id and poll_id Previously we had only a Rails validation which ensured poll participation uniqueness but this adds uniqueness control to the database level, so that uniqueness is guaranteed even when changing data with avoiding Rails validations. closes #7798 --- Changelog.md | 1 + ...pations_unique_index_on_author_and_poll.rb | 19 +++++++++++++++++++ spec/models/poll_participation_spec.rb | 10 ++++++++++ 3 files changed, 30 insertions(+) create mode 100644 db/migrate/20180425125409_add_poll_participations_unique_index_on_author_and_poll.rb diff --git a/Changelog.md b/Changelog.md index 204cbc645..145a6e5a6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,7 @@ # 0.7.6.0 ## Refactor +* Add unique index to poll participations on `poll_id` and `author_id` [#7798](https://github.com/diaspora/diaspora/pull/7798) ## Bug fixes diff --git a/db/migrate/20180425125409_add_poll_participations_unique_index_on_author_and_poll.rb b/db/migrate/20180425125409_add_poll_participations_unique_index_on_author_and_poll.rb new file mode 100644 index 000000000..2f7376f91 --- /dev/null +++ b/db/migrate/20180425125409_add_poll_participations_unique_index_on_author_and_poll.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddPollParticipationsUniqueIndexOnAuthorAndPoll < ActiveRecord::Migration[5.1] + def change + reversible do |change| + change.up do + duplicate_query = "WHERE a1.poll_id = a2.poll_id AND a1.author_id = a2.author_id AND a1.id > a2.id" + if AppConfig.postgres? + execute("DELETE FROM poll_participations AS a1 USING poll_participations AS a2 #{duplicate_query}") + else + execute("DELETE a1 FROM poll_participations a1, poll_participations a2 #{duplicate_query}") + end + end + end + + add_index :poll_participations, %i[poll_id author_id], unique: true + remove_index :poll_participations, :poll_id + end +end diff --git a/spec/models/poll_participation_spec.rb b/spec/models/poll_participation_spec.rb index 581be0663..bf3d54537 100644 --- a/spec/models/poll_participation_spec.rb +++ b/spec/models/poll_participation_spec.rb @@ -24,6 +24,16 @@ describe PollParticipation, type: :model do bob.participate_in_poll!(status, poll.poll_answers.first) }.to_not raise_error end + + it "has unique DB index for author-person" do + pp = FactoryGirl.create(:poll_participation) + pp2 = FactoryGirl.create(:poll_participation, author: pp.author) + expect { + # rubocop:disable Rails/SkipsModelValidations + pp2.update_attribute(:poll_id, pp.poll_id) + # rubocop:enable Rails/SkipsModelValidations + }.to raise_error ActiveRecord::RecordNotUnique + end end it_behaves_like "it is relayable" do From cb294fd3f4a856b7ef2f503268061fd340265ef2 Mon Sep 17 00:00:00 2001 From: cmrd Senya Date: Thu, 3 May 2018 19:44:32 +0300 Subject: [PATCH 2/2] Add completed_at to account_migrations Use completed_at datetime field as an indication of a performed migration closes #7805 --- Changelog.md | 1 + app/models/account_migration.rb | 3 ++- ...4_add_completed_at_to_account_migration.rb | 19 +++++++++++++++++++ spec/models/account_migration_spec.rb | 11 ++++++++--- 4 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20180430134444_add_completed_at_to_account_migration.rb diff --git a/Changelog.md b/Changelog.md index 145a6e5a6..262f7bcab 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ ## Refactor * Add unique index to poll participations on `poll_id` and `author_id` [#7798](https://github.com/diaspora/diaspora/pull/7798) +* Add 'completed at' date to account migrations [#7805](https://github.com/diaspora/diaspora/pull/7805) ## Bug fixes diff --git a/app/models/account_migration.rb b/app/models/account_migration.rb index 1822e947e..3d4a40d0b 100644 --- a/app/models/account_migration.rb +++ b/app/models/account_migration.rb @@ -40,10 +40,11 @@ class AccountMigration < ApplicationRecord dispatch if locally_initiated? dispatch_contacts if remotely_initiated? + update(completed_at: Time.zone.now) end def performed? - old_person.closed_account? + !completed_at.nil? end # We assume that migration message subscribers are people that are subscribed to a new user profile updates. diff --git a/db/migrate/20180430134444_add_completed_at_to_account_migration.rb b/db/migrate/20180430134444_add_completed_at_to_account_migration.rb new file mode 100644 index 000000000..13eff4c85 --- /dev/null +++ b/db/migrate/20180430134444_add_completed_at_to_account_migration.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddCompletedAtToAccountMigration < ActiveRecord::Migration[5.1] + def change + add_column :account_migrations, :completed_at, :datetime, default: nil + + reversible do |change| + change.up do + set_completed_at_for_closed_accounts + end + end + end + + def set_completed_at_for_closed_accounts + # rubocop:disable Rails/SkipsModelValidations + AccountMigration.joins(:old_person).where(people: {closed_account: true}).update_all(completed_at: Time.zone.now) + # rubocop:enable Rails/SkipsModelValidations + end +end diff --git a/spec/models/account_migration_spec.rb b/spec/models/account_migration_spec.rb index d0cad3e79..0bf1b36a6 100644 --- a/spec/models/account_migration_spec.rb +++ b/spec/models/account_migration_spec.rb @@ -61,9 +61,14 @@ describe AccountMigration, type: :model do }.to change(account_migration, :performed?).to be_truthy end - it "calls old_person.closed_account?" do - expect(account_migration.old_person).to receive(:closed_account?) - account_migration.performed? + it "is truthy when completed_at is set" do + expect(FactoryGirl.create(:account_migration, completed_at: Time.zone.now).performed?).to be_truthy + end + + it "is falsey when completed_at is null" do + account_migration = FactoryGirl.create(:account_migration, completed_at: nil) + account_migration.old_person.lock_access! + expect(account_migration.performed?).to be_falsey end end