From e809d40f4ed988ecd98ecbc162553a5b180b3083 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 16 Jan 2011 14:27:02 +0000 Subject: [PATCH] When destroying a user, remove all references to that user (#7296). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4726 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/user.rb | 36 +++++- test/unit/repository_test.rb | 11 +- test/unit/user_test.rb | 208 ++++++++++++++++++++++++++++++++++- 3 files changed, 245 insertions(+), 10 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 7d0014c8d..9bfd83029 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -48,8 +48,8 @@ class User < Principal has_many :issue_categories, :foreign_key => 'assigned_to_id', :dependent => :nullify has_many :changesets, :dependent => :nullify has_one :preference, :dependent => :destroy, :class_name => 'UserPreference' - has_one :rss_token, :dependent => :destroy, :class_name => 'Token', :conditions => "action='feeds'" - has_one :api_token, :dependent => :destroy, :class_name => 'Token', :conditions => "action='api'" + has_one :rss_token, :class_name => 'Token', :conditions => "action='feeds'" + has_one :api_token, :class_name => 'Token', :conditions => "action='api'" belongs_to :auth_source # Active non-anonymous users scope @@ -74,6 +74,8 @@ class User < Principal validates_confirmation_of :password, :allow_nil => true validates_inclusion_of :mail_notification, :in => MAIL_NOTIFICATION_OPTIONS.collect(&:first), :allow_blank => true + before_destroy :remove_references_before_destroy + def before_create self.mail_notification = Setting.default_notification_option if self.mail_notification.blank? true @@ -473,6 +475,31 @@ class User < Principal end private + + # Removes references that are not handled by associations + # Things that are not deleted are reassociated with the anonymous user + def remove_references_before_destroy + return if self.id.nil? + + substitute = User.anonymous + Attachment.update_all ['author_id = ?', substitute.id], ['author_id = ?', id] + Comment.update_all ['author_id = ?', substitute.id], ['author_id = ?', id] + Issue.update_all ['author_id = ?', substitute.id], ['author_id = ?', id] + Issue.update_all 'assigned_to_id = NULL', ['assigned_to_id = ?', id] + Journal.update_all ['user_id = ?', substitute.id], ['user_id = ?', id] + JournalDetail.update_all ['old_value = ?', substitute.id.to_s], ["property = 'attr' AND prop_key = 'assigned_to_id' AND old_value = ?", id.to_s] + JournalDetail.update_all ['value = ?', substitute.id.to_s], ["property = 'attr' AND prop_key = 'assigned_to_id' AND value = ?", id.to_s] + Message.update_all ['author_id = ?', substitute.id], ['author_id = ?', id] + News.update_all ['author_id = ?', substitute.id], ['author_id = ?', id] + # Remove private queries and keep public ones + Query.delete_all ['user_id = ? AND is_public = ?', id, false] + Query.update_all ['user_id = ?', substitute.id], ['user_id = ?', id] + TimeEntry.update_all ['user_id = ?', substitute.id], ['user_id = ?', id] + Token.delete_all ['user_id = ?', id] + Watcher.delete_all ['user_id = ?', id] + WikiContent.update_all ['author_id = ?', substitute.id], ['author_id = ?', id] + WikiContent::Version.update_all ['author_id = ?', substitute.id], ['author_id = ?', id] + end # Return password digest def self.hash_password(clear_password) @@ -498,4 +525,9 @@ class AnonymousUser < User def mail; nil end def time_zone; nil end def rss_key; nil end + + # Anonymous user can not be destroyed + def destroy + false + end end diff --git a/test/unit/repository_test.rb b/test/unit/repository_test.rb index 252230b20..8c4146ff0 100644 --- a/test/unit/repository_test.rb +++ b/test/unit/repository_test.rb @@ -62,12 +62,11 @@ class RepositoryTest < ActiveSupport::TestCase def test_should_not_create_with_disabled_scm # disable Subversion - Setting.enabled_scm = ['Darcs', 'Git'] - repository = Repository::Subversion.new(:project => Project.find(3), :url => "svn://localhost") - assert !repository.save - assert_equal I18n.translate('activerecord.errors.messages.invalid'), repository.errors.on(:type) - # re-enable Subversion for following tests - Setting.delete_all + with_settings :enabled_scm => ['Darcs', 'Git'] do + repository = Repository::Subversion.new(:project => Project.find(3), :url => "svn://localhost") + assert !repository.save + assert_equal I18n.translate('activerecord.errors.messages.invalid'), repository.errors.on(:type) + end end def test_scan_changesets_for_issue_ids diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 5f1e41a65..39a4d48f5 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -109,12 +109,216 @@ class UserTest < ActiveSupport::TestCase assert_equal "john", @admin.login end - def test_destroy - User.find(2).destroy + def test_destroy_should_delete_members_and_roles + members = Member.find_all_by_user_id(2) + ms = members.size + rs = members.collect(&:roles).flatten.size + + assert_difference 'Member.count', - ms do + assert_difference 'MemberRole.count', - rs do + User.find(2).destroy + end + end + assert_nil User.find_by_id(2) assert Member.find_all_by_user_id(2).empty? end + def test_destroy_should_update_attachments + attachment = Attachment.create!(:container => Project.find(1), + :file => uploaded_test_file("testfile.txt", "text/plain"), + :author_id => 2) + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous, attachment.reload.author + end + + def test_destroy_should_update_comments + comment = Comment.create!( + :commented => News.create!(:project_id => 1, :author_id => 1, :title => 'foo', :description => 'foo'), + :author => User.find(2), + :comments => 'foo' + ) + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous, comment.reload.author + end + + def test_destroy_should_update_issues + issue = Issue.create!(:project_id => 1, :author_id => 2, :tracker_id => 1, :subject => 'foo') + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous, issue.reload.author + end + + def test_destroy_should_unassign_issues + issue = Issue.create!(:project_id => 1, :author_id => 1, :tracker_id => 1, :subject => 'foo', :assigned_to_id => 2) + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_nil issue.reload.assigned_to + end + + def test_destroy_should_update_journals + issue = Issue.create!(:project_id => 1, :author_id => 2, :tracker_id => 1, :subject => 'foo') + issue.init_journal(User.find(2), "update") + issue.save! + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous, issue.journals.first.reload.user + end + + def test_destroy_should_update_journal_details_old_value + issue = Issue.create!(:project_id => 1, :author_id => 1, :tracker_id => 1, :subject => 'foo', :assigned_to_id => 2) + issue.init_journal(User.find(1), "update") + issue.assigned_to_id = nil + assert_difference 'JournalDetail.count' do + issue.save! + end + journal_detail = JournalDetail.first(:order => 'id DESC') + assert_equal '2', journal_detail.old_value + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous.id.to_s, journal_detail.reload.old_value + end + + def test_destroy_should_update_journal_details_value + issue = Issue.create!(:project_id => 1, :author_id => 1, :tracker_id => 1, :subject => 'foo') + issue.init_journal(User.find(1), "update") + issue.assigned_to_id = 2 + assert_difference 'JournalDetail.count' do + issue.save! + end + journal_detail = JournalDetail.first(:order => 'id DESC') + assert_equal '2', journal_detail.value + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous.id.to_s, journal_detail.reload.value + end + + def test_destroy_should_update_messages + board = Board.create!(:project_id => 1, :name => 'Board', :description => 'Board') + message = Message.create!(:board_id => board.id, :author_id => 2, :subject => 'foo', :content => 'foo') + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous, message.reload.author + end + + def test_destroy_should_update_news + news = News.create!(:project_id => 1, :author_id => 2, :title => 'foo', :description => 'foo') + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous, news.reload.author + end + + def test_destroy_should_delete_private_queries + query = Query.new(:name => 'foo', :is_public => false) + query.project_id = 1 + query.user_id = 2 + query.save! + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_nil Query.find_by_id(query.id) + end + + def test_destroy_should_update_public_queries + query = Query.new(:name => 'foo', :is_public => true) + query.project_id = 1 + query.user_id = 2 + query.save! + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous, query.reload.user + end + + def test_destroy_should_update_time_entries + entry = TimeEntry.new(:hours => '2', :spent_on => Date.today, :activity => TimeEntryActivity.create!(:name => 'foo')) + entry.project_id = 1 + entry.user_id = 2 + entry.save! + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous, entry.reload.user + end + + def test_destroy_should_delete_tokens + token = Token.create!(:user_id => 2, :value => 'foo') + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_nil Token.find_by_id(token.id) + end + + def test_destroy_should_delete_watchers + issue = Issue.create!(:project_id => 1, :author_id => 1, :tracker_id => 1, :subject => 'foo') + watcher = Watcher.create!(:user_id => 2, :watchable => issue) + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_nil Watcher.find_by_id(watcher.id) + end + + def test_destroy_should_update_wiki_contents + wiki_content = WikiContent.create!( + :text => 'foo', + :author_id => 2, + :page => WikiPage.create!(:title => 'Foo', :wiki => Wiki.create!(:project_id => 1, :start_page => 'Start')) + ) + wiki_content.text = 'bar' + assert_difference 'WikiContent::Version.count' do + wiki_content.save! + end + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_equal User.anonymous, wiki_content.reload.author + wiki_content.versions.each do |version| + assert_equal User.anonymous, version.reload.author + end + end + + def test_destroy_should_nullify_issue_categories + category = IssueCategory.create!(:project_id => 1, :assigned_to_id => 2, :name => 'foo') + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_nil category.reload.assigned_to_id + end + + def test_destroy_should_nullify_changesets + changeset = Changeset.create!( + :repository => Repository::Subversion.create!( + :project_id => 1, + :url => 'file:///var/svn' + ), + :revision => '12', + :committed_on => Time.now, + :committer => 'jsmith' + ) + assert_equal 2, changeset.user_id + + User.find(2).destroy + assert_nil User.find_by_id(2) + assert_nil changeset.reload.user_id + end + + def test_anonymous_user_should_not_be_destroyable + assert_no_difference 'User.count' do + assert_equal false, User.anonymous.destroy + end + end + def test_validate_login_presence @admin.login = "" assert !@admin.save -- 2.39.5