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

@@ -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

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

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

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,18 +23,22 @@ 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
+    
+    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
   def normal_ar_user_associates_to_delete
@@ -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

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'

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

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

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"

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

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

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} ]
     }

spec/integration/account_deletion_spec.rb

@@ -1,9 +1,10 @@
 require 'spec_helper'
 
 describe 'deleteing your account' do
+  context "user" do
     before do
       @bob2 = bob
-    @bobs_person_id = @bob2.person.id
+      @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}
@@ -19,7 +20,7 @@ describe 'deleteing your account' do
       @bob2.comment("here are some thoughts on your post", :post => @alices_post)
 
       #conversations
-    create_conversation_with_message(alice, @bob2, "Subject", "Hey @bob2")
+      create_conversation_with_message(alice, @bob2.person, "Subject", "Hey @bob2")
 
       #join tables
       @users_sv = ShareVisibility.where(:contact_id => @bobs_contact_ids).all
@@ -49,14 +50,10 @@ describe 'deleteing your account' do
       #authorization
       @authorization = Factory.create(:oauth_authorization, :resource_owner => @bob2)
 
-    AccountDeletion.new(@bob2.person.diaspora_handle).perform!
+      AccountDeleter.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 the user's preferences" do
       UserPreference.where(:id => @prefs.map{|pref| pref.id}).should be_empty
     end
@@ -82,19 +79,11 @@ describe 'deleteing your account' 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
-
-  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
+    it 'deletes all user contacts' do
       @bob2.contacts.should be_empty
     end
 
@@ -102,14 +91,48 @@ describe 'deleteing your account' 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
+    it "clears the account fields" do 
+      @bob2.send(:clearable_fields).each do |field|
+        @bob2.reload[field].should be_blank
+      end
     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
+
+  context 'remote person' do
+    before do
+      @person = remote_raphael
+      
+      #contacts
+      @contacts = @person.contacts
+
+      #posts
+      @posts = (1..3).map do
+        Factory.create(:status_message, :author => @person)
+      end
+
+      @persons_sv = @posts.each do |post|
+        @contacts.each do |contact|
+          ShareVisibility.create!(:contact_id => contact.id, :shareable => post)
+        end
+      end
+
+      #photos
+      @photo = Factory(:photo, :author => @person)
+
+      #mentions
+      @mentions = 3.times do
+        Factory.create(:mention, :person => @person)
+      end
+
+      #conversations
+      create_conversation_with_message(alice, @person, "Subject", "Hey @bob2")
+
+      AccountDeleter.new(@person.diaspora_handle).perform!
+      @person.reload
+    end
+
+      it_should_behave_like 'it removes the person associations'
   end
 end

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"

spec/models/account_deleter_spec.rb

@@ -4,34 +4,38 @@
 
 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
+
+
+    user_removal_methods = [:delete_standard_user_associations,
+     :disassociate_invitations,
+     :remove_share_visibilities_on_contacts_posts,
+     :disconnect_contacts,
+     :tombstone_user]
+
+    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
 
-    [: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,
-     :tombstone_user].each do |method|
+      (user_removal_methods + person_removal_methods).each do |method|
 
         it "calls ##{method.to_s}" do
           @account_deletion.should_receive(method)
@@ -39,29 +43,56 @@ describe AccountDeletion do
       end
     end
 
-  describe "#delete_standard_associations" do
+    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_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
+    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)
+
+      @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
 

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

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

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
+