From 8335c8dc1a30a2be87173e9d844b6890d1d00225 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Mon, 29 Mar 2021 14:03:19 +0300 Subject: [PATCH] FEATURE: Allow admins to pre-populate user fields (#12361) Admins can use bulk invites to pre-populate user fields. The imported CSV file must have a header with "email" column (first position) and names of the user fields (exact match). Under the hood, the bulk invite will create staged users and populate the user fields of those. --- .../discourse/app/routes/invites-show.js | 12 ++++ app/controllers/invites_controller.rb | 45 ++++++++---- app/jobs/regular/bulk_invite.rb | 71 +++++++++++++------ app/models/user.rb | 4 ++ spec/fixtures/csv/discourse_headers.csv | 3 + spec/jobs/bulk_invite_spec.rb | 46 +++++++----- spec/models/invite_spec.rb | 10 +++ spec/requests/invites_controller_spec.rb | 35 +++++++++ 8 files changed, 174 insertions(+), 52 deletions(-) create mode 100644 spec/fixtures/csv/discourse_headers.csv diff --git a/app/assets/javascripts/discourse/app/routes/invites-show.js b/app/assets/javascripts/discourse/app/routes/invites-show.js index d094036fe66..7a044a0e373 100644 --- a/app/assets/javascripts/discourse/app/routes/invites-show.js +++ b/app/assets/javascripts/discourse/app/routes/invites-show.js @@ -17,4 +17,16 @@ export default DiscourseRoute.extend({ return {}; } }, + + setupController(controller, model) { + this._super(...arguments); + + if (model.user_fields) { + controller.userFields.forEach((userField) => { + if (model.user_fields[userField.field.id]) { + userField.value = model.user_fields[userField.field.id]; + } + }); + } + }, }); diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 0e6a2b4f2b6..f6b89456285 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'csv' + class InvitesController < ApplicationController requires_login only: [:create, :destroy, :destroy_all_expired, :resend_invite, :resend_all_invites, :upload_csv] @@ -29,13 +31,19 @@ class InvitesController < ApplicationController hidden_email = email != invite.email - store_preloaded("invite_info", MultiJson.dump( + info = { invited_by: UserNameSerializer.new(invite.invited_by, scope: guardian, root: false), email: email, hidden_email: hidden_email, username: hidden_email ? '' : UserNameSuggester.suggest(invite.email), is_invite_link: invite.is_invite_link? - )) + } + + if staged_user = User.where(staged: true).with_email(invite.email).first + info[:user_fields] = staged_user.user_fields + end + + store_preloaded("invite_info", MultiJson.dump(info)) secure_session["invite-key"] = invite.invite_key @@ -266,35 +274,44 @@ class InvitesController < ApplicationController end def upload_csv - require 'csv' - guardian.ensure_can_bulk_invite_to_forum!(current_user) hijack do begin file = params[:file] || params[:files].first - count = 0 + csv_header = nil invites = [] - max_bulk_invites = SiteSetting.max_bulk_invites - CSV.foreach(file.tempfile) do |row| - count += 1 - invites.push(email: row[0], groups: row[1], topic_id: row[2]) if row[0].present? - break if count >= max_bulk_invites + + CSV.foreach(file.tempfile, encoding: "bom|utf-8") do |row| + # Try to extract a CSV header, if it exists + if csv_header.nil? + if row[0] == 'email' + csv_header = row + next + else + csv_header = ["email", "groups", "topic_id"] + end + end + + if row[0].present? + invites.push(csv_header.zip(row).map.to_h.filter { |k, v| v.present? }) + end + + break if invites.count >= SiteSetting.max_bulk_invites end if invites.present? Jobs.enqueue(:bulk_invite, invites: invites, current_user_id: current_user.id) - if count >= max_bulk_invites - render json: failed_json.merge(errors: [I18n.t("bulk_invite.max_rows", max_bulk_invites: max_bulk_invites)]), status: 422 + + if invites.count >= SiteSetting.max_bulk_invites + render json: failed_json.merge(errors: [I18n.t("bulk_invite.max_rows", max_bulk_invites: SiteSetting.max_bulk_invites)]), status: 422 else render json: success_json end else render json: failed_json.merge(errors: [I18n.t("bulk_invite.error")]), status: 422 end - rescue - render json: failed_json.merge(errors: [I18n.t("bulk_invite.error")]), status: 422 end end end diff --git a/app/jobs/regular/bulk_invite.rb b/app/jobs/regular/bulk_invite.rb index 2965abc4067..cb9b0587993 100644 --- a/app/jobs/regular/bulk_invite.rb +++ b/app/jobs/regular/bulk_invite.rb @@ -6,25 +6,27 @@ module Jobs def initialize super - @logs = [] - @sent = 0 - @failed = 0 - @groups = {} + + @logs = [] + @sent = 0 + @failed = 0 + @groups = {} + @user_fields = {} @valid_groups = {} end def execute(args) - invites = args[:invites] - raise Discourse::InvalidParameters.new(:invites) if invites.blank? + @invites = args[:invites] + raise Discourse::InvalidParameters.new(:invites) if @invites.blank? @current_user = User.find_by(id: args[:current_user_id]) raise Discourse::InvalidParameters.new(:current_user_id) unless @current_user + @guardian = Guardian.new(@current_user) - @total_invites = invites.length - process_invites(invites) + process_invites(@invites) - if @total_invites > Invite::BULK_INVITE_EMAIL_LIMIT + if @invites.length > Invite::BULK_INVITE_EMAIL_LIMIT ::Jobs.enqueue(:process_bulk_invite_emails) end ensure @@ -87,10 +89,22 @@ module Jobs topic end + def get_user_fields(fields) + user_fields = {} + + fields.each do |key, value| + @user_fields[key] ||= UserField.find_by(name: key)&.id || :nil + user_fields[@user_fields[key]] = value if @user_fields[key] != :nil + end + + user_fields + end + def send_invite(invite) email = invite[:email] groups = get_groups(invite[:groups]) topic = get_topic(invite[:topic_id]) + user_fields = get_user_fields(invite.except(:email, :groups, :topic_id)) begin if user = Invite.find_user_by_email(email) @@ -105,17 +119,34 @@ module Jobs end end end - else - if @total_invites > Invite::BULK_INVITE_EMAIL_LIMIT - invite = Invite.generate(@current_user, - email: email, - topic: topic, - group_ids: groups.map(&:id), - emailed_status: Invite.emailed_status_types[:bulk_pending] - ) - else - Invite.generate(@current_user, email: email, topic: topic, group_ids: groups.map(&:id)) + + if user_fields.present? + user_fields.each do |user_field, value| + user.set_user_field(user_field, value) + end + user.save_custom_fields end + else + if user_fields.present? + user = User.where(staged: true).find_by_email(email) + user ||= User.new(username: UserNameSuggester.suggest(email), email: email, staged: true) + user_fields.each do |user_field, value| + user.set_user_field(user_field, value) + end + user.save! + end + + invite_opts = { + email: email, + topic: topic, + group_ids: groups.map(&:id), + } + + if @invites.length > Invite::BULK_INVITE_EMAIL_LIMIT + invite_opts[:emailed_status] = Invite.emailed_status_types[:bulk_pending] + end + + Invite.generate(@current_user, invite_opts) end rescue => e save_log "Error inviting '#{email}' -- #{Rails::Html::FullSanitizer.new.sanitize(e.message)}" @@ -153,7 +184,7 @@ module Jobs group = @groups[group_name] unless group - group = Group.find_by("lower(name) = ?", group_name) + group = Group.find_by('lower(name) = ?', group_name) @groups[group_name] = group end diff --git a/app/models/user.rb b/app/models/user.rb index d8cbdc3fb7d..349cedf1776 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1173,6 +1173,10 @@ class User < ActiveRecord::Base end end + def set_user_field(field_id, value) + custom_fields["#{USER_FIELD_PREFIX}#{field_id}"] = value + end + def number_of_deleted_posts Post.with_deleted .where(user_id: self.id) diff --git a/spec/fixtures/csv/discourse_headers.csv b/spec/fixtures/csv/discourse_headers.csv new file mode 100644 index 00000000000..14215ae2eea --- /dev/null +++ b/spec/fixtures/csv/discourse_headers.csv @@ -0,0 +1,3 @@ +email,groups,location +test@example.com,discourse;ubuntu,usa +test2@example.com,discourse;ubuntu,europe diff --git a/spec/jobs/bulk_invite_spec.rb b/spec/jobs/bulk_invite_spec.rb index aad9f5ffe47..95ebd240c24 100644 --- a/spec/jobs/bulk_invite_spec.rb +++ b/spec/jobs/bulk_invite_spec.rb @@ -10,7 +10,7 @@ describe Jobs::BulkInvite do fab!(:group2) { Fabricate(:group, name: 'group2') } fab!(:topic) { Fabricate(:topic) } let(:staged_user) { Fabricate(:user, staged: true, active: false) } - let(:email) { "test@discourse.org" } + let(:email) { 'test@discourse.org' } let(:invites) { [{ email: staged_user.email }, { email: 'test2@discourse.org' }, { email: 'test@discourse.org', groups: 'GROUP1;group2', topic_id: topic.id }] } it 'raises an error when the invites array is missing' do @@ -30,13 +30,11 @@ describe Jobs::BulkInvite do ) expect(Invite.exists?(email: staged_user.email)).to eq(true) - expect(Invite.exists?(email: "test2@discourse.org")).to eq(true) + expect(Invite.exists?(email: 'test2@discourse.org')).to eq(true) invite = Invite.last expect(invite.email).to eq(email) - expect(invite.invited_groups.pluck(:group_id)).to contain_exactly( - group1.id, group2.id - ) + expect(invite.invited_groups.pluck(:group_id)).to contain_exactly(group1.id, group2.id) expect(invite.topic_invites.pluck(:topic_id)).to contain_exactly(topic.id) end @@ -49,12 +47,8 @@ describe Jobs::BulkInvite do ) invite = Invite.last - expect(invite.email).to eq(email) - - expect(invite.invited_groups.pluck(:group_id)).to contain_exactly( - group1.id - ) + expect(invite.invited_groups.pluck(:group_id)).to contain_exactly(group1.id) end it 'does not create invited groups record if the user can not manage the group' do @@ -66,16 +60,12 @@ describe Jobs::BulkInvite do ) invite = Invite.last - expect(invite.email).to eq(email) - - expect(invite.invited_groups.pluck(:group_id)).to contain_exactly( - group1.id - ) + expect(invite.invited_groups.pluck(:group_id)).to contain_exactly(group1.id) end it 'adds existing users to valid groups' do - existing_user = Fabricate(:user, email: "test@discourse.org") + existing_user = Fabricate(:user, email: 'test@discourse.org') group2.update!(automatic: true) @@ -87,10 +77,30 @@ describe Jobs::BulkInvite do end.to change { Invite.count }.by(2) expect(Invite.exists?(email: staged_user.email)).to eq(true) - expect(Invite.exists?(email: "test2@discourse.org")).to eq(true) + expect(Invite.exists?(email: 'test2@discourse.org')).to eq(true) expect(existing_user.reload.groups).to eq([group1]) end + it 'can create staged users and prepulate user fields' do + user_field = Fabricate(:user_field) + described_class.new.execute( + current_user_id: admin.id, + invites: [ + { email: 'test@discourse.org' }, # new user without user fields + { email: user.email, user_field.name => 'value 1' }, # existing user with user fields + { email: staged_user.email, user_field.name => 'value 2' }, # existing staged user with user fields + { email: 'test2@discourse.org', user_field.name => 'value 3' } # new staged user with user fields + ] + ) + + expect(Invite.count).to eq(3) + expect(User.where(staged: true).find_by_email('test@discourse.org')).to eq(nil) + expect(user.user_fields[user_field.id.to_s]).to eq('value 1') + expect(staged_user.user_fields[user_field.id.to_s]).to eq('value 2') + new_staged_user = User.where(staged: true).find_by_email('test2@discourse.org') + expect(new_staged_user.user_fields[user_field.id.to_s]).to eq('value 3') + end + context 'invites are more than 200' do let(:bulk_invites) { [] } @@ -107,7 +117,7 @@ describe Jobs::BulkInvite do ) invite = Invite.last - expect(invite.email).to eq("test_201@discourse.org") + expect(invite.email).to eq('test_201@discourse.org') expect(invite.emailed_status).to eq(Invite.emailed_status_types[:bulk_pending]) expect(Jobs::ProcessBulkInviteEmails.jobs.size).to eq(1) end diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 35875a3fde2..9850aba37ec 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -176,6 +176,16 @@ describe Invite do expect(invite.redeem).to be_blank end + it 'keeps custom fields' do + user_field = Fabricate(:user_field) + staged_user = Fabricate(:user, staged: true, email: invite.email) + staged_user.set_user_field(user_field.id, 'some value') + staged_user.save_custom_fields + + expect(invite.redeem).to eq(staged_user) + expect(staged_user.reload.user_fields[user_field.id.to_s]).to eq('some value') + end + it 'creates a notification for the invitee' do expect { invite.redeem }.to change { Notification.count } end diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index 53645fb52da..5bd12892242 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -27,6 +27,20 @@ describe InvitesController do expect(response.body).not_to include('i*****g@a***********e.ooo') end + it 'shows default user fields' do + user_field = Fabricate(:user_field) + staged_user = Fabricate(:user, staged: true, email: invite.email) + staged_user.set_user_field(user_field.id, 'some value') + staged_user.save_custom_fields + + get "/invites/#{invite.invite_key}" + expect(response.body).to have_tag("div#data-preloaded") do |element| + json = JSON.parse(element.current_scope.attribute('data-preloaded').value) + invite_info = JSON.parse(json['invite_info']) + expect(invite_info['user_fields'][user_field.id.to_s]).to eq('some value') + end + end + it 'fails for logged in users' do sign_in(Fabricate(:user)) @@ -691,6 +705,9 @@ describe InvitesController do let(:csv_file) { File.new("#{Rails.root}/spec/fixtures/csv/discourse.csv") } let(:file) { Rack::Test::UploadedFile.new(File.open(csv_file)) } + let(:csv_file_with_headers) { File.new("#{Rails.root}/spec/fixtures/csv/discourse_headers.csv") } + let(:file_with_headers) { Rack::Test::UploadedFile.new(File.open(csv_file_with_headers)) } + it 'fails if you cannot bulk invite to the forum' do sign_in(Fabricate(:user)) post '/invites/upload_csv.json', params: { file: file, name: 'discourse.csv' } @@ -713,6 +730,24 @@ describe InvitesController do expect(Jobs::BulkInvite.jobs.size).to eq(1) expect(response.parsed_body['errors'][0]).to eq(I18n.t('bulk_invite.max_rows', max_bulk_invites: SiteSetting.max_bulk_invites)) end + + it 'can import user fields' do + Jobs.run_immediately! + user_field = Fabricate(:user_field, name: "location") + Fabricate(:group, name: 'discourse') + Fabricate(:group, name: 'ubuntu') + + sign_in(admin) + + post '/invites/upload_csv.json', params: { file: file_with_headers, name: 'discourse_headers.csv' } + expect(response.status).to eq(200) + + user = User.where(staged: true).find_by_email('test@example.com') + expect(user.user_fields[user_field.id.to_s]).to eq('usa') + + user2 = User.where(staged: true).find_by_email('test2@example.com') + expect(user2.user_fields[user_field.id.to_s]).to eq('europe') + end end end end