From ae5ba97f1e1f39a1abbaf434bef97a9d12a4aa13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Thu, 9 Feb 2017 18:43:50 +0100 Subject: [PATCH 1/4] adds erased scope to User --- app/models/user.rb | 1 + spec/models/user_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index dc035c6d9..54921fd10 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -56,6 +56,7 @@ class User < ActiveRecord::Base scope :by_document, -> (document_type, document_number) { where(document_type: document_type, document_number: document_number) } scope :email_digest, -> { where(email_digest: true) } scope :active, -> { where(erased_at: nil) } + scope :erased, -> { where.not(erased_at: nil) } before_validation :clean_document_number diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 789341fe9..9088afaea 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -370,6 +370,20 @@ describe User do end end + + describe "erased" do + + it "returns users that have been erased" do + user1 = create(:user, erased_at: Time.current) + user2 = create(:user, erased_at: Time.current) + user3 = create(:user, erased_at: nil) + + expect(User.erased).to include(user1) + expect(User.erased).to include(user2) + expect(User.erased).to_not include(user3) + end + + end end describe "self.search" do From 7dc267935e0040102e6021ac950ce9a9e5a2cb0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Thu, 9 Feb 2017 18:48:26 +0100 Subject: [PATCH 2/4] adds field to track former users ids --- .../20170208160130_add_former_users_data_log_to_users.rb | 5 +++++ db/schema.rb | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20170208160130_add_former_users_data_log_to_users.rb diff --git a/db/migrate/20170208160130_add_former_users_data_log_to_users.rb b/db/migrate/20170208160130_add_former_users_data_log_to_users.rb new file mode 100644 index 000000000..2cd3ca956 --- /dev/null +++ b/db/migrate/20170208160130_add_former_users_data_log_to_users.rb @@ -0,0 +1,5 @@ +class AddFormerUsersDataLogToUsers < ActiveRecord::Migration + def change + add_column :users, :former_users_data_log, :text, default: "" + end +end diff --git a/db/schema.rb b/db/schema.rb index 274352986..d765a7c1d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170208114548) do +ActiveRecord::Schema.define(version: 20170208160130) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -751,6 +751,7 @@ ActiveRecord::Schema.define(version: 20170208114548) do t.boolean "official_position_badge", default: false t.datetime "password_changed_at", default: '2016-11-23 10:59:20', null: false t.boolean "created_from_signature", default: false + t.text "former_users_data_log", default: "" end add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree From 35078848c39569390be0b3e23915863133984256 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Thu, 9 Feb 2017 20:09:06 +0100 Subject: [PATCH 3/4] adds methods to take votes from other user (generic & erased) --- app/models/user.rb | 16 +++++ spec/models/user_spec.rb | 130 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index 54921fd10..c3038c88a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -194,6 +194,22 @@ class User < ActiveRecord::Base erased_at.present? end + def take_votes_if_erased_document(document_number, document_type) + erased_user = User.erased.where(document_number: document_number).where(document_type: document_type).first + if erased_user.present? + self.take_votes_from(erased_user) + erased_user.update(document_number: nil, document_type: nil) + end + end + + def take_votes_from(other_user) + return if other_user.blank? + Poll::Voter.where(user_id: other_user.id).update_all(user_id: self.id) + Budget::Ballot.where(user_id: other_user.id).update_all(user_id: self.id) + Vote.where("voter_id = ? AND voter_type = ?", other_user.id, "User").update_all(voter_id: self.id) + self.update(former_users_data_log: "#{self.former_users_data_log} | id: #{other_user.id} - #{Time.current.strftime('%Y-%m-%d %H:%M:%S')}") + end + def locked? Lock.find_or_create_by(user: self).locked? end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9088afaea..7a78037dd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -502,4 +502,134 @@ describe User do end + describe "#take_votes_from" do + it "logs info of previous users" do + user = create(:user, :level_three) + other_user = create(:user, :level_three) + another_user = create(:user) + + expect(user.former_users_data_log).to be_blank + + user.take_votes_from other_user + + expect(user.former_users_data_log).to include("id: #{other_user.id}") + + user.take_votes_from another_user + + expect(user.former_users_data_log).to include("id: #{other_user.id}") + expect(user.former_users_data_log).to include("| id: #{another_user.id}") + end + + it "reassignes votes from other user" do + other_user = create(:user, :level_three) + user = create(:user, :level_three) + + v1 = create(:vote, voter: other_user, votable: create(:debate)) + v2 = create(:vote, voter: other_user, votable: create(:proposal)) + v3 = create(:vote, voter: other_user, votable: create(:comment)) + + create(:vote) + + expect(other_user.votes.count).to eq(3) + expect(user.votes.count).to eq(0) + + user.take_votes_from other_user + + expect(other_user.votes.count).to eq(0) + expect(user.vote_ids.sort).to eq([v1.id, v2.id, v3.id].sort) + end + + it "reassignes budget ballots from other user" do + other_user = create(:user, :level_three) + user = create(:user, :level_three) + + b1 = create(:budget_ballot, user: other_user) + b2 = create(:budget_ballot, user: other_user) + + create(:budget_ballot) + + expect(Budget::Ballot.where(user: other_user).count).to eq(2) + expect(Budget::Ballot.where(user: user).count).to eq(0) + + user.take_votes_from other_user + + expect(Budget::Ballot.where(user: other_user).count).to eq(0) + expect(Budget::Ballot.where(user: user).sort).to eq([b1, b2].sort) + end + + it "reassignes poll voters from other user" do + other_user = create(:user, :level_three) + user = create(:user, :level_three) + + v1 = create(:poll_voter, user: other_user) + v2 = create(:poll_voter, user: other_user) + + create(:poll_voter) + + expect(Poll::Voter.where(user: other_user).count).to eq(2) + expect(Poll::Voter.where(user: user).count).to eq(0) + + user.take_votes_from other_user + + expect(Poll::Voter.where(user: other_user).count).to eq(0) + expect(Poll::Voter.where(user: user).sort).to eq([v1, v2].sort) + end + end + + describe "#take_votes_if_erased_document" do + it "does nothing if no erased user with received document" do + user_1 = create(:user, :level_three) + user_2 = create(:user, :level_three) + + create(:vote, voter: user_1) + create(:budget_ballot, user: user_1) + create(:poll_voter, user: user_1) + + user_2.take_votes_if_erased_document(111, 1) + + expect(user_1.votes.count).to eq(1) + expect(user_2.votes.count).to eq(0) + + expect(Budget::Ballot.where(user: user_1).count).to eq(1) + expect(Budget::Ballot.where(user: user_2).count).to eq(0) + + expect(Poll::Voter.where(user: user_1).count).to eq(1) + expect(Poll::Voter.where(user: user_2).count).to eq(0) + end + + it "takes votes from erased user with received document" do + user_1 = create(:user, :level_two, document_number: "12345777", document_type: "1") + user_2 = create(:user) + + create(:vote, voter: user_1) + create(:budget_ballot, user: user_1) + create(:poll_voter, user: user_1) + + user_1.erase + + user_2.take_votes_if_erased_document("12345777", "1") + + expect(user_1.votes.count).to eq(0) + expect(user_2.votes.count).to eq(1) + + expect(Budget::Ballot.where(user: user_1).count).to eq(0) + expect(Budget::Ballot.where(user: user_2).count).to eq(1) + + expect(Poll::Voter.where(user: user_1).count).to eq(0) + expect(Poll::Voter.where(user: user_2).count).to eq(1) + end + + it "removes document from erased user and logs info" do + user_1 = create(:user, document_number: "12345777", document_type: "1") + user_2 = create(:user) + user_1.erase + + user_2.take_votes_if_erased_document("12345777", "1") + + expect(user_2.reload.former_users_data_log).to include("id: #{user_1.id}") + expect(user_1.reload.document_number).to be_blank + end + + end + end From 590a7bce4498fb10516a34de65634ea92ac831c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Thu, 9 Feb 2017 20:20:30 +0100 Subject: [PATCH 4/4] reassigns erased user votes when returning user is verified --- app/models/verification/residence.rb | 5 ++- spec/features/verification/residence_spec.rb | 45 +++++++++++--------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/app/models/verification/residence.rb b/app/models/verification/residence.rb index 4f6cbf68a..ea000677f 100644 --- a/app/models/verification/residence.rb +++ b/app/models/verification/residence.rb @@ -26,6 +26,9 @@ class Verification::Residence def save return false unless valid? + + user.take_votes_if_erased_document(document_number, document_type) + user.update(document_number: document_number, document_type: document_type, geozone: self.geozone, @@ -40,7 +43,7 @@ class Verification::Residence end def document_number_uniqueness - errors.add(:document_number, I18n.t('errors.messages.taken')) if User.where(document_number: document_number).any? + errors.add(:document_number, I18n.t('errors.messages.taken')) if User.active.where(document_number: document_number).any? end def store_failed_attempt diff --git a/spec/features/verification/residence_spec.rb b/spec/features/verification/residence_spec.rb index 58d2c8092..ed2ffd083 100644 --- a/spec/features/verification/residence_spec.rb +++ b/spec/features/verification/residence_spec.rb @@ -22,6 +22,31 @@ feature 'Residence' do expect(page).to have_content 'Residence verified' end + scenario 'When trying to verify a deregistered account old votes are reassigned' do + erased_user = create(:user, document_number: '12345678Z', document_type: '1', erased_at: Time.current) + vote = create(:vote, voter: erased_user) + new_user = create(:user) + + login_as(new_user) + + visit account_path + click_link 'Verify my account' + + fill_in 'residence_document_number', with: '12345678Z' + select 'DNI', from: 'residence_document_type' + select_date '31-December-1980', from: 'residence_date_of_birth' + fill_in 'residence_postal_code', with: '28013' + check 'residence_terms_of_service' + + click_button 'Verify residence' + + expect(page).to have_content 'Residence verified' + + expect(vote.reload.voter).to eq(new_user) + expect(erased_user.reload.document_number).to be_blank + expect(new_user.reload.document_number).to eq('12345678Z') + end + scenario 'Error on verify' do user = create(:user) login_as(user) @@ -102,24 +127,4 @@ feature 'Residence' do expect(page).to have_content "You have reached the maximum number of attempts. Please try again later." expect(current_path).to eq(account_path) end - - scenario 'Error when trying to verify a deregistered account' do - create(:user, document_number: '12345678Z', document_type: '1', erased_at: Time.current) - - login_as(create(:user)) - - visit account_path - click_link 'Verify my account' - - fill_in 'residence_document_number', with: "12345678Z" - select 'DNI', from: 'residence_document_type' - select_date '31-December-1980', from: 'residence_date_of_birth' - fill_in 'residence_postal_code', with: '28013' - check 'residence_terms_of_service' - - click_button 'Verify residence' - - expect(page).to_not have_content 'Residence verified' - expect(page).to have_content 'has already been taken' - end end