diff --git a/app/assets/javascripts/discourse/templates/components/reviewable-user.hbs b/app/assets/javascripts/discourse/templates/components/reviewable-user.hbs index 0afc052a447..a0f6143f21f 100644 --- a/app/assets/javascripts/discourse/templates/components/reviewable-user.hbs +++ b/app/assets/javascripts/discourse/templates/components/reviewable-user.hbs @@ -1,7 +1,7 @@
- {{reviewable.username}} - {{reviewable.email}} + {{reviewable.payload.username}} + {{reviewable.payload.email}}
{{yield}} diff --git a/app/jobs/regular/create_user_reviewable.rb b/app/jobs/regular/create_user_reviewable.rb index 911728ddacc..2546f7fe636 100644 --- a/app/jobs/regular/create_user_reviewable.rb +++ b/app/jobs/regular/create_user_reviewable.rb @@ -5,7 +5,16 @@ class Jobs::CreateUserReviewable < Jobs::Base if user = User.find_by(id: args[:user_id]) return if user.approved? - reviewable = ReviewableUser.create!(target: user, created_by: Discourse.system_user, reviewable_by_moderator: true) + reviewable = ReviewableUser.create!( + target: user, + created_by: Discourse.system_user, + reviewable_by_moderator: true, + payload: { + username: user.username, + name: user.name, + email: user.email + } + ) reviewable.add_score( Discourse.system_user, ReviewableScore.types[:needs_approval], diff --git a/app/models/reviewable_user.rb b/app/models/reviewable_user.rb index 1f4f7a05a38..f1ebf49c520 100644 --- a/app/models/reviewable_user.rb +++ b/app/models/reviewable_user.rb @@ -34,7 +34,7 @@ class ReviewableUser < Reviewable end def perform_reject(performed_by, args) - destroyer = UserDestroyer.new(performed_by) + destroyer = UserDestroyer.new(performed_by) unless args[:skip_delete] # If a user has posts, we won't delete them to preserve their content. # However the reviable record will be "rejected" and they will remain diff --git a/app/models/user.rb b/app/models/user.rb index 566ddeb7164..f1f79896473 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1220,7 +1220,6 @@ class User < ActiveRecord::Base Jobs.enqueue(:create_user_reviewable, user_id: self.id) end - protected def badge_grant diff --git a/app/serializers/reviewable_user_serializer.rb b/app/serializers/reviewable_user_serializer.rb index 1991a4f4ead..f41320a994f 100644 --- a/app/serializers/reviewable_user_serializer.rb +++ b/app/serializers/reviewable_user_serializer.rb @@ -1,6 +1,6 @@ class ReviewableUserSerializer < ReviewableSerializer - target_attributes( + payload_attributes( :username, :email, :name diff --git a/app/services/user_destroyer.rb b/app/services/user_destroyer.rb index be4e91e5caa..dc554f3a6be 100644 --- a/app/services/user_destroyer.rb +++ b/app/services/user_destroyer.rb @@ -28,6 +28,10 @@ class UserDestroyer Draft.where(user_id: user.id).delete_all Reviewable.where(created_by_id: user.id).delete_all + if reviewable = Reviewable.find_by(target: user) + reviewable.perform(@actor, :reject, skip_delete: true) + end + if opts[:delete_posts] user.posts.each do |post| diff --git a/db/migrate/20190403202001_fix_reviewable_users.rb b/db/migrate/20190403202001_fix_reviewable_users.rb new file mode 100644 index 00000000000..eb953264949 --- /dev/null +++ b/db/migrate/20190403202001_fix_reviewable_users.rb @@ -0,0 +1,20 @@ +class FixReviewableUsers < ActiveRecord::Migration[5.2] + def up + execute(<<~SQL) + UPDATE reviewables + SET payload = json_build_object( + 'username', u.username, + 'name', u.name, + 'email', ue.email + )::jsonb + FROM reviewables AS r + LEFT OUTER JOIN users AS u ON u.id = r.target_id + LEFT OUTER JOIN user_emails AS ue ON ue.user_id = u.id AND ue.primary + WHERE r.id = reviewables.id + AND r.type = 'ReviewableUser' + SQL + end + + def down + end +end diff --git a/spec/jobs/create_user_reviewable_spec.rb b/spec/jobs/create_user_reviewable_spec.rb new file mode 100644 index 00000000000..ffb2c3b8922 --- /dev/null +++ b/spec/jobs/create_user_reviewable_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' +require_dependency 'jobs/regular/create_user_reviewable' + +describe Jobs::CreateUserReviewable do + + let(:user) { Fabricate(:user) } + + it "creates the reviewable" do + described_class.new.execute(user_id: user.id) + + reviewable = Reviewable.find_by(target: user) + expect(reviewable).to be_present + expect(reviewable.pending?).to eq(true) + expect(reviewable.reviewable_scores).to be_present + expect(reviewable.payload['username']).to eq(user.username) + expect(reviewable.payload['name']).to eq(user.name) + expect(reviewable.payload['email']).to eq(user.email) + end +end diff --git a/spec/models/reviewable_user_spec.rb b/spec/models/reviewable_user_spec.rb index 07821378183..95e6c73e995 100644 --- a/spec/models/reviewable_user_spec.rb +++ b/spec/models/reviewable_user_spec.rb @@ -36,6 +36,17 @@ RSpec.describe ReviewableUser, type: :model do end end + context "when a user is deleted" do + it "should reject the reviewable" do + Jobs::CreateUserReviewable.new.execute(user_id: user.id) + reviewable = Reviewable.find_by(target: user) + expect(reviewable.pending?).to eq(true) + + UserDestroyer.new(Discourse.system_user).destroy(user) + expect(reviewable.reload.rejected?).to eq(true) + end + end + context "perform" do let(:reviewable) { Fabricate(:reviewable) } context "approve" do diff --git a/spec/serializers/reviewable_user_serializer_spec.rb b/spec/serializers/reviewable_user_serializer_spec.rb index 5b117d01473..88991ea19ae 100644 --- a/spec/serializers/reviewable_user_serializer_spec.rb +++ b/spec/serializers/reviewable_user_serializer_spec.rb @@ -2,15 +2,18 @@ require 'rails_helper' describe ReviewableUserSerializer do - let(:reviewable) { Fabricate(:reviewable) } + let(:user) { Fabricate(:user) } let(:admin) { Fabricate(:admin) } it "includes the user fields for review" do + Jobs::CreateUserReviewable.new.execute(user_id: user.id) + reviewable = Reviewable.find_by(target: user) + json = ReviewableUserSerializer.new(reviewable, scope: Guardian.new(admin), root: nil).as_json expect(json[:user_id]).to eq(reviewable.target_id) - expect(json[:username]).to eq(reviewable.target.username) - expect(json[:email]).to eq(reviewable.target.email) - expect(json[:name]).to eq(reviewable.target.name) + expect(json[:payload]['username']).to eq(user.username) + expect(json[:payload]['email']).to eq(user.email) + expect(json[:payload]['name']).to eq(user.name) expect(json[:topic_url]).to be_blank end