Commit a9d940bf authored by Jarka Kadlecova's avatar Jarka Kadlecova

Use Ghost user when edited_by, merged_by deleted

parent 0cd42fea
...@@ -4,4 +4,8 @@ module Editable ...@@ -4,4 +4,8 @@ module Editable
def is_edited? def is_edited?
last_edited_at.present? && last_edited_at != created_at last_edited_at.present? && last_edited_at != created_at
end end
def last_edited_by
super || User.ghost
end
end end
...@@ -385,9 +385,11 @@ class User < ActiveRecord::Base ...@@ -385,9 +385,11 @@ class User < ActiveRecord::Base
# Return (create if necessary) the ghost user. The ghost user # Return (create if necessary) the ghost user. The ghost user
# owns records previously belonging to deleted users. # owns records previously belonging to deleted users.
def ghost def ghost
unique_internal(where(ghost: true), 'ghost', 'ghost%s@example.com') do |u| email = 'ghost%s@example.com'
unique_internal(where(ghost: true), 'ghost', email) do |u|
u.bio = 'This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.' u.bio = 'This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.'
u.name = 'Ghost User' u.name = 'Ghost User'
u.notification_email = email
end end
end end
end end
......
...@@ -50,10 +50,12 @@ module Users ...@@ -50,10 +50,12 @@ module Users
def migrate_issues def migrate_issues
user.issues.update_all(author_id: ghost_user.id) user.issues.update_all(author_id: ghost_user.id)
Issue.where(last_edited_by_id: user.id).update_all(last_edited_by_id: ghost_user.id)
end end
def migrate_merge_requests def migrate_merge_requests
user.merge_requests.update_all(author_id: ghost_user.id) user.merge_requests.update_all(author_id: ghost_user.id)
MergeRequest.where(merge_user_id: user.id).update_all(merge_user_id: ghost_user.id)
end end
def migrate_notes def migrate_notes
......
---
title: Use Ghost user for last_edited_by and merge_user when original user is deleted
merge_request: 12933
author:
...@@ -512,6 +512,36 @@ describe Projects::IssuesController do ...@@ -512,6 +512,36 @@ describe Projects::IssuesController do
end end
end end
describe 'GET #realtime_changes' do
it_behaves_like 'restricted action', success: 200
def go(id:)
get :realtime_changes,
namespace_id: project.namespace.to_param,
project_id: project,
id: id
end
context 'when an issue was edited by a deleted user' do
let(:deleted_user) { create(:user) }
before do
project.team << [user, :developer]
issue.update!(last_edited_by: deleted_user, last_edited_at: Time.now)
deleted_user.destroy
sign_in(user)
end
it 'returns 200' do
go(id: issue.iid)
expect(response).to have_http_status(200)
end
end
end
describe 'GET #edit' do describe 'GET #edit' do
it_behaves_like 'restricted action', success: 200 it_behaves_like 'restricted action', success: 200
......
require 'rails_helper'
feature 'Issue Detail', js: true, feature: true do
let(:user) { create(:user) }
let(:project) { create(:project, :public) }
let(:issue) { create(:issue, project: project, author: user) }
context 'when user displays the issue' do
before do
visit project_issue_path(project, issue)
wait_for_requests
end
it 'shows the issue' do
page.within('.issuable-details') do
expect(find('h2')).to have_content(issue.title)
end
end
end
context 'when edited by a user who is later deleted' do
before do
sign_in(user)
visit project_issue_path(project, issue)
wait_for_requests
click_link 'Edit'
fill_in 'issue-title', with: 'issue title'
click_button 'Save'
visit profile_account_path
click_link 'Delete account'
visit project_issue_path(project, issue)
end
it 'shows the issue' do
page.within('.issuable-details') do
expect(find('h2')).to have_content(issue.reload.title)
end
end
end
end
...@@ -244,5 +244,25 @@ describe IssuablesHelper do ...@@ -244,5 +244,25 @@ describe IssuablesHelper do
it { expect(helper.updated_at_by(unedited_issuable)).to eq({}) } it { expect(helper.updated_at_by(unedited_issuable)).to eq({}) }
it { expect(helper.updated_at_by(edited_issuable)).to eq(edited_updated_at_by) } it { expect(helper.updated_at_by(edited_issuable)).to eq(edited_updated_at_by) }
context 'when updated by a deleted user' do
let(:edited_updated_at_by) do
{
updatedAt: edited_issuable.updated_at.to_time.iso8601,
updatedBy: {
name: User.ghost.name,
path: user_path(User.ghost)
}
}
end
before do
user.destroy
end
it 'returns "Ghost user" as edited_by' do
expect(helper.updated_at_by(edited_issuable.reload)).to eq(edited_updated_at_by)
end
end
end end
end end
...@@ -7,16 +7,32 @@ describe Users::MigrateToGhostUserService, services: true do ...@@ -7,16 +7,32 @@ describe Users::MigrateToGhostUserService, services: true do
context "migrating a user's associated records to the ghost user" do context "migrating a user's associated records to the ghost user" do
context 'issues' do context 'issues' do
include_examples "migrating a deleted user's associated records to the ghost user", Issue do context 'deleted user is present as both author and edited_user' do
let(:created_record) { create(:issue, project: project, author: user) } include_examples "migrating a deleted user's associated records to the ghost user", Issue, [:author, :last_edited_by] do
let(:assigned_record) { create(:issue, project: project, assignee: user) } let(:created_record) do
create(:issue, project: project, author: user, last_edited_by: user)
end
end
end
context 'deleted user is present only as edited_user' do
include_examples "migrating a deleted user's associated records to the ghost user", Issue, [:last_edited_by] do
let(:created_record) { create(:issue, project: project, author: create(:user), last_edited_by: user) }
end
end end
end end
context 'merge requests' do context 'merge requests' do
include_examples "migrating a deleted user's associated records to the ghost user", MergeRequest do context 'deleted user is present as both author and merge_user' do
let(:created_record) { create(:merge_request, source_project: project, author: user, target_branch: "first") } include_examples "migrating a deleted user's associated records to the ghost user", MergeRequest, [:author, :merge_user] do
let(:assigned_record) { create(:merge_request, source_project: project, assignee: user, target_branch: 'second') } let(:created_record) { create(:merge_request, source_project: project, author: user, merge_user: user, target_branch: "first") }
end
end
context 'deleted user is present only as both merge_user' do
include_examples "migrating a deleted user's associated records to the ghost user", MergeRequest, [:merge_user] do
let(:created_record) { create(:merge_request, source_project: project, merge_user: user, target_branch: "first") }
end
end end
end end
...@@ -33,9 +49,8 @@ describe Users::MigrateToGhostUserService, services: true do ...@@ -33,9 +49,8 @@ describe Users::MigrateToGhostUserService, services: true do
end end
context 'award emoji' do context 'award emoji' do
include_examples "migrating a deleted user's associated records to the ghost user", AwardEmoji do include_examples "migrating a deleted user's associated records to the ghost user", AwardEmoji, [:user] do
let(:created_record) { create(:award_emoji, user: user) } let(:created_record) { create(:award_emoji, user: user) }
let(:author_alias) { :user }
context "when the awardable already has an award emoji of the same name assigned to the ghost user" do context "when the awardable already has an award emoji of the same name assigned to the ghost user" do
let(:awardable) { create(:issue) } let(:awardable) { create(:issue) }
......
require "spec_helper" require "spec_helper"
shared_examples "migrating a deleted user's associated records to the ghost user" do |record_class| shared_examples "migrating a deleted user's associated records to the ghost user" do |record_class, fields|
record_class_name = record_class.to_s.titleize.downcase record_class_name = record_class.to_s.titleize.downcase
let(:project) { create(:project) } let(:project) { create(:project) }
...@@ -11,6 +11,7 @@ shared_examples "migrating a deleted user's associated records to the ghost user ...@@ -11,6 +11,7 @@ shared_examples "migrating a deleted user's associated records to the ghost user
context "for a #{record_class_name} the user has created" do context "for a #{record_class_name} the user has created" do
let!(:record) { created_record } let!(:record) { created_record }
let(:migrated_fields) { fields || [:author] }
it "does not delete the #{record_class_name}" do it "does not delete the #{record_class_name}" do
service.execute service.execute
...@@ -18,22 +19,20 @@ shared_examples "migrating a deleted user's associated records to the ghost user ...@@ -18,22 +19,20 @@ shared_examples "migrating a deleted user's associated records to the ghost user
expect(record_class.find_by_id(record.id)).to be_present expect(record_class.find_by_id(record.id)).to be_present
end end
it "migrates the #{record_class_name} so that the 'Ghost User' is the #{record_class_name} owner" do it "blocks the user before migrating #{record_class_name}s to the 'Ghost User'" do
service.execute service.execute
migrated_record = record_class.find_by_id(record.id) expect(user).to be_blocked
if migrated_record.respond_to?(:author)
expect(migrated_record.author).to eq(User.ghost)
else
expect(migrated_record.send(author_alias)).to eq(User.ghost)
end
end end
it "blocks the user before migrating #{record_class_name}s to the 'Ghost User'" do it 'migrates all associated fields to te "Ghost user"' do
service.execute service.execute
expect(user).to be_blocked migrated_record = record_class.find_by_id(record.id)
migrated_fields.each do |field|
expect(migrated_record.public_send(field)).to eq(User.ghost)
end
end end
context "race conditions" do context "race conditions" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment