From 05612ef7337d1dc843949e780259ad3c36ddc4be Mon Sep 17 00:00:00 2001 From: Ilya Zhitomirskiy Date: Wed, 2 Nov 2011 18:49:57 -0700 Subject: [PATCH 01/17] ms iz wip --- app/models/account_deletion.rb | 85 ++++++++++++++ app/models/contact.rb | 5 +- app/models/invitation.rb | 11 ++ app/models/user.rb | 1 + spec/factories.rb | 19 ++++ spec/helper_methods.rb | 11 ++ spec/integration/account_deletion_spec.rb | 54 +++++++++ spec/misc_spec.rb | 18 +++ spec/models/account_deletion_spec.rb | 130 ++++++++++++++++++++++ spec/models/contact_spec.rb | 10 ++ spec/models/invitation_spec.rb | 11 ++ spec/models/tag_following_spec.rb | 2 +- 12 files changed, 355 insertions(+), 2 deletions(-) create mode 100644 app/models/account_deletion.rb create mode 100644 spec/integration/account_deletion_spec.rb create mode 100644 spec/models/account_deletion_spec.rb diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb new file mode 100644 index 000000000..0b6070bf9 --- /dev/null +++ b/app/models/account_deletion.rb @@ -0,0 +1,85 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +class AccountDeletion + attr_accessor :person, :user + + def initialize(diaspora_id) + self.person = Person.where(:diaspora_handle => diaspora_id).first + self.user = self.person.owner + end + + def perform! + delete_standard_associations + disassociate_invitations + delete_mentions + delete_contacts_of_me + disconnect_contacts + delete_posts + end + + #user deletions + def normal_ar_user_associates_to_delete + [:tag_followings, :authorizations, :invitations_to_me, :services, :aspects, :user_preferences, :notifications] + end + + def special_ar_user_associations + [:invitations_from_me, :person, :contacts] + end + + def ignored_ar_user_associations + [:followed_tags, :invited_by, :contact_people, :applications, :aspect_memberships] + end + + def delete_standard_associations + normal_ar_user_associates_to_delete.each do |asso| + user.send(asso).delete_all + end + end + + def disassociate_invitations + user.invitations_from_me.each do |inv| + inv.convert_to_admin! + end + end + + def disconnect_contacts + user.contacts.delete_all + end + + + #person deletion +# def delete_posts +# end + +# def delete_photos +# end + +# def comments +# end + +# def delete_notification_actors +# end + + def delete_posts + self.person.posts.delete_all + end + + def delete_photos + self.person.photos.delete_all + end + + def delete_mentions + self.person.mentions.delete_all + end + +# def reset_profile +# end + + def delete_contacts_of_me + Contact.all_contacts_of_person(self.person).delete_all + end + #private + +end diff --git a/app/models/contact.rb b/app/models/contact.rb index 518b11e89..8eae319fb 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -23,7 +23,10 @@ class Contact < ActiveRecord::Base before_destroy :destroy_notifications, :repopulate_cache! - # contact.sharing is true when contact.person is sharing with contact.user + + scope :all_contacts_of_person, lambda {|x| where(:person_id => x.id)} + + # contact.sharing is true when contact.person is sharing with contact.user scope :sharing, lambda { where(:sharing => true) } diff --git a/app/models/invitation.rb b/app/models/invitation.rb index 3be97a991..fc364b16c 100644 --- a/app/models/invitation.rb +++ b/app/models/invitation.rb @@ -99,6 +99,17 @@ class Invitation < ActiveRecord::Base self end + + # converts a personal invitation to an admin invite + # used in account deletion + # @return [Invitation] self + def convert_to_admin! + self.admin = true + self.sender = nil + self.aspect = nil + self.save + self + end # @return [Invitation] self def resend self.send! diff --git a/app/models/user.rb b/app/models/user.rb index 654bb7bb1..e49025acd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -44,6 +44,7 @@ class User < ActiveRecord::Base has_many :tag_followings, :dependent => :destroy has_many :followed_tags, :through => :tag_followings, :source => :tag, :order => 'tags.name' has_many :blocks + has_many :notifications, :foreign_key => :recipient_id has_many :authorizations, :class_name => 'OAuth2::Provider::Models::ActiveRecord::Authorization', :foreign_key => :resource_owner_id has_many :applications, :through => :authorizations, :source => :client diff --git a/spec/factories.rb b/spec/factories.rb index 74578cf6e..24af78f8c 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -166,3 +166,22 @@ end Factory.define(:oauth_access_token, :class => OAuth2::Provider.access_token_class) do |a| a.association(:authorization, :factory => :oauth_authorization) end + +Factory.define(:tag, :class => ActsAsTaggableOn::Tag) do |t| + t.name "partytimeexcellent" +end + +Factory.define(:tag_following) do |a| + a.association(:tag, :factory => :tag) + a.association(:user, :factory => :user) +end + +Factory.define(:contact) do |c| + c.association(:person, :factory => :person) + c.association(:user, :factory => :user) +end + +Factory.define(:mention) do |c| + c.association(:person, :factory => :person) + c.association(:post, :factory => :status_message) +end diff --git a/spec/helper_methods.rb b/spec/helper_methods.rb index 4c2fa3196..921b7ffbc 100644 --- a/spec/helper_methods.rb +++ b/spec/helper_methods.rb @@ -61,4 +61,15 @@ module HelperMethods fixture_name = File.join(File.dirname(__FILE__), 'fixtures', fixture_filename) File.open(fixture_name) end + + def create_conversation_with_message(sender, recipient, subject, text) + create_hash = { + :author => sender.person, + :participant_ids => [sender.person.id, recipient.person.id], + :subject => subject, + :messages_attributes => [ {:author => sender.person, :text => text} ] + } + + Conversation.create!(create_hash) + end end diff --git a/spec/integration/account_deletion_spec.rb b/spec/integration/account_deletion_spec.rb new file mode 100644 index 000000000..f442710f0 --- /dev/null +++ b/spec/integration/account_deletion_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe 'deleteing your account' do + before do + @bob2 = bob + @bobs_person_id = @bob2.person.id + @alices_post = alice.post(:status_message, :text => "@{@bob2 Grimn; #{@bob2.person.diaspora_handle}} you are silly", :to => alice.aspects.find_by_name('generic')) + + @bobs_contact_ids = @bob2.contacts.map {|c| c.id} + + #@bob2's own content + @bob2.post(:status_message, :text => 'asldkfjs', :to => @bob2.aspects.first) + f = Factory(:photo, :author => @bob2.person) + + #objects on post + @bob2.like(true, :target => @alices_post) + @bob2.comment("here are some thoughts on your post", :post => @alices_post) + + #conversations + create_conversation_with_message(alice, @bob2, "Subject", "Hey @bob2") + + AccountDeletion.new(@bob2.person.diaspora_handle).perform! + + @bob2.reload + end + + it 'deletes all of @bob2s posts' do + @bob2.posts.should be_empty + end + + it 'deletes all of @bob2s share visiblites' do + ShareVisibility.where(:contact_id => @bobs_contact_ids).should be_empty + end + + it 'deletes all photos' do + Photo.where(:author_id => @bobs_person_id).should be_empty + end + + it 'deletes all mentions ' do + @bob2.person.mentions.should be_empty + end + + it 'deletes all aspects' do + @bob2.aspects.should be_empty + end + + it 'deletes all contacts' do + @bob2.contacts.should be_empty + end + + it 'deletes the converersation visibilities' do + pending + end +end diff --git a/spec/misc_spec.rb b/spec/misc_spec.rb index 1c5ce9edb..8007c3753 100644 --- a/spec/misc_spec.rb +++ b/spec/misc_spec.rb @@ -61,4 +61,22 @@ describe 'making sure the spec runner works' do alice.comment "yo", :post => person_status end end + + describe '#post' do + it 'creates a notification with a mention' do + lambda{ + alice.post(:status_message, :text => "@{Bob Grimn; #{bob.person.diaspora_handle}} you are silly", :to => alice.aspects.find_by_name('generic')) + }.should change(Notification, :count).by(1) + end + end + + describe "#create_conversation_with_message" do + it 'creates a conversation and a message' do + conversation = create_conversation_with_message(alice, bob, "Subject", "Hey Bob") + + conversation.participants.should == [alice.person, bob.person] + conversation.subject.should == "Subject" + conversation.messages.first.text.should == "Hey Bob" + end + end end diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb new file mode 100644 index 000000000..a75ecd5ee --- /dev/null +++ b/spec/models/account_deletion_spec.rb @@ -0,0 +1,130 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +require 'spec_helper' + +describe AccountDeletion do + before do + @account_deletion = AccountDeletion.new(bob.person.diaspora_handle) + @account_deletion.user = bob + end + + it 'works' do + pending + end + + it "attaches the user" do + AccountDeletion.new(bob.person.diaspora_handle).user.should == bob + AccountDeletion.new(remote_raphael.diaspora_handle).user.should == nil + end + + describe '#perform' do + it 'calls delete_standard_associations' do + @account_deletion.should_receive(:delete_standard_associations) + @account_deletion.perform! + end + + it 'calls disassociate_invitations' do + @account_deletion.should_receive(:disassociate_invitations) + @account_deletion.perform! + end + + it 'calls delete_contacts_of_me' do + @account_deletion.should_receive(:delete_contacts_of_me) + @account_deletion.perform! + end + + it 'calls delete_contacts_of_me' do + @account_deletion.should_receive(:delete_mentions) + @account_deletion.perform! + end + + it 'calls disconnect_contacts' do + @account_deletion.should_receive(:disconnect_contacts) + @account_deletion.perform! + end + + it 'calls delete_posts' do + @account_deletion.should_receive(:delete_posts) + @account_deletion.perform! + end + end + + describe "#delete_standard_associations" do + it 'removes all standard user associaltions' do + + @account_deletion.normal_ar_user_associates_to_delete.each do |asso| + association_mock = mock + association_mock.should_receive(:delete_all) + bob.should_receive(asso).and_return(association_mock) + end + + @account_deletion.delete_standard_associations + end + end + + + describe '#delete_posts' do + it 'deletes all posts' do + @account_deletion.person.posts.should_receive(:delete_all) + @account_deletion.delete_posts + end + end + + describe '#delete_photos' do + it 'deletes all photos' do + @account_deletion.person.photos.should_receive(:delete_all) + @account_deletion.delete_posts + end + end + + describe "#disassociate_invitations" do + it "sets invitations_from_me to be admin invitations" do + invites = [mock] + bob.stub(:invitations_from_me).and_return(invites) + invites.first.should_receive(:convert_to_admin!) + @account_deletion.disassociate_invitations + end + end + + describe "#normal_ar_user_associates_to_delete" do + it "has the regular associations" do + @account_deletion.normal_ar_user_associates_to_delete.should == + [:tag_followings, :authorizations, :invitations_to_me, :services, :aspects, :user_preferences, :notifications] + end + end + + context 'person associations' do + describe '#delete mentions' do + it 'deletes the mentions for people' do + mentions = mock + @account_deletion.person.should_receive(:mentions).and_return(mentions) + mentions.should_receive(:delete_all) + @account_deletion.delete_mentions + end + end + + describe '#disconnect_contacts' do + it "deletes all of user's contacts" do + bob.contacts.should_receive(:delete_all) + @account_deletion.disconnect_contacts + end + end + + describe '#delete_contacts_of_me' do + it 'deletes all the local contact objects where deleted account is the person' do + contacts = mock + Contact.should_receive(:all_contacts_of_person).with(bob.person).and_return(contacts) + contacts.should_receive(:delete_all) + @account_deletion.delete_contacts_of_me + end + end + end + + it 'has all user association keys accounted for' do + all_keys = (@account_deletion.normal_ar_user_associates_to_delete + @account_deletion.special_ar_user_associations + @account_deletion.ignored_ar_user_associations) + all_keys.sort{|x, y| x.to_s <=> y.to_s}.should == User.reflections.keys.sort{|x, y| x.to_s <=> y.to_s} + end +end + diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 1a3c507aa..632969382 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -81,6 +81,16 @@ describe Contact do }.by(2) end end + + describe "all_contacts_of_person" do + it 'returns all contacts where the person is the passed in person' do + person = Factory.create(:person) + contact1 = Factory(:contact, :person => person) + contact2 = Factory(:contact) + contacts = Contact.all_contacts_of_person(person) + contacts.should == [contact1] + end + end end describe '#contacts' do diff --git a/spec/models/invitation_spec.rb b/spec/models/invitation_spec.rb index 4681ba9c5..ad19d8b59 100644 --- a/spec/models/invitation_spec.rb +++ b/spec/models/invitation_spec.rb @@ -75,6 +75,17 @@ describe Invitation do }.should_not change(User, :count) end end + + describe '#convert_to_admin!' do + it 'reset sender and aspect to nil, and sets admin flag to true' do + invite = Factory(:invitation) + invite.convert_to_admin! + invite.reload + invite.admin?.should be_true + invite.sender_id.should be_nil + invite.aspect_id.should be_nil + end + end describe '.batch_invite' do before do diff --git a/spec/models/tag_following_spec.rb b/spec/models/tag_following_spec.rb index 3b1614dfa..611c4fee3 100644 --- a/spec/models/tag_following_spec.rb +++ b/spec/models/tag_following_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe TagFollowing do before do - @tag = ActsAsTaggableOn::Tag.create(:name => "partytimeexcellent") + @tag = Factory.create(:tag) TagFollowing.create!(:tag => @tag, :user => alice) end From 3035f173bd3503b15cae58510588c5a75ab31506 Mon Sep 17 00:00:00 2001 From: Ilya Zhitomirskiy Date: Thu, 3 Nov 2011 20:43:47 -0700 Subject: [PATCH 02/17] ms iz wip, clearing profile, started deleting post visibilities --- app/models/account_deletion.rb | 14 +++++-- app/models/person.rb | 7 ++++ app/models/profile.rb | 12 ++++++ app/models/share_visibility.rb | 7 ++++ ...84050_add_closed_account_flag_to_person.rb | 9 +++++ db/schema.rb | 11 +++--- spec/factories.rb | 9 +++++ spec/integration/account_deletion_spec.rb | 7 ++++ spec/models/account_deletion_spec.rb | 18 ++++++--- spec/models/person_spec.rb | 15 +++++++ spec/models/profile_spec.rb | 39 +++++++++++++++++++ spec/models/share_visibility_spec.rb | 23 +++++++++++ 12 files changed, 157 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20111103184050_add_closed_account_flag_to_person.rb diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb index 0b6070bf9..3c51e2a66 100644 --- a/app/models/account_deletion.rb +++ b/app/models/account_deletion.rb @@ -17,6 +17,7 @@ class AccountDeletion delete_contacts_of_me disconnect_contacts delete_posts + tombstone_person_and_profile end #user deletions @@ -58,6 +59,12 @@ class AccountDeletion # def comments # end + # + def remove_share_visibilities + #my_contacts = user.contacts.map{|x| x.id} + #others_contacts = person.contacts{|x| x.id} + #ShareVisibility.where(:contact_id => my_contacts + others_contacts) + end # def delete_notification_actors # end @@ -74,12 +81,11 @@ class AccountDeletion self.person.mentions.delete_all end -# def reset_profile -# end + def tombstone_person_and_profile + self.person.close_account! + end def delete_contacts_of_me Contact.all_contacts_of_person(self.person).delete_all end - #private - end diff --git a/app/models/person.rb b/app/models/person.rb index 0d6d40829..eab06e394 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -288,6 +288,13 @@ class Person < ActiveRecord::Base self.update_attributes(:url => newuri) end + def close_account! + self.profile.tombstone! + self.closed_account = true + self.save + self + end + protected def clean_url diff --git a/app/models/profile.rb b/app/models/profile.rb index 7764a4f28..fc41de397 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -146,6 +146,14 @@ class Profile < ActiveRecord::Base self.full_name end + def tombstone! + self.taggings.delete_all + clearable_profile_fields.each do |field| + self[field] = nil + end + self.save + end + protected def strip_names self.first_name.strip! if self.first_name @@ -166,6 +174,10 @@ class Profile < ActiveRecord::Base end private + def clearable_profile_fields + self.attributes.keys - Profile.protected_attributes.to_a - ["created_at", "updated_at", "person_id"] + end + def absolutify_local_url url pod_url = AppConfig[:pod_url].dup pod_url.chop! if AppConfig[:pod_url][-1,1] == '/' diff --git a/app/models/share_visibility.rb b/app/models/share_visibility.rb index 6fd3c0712..eb80309f4 100644 --- a/app/models/share_visibility.rb +++ b/app/models/share_visibility.rb @@ -6,6 +6,13 @@ class ShareVisibility < ActiveRecord::Base belongs_to :contact belongs_to :shareable, :polymorphic => :true + scope :for_a_users_contacts, lambda { |user| + where(:contact_id => user.contacts.map {|c| c.id}) + } + + alias :for_a_users_contacts :for_contacts_of_a_person + + # Perform a batch import, given a set of contacts and a shareable # @note performs a bulk insert in mySQL; performs linear insertions in postgres # @param contacts [Array] Recipients diff --git a/db/migrate/20111103184050_add_closed_account_flag_to_person.rb b/db/migrate/20111103184050_add_closed_account_flag_to_person.rb new file mode 100644 index 000000000..0a845f835 --- /dev/null +++ b/db/migrate/20111103184050_add_closed_account_flag_to_person.rb @@ -0,0 +1,9 @@ +class AddClosedAccountFlagToPerson < ActiveRecord::Migration + def self.up + add_column :people, :closed_account, :boolean, :default => false + end + + def self.down + remove_column :people, :closed_account + end +end diff --git a/db/schema.rb b/db/schema.rb index 4a3992302..7c2d8a731 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20111101202137) do +ActiveRecord::Schema.define(:version => 20111103184050) do create_table "aspect_memberships", :force => true do |t| t.integer "aspect_id", :null => false @@ -238,13 +238,14 @@ ActiveRecord::Schema.define(:version => 20111101202137) do add_index "oauth_clients", ["nonce"], :name => "index_oauth_clients_on_nonce", :unique => true create_table "people", :force => true do |t| - t.string "guid", :null => false - t.text "url", :null => false - t.string "diaspora_handle", :null => false - t.text "serialized_public_key", :null => false + t.string "guid", :null => false + t.text "url", :null => false + t.string "diaspora_handle", :null => false + t.text "serialized_public_key", :null => false t.integer "owner_id" t.datetime "created_at" t.datetime "updated_at" + t.boolean "closed_account", :default => false end add_index "people", ["diaspora_handle"], :name => "index_people_on_diaspora_handle", :unique => true diff --git a/spec/factories.rb b/spec/factories.rb index 24af78f8c..d429b97fa 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -13,9 +13,18 @@ end Factory.define :profile do |p| p.sequence(:first_name) { |n| "Robert#{n}#{r_str}" } p.sequence(:last_name) { |n| "Grimm#{n}#{r_str}" } + p.bio "I am a cat lover and I love to run" + p.gender "robot" + p.location "Earth" p.birthday Date.today end +Factory.define :profile_with_image_url, :parent => :profile do |p| + p.image_url "http://example.com/image.jpg" + p.image_url_medium "http://example.com/image_mid.jpg" + p.image_url_small "http://example.com/image_small.jpg" +end + Factory.define :person do |p| p.sequence(:diaspora_handle) { |n| "bob-person-#{n}#{r_str}@example.net" } p.sequence(:url) { |n| AppConfig[:pod_url] } diff --git a/spec/integration/account_deletion_spec.rb b/spec/integration/account_deletion_spec.rb index f442710f0..053ace48b 100644 --- a/spec/integration/account_deletion_spec.rb +++ b/spec/integration/account_deletion_spec.rb @@ -30,6 +30,7 @@ describe 'deleteing your account' do it 'deletes all of @bob2s share visiblites' do ShareVisibility.where(:contact_id => @bobs_contact_ids).should be_empty + ShareVisibility.where(:contact_id => bob.person.contacts.map(&:id)).should be_empty end it 'deletes all photos' do @@ -48,6 +49,12 @@ describe 'deleteing your account' do @bob2.contacts.should be_empty end + it 'sets the person object as closed and the profile is cleared' do + @bob2.person.reload.closed_account.should be_true + + @bob2.person.profile.reload.first_name.should be_blank + end + it 'deletes the converersation visibilities' do pending end diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index a75ecd5ee..9e4dc5c40 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -10,10 +10,6 @@ describe AccountDeletion do @account_deletion.user = bob end - it 'works' do - pending - end - it "attaches the user" do AccountDeletion.new(bob.person.diaspora_handle).user.should == bob AccountDeletion.new(remote_raphael.diaspora_handle).user.should == nil @@ -49,6 +45,11 @@ describe AccountDeletion do @account_deletion.should_receive(:delete_posts) @account_deletion.perform! end + + it 'calls tombstone_person_and_profile' do + @account_deletion.should_receive(:tombstone_person_and_profile) + @account_deletion.perform! + end end describe "#delete_standard_associations" do @@ -75,7 +76,7 @@ describe AccountDeletion do describe '#delete_photos' do it 'deletes all photos' do @account_deletion.person.photos.should_receive(:delete_all) - @account_deletion.delete_posts + @account_deletion.delete_photos end end @@ -120,6 +121,13 @@ describe AccountDeletion do @account_deletion.delete_contacts_of_me end end + + describe '#tombstone_person_and_profile' do + it 'calls close_account! on person' do + @account_deletion.person.should_receive(:close_account!) + @account_deletion.tombstone_person_and_profile + end + end end it 'has all user association keys accounted for' do diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 48d5f515d..ae1111aef 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -536,4 +536,19 @@ describe Person do end end end + + describe "#close_account!" do + before do + @person = Factory(:person) + end + it 'sets the closed_account flag' do + @person.close_account! + @person.reload.closed_account.should be_true + end + + it 'calls Profile#tombstone!' do + @person.profile.should_receive(:tombstone!) + @person.close_account! + end + end end diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index c93984dcb..9a703fe78 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -275,4 +275,43 @@ describe Profile do end end + + describe "#tombstone!" do + before do + @profile = bob.person.profile + end + it "clears the profile fields" do + attributes = @profile.send(:clearable_profile_fields) + + @profile.tombstone! + @profile.reload + attributes.each{ |attr| + @profile[attr.to_sym].should be_blank + } + end + + it 'removes all the tags from the profile' do + @profile.taggings.should_receive(:delete_all) + @profile.tombstone! + end + end + + describe "#clearable_profile_fields" do + it 'returns the current profile fields' do + profile = Factory.build :profile + profile.send(:clearable_profile_fields).sort.should == + ["diaspora_handle", + "first_name", + "last_name", + "image_url", + "image_url_small", + "image_url_medium", + "birthday", + "gender", + "bio", + "searchable", + "location", + "full_name"].sort + end + end end diff --git a/spec/models/share_visibility_spec.rb b/spec/models/share_visibility_spec.rb index 805c1bbdb..4c72adf21 100644 --- a/spec/models/share_visibility_spec.rb +++ b/spec/models/share_visibility_spec.rb @@ -25,5 +25,28 @@ describe ShareVisibility do ShareVisibility.batch_import([@contact.id], @post) }.should_not raise_error end + + context "scopes" do + describe '.for_a_users_contacts' do + before do + alice.post(:status_message, :text => "Hey", :to => alice.aspects.first) + end + + it 'searches for share visibilies for all users contacts' do + contact_ids = alice.contacts.map{|c| c.id} + ShareVisibility.for_a_users_contacts(alice).should == ShareVisibility.where(:contact_id => contact_ids).all + end + end + + describe '.for_contacts_of_a_person' do + it 'searches for share visibilties generated by a person' do + + contact_ids = alice.person.contacts.map{|c| c.id} + + ShareVisibility.for_contacts_of_a_person(alice.person) == ShareVisibility.where(:contact_id => contact_ids).all + + end + end + end end end From 644e382cfd1b91f90c9e6eabc6a5f962f74704f5 Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Mon, 7 Nov 2011 16:30:31 -0800 Subject: [PATCH 03/17] DG IZ; remove more stuff associated with user; removed dependant destroys --- app/models/account_deletion.rb | 40 ++++----- app/models/share_visibility.rb | 6 +- app/models/user.rb | 10 +-- spec/integration/account_deletion_spec.rb | 55 ++++++++++-- spec/models/account_deletion_spec.rb | 101 ++++++++++++---------- 5 files changed, 128 insertions(+), 84 deletions(-) diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb index 3c51e2a66..4ab5a5536 100644 --- a/app/models/account_deletion.rb +++ b/app/models/account_deletion.rb @@ -15,14 +15,17 @@ class AccountDeletion disassociate_invitations delete_mentions delete_contacts_of_me + remove_share_visibilities + remove_conversation_visibilities disconnect_contacts + delete_photos delete_posts tombstone_person_and_profile end #user deletions def normal_ar_user_associates_to_delete - [:tag_followings, :authorizations, :invitations_to_me, :services, :aspects, :user_preferences, :notifications] + [:tag_followings, :authorizations, :invitations_to_me, :services, :aspects, :user_preferences, :notifications, :blocks] end def special_ar_user_associations @@ -32,10 +35,10 @@ class AccountDeletion def ignored_ar_user_associations [:followed_tags, :invited_by, :contact_people, :applications, :aspect_memberships] end - + def delete_standard_associations normal_ar_user_associates_to_delete.each do |asso| - user.send(asso).delete_all + user.send(asso).destroy_all end end @@ -46,39 +49,28 @@ class AccountDeletion end def disconnect_contacts - user.contacts.delete_all + user.contacts.destroy_all end - - #person deletion -# def delete_posts -# end - -# def delete_photos -# end - -# def comments -# end - # def remove_share_visibilities - #my_contacts = user.contacts.map{|x| x.id} - #others_contacts = person.contacts{|x| x.id} - #ShareVisibility.where(:contact_id => my_contacts + others_contacts) + ShareVisibility.for_a_users_contacts(user).destroy_all + ShareVisibility.for_contacts_of_a_person(person).destroy_all end -# def delete_notification_actors -# end + def remove_conversation_visibilities + ConversationVisibility.where(:person_id => person.id).destroy_all + end def delete_posts - self.person.posts.delete_all + self.person.posts.destroy_all end def delete_photos - self.person.photos.delete_all + self.person.photos.destroy_all end def delete_mentions - self.person.mentions.delete_all + self.person.mentions.destroy_all end def tombstone_person_and_profile @@ -86,6 +78,6 @@ class AccountDeletion end def delete_contacts_of_me - Contact.all_contacts_of_person(self.person).delete_all + Contact.all_contacts_of_person(self.person).destroy_all end end diff --git a/app/models/share_visibility.rb b/app/models/share_visibility.rb index eb80309f4..b46fb3342 100644 --- a/app/models/share_visibility.rb +++ b/app/models/share_visibility.rb @@ -9,9 +9,9 @@ class ShareVisibility < ActiveRecord::Base scope :for_a_users_contacts, lambda { |user| where(:contact_id => user.contacts.map {|c| c.id}) } - - alias :for_a_users_contacts :for_contacts_of_a_person - + scope :for_contacts_of_a_person, lambda { |person| + where(:contact_id => person.contacts.map {|c| c.id}) + } # Perform a batch import, given a set of contacts and a shareable # @note performs a bulk insert in mySQL; performs linear insertions in postgres diff --git a/app/models/user.rb b/app/models/user.rb index e49025acd..269c00797 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -33,15 +33,15 @@ class User < ActiveRecord::Base has_one :person, :foreign_key => :owner_id delegate :public_key, :posts, :photos, :owns?, :diaspora_handle, :name, :public_url, :profile, :first_name, :last_name, :to => :person - has_many :invitations_from_me, :class_name => 'Invitation', :foreign_key => :sender_id, :dependent => :destroy - has_many :invitations_to_me, :class_name => 'Invitation', :foreign_key => :recipient_id, :dependent => :destroy + has_many :invitations_from_me, :class_name => 'Invitation', :foreign_key => :sender_id + has_many :invitations_to_me, :class_name => 'Invitation', :foreign_key => :recipient_id has_many :aspects, :order => 'order_id ASC' has_many :aspect_memberships, :through => :aspects has_many :contacts has_many :contact_people, :through => :contacts, :source => :person - has_many :services, :dependent => :destroy - has_many :user_preferences, :dependent => :destroy - has_many :tag_followings, :dependent => :destroy + has_many :services + has_many :user_preferences + has_many :tag_followings has_many :followed_tags, :through => :tag_followings, :source => :tag, :order => 'tags.name' has_many :blocks has_many :notifications, :foreign_key => :recipient_id diff --git a/spec/integration/account_deletion_spec.rb b/spec/integration/account_deletion_spec.rb index 053ace48b..13ccf1007 100644 --- a/spec/integration/account_deletion_spec.rb +++ b/spec/integration/account_deletion_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'deleteing your account' do - before do + before :all do @bob2 = bob @bobs_person_id = @bob2.person.id @alices_post = alice.post(:status_message, :text => "@{@bob2 Grimn; #{@bob2.person.diaspora_handle}} you are silly", :to => alice.aspects.find_by_name('generic')) @@ -19,8 +19,32 @@ describe 'deleteing your account' do #conversations create_conversation_with_message(alice, @bob2, "Subject", "Hey @bob2") - AccountDeletion.new(@bob2.person.diaspora_handle).perform! + #join tables + @users_sv = ShareVisibility.where(:contact_id => @bobs_contact_ids).all + @persons_sv = ShareVisibility.where(:contact_id => bob.person.contacts.map(&:id)).all + #user associated objects + @prefs = [] + %w{mentioned liked reshared}.each do |pref| + @prefs << @bob2.user_preferences.create!(:email_type => pref) + end + + # notifications + @notifications = [] + 3.times do |n| + @notifications << Factory(:notification, :recipient => @bob2) + end + + # services + @services = [] + 3.times do |n| + @services << Factory(:service, :user => @bob2) + end + + # block + @block = @bob2.blocks.create!(:person => eve.person) + + AccountDeletion.new(@bob2.person.diaspora_handle).perform! @bob2.reload end @@ -28,16 +52,32 @@ describe 'deleteing your account' do @bob2.posts.should be_empty end + it "deletes all of the user's preferences" do + UserPreference.where(:id => @prefs.map{|pref| pref.id}).should be_empty + end + + it "deletes all of the user's notifications" do + Notification.where(:id => @notifications.map{|n| n.id}).should be_empty + end + + it "deletes all of the users's blocked users" do + Block.where(:id => @block.id).should be_empty + end + + it "deletes all of the user's services" do + Service.where(:id => @services.map{|s| s.id}).should be_empty + end + it 'deletes all of @bob2s share visiblites' do - ShareVisibility.where(:contact_id => @bobs_contact_ids).should be_empty - ShareVisibility.where(:contact_id => bob.person.contacts.map(&:id)).should be_empty + ShareVisibility.where(:id => @users_sv.map{|sv| sv.id}).should be_empty + ShareVisibility.where(:id => @persons_sv.map{|sv| sv.id}).should be_empty end it 'deletes all photos' do Photo.where(:author_id => @bobs_person_id).should be_empty end - it 'deletes all mentions ' do + it 'deletes all mentions' do @bob2.person.mentions.should be_empty end @@ -55,7 +95,8 @@ describe 'deleteing your account' do @bob2.person.profile.reload.first_name.should be_blank end - it 'deletes the converersation visibilities' do - pending + it 'deletes only the converersation visibility for the deleted user' do + ConversationVisibility.where(:person_id => alice.person.id).should_not be_empty + ConversationVisibility.where(:person_id => bob.person.id).should be_empty end end diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index 9e4dc5c40..4b10fdcbf 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -16,48 +16,33 @@ describe AccountDeletion do end describe '#perform' do - it 'calls delete_standard_associations' do - @account_deletion.should_receive(:delete_standard_associations) + after do @account_deletion.perform! end - it 'calls disassociate_invitations' do - @account_deletion.should_receive(:disassociate_invitations) - @account_deletion.perform! - end + [:delete_standard_associations, + :disassociate_invitations, + :delete_standard_associations, + :delete_contacts_of_me, + :delete_mentions, + :disconnect_contacts, + :delete_photos, + :delete_posts, + :tombstone_person_and_profile, + :remove_share_visibilities, + :remove_conversation_visibilities].each do |method| - it 'calls delete_contacts_of_me' do - @account_deletion.should_receive(:delete_contacts_of_me) - @account_deletion.perform! - end - - it 'calls delete_contacts_of_me' do - @account_deletion.should_receive(:delete_mentions) - @account_deletion.perform! - end - - it 'calls disconnect_contacts' do - @account_deletion.should_receive(:disconnect_contacts) - @account_deletion.perform! - end - - it 'calls delete_posts' do - @account_deletion.should_receive(:delete_posts) - @account_deletion.perform! - end - - it 'calls tombstone_person_and_profile' do - @account_deletion.should_receive(:tombstone_person_and_profile) - @account_deletion.perform! + it "calls ##{method.to_s}" do + @account_deletion.should_receive(method) + end end end - + describe "#delete_standard_associations" do it 'removes all standard user associaltions' do - @account_deletion.normal_ar_user_associates_to_delete.each do |asso| association_mock = mock - association_mock.should_receive(:delete_all) + association_mock.should_receive(:destroy_all) bob.should_receive(asso).and_return(association_mock) end @@ -65,17 +50,16 @@ describe AccountDeletion do end end - describe '#delete_posts' do it 'deletes all posts' do - @account_deletion.person.posts.should_receive(:delete_all) + @account_deletion.person.posts.should_receive(:destroy_all) @account_deletion.delete_posts end end describe '#delete_photos' do it 'deletes all photos' do - @account_deletion.person.photos.should_receive(:delete_all) + @account_deletion.person.photos.should_receive(:destroy_all) @account_deletion.delete_photos end end @@ -89,26 +73,19 @@ describe AccountDeletion do end end - describe "#normal_ar_user_associates_to_delete" do - it "has the regular associations" do - @account_deletion.normal_ar_user_associates_to_delete.should == - [:tag_followings, :authorizations, :invitations_to_me, :services, :aspects, :user_preferences, :notifications] - end - end - context 'person associations' do describe '#delete mentions' do it 'deletes the mentions for people' do mentions = mock @account_deletion.person.should_receive(:mentions).and_return(mentions) - mentions.should_receive(:delete_all) + mentions.should_receive(:destroy_all) @account_deletion.delete_mentions end end describe '#disconnect_contacts' do it "deletes all of user's contacts" do - bob.contacts.should_receive(:delete_all) + bob.contacts.should_receive(:destroy_all) @account_deletion.disconnect_contacts end end @@ -117,7 +94,7 @@ describe AccountDeletion do it 'deletes all the local contact objects where deleted account is the person' do contacts = mock Contact.should_receive(:all_contacts_of_person).with(bob.person).and_return(contacts) - contacts.should_receive(:delete_all) + contacts.should_receive(:destroy_all) @account_deletion.delete_contacts_of_me end end @@ -128,6 +105,40 @@ describe AccountDeletion do @account_deletion.tombstone_person_and_profile end end + describe "#remove_conversation_visibilities" do + it "removes the conversation visibility for the deleted user" do + vis = stub + ConversationVisibility.should_receive(:where).with(hash_including(:person_id => bob.person.id)).and_return(vis) + vis.should_receive(:destroy_all) + @account_deletion.remove_conversation_visibilities + end + end + end + + describe "#remove_share_visibilities" do + before do + @s_vis = stub + end + + after do + @account_deletion.remove_share_visibilities + end + + it 'removes the share visibilities for a person ' do + ShareVisibility.should_receive(:for_contacts_of_a_person).with(bob.person).and_return(@s_vis) + @s_vis.should_receive(:destroy_all) + end + + it 'removes the share visibilities for a user' do + ShareVisibility.should_receive(:for_a_users_contacts).with(bob).and_return(@s_vis) + @s_vis.should_receive(:destroy_all) + end + + it 'does not remove share visibilities for a user if the user is not present' do + pending + ShareVisibility.should_receive(:for_a_users_contacts).with(bob).and_return(@s_vis) + @s_vis.should_receive(:destroy_all) + end end it 'has all user association keys accounted for' do From cd6f97fa0eadbf48d1f291c89686b6eebc540d64 Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Mon, 7 Nov 2011 17:27:26 -0800 Subject: [PATCH 04/17] DG IZ; remove authorizations from user; double check that aspect_visibilities are deleted --- app/models/account_deletion.rb | 12 ++++++++++++ spec/integration/account_deletion_spec.rb | 15 ++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb index 4ab5a5536..8ece18e0e 100644 --- a/app/models/account_deletion.rb +++ b/app/models/account_deletion.rb @@ -3,6 +3,18 @@ # the COPYRIGHT file. class AccountDeletion + + # Things that are not removed from the database: + # - Comments + # - Likes + # - Messages + # - NotificationActors + # + # Given that the User in question will be tombstoned, all of the + # above will come from an anonomized account (via the UI). + # The deleted user will appear as "Deleted Account" in + # the interface. + attr_accessor :person, :user def initialize(diaspora_id) diff --git a/spec/integration/account_deletion_spec.rb b/spec/integration/account_deletion_spec.rb index 13ccf1007..258c5b84d 100644 --- a/spec/integration/account_deletion_spec.rb +++ b/spec/integration/account_deletion_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'deleteing your account' do - before :all do + before do @bob2 = bob @bobs_person_id = @bob2.person.id @alices_post = alice.post(:status_message, :text => "@{@bob2 Grimn; #{@bob2.person.diaspora_handle}} you are silly", :to => alice.aspects.find_by_name('generic')) @@ -12,6 +12,8 @@ describe 'deleteing your account' do @bob2.post(:status_message, :text => 'asldkfjs', :to => @bob2.aspects.first) f = Factory(:photo, :author => @bob2.person) + @aspect_vis = AspectVisibility.where(:aspect_id => @bob2.aspects.map(&:id)) + #objects on post @bob2.like(true, :target => @alices_post) @bob2.comment("here are some thoughts on your post", :post => @alices_post) @@ -44,6 +46,9 @@ describe 'deleteing your account' do # block @block = @bob2.blocks.create!(:person => eve.person) + #authorization + @authorization = Factory.create(:oauth_authorization, :resource_owner => @bob2) + AccountDeletion.new(@bob2.person.diaspora_handle).perform! @bob2.reload end @@ -73,6 +78,10 @@ describe 'deleteing your account' do ShareVisibility.where(:id => @persons_sv.map{|sv| sv.id}).should be_empty end + it 'deletes all of @bob2s aspect visiblites' do + AspectVisibility.where(:id => @aspect_vis.map(&:id)).should be_empty + end + it 'deletes all photos' do Photo.where(:author_id => @bobs_person_id).should be_empty end @@ -89,6 +98,10 @@ describe 'deleteing your account' do @bob2.contacts.should be_empty end + it 'deletes all the authorizations' do + OAuth2::Provider.authorization_class.where(:id => @authorization.id).should be_empty + end + it 'sets the person object as closed and the profile is cleared' do @bob2.person.reload.closed_account.should be_true From e6ed2d397a9c0e280083934dbf2d387e8a4a226c Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Tue, 8 Nov 2011 13:48:38 -0800 Subject: [PATCH 05/17] added close account to user --- app/models/account_deletion.rb | 5 +++ app/models/person.rb | 7 +--- app/models/profile.rb | 4 +- app/models/user.rb | 12 ++++++ spec/models/account_deletion_spec.rb | 10 ++++- spec/models/profile_spec.rb | 9 ++--- spec/models/user_spec.rb | 56 +++++++++++++++++++++++++++- 7 files changed, 89 insertions(+), 14 deletions(-) diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb index 8ece18e0e..1a40c89c1 100644 --- a/app/models/account_deletion.rb +++ b/app/models/account_deletion.rb @@ -33,6 +33,7 @@ class AccountDeletion delete_photos delete_posts tombstone_person_and_profile + tombstone_user end #user deletions @@ -89,6 +90,10 @@ class AccountDeletion self.person.close_account! end + def tombstone_user + self.user.close_account! + end + def delete_contacts_of_me Contact.all_contacts_of_person(self.person).destroy_all end diff --git a/app/models/person.rb b/app/models/person.rb index eab06e394..4db5da134 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -40,7 +40,7 @@ class Person < ActiveRecord::Base before_destroy :remove_all_traces before_validation :clean_url - + validates :url, :presence => true validates :profile, :presence => true validates :serialized_public_key, :presence => true @@ -78,7 +78,7 @@ class Person < ActiveRecord::Base super self.profile ||= Profile.new unless profile_set end - + def self.find_from_id_or_username(params) p = if params[:id].present? Person.where(:id => params[:id]).first @@ -91,7 +91,6 @@ class Person < ActiveRecord::Base p end - def self.search_query_string(query) query = query.downcase like_operator = postgres? ? "ILIKE" : "LIKE" @@ -276,8 +275,6 @@ class Person < ActiveRecord::Base end end - - # @param person [Person] # @param url [String] def update_url(url) diff --git a/app/models/profile.rb b/app/models/profile.rb index fc41de397..cdbb1b2be 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -148,7 +148,7 @@ class Profile < ActiveRecord::Base def tombstone! self.taggings.delete_all - clearable_profile_fields.each do |field| + clearable_fields.each do |field| self[field] = nil end self.save @@ -174,7 +174,7 @@ class Profile < ActiveRecord::Base end private - def clearable_profile_fields + def clearable_fields self.attributes.keys - Profile.protected_attributes.to_a - ["created_at", "updated_at", "person_id"] end diff --git a/app/models/user.rb b/app/models/user.rb index 269c00797..d0a8be3db 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -488,4 +488,16 @@ class User < ActiveRecord::Base errors[:base] << 'That username has already been taken' end end + + def close_account! + clearable_fields.each do |field| + self[field] = nil + end + self.save + end + + private + def clearable_fields + self.attributes.keys - ["username", "encrypted_password", "created_at", "updated_at"] + end end diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index 4b10fdcbf..1a4ed4699 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -30,7 +30,8 @@ describe AccountDeletion do :delete_posts, :tombstone_person_and_profile, :remove_share_visibilities, - :remove_conversation_visibilities].each do |method| + :remove_conversation_visibilities, + :tombstone_user].each do |method| it "calls ##{method.to_s}" do @account_deletion.should_receive(method) @@ -141,6 +142,13 @@ describe AccountDeletion do end end + describe "#tombstone_user" do + it 'calls strip_model on user' do + bob.should_receive(:close_account!) + @account_deletion.tombstone_user + end + end + it 'has all user association keys accounted for' do all_keys = (@account_deletion.normal_ar_user_associates_to_delete + @account_deletion.special_ar_user_associations + @account_deletion.ignored_ar_user_associations) all_keys.sort{|x, y| x.to_s <=> y.to_s}.should == User.reflections.keys.sort{|x, y| x.to_s <=> y.to_s} diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index 9a703fe78..2905c6b05 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -264,7 +264,6 @@ describe Profile do end describe '#receive' do - it 'updates the profile in place' do local_luke, local_leia, remote_raphael = set_up_friends new_profile = Factory.build :profile @@ -281,8 +280,8 @@ describe Profile do @profile = bob.person.profile end it "clears the profile fields" do - attributes = @profile.send(:clearable_profile_fields) - + attributes = @profile.send(:clearable_fields) + @profile.tombstone! @profile.reload attributes.each{ |attr| @@ -296,10 +295,10 @@ describe Profile do end end - describe "#clearable_profile_fields" do + describe "#clearable_fields" do it 'returns the current profile fields' do profile = Factory.build :profile - profile.send(:clearable_profile_fields).sort.should == + profile.send(:clearable_fields).sort.should == ["diaspora_handle", "first_name", "last_name", diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 431a745be..a2ef86505 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1003,6 +1003,60 @@ describe User do user = Factory :user Resque.should_receive(:enqueue).with(Jobs::ResetPassword, user.id) user.send_reset_password_instructions + + context "close account" do + before do + @user = bob + end + + describe "#close_account!" do + it 'resets the password to a random string' do + random_pass = "12345678909876543210" + ActiveSupport::SecureRandom.should_receive(:hex).and_return(random_pass) + @user.close_account! + @user.valid_password?(random_pass) + end + + it 'clears all the clearable fields' do + attributes = @user.send(:clearable_fields) + @user.close_account! + + attributes.each do |attr| + @user.send(attr.to_sym).should be_blank + end + end + end + + describe "#clearable_attributes" do + it 'has all the attributes' do + user = Factory.build :user + user.send(:clearable_fields).sort.should == %w{ + serialized_private_key + getting_started + disable_mail + language + email + invitation_token + invitation_sent_at + reset_password_token + remember_token + remember_created_at + sign_in_count + current_sign_in_at + last_sign_in_at + current_sign_in_ip + last_sign_in_ip + invitation_service + invitation_identifier + invitation_limit + invited_by_id + invited_by_type + authentication_token + unconfirmed_email + confirm_email_token + locked_at + show_community_spotlight_in_stream + }.sort + end end end -end From 03ca34767a0f5382f15b2c1d8e2f3f5b703a90a5 Mon Sep 17 00:00:00 2001 From: Ilya Zhitomirskiy Date: Tue, 8 Nov 2011 18:23:11 -0800 Subject: [PATCH 06/17] ms iz rendering nothing for hcard and webfinger if account is closed, not showing aspect dropdown if the user account is closed --- app/controllers/people_controller.rb | 7 +- app/controllers/publics_controller.rb | 12 ++ app/helpers/aspects_helper.rb | 2 + ...account_deletion.rb => account_deleter.rb} | 63 +++--- app/models/contact.rb | 9 +- app/models/jobs/delete_account.rb | 3 +- app/models/user.rb | 8 +- config/locales/diaspora/en.yml | 1 + spec/controllers/people_controller_spec.rb | 7 + spec/controllers/publics_controller_spec.rb | 12 ++ spec/helper_methods.rb | 4 +- spec/integration/account_deletion_spec.rb | 195 ++++++++++-------- spec/misc_spec.rb | 2 +- ...letion_spec.rb => account_deleter_spec.rb} | 133 +++++++----- spec/models/contact_spec.rb | 9 + spec/models/user_spec.rb | 9 +- spec/shared_behaviors/account_deletion.rb | 42 ++++ 17 files changed, 338 insertions(+), 180 deletions(-) rename app/models/{account_deletion.rb => account_deleter.rb} (65%) rename spec/models/{account_deletion_spec.rb => account_deleter_spec.rb} (54%) create mode 100644 spec/shared_behaviors/account_deletion.rb diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index dc7e1416b..ac9c708d2 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -90,6 +90,11 @@ class PeopleController < ApplicationController raise ActiveRecord::RecordNotFound end + if @person.closed_account? + redirect_to :back, :notice => t("people.show.closed_account") + return + end + @post_type = :all @aspect = :profile @share_with = (params[:share_with] == 'true') @@ -177,6 +182,4 @@ class PeopleController < ApplicationController def remote_profile_with_no_user_session? @person && @person.remote? && !user_signed_in? end - - end diff --git a/app/controllers/publics_controller.rb b/app/controllers/publics_controller.rb index a24525534..c6d8bbc76 100644 --- a/app/controllers/publics_controller.rb +++ b/app/controllers/publics_controller.rb @@ -32,6 +32,12 @@ class PublicsController < ApplicationController def hcard @person = Person.where(:guid => params[:guid]).first + + if @person && @person.closed_account? + render :nothing => true, :status => 404 + return + end + unless @person.nil? || @person.owner.nil? render 'publics/hcard' else @@ -45,6 +51,12 @@ class PublicsController < ApplicationController def webfinger @person = Person.local_by_account_identifier(params[:q]) if params[:q] + + if @person && @person.closed_account? + render :nothing => true, :status => 404 + return + end + unless @person.nil? render 'webfinger', :content_type => 'application/xrd+xml' else diff --git a/app/helpers/aspects_helper.rb b/app/helpers/aspects_helper.rb index 906ddc272..f02505b01 100644 --- a/app/helpers/aspects_helper.rb +++ b/app/helpers/aspects_helper.rb @@ -39,6 +39,8 @@ module AspectsHelper end def aspect_membership_button(aspect, contact, person) + return if person && person.closed_account? + if contact.nil? || !contact.aspect_memberships.detect{ |am| am.aspect_id == aspect.id} add_to_aspect_button(aspect.id, person.id) else diff --git a/app/models/account_deletion.rb b/app/models/account_deleter.rb similarity index 65% rename from app/models/account_deletion.rb rename to app/models/account_deleter.rb index 1a40c89c1..d28043d82 100644 --- a/app/models/account_deletion.rb +++ b/app/models/account_deleter.rb @@ -2,7 +2,7 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -class AccountDeletion +class AccountDeleter # Things that are not removed from the database: # - Comments @@ -23,17 +23,21 @@ class AccountDeletion end def perform! - delete_standard_associations - disassociate_invitations - delete_mentions - delete_contacts_of_me - remove_share_visibilities + #person + delete_standard_person_associations remove_conversation_visibilities - disconnect_contacts - delete_photos - delete_posts + remove_share_visibilities_on_persons_posts + delete_contacts_of_me tombstone_person_and_profile - tombstone_user + + if self.user + #user deletion methods + remove_share_visibilities_on_contacts_posts + delete_standard_user_associations + disassociate_invitations + disconnect_contacts + tombstone_user + end end #user deletions @@ -49,9 +53,15 @@ class AccountDeletion [:followed_tags, :invited_by, :contact_people, :applications, :aspect_memberships] end - def delete_standard_associations + def delete_standard_user_associations normal_ar_user_associates_to_delete.each do |asso| - user.send(asso).destroy_all + self.user.send(asso).each{|model| model.delete} + end + end + + def delete_standard_person_associations + normal_ar_person_associates_to_delete.each do |asso| + self.person.send(asso).delete_all end end @@ -65,27 +75,20 @@ class AccountDeletion user.contacts.destroy_all end - def remove_share_visibilities - ShareVisibility.for_a_users_contacts(user).destroy_all + # Currently this would get deleted due to the db foreign key constrainsts, + # but we'll keep this method here for completeness + def remove_share_visibilities_on_persons_posts ShareVisibility.for_contacts_of_a_person(person).destroy_all end + def remove_share_visibilities_on_contacts_posts + ShareVisibility.for_a_users_contacts(user).destroy_all + end + def remove_conversation_visibilities ConversationVisibility.where(:person_id => person.id).destroy_all end - def delete_posts - self.person.posts.destroy_all - end - - def delete_photos - self.person.photos.destroy_all - end - - def delete_mentions - self.person.mentions.destroy_all - end - def tombstone_person_and_profile self.person.close_account! end @@ -97,4 +100,12 @@ class AccountDeletion def delete_contacts_of_me Contact.all_contacts_of_person(self.person).destroy_all end + + def normal_ar_person_associates_to_delete + [:posts, :photos, :mentions] + end + + def ignored_or_special_ar_person_associations + [:comments, :contacts, :notification_actors, :notifications, :owner, :profile ] + end end diff --git a/app/models/contact.rb b/app/models/contact.rb index 8eae319fb..22c5a5f06 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -15,7 +15,8 @@ class Contact < ActiveRecord::Base has_many :posts, :through => :share_visibilities, :source => :shareable, :source_type => 'Post' validate :not_contact_for_self, - :not_blocked_user + :not_blocked_user, + :not_contact_with_closed_account validates_presence_of :user validates_uniqueness_of :person_id, :scope => :user_id @@ -97,6 +98,12 @@ class Contact < ActiveRecord::Base end private + def not_contact_with_closed_account + if person_id && person.closed_account? + errors[:base] << 'Cannot be in contact with a closed account' + end + end + def not_contact_for_self if person_id && person.owner == user errors[:base] << 'Cannot create self-contact' diff --git a/app/models/jobs/delete_account.rb b/app/models/jobs/delete_account.rb index 0d9b42f8f..acc813a7d 100644 --- a/app/models/jobs/delete_account.rb +++ b/app/models/jobs/delete_account.rb @@ -8,8 +8,7 @@ module Jobs @queue = :delete_account def self.perform(user_id) user = User.find(user_id) - user.remove_all_traces - user.destroy + AccountDeleter.new(user.person.diaspora_handle).perform! end end end diff --git a/app/models/user.rb b/app/models/user.rb index d0a8be3db..928a6a7cb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -493,11 +493,15 @@ class User < ActiveRecord::Base clearable_fields.each do |field| self[field] = nil end - self.save + + random_password = ActiveSupport::SecureRandom.hex(20) + self.password = random_password + self.password_confirmation = random_password + self.save(:validate => false) end private def clearable_fields - self.attributes.keys - ["username", "encrypted_password", "created_at", "updated_at"] + self.attributes.keys - ["id", "username", "encrypted_password", "created_at", "updated_at"] end end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 87a6fd482..1d6871384 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -551,6 +551,7 @@ en: message: "Message" mention: "Mention" ignoring: "You are ignoring all posts from %{name}." + closed_account: "This account has been closed." sub_header: you_have_no_tags: "you have no tags!" add_some: "add some" diff --git a/spec/controllers/people_controller_spec.rb b/spec/controllers/people_controller_spec.rb index 6dfc245af..3c4630b7e 100644 --- a/spec/controllers/people_controller_spec.rb +++ b/spec/controllers/people_controller_spec.rb @@ -161,6 +161,13 @@ describe PeopleController do response.code.should == "404" end + it 'redirects home for closed account' do + @person = Factory.create(:person, :closed_account => true) + get :show, :id => @person.id + response.should be_redirect + flash[:notice].should_not be_blank + end + it 'does not allow xss attacks' do user2 = bob profile = user2.profile diff --git a/spec/controllers/publics_controller_spec.rb b/spec/controllers/publics_controller_spec.rb index 903fb9162..ddff1c413 100644 --- a/spec/controllers/publics_controller_spec.rb +++ b/spec/controllers/publics_controller_spec.rb @@ -97,6 +97,12 @@ describe PublicsController do assigns[:person].should be_nil response.should be_not_found end + + it 'finds nothing for closed accounts' do + @user.person.update_attributes(:closed_account => true) + get :hcard, :guid => @user.person.guid.to_s + response.should be_not_found + end end describe '#webfinger' do @@ -127,6 +133,12 @@ describe PublicsController do get :webfinger, :q => @user.diaspora_handle response.body.should include "http://webfinger.net/rel/profile-page" end + + it 'finds nothing for closed accounts' do + @user.person.update_attributes(:closed_account => true) + get :webfinger, :q => @user.diaspora_handle + response.should be_not_found + end end describe '#hub' do diff --git a/spec/helper_methods.rb b/spec/helper_methods.rb index 921b7ffbc..471526452 100644 --- a/spec/helper_methods.rb +++ b/spec/helper_methods.rb @@ -62,10 +62,10 @@ module HelperMethods File.open(fixture_name) end - def create_conversation_with_message(sender, recipient, subject, text) + def create_conversation_with_message(sender, recipient_person, subject, text) create_hash = { :author => sender.person, - :participant_ids => [sender.person.id, recipient.person.id], + :participant_ids => [sender.person.id, recipient_person.id], :subject => subject, :messages_attributes => [ {:author => sender.person, :text => text} ] } diff --git a/spec/integration/account_deletion_spec.rb b/spec/integration/account_deletion_spec.rb index 258c5b84d..ae350f1f5 100644 --- a/spec/integration/account_deletion_spec.rb +++ b/spec/integration/account_deletion_spec.rb @@ -1,115 +1,138 @@ require 'spec_helper' describe 'deleteing your account' do - before do - @bob2 = bob - @bobs_person_id = @bob2.person.id - @alices_post = alice.post(:status_message, :text => "@{@bob2 Grimn; #{@bob2.person.diaspora_handle}} you are silly", :to => alice.aspects.find_by_name('generic')) + context "user" do + before do + @bob2 = bob + @person = @bob2.person + @alices_post = alice.post(:status_message, :text => "@{@bob2 Grimn; #{@bob2.person.diaspora_handle}} you are silly", :to => alice.aspects.find_by_name('generic')) - @bobs_contact_ids = @bob2.contacts.map {|c| c.id} + @bobs_contact_ids = @bob2.contacts.map {|c| c.id} - #@bob2's own content - @bob2.post(:status_message, :text => 'asldkfjs', :to => @bob2.aspects.first) - f = Factory(:photo, :author => @bob2.person) + #@bob2's own content + @bob2.post(:status_message, :text => 'asldkfjs', :to => @bob2.aspects.first) + f = Factory(:photo, :author => @bob2.person) - @aspect_vis = AspectVisibility.where(:aspect_id => @bob2.aspects.map(&:id)) + @aspect_vis = AspectVisibility.where(:aspect_id => @bob2.aspects.map(&:id)) - #objects on post - @bob2.like(true, :target => @alices_post) - @bob2.comment("here are some thoughts on your post", :post => @alices_post) + #objects on post + @bob2.like(true, :target => @alices_post) + @bob2.comment("here are some thoughts on your post", :post => @alices_post) - #conversations - create_conversation_with_message(alice, @bob2, "Subject", "Hey @bob2") + #conversations + create_conversation_with_message(alice, @bob2.person, "Subject", "Hey @bob2") - #join tables - @users_sv = ShareVisibility.where(:contact_id => @bobs_contact_ids).all - @persons_sv = ShareVisibility.where(:contact_id => bob.person.contacts.map(&:id)).all + #join tables + @users_sv = ShareVisibility.where(:contact_id => @bobs_contact_ids).all + @persons_sv = ShareVisibility.where(:contact_id => bob.person.contacts.map(&:id)).all - #user associated objects - @prefs = [] - %w{mentioned liked reshared}.each do |pref| - @prefs << @bob2.user_preferences.create!(:email_type => pref) + #user associated objects + @prefs = [] + %w{mentioned liked reshared}.each do |pref| + @prefs << @bob2.user_preferences.create!(:email_type => pref) + end + + # notifications + @notifications = [] + 3.times do |n| + @notifications << Factory(:notification, :recipient => @bob2) + end + + # services + @services = [] + 3.times do |n| + @services << Factory(:service, :user => @bob2) + end + + # block + @block = @bob2.blocks.create!(:person => eve.person) + + #authorization + @authorization = Factory.create(:oauth_authorization, :resource_owner => @bob2) + + AccountDeleter.new(@bob2.person.diaspora_handle).perform! + @bob2.reload end - # notifications - @notifications = [] - 3.times do |n| - @notifications << Factory(:notification, :recipient => @bob2) + it "deletes all of the user's preferences" do + UserPreference.where(:id => @prefs.map{|pref| pref.id}).should be_empty end - # services - @services = [] - 3.times do |n| - @services << Factory(:service, :user => @bob2) + it "deletes all of the user's notifications" do + Notification.where(:id => @notifications.map{|n| n.id}).should be_empty end - # block - @block = @bob2.blocks.create!(:person => eve.person) + it "deletes all of the users's blocked users" do + Block.where(:id => @block.id).should be_empty + end - #authorization - @authorization = Factory.create(:oauth_authorization, :resource_owner => @bob2) + it "deletes all of the user's services" do + Service.where(:id => @services.map{|s| s.id}).should be_empty + end - AccountDeletion.new(@bob2.person.diaspora_handle).perform! - @bob2.reload + it 'deletes all of @bob2s share visiblites' do + ShareVisibility.where(:id => @users_sv.map{|sv| sv.id}).should be_empty + ShareVisibility.where(:id => @persons_sv.map{|sv| sv.id}).should be_empty + end + + it 'deletes all of @bob2s aspect visiblites' do + AspectVisibility.where(:id => @aspect_vis.map(&:id)).should be_empty + end + + it 'deletes all aspects' do + @bob2.aspects.should be_empty + end + + it 'deletes all user contacts' do + @bob2.contacts.should be_empty + end + + it 'deletes all the authorizations' do + OAuth2::Provider.authorization_class.where(:id => @authorization.id).should be_empty + end + + it "clears the account fields" do + @bob2.send(:clearable_fields).each do |field| + @bob2.reload[field].should be_blank + end + end + + it_should_behave_like 'it removes the person associations' end - it 'deletes all of @bob2s posts' do - @bob2.posts.should be_empty - end + context 'remote person' do + before do + @person = remote_raphael + + #contacts + @contacts = @person.contacts - it "deletes all of the user's preferences" do - UserPreference.where(:id => @prefs.map{|pref| pref.id}).should be_empty - end + #posts + @posts = (1..3).map do + Factory.create(:status_message, :author => @person) + end - it "deletes all of the user's notifications" do - Notification.where(:id => @notifications.map{|n| n.id}).should be_empty - end + @persons_sv = @posts.each do |post| + @contacts.each do |contact| + ShareVisibility.create!(:contact_id => contact.id, :shareable => post) + end + end - it "deletes all of the users's blocked users" do - Block.where(:id => @block.id).should be_empty - end + #photos + @photo = Factory(:photo, :author => @person) - it "deletes all of the user's services" do - Service.where(:id => @services.map{|s| s.id}).should be_empty - end + #mentions + @mentions = 3.times do + Factory.create(:mention, :person => @person) + end - it 'deletes all of @bob2s share visiblites' do - ShareVisibility.where(:id => @users_sv.map{|sv| sv.id}).should be_empty - ShareVisibility.where(:id => @persons_sv.map{|sv| sv.id}).should be_empty - end + #conversations + create_conversation_with_message(alice, @person, "Subject", "Hey @bob2") - it 'deletes all of @bob2s aspect visiblites' do - AspectVisibility.where(:id => @aspect_vis.map(&:id)).should be_empty - end + AccountDeleter.new(@person.diaspora_handle).perform! + @person.reload + end - it 'deletes all photos' do - Photo.where(:author_id => @bobs_person_id).should be_empty - end - - it 'deletes all mentions' do - @bob2.person.mentions.should be_empty - end - - it 'deletes all aspects' do - @bob2.aspects.should be_empty - end - - it 'deletes all contacts' do - @bob2.contacts.should be_empty - end - - it 'deletes all the authorizations' do - OAuth2::Provider.authorization_class.where(:id => @authorization.id).should be_empty - end - - it 'sets the person object as closed and the profile is cleared' do - @bob2.person.reload.closed_account.should be_true - - @bob2.person.profile.reload.first_name.should be_blank - end - - it 'deletes only the converersation visibility for the deleted user' do - ConversationVisibility.where(:person_id => alice.person.id).should_not be_empty - ConversationVisibility.where(:person_id => bob.person.id).should be_empty + it_should_behave_like 'it removes the person associations' end end diff --git a/spec/misc_spec.rb b/spec/misc_spec.rb index 8007c3753..642ccc477 100644 --- a/spec/misc_spec.rb +++ b/spec/misc_spec.rb @@ -72,7 +72,7 @@ describe 'making sure the spec runner works' do describe "#create_conversation_with_message" do it 'creates a conversation and a message' do - conversation = create_conversation_with_message(alice, bob, "Subject", "Hey Bob") + conversation = create_conversation_with_message(alice, bob.person, "Subject", "Hey Bob") conversation.participants.should == [alice.person, bob.person] conversation.subject.should == "Subject" diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deleter_spec.rb similarity index 54% rename from spec/models/account_deletion_spec.rb rename to spec/models/account_deleter_spec.rb index 1a4ed4699..38fcdd345 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deleter_spec.rb @@ -4,64 +4,95 @@ require 'spec_helper' -describe AccountDeletion do +describe AccountDeleter do before do - @account_deletion = AccountDeletion.new(bob.person.diaspora_handle) + @account_deletion = AccountDeleter.new(bob.person.diaspora_handle) @account_deletion.user = bob end it "attaches the user" do - AccountDeletion.new(bob.person.diaspora_handle).user.should == bob - AccountDeletion.new(remote_raphael.diaspora_handle).user.should == nil + AccountDeleter.new(bob.person.diaspora_handle).user.should == bob + AccountDeleter.new(remote_raphael.diaspora_handle).user.should == nil end describe '#perform' do - after do - @account_deletion.perform! - end - [:delete_standard_associations, + + user_removal_methods = [:delete_standard_user_associations, :disassociate_invitations, - :delete_standard_associations, - :delete_contacts_of_me, - :delete_mentions, + :remove_share_visibilities_on_contacts_posts, :disconnect_contacts, - :delete_photos, - :delete_posts, - :tombstone_person_and_profile, - :remove_share_visibilities, - :remove_conversation_visibilities, - :tombstone_user].each do |method| + :tombstone_user] - it "calls ##{method.to_s}" do - @account_deletion.should_receive(method) + person_removal_methods = [:delete_contacts_of_me, + :delete_standard_person_associations, + :tombstone_person_and_profile, + :remove_share_visibilities_on_persons_posts, + :remove_conversation_visibilities] + + context "user deletion" do + after do + @account_deletion.perform! + end + + (user_removal_methods + person_removal_methods).each do |method| + + it "calls ##{method.to_s}" do + @account_deletion.should_receive(method) + end end end + + context "person deletion" do + before do + @person_deletion = AccountDeleter.new(remote_raphael.diaspora_handle) + end + + after do + @person_deletion.perform! + end + + (user_removal_methods).each do |method| + + it "does not call ##{method.to_s}" do + @person_deletion.should_not_receive(method) + end + end + + (person_removal_methods).each do |method| + + it "calls ##{method.to_s}" do + @person_deletion.should_receive(method) + end + end + end + end - describe "#delete_standard_associations" do + describe "#delete_standard_user_associations" do it 'removes all standard user associaltions' do @account_deletion.normal_ar_user_associates_to_delete.each do |asso| association_mock = mock - association_mock.should_receive(:destroy_all) - bob.should_receive(asso).and_return(association_mock) + association_mock.should_receive(:delete) + bob.should_receive(asso).and_return([association_mock]) end - @account_deletion.delete_standard_associations + @account_deletion.delete_standard_user_associations end end - describe '#delete_posts' do - it 'deletes all posts' do - @account_deletion.person.posts.should_receive(:destroy_all) - @account_deletion.delete_posts + describe "#delete_standard_person_associations" do + before do + @account_deletion.person = bob.person end - end + it 'removes all standard person associaltions' do + @account_deletion.normal_ar_person_associates_to_delete.each do |asso| + association_mock = mock + association_mock.should_receive(:delete_all) + bob.person.should_receive(asso).and_return(association_mock) + end - describe '#delete_photos' do - it 'deletes all photos' do - @account_deletion.person.photos.should_receive(:destroy_all) - @account_deletion.delete_photos + @account_deletion.delete_standard_person_associations end end @@ -75,15 +106,6 @@ describe AccountDeletion do end context 'person associations' do - describe '#delete mentions' do - it 'deletes the mentions for people' do - mentions = mock - @account_deletion.person.should_receive(:mentions).and_return(mentions) - mentions.should_receive(:destroy_all) - @account_deletion.delete_mentions - end - end - describe '#disconnect_contacts' do it "deletes all of user's contacts" do bob.contacts.should_receive(:destroy_all) @@ -116,29 +138,23 @@ describe AccountDeletion do end end - describe "#remove_share_visibilities" do - before do - @s_vis = stub - end - - after do - @account_deletion.remove_share_visibilities - end - + describe "#remove_person_share_visibilities" do it 'removes the share visibilities for a person ' do + @s_vis = stub ShareVisibility.should_receive(:for_contacts_of_a_person).with(bob.person).and_return(@s_vis) @s_vis.should_receive(:destroy_all) - end + @account_deletion.remove_share_visibilities_on_persons_posts + end + end + + describe "#remove_share_visibilities_by_contacts_of_user" do it 'removes the share visibilities for a user' do + @s_vis = stub ShareVisibility.should_receive(:for_a_users_contacts).with(bob).and_return(@s_vis) @s_vis.should_receive(:destroy_all) - end - it 'does not remove share visibilities for a user if the user is not present' do - pending - ShareVisibility.should_receive(:for_a_users_contacts).with(bob).and_return(@s_vis) - @s_vis.should_receive(:destroy_all) + @account_deletion.remove_share_visibilities_on_contacts_posts end end @@ -153,5 +169,10 @@ describe AccountDeletion do all_keys = (@account_deletion.normal_ar_user_associates_to_delete + @account_deletion.special_ar_user_associations + @account_deletion.ignored_ar_user_associations) all_keys.sort{|x, y| x.to_s <=> y.to_s}.should == User.reflections.keys.sort{|x, y| x.to_s <=> y.to_s} end + + it 'has all person association keys accounted for' do + all_keys = (@account_deletion.normal_ar_person_associates_to_delete + @account_deletion.ignored_or_special_ar_person_associations) + all_keys.sort{|x, y| x.to_s <=> y.to_s}.should == Person.reflections.keys.sort{|x, y| x.to_s <=> y.to_s} + end end diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 632969382..d93b17188 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -44,6 +44,15 @@ describe Contact do contact.person = person contact.should_not be_valid end + + it "validates that the person's account is not closed" do + person = Factory.create(:person, :closed_account => true) + + contact = alice.contacts.new(:person=>person) + + contact.should_not be_valid + contact.errors.full_messages.should include "Cannot be in contact with a closed account" + end end context 'scope' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a2ef86505..3a417e472 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1010,6 +1010,9 @@ describe User do end describe "#close_account!" do + before do + @user = Factory.create(:user) + end it 'resets the password to a random string' do random_pass = "12345678909876543210" ActiveSupport::SecureRandom.should_receive(:hex).and_return(random_pass) @@ -1018,9 +1021,11 @@ describe User do end it 'clears all the clearable fields' do + @user.reload attributes = @user.send(:clearable_fields) @user.close_account! + @user.reload attributes.each do |attr| @user.send(attr.to_sym).should be_blank end @@ -1028,8 +1033,8 @@ describe User do end describe "#clearable_attributes" do - it 'has all the attributes' do - user = Factory.build :user + it 'returns the clearable fields' do + user = Factory.create :user user.send(:clearable_fields).sort.should == %w{ serialized_private_key getting_started diff --git a/spec/shared_behaviors/account_deletion.rb b/spec/shared_behaviors/account_deletion.rb new file mode 100644 index 000000000..d685432e6 --- /dev/null +++ b/spec/shared_behaviors/account_deletion.rb @@ -0,0 +1,42 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +require 'spec_helper' + +describe 'deleteing your account' do + shared_examples_for 'it removes the person associations' do + it "removes all of the person's posts" do + Post.where(:author_id => @person.id).count.should == 0 + end + + it 'deletes all person contacts' do + Contact.where(:person_id => @person.id).should be_empty + end + + it 'deletes all mentions' do + @person.mentions.should be_empty + end + + it "removes all of the person's photos" do + Photo.where(:author_id => @person.id).should be_empty + end + + it 'sets the person object as closed and the profile is cleared' do + @person.reload.closed_account.should be_true + + @person.profile.reload.first_name.should be_blank + @person.profile.reload.last_name.should be_blank + end + + it 'deletes only the converersation visibility for the deleted user' do + ConversationVisibility.where(:person_id => alice.person.id).should_not be_empty + ConversationVisibility.where(:person_id => @person.id).should be_empty + end + + it "deletes the share visibilities on the person's posts" do + ShareVisibility.for_contacts_of_a_person(@person).should be_empty + end + end +end + From 7667029e71b25eb9490cef22bca97c48494569be Mon Sep 17 00:00:00 2001 From: Ilya Zhitomirskiy Date: Tue, 8 Nov 2011 18:51:54 -0800 Subject: [PATCH 07/17] wip, stated the serialized model --- app/models/account_deletion.rb | 7 +++++++ .../20111109023618_create_account_deletions.rb | 12 ++++++++++++ db/schema.rb | 7 ++++++- spec/models/account_deletion_spec.rb | 14 ++++++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 app/models/account_deletion.rb create mode 100644 db/migrate/20111109023618_create_account_deletions.rb create mode 100644 spec/models/account_deletion_spec.rb diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb new file mode 100644 index 000000000..f62c83a72 --- /dev/null +++ b/app/models/account_deletion.rb @@ -0,0 +1,7 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +class AccountDeletion < ActiveRecord::Base + +end diff --git a/db/migrate/20111109023618_create_account_deletions.rb b/db/migrate/20111109023618_create_account_deletions.rb new file mode 100644 index 000000000..4785c1402 --- /dev/null +++ b/db/migrate/20111109023618_create_account_deletions.rb @@ -0,0 +1,12 @@ +class CreateAccountDeletions < ActiveRecord::Migration + def self.up + create_table :account_deletions do |t| + t.string :diaspora_id + t.integer :person_id + end + end + + def self.down + drop_table :account_deletions + end +end diff --git a/db/schema.rb b/db/schema.rb index 7c2d8a731..1d3af3eaf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,12 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20111103184050) do +ActiveRecord::Schema.define(:version => 20111109023618) do + + create_table "account_deletion", :force => true do |t| + t.string "diaspora_id" + t.integer "person_id" + end create_table "aspect_memberships", :force => true do |t| t.integer "aspect_id", :null => false diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb new file mode 100644 index 000000000..f5fb204f5 --- /dev/null +++ b/spec/models/account_deletion_spec.rb @@ -0,0 +1,14 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +require 'spec_helper' + +describe AccountDeletion do + + it 'gets initialized with diaspora_id' do + AccountDeletion.new(:diaspora_id => alice.person.diaspora_handle).should be_true + end + + +end From 0bd101dca9cfc26c617878d9d8032e50d63daff3 Mon Sep 17 00:00:00 2001 From: Ilya Zhitomirskiy Date: Thu, 10 Nov 2011 10:57:48 -0800 Subject: [PATCH 08/17] change close account to clear profile, still need to dispatch account deletion xml --- app/controllers/users_controller.rb | 2 +- app/models/account_deleter.rb | 9 +-- app/models/account_deletion.rb | 39 ++++++++++ app/models/jobs/delete_account.rb | 6 +- app/models/person.rb | 11 ++- app/models/status_message.rb | 4 ++ app/models/user.rb | 8 ++- ...20111109023618_create_account_deletions.rb | 2 +- db/schema.rb | 4 +- spec/controllers/users_controller_spec.rb | 10 +-- spec/models/account_deleter_spec.rb | 11 ++- spec/models/account_deletion_spec.rb | 72 +++++++++++++++++-- spec/models/jobs/delete_account_spec.rb | 25 ++----- spec/models/person_spec.rb | 23 ++++-- spec/models/status_message_spec.rb | 9 +++ spec/models/user_spec.rb | 24 +++++-- 16 files changed, 204 insertions(+), 55 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5777c8e33..62c91f76b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -82,7 +82,7 @@ class UsersController < ApplicationController def destroy if params[:user] && params[:user][:current_password] && current_user.valid_password?(params[:user][:current_password]) Resque.enqueue(Jobs::DeleteAccount, current_user.id) - current_user.lock_access! + current_user.close_account! sign_out current_user flash[:notice] = I18n.t 'users.destroy.success' redirect_to multi_path diff --git a/app/models/account_deleter.rb b/app/models/account_deleter.rb index d28043d82..9c14ed68f 100644 --- a/app/models/account_deleter.rb +++ b/app/models/account_deleter.rb @@ -17,8 +17,8 @@ class AccountDeleter attr_accessor :person, :user - def initialize(diaspora_id) - self.person = Person.where(:diaspora_handle => diaspora_id).first + def initialize(diaspora_handle) + self.person = Person.where(:diaspora_handle => diaspora_handle).first self.user = self.person.owner end @@ -90,11 +90,12 @@ class AccountDeleter end def tombstone_person_and_profile - self.person.close_account! + self.person.lock_access! + self.person.clear_profile! end def tombstone_user - self.user.close_account! + self.user.clear_account! end def delete_contacts_of_me diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb index f62c83a72..77918103b 100644 --- a/app/models/account_deletion.rb +++ b/app/models/account_deletion.rb @@ -3,5 +3,44 @@ # the COPYRIGHT file. class AccountDeletion < ActiveRecord::Base + include ROXML + include Diaspora::Webhooks + + belongs_to :person + after_create :queue_delete_account + + attr_accessible :person + + xml_attr :diaspora_handle + + + def person=(person) + self[:diaspora_handle] = person.diaspora_handle + self[:person_id] = person.id + end + + def diaspora_handle=(diaspora_handle) + self[:diaspora_handle] = diaspora_handle + self[:person_id] ||= Person.find_by_diaspora_handle(diaspora_handle).id + end + + + + def queue_delete_account + Resque.enqueue(Jobs::DeleteAccount, self.id) + end + + def perform! + AccountDeleter.new(self.diaspora_handle).perform! + self.dispatch if person.local? + end + + def subscribers(user) + person.owner.contact_people | Person.who_have_reshared_a_users_posts(person.owner) + end + + def dispatch + Postzord::Dispatcher.build(person.owner, self).post + end end diff --git a/app/models/jobs/delete_account.rb b/app/models/jobs/delete_account.rb index acc813a7d..0d856dda4 100644 --- a/app/models/jobs/delete_account.rb +++ b/app/models/jobs/delete_account.rb @@ -6,9 +6,9 @@ module Jobs class DeleteAccount < Base @queue = :delete_account - def self.perform(user_id) - user = User.find(user_id) - AccountDeleter.new(user.person.diaspora_handle).perform! + def self.perform(account_deletion_id) + account_deletion = AccountDeletion.find(account_deletion_id) + account_deletion.perform! end end end diff --git a/app/models/person.rb b/app/models/person.rb index 4db5da134..9899f8528 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -61,6 +61,10 @@ class Person < ActiveRecord::Base scope :profile_tagged_with, lambda{|tag_name| joins(:profile => :tags).where(:profile => {:tags => {:name => tag_name}}).where('profiles.searchable IS TRUE') } + scope :who_have_reshared_a_users_posts, lambda{|user| + joins(:posts).where(:posts => {:root_guid => StatusMessage.guids_for_author(user.person), :type => 'Reshare'} ) + } + def self.community_spotlight AppConfig[:community_spotlight].present? ? Person.where(:diaspora_handle => AppConfig[:community_spotlight]) : [] end @@ -285,10 +289,13 @@ class Person < ActiveRecord::Base self.update_attributes(:url => newuri) end - def close_account! - self.profile.tombstone! + def lock_access! self.closed_account = true self.save + end + + def clear_profile! + self.profile.tombstone! self end diff --git a/app/models/status_message.rb b/app/models/status_message.rb index af7250069..dd32b6cc0 100644 --- a/app/models/status_message.rb +++ b/app/models/status_message.rb @@ -42,6 +42,10 @@ class StatusMessage < Post joins(:likes).where(:likes => {:author_id => person.id}) } + def self.guids_for_author(person) + Post.connection.select_values(Post.where(:author_id => person.id).select('posts.guid').to_sql) + end + def self.user_tag_stream(user, tag_ids) owned_or_visible_by_user(user). tag_stream(tag_ids) diff --git a/app/models/user.rb b/app/models/user.rb index 928a6a7cb..8a35d1cfa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -490,6 +490,12 @@ class User < ActiveRecord::Base end def close_account! + AccountDeletion.create(:person => self.person) + self.person.lock_access! + self.lock_access! + end + + def clear_account! clearable_fields.each do |field| self[field] = nil end @@ -502,6 +508,6 @@ class User < ActiveRecord::Base private def clearable_fields - self.attributes.keys - ["id", "username", "encrypted_password", "created_at", "updated_at"] + self.attributes.keys - ["id", "username", "encrypted_password", "created_at", "updated_at", "locked_at"] end end diff --git a/db/migrate/20111109023618_create_account_deletions.rb b/db/migrate/20111109023618_create_account_deletions.rb index 4785c1402..6e2d1f9da 100644 --- a/db/migrate/20111109023618_create_account_deletions.rb +++ b/db/migrate/20111109023618_create_account_deletions.rb @@ -1,7 +1,7 @@ class CreateAccountDeletions < ActiveRecord::Migration def self.up create_table :account_deletions do |t| - t.string :diaspora_id + t.string :diaspora_handle t.integer :person_id end end diff --git a/db/schema.rb b/db/schema.rb index 1d3af3eaf..67b994b0e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,8 +12,8 @@ ActiveRecord::Schema.define(:version => 20111109023618) do - create_table "account_deletion", :force => true do |t| - t.string "diaspora_id" + create_table "account_deletions", :force => true do |t| + t.string "diaspora_handle" t.integer "person_id" end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 568fdcff7..1f47f3c70 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -10,6 +10,7 @@ describe UsersController do @aspect = @user.aspects.first @aspect1 = @user.aspects.create(:name => "super!!") sign_in :user, @user + @controller.stub(:current_user).and_return(@user) end describe '#export' do @@ -192,15 +193,16 @@ describe UsersController do delete :destroy, :user => { :current_password => "stuff" } end + it 'closes the account' do + alice.should_receive(:close_account!) + delete :destroy, :user => { :current_password => "bluepin7" } + end + it 'enqueues a delete job' do Resque.should_receive(:enqueue).with(Jobs::DeleteAccount, alice.id) delete :destroy, :user => { :current_password => "bluepin7" } end - it 'locks the user out' do - delete :destroy, :user => { :current_password => "bluepin7" } - alice.reload.access_locked?.should be_true - end end describe '#confirm_email' do diff --git a/spec/models/account_deleter_spec.rb b/spec/models/account_deleter_spec.rb index 38fcdd345..d8137fa97 100644 --- a/spec/models/account_deleter_spec.rb +++ b/spec/models/account_deleter_spec.rb @@ -123,8 +123,13 @@ describe AccountDeleter do end describe '#tombstone_person_and_profile' do - it 'calls close_account! on person' do - @account_deletion.person.should_receive(:close_account!) + it 'calls clear_profile! on person' do + @account_deletion.person.should_receive(:clear_profile!) + @account_deletion.tombstone_person_and_profile + end + + it 'calls lock_access! on person' do + @account_deletion.person.should_receive(:lock_access!) @account_deletion.tombstone_person_and_profile end end @@ -160,7 +165,7 @@ describe AccountDeleter do describe "#tombstone_user" do it 'calls strip_model on user' do - bob.should_receive(:close_account!) + bob.should_receive(:clear_account!) @account_deletion.tombstone_user end end diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index f5fb204f5..50f02ec15 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -5,10 +5,74 @@ require 'spec_helper' describe AccountDeletion do - - it 'gets initialized with diaspora_id' do - AccountDeletion.new(:diaspora_id => alice.person.diaspora_handle).should be_true + it 'assigns the diaspora_handle from the person object' do + a = AccountDeletion.new(:person => alice.person) + a.diaspora_handle.should == alice.person.diaspora_handle end - + it 'fires a resque job after creation'do + Resque.should_receive(:enqueue).with(Jobs::DeleteAccount, anything) + + AccountDeletion.create(:person => alice.person) + end + + describe "#perform!" do + before do + @ad = AccountDeletion.new(:person => alice.person) + end + + it 'creates a deleter' do + AccountDeleter.should_receive(:new).with(alice.person.diaspora_handle).and_return(stub(:perform! => true)) + @ad.perform! + end + + it 'dispatches the account deletion if the user exists' do + @ad.should_receive(:dispatch) + @ad.perform! + end + + it 'does not dispatch an account deletion for non-local people' do + deletion = AccountDeletion.new(:person => remote_raphael) + deletion.should_not_receive(:dispatch) + deletion.perform! + end + end + + describe '#dispatch' do + it "sends the account deletion xml" do + @ad = AccountDeletion.new(:person => alice.person) + @ad.send(:dispatch) + + end + end + + describe "#subscribers" do + it 'includes all contacts' do + @ad = AccountDeletion.new(:person => alice.person) + @ad.person.owner.should_receive(:contact_people).and_return([remote_raphael]) + @ad.subscribers(alice).should include(remote_raphael) + end + + it 'includes resharers' do + @ad = AccountDeletion.new(:person => alice.person) + p = Factory(:person) + Person.should_receive(:who_have_reshared_a_users_posts).with(alice).and_return([p]) + @ad.subscribers(alice).should include(p) + end + end + + describe 'serialization' do + before do + account_deletion = AccountDeletion.new(:person => alice.person) + @xml = account_deletion.to_xml.to_s + end + + it 'should have a diaspora_handle' do + @xml.include?(alice.person.diaspora_handle).should == true + end + + it 'marshals the xml' do + AccountDeletion.from_xml(@xml).should be_valid + end + end end diff --git a/spec/models/jobs/delete_account_spec.rb b/spec/models/jobs/delete_account_spec.rb index 0bf04b5c1..507f0d2b9 100644 --- a/spec/models/jobs/delete_account_spec.rb +++ b/spec/models/jobs/delete_account_spec.rb @@ -6,25 +6,12 @@ require 'spec_helper' describe Jobs::DeleteAccount do describe '#perform' do - it 'calls remove_all_traces' do - stub_find_for(bob) - bob.should_receive(:remove_all_traces) - Jobs::DeleteAccount.perform(bob.id) - end - - it 'calls destroy' do - stub_find_for(bob) - bob.should_receive(:destroy) - Jobs::DeleteAccount.perform(bob.id) - end - def stub_find_for model - model.class.stub!(:find) do |id, conditions| - if id == model.id - model - else - model.class.find_by_id(id) - end - end + it 'performs the account deletion' do + account_deletion = stub + AccountDeletion.stub(:find).and_return(account_deletion) + account_deletion.should_receive(:perform!) + + Jobs::DeleteAccount.perform(1) end end end diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index ae1111aef..f37f80c51 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -91,6 +91,14 @@ describe Person do Person.all_from_aspects(aspect_ids, bob).map(&:id).should == [] end end + + describe ".who_have_reshared a user's posts" do + it 'pulls back users who reshared the status message of a user' do + sm = Factory.create(:status_message, :author => alice.person, :public => true) + reshare = Factory.create(:reshare, :root => sm) + Person.who_have_reshared_a_users_posts(alice).should == [reshare.author] + end + end end describe "delegating" do @@ -537,18 +545,21 @@ describe Person do end end - describe "#close_account!" do + describe '#lock_access!' do + it 'sets the closed_account flag' do + @person.lock_access! + @person.reload.closed_account.should be_true + end + end + + describe "#clear_profile!!" do before do @person = Factory(:person) end - it 'sets the closed_account flag' do - @person.close_account! - @person.reload.closed_account.should be_true - end it 'calls Profile#tombstone!' do @person.profile.should_receive(:tombstone!) - @person.close_account! + @person.clear_profile! end end end diff --git a/spec/models/status_message_spec.rb b/spec/models/status_message_spec.rb index 9c3450249..8202c021b 100644 --- a/spec/models/status_message_spec.rb +++ b/spec/models/status_message_spec.rb @@ -66,6 +66,15 @@ describe StatusMessage do end end + describe ".guids_for_author" do + it 'returns an array of the status_message guids' do + sm1 = Factory(:status_message, :author => alice.person) + sm2 = Factory(:status_message, :author => bob.person) + guids = StatusMessage.guids_for_author(alice.person) + guids.should == [sm1.guid] + end + end + describe '.before_create' do it 'calls build_tags' do status = Factory.build(:status_message) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3a417e472..43f642501 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1010,20 +1010,35 @@ describe User do end describe "#close_account!" do - before do - @user = Factory.create(:user) + it 'locks the user out' do + @user.close_account! + @user.reload.access_locked?.should be_true end + + it 'creates an account deletion' do + expect{ + @user.close_account! + }.to change(AccountDeletion, :count).by(1) + end + + it 'calls person#lock_access!' do + @user.person.should_receive(:lock_access!) + @user.close_account! + end + end + + describe "#clear_account!" do it 'resets the password to a random string' do random_pass = "12345678909876543210" ActiveSupport::SecureRandom.should_receive(:hex).and_return(random_pass) - @user.close_account! + @user.clear_account! @user.valid_password?(random_pass) end it 'clears all the clearable fields' do @user.reload attributes = @user.send(:clearable_fields) - @user.close_account! + @user.clear_account! @user.reload attributes.each do |attr| @@ -1059,7 +1074,6 @@ describe User do authentication_token unconfirmed_email confirm_email_token - locked_at show_community_spotlight_in_stream }.sort end From f800d50a2b80809f4a3a21c5ba2f74554a2a9c4f Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Thu, 10 Nov 2011 16:14:37 -0800 Subject: [PATCH 09/17] i think this works --- app/controllers/users_controller.rb | 1 - app/models/account_deletion.rb | 7 +++++-- app/models/user.rb | 2 +- lib/postzord/receiver/public.rb | 2 ++ spec/models/account_deletion_spec.rb | 22 +++++++++++++++------- 5 files changed, 23 insertions(+), 11 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 62c91f76b..4f47ee730 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -81,7 +81,6 @@ class UsersController < ApplicationController def destroy if params[:user] && params[:user][:current_password] && current_user.valid_password?(params[:user][:current_password]) - Resque.enqueue(Jobs::DeleteAccount, current_user.id) current_user.close_account! sign_out current_user flash[:notice] = I18n.t 'users.destroy.success' diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb index 77918103b..5e71fd0c1 100644 --- a/app/models/account_deletion.rb +++ b/app/models/account_deletion.rb @@ -14,7 +14,6 @@ class AccountDeletion < ActiveRecord::Base xml_attr :diaspora_handle - def person=(person) self[:diaspora_handle] = person.diaspora_handle self[:person_id] = person.id @@ -37,10 +36,14 @@ class AccountDeletion < ActiveRecord::Base end def subscribers(user) - person.owner.contact_people | Person.who_have_reshared_a_users_posts(person.owner) + person.owner.contact_people.remote | Person.who_have_reshared_a_users_posts(person.owner).remote end def dispatch Postzord::Dispatcher.build(person.owner, self).post end + + def public? + true + end end diff --git a/app/models/user.rb b/app/models/user.rb index 8a35d1cfa..61803e285 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -490,9 +490,9 @@ class User < ActiveRecord::Base end def close_account! - AccountDeletion.create(:person => self.person) self.person.lock_access! self.lock_access! + AccountDeletion.create(:person => self.person) end def clear_account! diff --git a/lib/postzord/receiver/public.rb b/lib/postzord/receiver/public.rb index 277de7bb1..cd93179a2 100644 --- a/lib/postzord/receiver/public.rb +++ b/lib/postzord/receiver/public.rb @@ -23,6 +23,8 @@ class Postzord::Receiver::Public < Postzord::Receiver if @object.respond_to?(:relayable?) receive_relayable + elsif @object.is_a?(AccountDeletion) + #nothing else Resque.enqueue(Jobs::ReceiveLocalBatch, @object.class.to_s, @object.id, self.recipient_user_ids) true diff --git a/spec/models/account_deletion_spec.rb b/spec/models/account_deletion_spec.rb index 50f02ec15..5bd96f79f 100644 --- a/spec/models/account_deletion_spec.rb +++ b/spec/models/account_deletion_spec.rb @@ -42,22 +42,30 @@ describe AccountDeletion do it "sends the account deletion xml" do @ad = AccountDeletion.new(:person => alice.person) @ad.send(:dispatch) + end + it 'creates a public postzord' do + Postzord::Dispatcher::Public.should_receive(:new).and_return(stub.as_null_object) + @ad = AccountDeletion.new(:person => alice.person) + @ad.send(:dispatch) end end describe "#subscribers" do - it 'includes all contacts' do + it 'includes all remote contacts' do @ad = AccountDeletion.new(:person => alice.person) - @ad.person.owner.should_receive(:contact_people).and_return([remote_raphael]) - @ad.subscribers(alice).should include(remote_raphael) + alice.share_with(remote_raphael, alice.aspects.first) + + @ad.subscribers(alice).should == [remote_raphael] end - it 'includes resharers' do + it 'includes remote resharers' do @ad = AccountDeletion.new(:person => alice.person) - p = Factory(:person) - Person.should_receive(:who_have_reshared_a_users_posts).with(alice).and_return([p]) - @ad.subscribers(alice).should include(p) + sm = Factory( :status_message, :public => true, :author => alice.person) + r1 = Factory( :reshare, :author => remote_raphael, :root => sm) + r2 = Factory( :reshare, :author => local_luke.person, :root => sm) + + @ad.subscribers(alice).should == [remote_raphael] end end From 1552b5bcc2675f94e3c01800fc1c4a27b59b407d Mon Sep 17 00:00:00 2001 From: Ilya Zhitomirskiy Date: Thu, 10 Nov 2011 17:07:14 -0800 Subject: [PATCH 10/17] keep the pivate key, required for retries --- app/models/user.rb | 2 +- spec/models/user_spec.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 61803e285..303d86d6e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -508,6 +508,6 @@ class User < ActiveRecord::Base private def clearable_fields - self.attributes.keys - ["id", "username", "encrypted_password", "created_at", "updated_at", "locked_at"] + self.attributes.keys - ["id", "username", "encrypted_password", "created_at", "updated_at", "locked_at", "serialized_private_key"] end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 43f642501..ec0df5cba 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1051,7 +1051,6 @@ describe User do it 'returns the clearable fields' do user = Factory.create :user user.send(:clearable_fields).sort.should == %w{ - serialized_private_key getting_started disable_mail language From e57698bbe857fa2b824e9ac4d1215bf406ceee56 Mon Sep 17 00:00:00 2001 From: Ilya Zhitomirskiy Date: Thu, 10 Nov 2011 17:27:53 -0800 Subject: [PATCH 11/17] setting the account deletion name in the xml --- app/models/account_deletion.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb index 5e71fd0c1..961aa7f58 100644 --- a/app/models/account_deletion.rb +++ b/app/models/account_deletion.rb @@ -12,7 +12,8 @@ class AccountDeletion < ActiveRecord::Base attr_accessible :person - xml_attr :diaspora_handle + xml_name :account_deletion + xml_attr :diaspora_handle def person=(person) self[:diaspora_handle] = person.diaspora_handle @@ -31,8 +32,8 @@ class AccountDeletion < ActiveRecord::Base end def perform! - AccountDeleter.new(self.diaspora_handle).perform! self.dispatch if person.local? + #AccountDeleter.new(self.diaspora_handle).perform! end def subscribers(user) From ee12f19ff6762fbb19a77a7eaadf52a6f74f4a3f Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Fri, 11 Nov 2011 10:45:01 -0800 Subject: [PATCH 12/17] dispatch before deleting --- app/models/account_deletion.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/account_deletion.rb b/app/models/account_deletion.rb index 961aa7f58..9bd2a9e3b 100644 --- a/app/models/account_deletion.rb +++ b/app/models/account_deletion.rb @@ -33,7 +33,7 @@ class AccountDeletion < ActiveRecord::Base def perform! self.dispatch if person.local? - #AccountDeleter.new(self.diaspora_handle).perform! + AccountDeleter.new(self.diaspora_handle).perform! end def subscribers(user) From 90458381f50607a9fc1b5d8b4687952a631b102a Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Fri, 11 Nov 2011 12:12:49 -0800 Subject: [PATCH 13/17] basic page for deleting account with temporary text and cute cat picture --- app/controllers/users_controller.rb | 3 +- app/views/users/edit.html.haml | 45 ++++++++++++++++++++---- config/locales/diaspora/en.yml | 1 + public/stylesheets/sass/application.sass | 3 ++ 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4f47ee730..aea10ee1d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -83,8 +83,7 @@ class UsersController < ApplicationController if params[:user] && params[:user][:current_password] && current_user.valid_password?(params[:user][:current_password]) current_user.close_account! sign_out current_user - flash[:notice] = I18n.t 'users.destroy.success' - redirect_to multi_path + redirect_to(multi_path, :notice => I18n.t('users.destroy.success')) else if params[:user].present? && params[:user][:current_password].present? flash[:error] = t 'users.destroy.wrong_password' diff --git a/app/views/users/edit.html.haml b/app/views/users/edit.html.haml index d0e6ac949..751fdcea9 100644 --- a/app/views/users/edit.html.haml +++ b/app/views/users/edit.html.haml @@ -166,11 +166,42 @@ .span-5.last %h3 = t('.close_account') - = form_for 'user', :url => user_path, :html => { :method => :delete } do |f| - = f.error_messages + .button + =link_to 'Close Account', '#close_account_pane', :rel => 'facebox' - %p - = f.label :close_account_password, t('.current_password'), :for => :close_account_password - = f.password_field :current_password, :id => :close_account_password - %p - = f.submit t('.close_account'), :confirm => t('are_you_sure') + .hidden#close_account_pane{:rel => 'facebox'} + #inner_account_delete + %h1 + Hey, please don't go! + %p + We want you to help us make Diaspora better, so you should help us out instead of leaving. + if you do want to leave, we want you to know what happens next. + .span-10 + = image_tag 'http://itstrulyrandom.com/wp-content/uploads/2008/03/sadcat.jpg' + %br + %small + %b + Mr Wiggles will be sad to see you go + .span-10.last + %ul + %li + We delete all of your posts, profile data, as soon as humanly possible. + Your comments will hang around, but be associated with your Diaspora Handle. + %li + You will get signed out and locked out of your account. + %li + This will lock your username if you decided to sign back up. + %li + Currently, there is no turning back + %p + %b + If you really want this, type in your password below and click 'Close Account' + + = form_for 'user', :url => user_path, :html => { :method => :delete } do |f| + = f.error_messages + + %p + = f.label :close_account_password, t('.current_password'), :for => :close_account_password + = f.password_field :current_password, :id => :close_account_password + %p + = f.submit t('.close_account'), :confirm => t('are_you_sure_delete_account') diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 1d6871384..21624ef2e 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -27,6 +27,7 @@ en: password: "Password" password_confirmation: "Password confirmation" are_you_sure: "Are you sure?" + are_you_sure_delete_account: "Are you sure you want to close your account? This can't be undone!" fill_me_out: "Fill me out" back: "Back" public: "Public" diff --git a/public/stylesheets/sass/application.sass b/public/stylesheets/sass/application.sass index 2910d0af6..43b1767f0 100644 --- a/public/stylesheets/sass/application.sass +++ b/public/stylesheets/sass/application.sass @@ -2281,6 +2281,9 @@ ul.show_comments, :position relative :top 10px +#inner_account_delete + :width 810px + #aspect_edit_pane :width 810px .person_tiles From 70dcc5beccdb02cf62c9f7dd26161b56a388043e Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Tue, 6 Dec 2011 16:30:56 -0800 Subject: [PATCH 14/17] rebased this branch onto master --- spec/models/user_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ec0df5cba..f18eeb66a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -907,6 +907,7 @@ describe User do fantasy_resque do @invitation = Factory.create(:invitation, :sender => eve, :identifier => 'invitee@example.org', :aspect => eve.aspects.first) end + @invitation.reload @form_params = { :invitation_token => "abc", @@ -1003,6 +1004,8 @@ describe User do user = Factory :user Resque.should_receive(:enqueue).with(Jobs::ResetPassword, user.id) user.send_reset_password_instructions + end + end context "close account" do before do @@ -1078,3 +1081,4 @@ describe User do end end end +end From 7bf1970dd8092a29081474a3e23b68fbcadd3038 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Tue, 6 Dec 2011 16:58:00 -0800 Subject: [PATCH 15/17] adding account deletion factory, fixed a user spec --- spec/factories.rb | 7 +++++++ spec/models/user_spec.rb | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/spec/factories.rb b/spec/factories.rb index d429b97fa..69a563615 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -37,6 +37,13 @@ Factory.define :person do |p| end end +Factory.define :account_deletion do |d| + d.association :person + d.after_build do |delete| + delete.diaspora_handle= delete.person.diaspora_handle + end +end + Factory.define :searchable_person, :parent => :person do |p| p.after_build do |person| person.profile = Factory.build(:profile, :person => person, :searchable => true) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f18eeb66a..a10ca23cf 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -625,7 +625,8 @@ describe User do describe '#disconnect_everyone' do it 'has no error on a local friend who has deleted his account' do - Jobs::DeleteAccount.perform(alice.id) + d = Factory(:account_deletion, :person => alice.person) + Jobs::DeleteAccount.perform(d.id) lambda { bob.disconnect_everyone }.should_not raise_error From 342593988b8a080ae1b77fb96ee07649efe0991e Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Wed, 7 Dec 2011 17:15:46 -0800 Subject: [PATCH 16/17] MS DG; fixed close account cukes --- app/views/users/edit.html.haml | 5 ++--- features/closes_account.feature | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/views/users/edit.html.haml b/app/views/users/edit.html.haml index 751fdcea9..3175c1275 100644 --- a/app/views/users/edit.html.haml +++ b/app/views/users/edit.html.haml @@ -164,10 +164,9 @@ = link_to t('.download_photos'), "#", :class => "button", :id => "photo-export-button", :title => t('.photo_export_unavailable') .span-5.last - %h3 + %h3 = t('.close_account') - .button - =link_to 'Close Account', '#close_account_pane', :rel => 'facebox' + =link_to 'Close Account', '#close_account_pane', :rel => 'facebox', :class => "button" .hidden#close_account_pane{:rel => 'facebox'} #inner_account_delete diff --git a/features/closes_account.feature b/features/closes_account.feature index 6f6b0b24d..11242455a 100644 --- a/features/closes_account.feature +++ b/features/closes_account.feature @@ -8,22 +8,24 @@ Feature: Close Account Given I am signed in When I click on my name in the header And I follow "Settings" - And I put in my password in "close_account_password" + And I follow "Close Account" + And I put in my password in "close_account_password" in the modal window And I preemptively confirm the alert - And I press "Close Account" + And I press "Close Account" in the modal window Then I should be on the new user session page When I try to sign in manually Then I should be on the new user session page When I wait for the ajax to finish - Then I should see "Invalid email or password." + Then I should see "Your account is locked." Scenario: user is forced to enter something in the password field on closing account Given I am signed in When I click on my name in the header And I follow "Settings" + And I follow "Close Account" And I preemptively confirm the alert - And I press "Close Account" + And I press "Close Account" in the modal window Then I should be on the edit user page And I should see "Please enter your current password to close your account." @@ -31,9 +33,10 @@ Feature: Close Account Given I am signed in When I click on my name in the header And I follow "Settings" + And I follow "Close Account" And I preemptively confirm the alert - And I fill in "close_account_password" with "none sense" - And I press "Close Account" + And I fill in "close_account_password" with "none sense" in the modal window + And I press "Close Account" in the modal window Then I should be on the edit user page And I should see "The entered password didn't match your current password." @@ -50,9 +53,10 @@ Feature: Close Account Then I sign in as "bob@bob.bob" When I click on my name in the header And I follow "Settings" - And I put in my password in "close_account_password" + And I follow "Close Account" + And I put in my password in "close_account_password" in the modal window And I preemptively confirm the alert - And I press "Close Account" + And I press "Close Account" in the modal window Then I sign in as "alice@alice.alice" And I am on the home page Then I should see "Hi, Bob Jones long time no see" From c0ac664d640889e7a95ea4809553a1fe90ce3a39 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Thu, 8 Dec 2011 15:47:32 -0800 Subject: [PATCH 17/17] translated close account --- app/views/users/edit.html.haml | 23 +++++++++++------------ config/locales/diaspora/en.yml | 12 +++++++++++- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/app/views/users/edit.html.haml b/app/views/users/edit.html.haml index 3175c1275..6f92f1ce7 100644 --- a/app/views/users/edit.html.haml +++ b/app/views/users/edit.html.haml @@ -165,36 +165,35 @@ .span-5.last %h3 - = t('.close_account') + = t('.close_account_text') =link_to 'Close Account', '#close_account_pane', :rel => 'facebox', :class => "button" .hidden#close_account_pane{:rel => 'facebox'} #inner_account_delete %h1 - Hey, please don't go! + = t('.close_account.dont_go') %p - We want you to help us make Diaspora better, so you should help us out instead of leaving. - if you do want to leave, we want you to know what happens next. + = t('.close_account.make_diaspora_better') .span-10 = image_tag 'http://itstrulyrandom.com/wp-content/uploads/2008/03/sadcat.jpg' %br %small %b - Mr Wiggles will be sad to see you go + = t('.close_account.mr_wiggles') .span-10.last %ul %li - We delete all of your posts, profile data, as soon as humanly possible. - Your comments will hang around, but be associated with your Diaspora Handle. + = t('.close_account.what_we_delete') %li - You will get signed out and locked out of your account. + = t('.close_account.locked_out') %li - This will lock your username if you decided to sign back up. + = t('.close_account.lock_username') %li - Currently, there is no turning back + = t('.close_account.no_turning_back') %p %b - If you really want this, type in your password below and click 'Close Account' + = t('.close_account.no_turning_back') + = form_for 'user', :url => user_path, :html => { :method => :delete } do |f| = f.error_messages @@ -203,4 +202,4 @@ = f.label :close_account_password, t('.current_password'), :for => :close_account_password = f.password_field :current_password, :id => :close_account_password %p - = f.submit t('.close_account'), :confirm => t('are_you_sure_delete_account') + = f.submit t('.close_account_text'), :confirm => t('are_you_sure_delete_account') diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 21624ef2e..6bd15c7fd 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -932,7 +932,7 @@ en: edit: export_data: "Export Data" photo_export_unavailable: "Photo exporting currently unavailable" - close_account: "Close Account" + close_account_text: "Close Account" change_language: "Change language" change_password: "Change password" change_email: "Change email" @@ -958,6 +958,16 @@ en: show_getting_started: 'Re-enable Getting Started' getting_started: 'New User Prefrences' + close_account: + dont_go: "Hey, please don't go!" + make_diaspora_better: "We want you to help us make Diaspora better, so you should help us out instead of leaving. If you do want to leave, we want you to know what happens next." + mr_wiggles: 'Mr Wiggles will be sad to see you go' + what_we_delete: "We delete all of your posts, profile data, as soon as humanly possible. Your comments will hang around, but be associated with your Diaspora Handle." + locked_out: "You will get signed out and locked out of your account." + lock_username: "This will lock your username if you decided to sign back up." + no_turning_back: "Currently, there is no turning back." + if_you_want_this: "If you really want this, type in your password below and click 'Close Account'" + privacy_settings: title: "Privacy Settings" ignored_users: "Ignored Users"