From b80dae1436af2df549bc29fbe127d1771b08990e Mon Sep 17 00:00:00 2001 From: Andy Zhao <17884966+Zhao-Andy@users.noreply.github.com> Date: Mon, 26 Oct 2020 18:00:56 -0400 Subject: [PATCH] Update data exporter to handle admin send (#10274) * Update data exporter to handle admin send * Match button with everything else * Use proper redirect path * Stub SiteConfig definition instead of setting it Co-authored-by: Mac Siri * Removed if statement by accident oops * Remove non-functional boolean param and pass email directly * Use refinement to conv to boolean instead of JSON.parse * Rename to StringToBoolean * Use 'using' in proper scope (not in method) * Rename to_bool to to_boolean * Refactor if statement, thanks rhymes! * Fix small bugs in tests * Remove tracking for export_email b/c no @user Co-authored-by: Mac Siri --- app/controllers/admin/users_controller.rb | 16 ++++++ app/controllers/users_controller.rb | 2 +- app/mailers/notify_mailer.rb | 3 +- app/refinements/string_to_boolean.rb | 7 +++ app/services/exporter/service.rb | 8 +-- app/views/admin/events/new.html.erb | 55 +++++++++++++++++++ .../admin/moderator_actions/index.html.erb | 2 +- app/views/admin/users/_data_export.html.erb | 21 +++++++ app/views/admin/users/_notes.html.erb | 4 +- app/views/admin/users/edit.html.erb | 9 +-- app/workers/export_content_worker.rb | 4 +- config/routes.rb | 1 + spec/mailers/notify_mailer_spec.rb | 8 +-- spec/refinements/string_to_boolean_spec.rb | 10 ++++ spec/requests/admin/users_spec.rb | 10 +++- spec/services/exporter/service_spec.rb | 30 +++++++--- spec/workers/export_content_worker_spec.rb | 4 +- 17 files changed, 161 insertions(+), 33 deletions(-) create mode 100644 app/refinements/string_to_boolean.rb create mode 100644 app/views/admin/events/new.html.erb create mode 100644 app/views/admin/users/_data_export.html.erb create mode 100644 spec/refinements/string_to_boolean_spec.rb diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index e5e12066e..1046c101e 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -1,6 +1,7 @@ module Admin class UsersController < Admin::ApplicationController layout "admin" + using StringToBoolean after_action only: %i[update user_status banish full_delete merge] do Audit::Logger.log(:moderator, current_user, params.dup) @@ -50,6 +51,21 @@ module Admin redirect_to "/admin/users/#{@user.id}/edit" end + def export_data + user = User.find(params[:id]) + send_to_admin = params[:send_to_admin].to_boolean + if send_to_admin + email = SiteConfig.email_addresses[:default] + receiver = "admin" + else + email = user.email + receiver = "user" + end + ExportContentWorker.perform_async(user.id, email) + flash[:success] = "Data exported to the #{receiver}. The job will complete momentarily." + redirect_to edit_admin_user_path(user.id) + end + def banish Moderator::BanishUserWorker.perform_async(current_user.id, params[:id].to_i) flash[:success] = "This user is being banished in the background. The job will complete soon." diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 14265ae6c..2e8a477d1 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -47,7 +47,7 @@ class UsersController < ApplicationController end if @user.export_requested? notice += " The export will be emailed to you shortly." - ExportContentWorker.perform_async(@user.id) + ExportContentWorker.perform_async(@user.id, @user.email) end cookies.permanent[:user_experience_level] = @user.experience_level.to_s if @user.experience_level.present? flash[:settings_notice] = notice diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb index 17b2259e6..61e4d0bae 100644 --- a/app/mailers/notify_mailer.rb +++ b/app/mailers/notify_mailer.rb @@ -116,12 +116,11 @@ class NotifyMailer < ApplicationMailer end def export_email - @user = params[:user] attachment = params[:attachment] export_filename = "devto-export-#{Date.current.iso8601}.zip" attachments[export_filename] = attachment - mail(to: @user.email, subject: "The export of your content is ready") + mail(to: params[:email], subject: "The export of your content is ready") end def tag_moderator_confirmation_email diff --git a/app/refinements/string_to_boolean.rb b/app/refinements/string_to_boolean.rb new file mode 100644 index 000000000..4218d3267 --- /dev/null +++ b/app/refinements/string_to_boolean.rb @@ -0,0 +1,7 @@ +module StringToBoolean + refine String do + def to_boolean + ActiveModel::Type::Boolean.new.cast(self) + end + end +end diff --git a/app/services/exporter/service.rb b/app/services/exporter/service.rb index eae7d8240..a7b34a56a 100644 --- a/app/services/exporter/service.rb +++ b/app/services/exporter/service.rb @@ -13,7 +13,7 @@ module Exporter @user = user end - def export(send_email: false, config: {}) + def export(email, config: {}) exports = {} # export content with filenames @@ -26,7 +26,7 @@ module Exporter zipped_exports = zip_exports(exports) - send_exports_by_email(zipped_exports) if send_email + send_exports_by_email(zipped_exports, email) update_user_export_fields @@ -53,9 +53,9 @@ module Exporter buffer end - def send_exports_by_email(zipped_exports) + def send_exports_by_email(zipped_exports, email) zipped_exports.rewind - NotifyMailer.with(user: user, attachment: zipped_exports.read).export_email.deliver_now + NotifyMailer.with(email: email, attachment: zipped_exports.read).export_email.deliver_now end def update_user_export_fields diff --git a/app/views/admin/events/new.html.erb b/app/views/admin/events/new.html.erb new file mode 100644 index 000000000..cca524004 --- /dev/null +++ b/app/views/admin/events/new.html.erb @@ -0,0 +1,55 @@ +

Create Event

+ +
+ <%# form_with url: admin_ %> +
+
+ <%= f.label :cover_image %>: + <%= f.file_field :cover_image, class: "form-control" %> +
+
+ <%= f.label :profile_image %> (for live notification): + <%= f.file_field :profile_image, class: "form-control" %> + event profile image +
+
+ <%= f.label :title %> + <%= f.text_field :title, maxlength: 90, size: 40, required: true, class: "form-control" %> +
+
+ <%= f.label :host_name %> + <%= f.text_field :host_name, size: 40, required: true, class: "form-control" %> +
+
+ <%= f.label :category %> + <%= f.select :category, ["AMA", "Workshop", "Talk", "Town Hall"], required: true %> +
+
+ <%= f.label :starts_at %> + <%= f.datetime_select :starts_at, required: true, include_blank: true, start_year: Time.current.year, end_year: Time.current.year + 2, class: "form-control" %> UTC Time Only (4 hours ahead of eastern time) +
+
+ <%= f.label :ends_at %> + <%= f.datetime_select :ends_at, required: true, include_blank: true, start_year: Time.current.year, end_year: Time.current.year + 2, class: "form-control" %> UTC Time Only (4 hours ahead of eastern time) +
+
+ <%= f.label :location_name %> + <%= f.text_field :location_name, required: true, class: "form-control" %> +
+
+ <%= f.label :location_url %> + <%= f.text_field :location_url, required: true, class: "form-control" %> +
+
+ <%= f.label :description_markdown %> + <%= f.text_area :description_markdown, size: "45x10", required: true, class: "form-control" %> +
+
+ <%= f.label :published %> + <%= f.check_box :published %> +
+
+ <%= f.label :live_now %> + <%= f.check_box :live_now %> +
+<%= f.submit class: "btn btn-primary" %> \ No newline at end of file diff --git a/app/views/admin/moderator_actions/index.html.erb b/app/views/admin/moderator_actions/index.html.erb index 8fb35db2e..f7f101cfc 100644 --- a/app/views/admin/moderator_actions/index.html.erb +++ b/app/views/admin/moderator_actions/index.html.erb @@ -8,7 +8,7 @@ <%= paginate @moderator_actions %> - +
diff --git a/app/views/admin/users/_data_export.html.erb b/app/views/admin/users/_data_export.html.erb new file mode 100644 index 000000000..d0153d9fe --- /dev/null +++ b/app/views/admin/users/_data_export.html.erb @@ -0,0 +1,21 @@ +
+
+

Data Export

+ +
+
+

+ You can export a user's data here. Currently we only support exporting a user's posts and comments. +

+

+ You have the option of exporting to your admin account to send to the user, or exporting to the user directly. +

+

+ Exporting the data to your admin account will send it to your default admin email (<%= SiteConfig.email_addresses[:default] %>), and exporting to the user will send it to their email (<%= @user.email %>). +

+
+ <%= button_to "Export to Admin", export_data_admin_user_path(@user.id), data: { confirm: "Are you sure you want to export this user's content to the ADMIN?" }, class: "crayons-btn", params: { send_to_admin: true }, style: "margin-right: 8px;" %> + <%= button_to "Export to User", export_data_admin_user_path(@user.id), data: { confirm: "Are you sure you want to export this user's content to the USER?" }, class: "crayons-btn", params: { send_to_admin: false } %> +
+
+
diff --git a/app/views/admin/users/_notes.html.erb b/app/views/admin/users/_notes.html.erb index eb3a66c5b..a678eab60 100644 --- a/app/views/admin/users/_notes.html.erb +++ b/app/views/admin/users/_notes.html.erb @@ -1,6 +1,6 @@

Recent Notes (last 10)

- + <% unless @notes.load.empty? %> <% @notes.each do |note| %>

@@ -21,6 +21,6 @@ <%= f.label "Add new note: ", class: "d-block" %> <%= f.text_area :new_note, class: "form-control" %>

- <%= f.submit "Submit Note", class: "btn btn-primary float-right" %> + <%= f.submit "Submit Note", class: "crayons-btn float-right" %> <% end %> diff --git a/app/views/admin/users/edit.html.erb b/app/views/admin/users/edit.html.erb index 52f26cb81..ecf342aee 100644 --- a/app/views/admin/users/edit.html.erb +++ b/app/views/admin/users/edit.html.erb @@ -9,7 +9,7 @@ <%= link_to "Unlock access", unlock_access_admin_user_path(@user), method: :patch, class: "btn btn-success" %> <% end %> - Admin Profile + Admin Profile

Member since <%= @user.created_at.strftime("%b %e '%y") %>

@@ -43,18 +43,19 @@ <%= f.label "Reason for action:" %> <%= f.text_area :note_for_current_role, required: true, class: "form-control" %> - <%= f.submit "Update User Status", class: "btn btn-primary float-right" %> + <%= f.submit "Update User Status", class: "crayons-btn float-right" %> <% end %> <%= render "notes" %> <%= render "negative_reactions" %> <%= render "reports" %> +<%= render "data_export" %>

Remove Identity

- +

Removing a social account identity can solve certain sign in issues, for example:

@@ -92,7 +93,7 @@

Destructive Actions

- +
diff --git a/app/workers/export_content_worker.rb b/app/workers/export_content_worker.rb index 95293d60a..5cf385f5f 100644 --- a/app/workers/export_content_worker.rb +++ b/app/workers/export_content_worker.rb @@ -3,9 +3,9 @@ class ExportContentWorker sidekiq_options queue: :medium_priority, retry: 10, lock: :until_executed - def perform(user_id) + def perform(user_id, email) user = User.find_by(id: user_id) - Exporter::Service.new(user).export(send_email: true) if user + Exporter::Service.new(user).export(email) if user end end diff --git a/config/routes.rb b/config/routes.rb index c0e63ca1e..b76cdb175 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -103,6 +103,7 @@ Rails.application.routes.draw do resources :users, only: %i[index show edit update] do member do post "banish" + post "export_data" post "full_delete" patch "user_status" post "merge" diff --git a/spec/mailers/notify_mailer_spec.rb b/spec/mailers/notify_mailer_spec.rb index b90bbbd64..f0da96db0 100644 --- a/spec/mailers/notify_mailer_spec.rb +++ b/spec/mailers/notify_mailer_spec.rb @@ -403,7 +403,7 @@ RSpec.describe NotifyMailer, type: :mailer do end describe "#export_email" do - let(:email) { described_class.with(user: user, attachment: "attachment").export_email } + let(:email) { described_class.with(email: user.email, attachment: "attachment").export_email } it "renders proper subject" do expect(email.subject).to include("export of your content is ready") @@ -431,12 +431,6 @@ RSpec.describe NotifyMailer, type: :mailer do it "includes the tracking pixel" do expect(email.html_part.body).to include("open.gif") end - - it "includes UTM params" do - expect(email.html_part.body).to include(CGI.escape("utm_medium=email")) - expect(email.html_part.body).to include(CGI.escape("utm_source=notify_mailer")) - expect(email.html_part.body).to include(CGI.escape("utm_campaign=export_email")) - end end describe "#tag_moderator_confirmation_email" do diff --git a/spec/refinements/string_to_boolean_spec.rb b/spec/refinements/string_to_boolean_spec.rb new file mode 100644 index 000000000..4d6fae4ca --- /dev/null +++ b/spec/refinements/string_to_boolean_spec.rb @@ -0,0 +1,10 @@ +require "rails_helper" + +RSpec.describe StringToBoolean, type: :refinement do + describe "#to_boolean" do + using described_class + it "converts a string to a boolean" do + expect("true".to_boolean).to be true + end + end +end diff --git a/spec/requests/admin/users_spec.rb b/spec/requests/admin/users_spec.rb index 77a3ade19..b79823b67 100644 --- a/spec/requests/admin/users_spec.rb +++ b/spec/requests/admin/users_spec.rb @@ -11,7 +11,7 @@ RSpec.describe "admin/users", type: :request do sign_in(admin) end - describe "GETS /admin/users" do + describe "GET /admin/users" do it "renders to appropriate page" do get "/admin/users" expect(response.body).to include(user.username) @@ -149,4 +149,12 @@ RSpec.describe "admin/users", type: :request do end.to change { user.reload.access_locked? }.from(true).to(false) end end + + describe "POST admin/users/:id/export_data" do + it "redirects properly to the user edit page" do + sign_in admin + post export_data_admin_user_path(user), params: { send_to_admin: "true" } + expect(response).to redirect_to edit_admin_user_path(user) + end + end end diff --git a/spec/services/exporter/service_spec.rb b/spec/services/exporter/service_spec.rb index 6e2581fa2..488cdb232 100644 --- a/spec/services/exporter/service_spec.rb +++ b/spec/services/exporter/service_spec.rb @@ -47,14 +47,14 @@ RSpec.describe Exporter::Service, type: :service do describe "#export" do it "exports a zip file with files" do service = valid_instance(article.user) - zipped_exports = service.export + zipped_exports = service.export(article.user.email) exports = extract_zipped_exports(zipped_exports) expect(exports.keys).to eq(["articles.json", "comments.json"]) end it "passes configuration to an exporter" do service = valid_instance(article.user) - zipped_exports = service.export(config: { articles: { slug: article.slug } }) + zipped_exports = service.export(article.user.email, config: { articles: { slug: article.slug } }) exports = extract_zipped_exports(zipped_exports) expect(exports.length).to eq(described_class::EXPORTERS.size) end @@ -64,20 +64,26 @@ RSpec.describe Exporter::Service, type: :service do config = double allow(config).to receive(:fetch).with(:articles, {}).and_return(slug: article.slug) allow(config).to receive(:fetch).with(:comments, {}).and_return({}) - service.export(config: config) + service.export(article.user.email, config: config) expect(config).to have_received(:fetch).with(:articles, {}) end context "when emailing the user" do it "delivers one email" do service = valid_instance(article.user) - service.export(send_email: true) + service.export(article.user.email) expect(ActionMailer::Base.deliveries.count).to eq(1) end + it "delivers one email to the user's email" do + service = valid_instance(article.user) + service.export(article.user.email) + expect(ActionMailer::Base.deliveries.last.to.first).to eq article.user.email + end + it "delivers an email with the export" do service = valid_instance(article.user) - zipped_export = service.export(send_email: true) + zipped_export = service.export(article.user.email) attachment = ActionMailer::Base.deliveries.last.attachments[0].decoded exports = extract_zipped_exports(zipped_export) @@ -85,16 +91,26 @@ RSpec.describe Exporter::Service, type: :service do end end + context "when emailing an admin" do + it "delivers one email to the default admin email" do + admin_email = "admin@example.com" + allow(SiteConfig).to receive(:email_addresses).and_return({ default: admin_email }) + service = valid_instance(article.user) + service.export(admin_email) + expect(ActionMailer::Base.deliveries.last.to.first).to eq admin_email + end + end + it "sets the requested flag as false" do service = valid_instance(article.user) - service.export + service.export(article.user.email) expect(user.export_requested).to be(false) end it "sets the exported at datetime as the current one" do Timecop.freeze(Time.current) do service = valid_instance(article.user) - service.export + service.export(article.user.email) expect(user.exported_at).to eq(Time.current) end end diff --git a/spec/workers/export_content_worker_spec.rb b/spec/workers/export_content_worker_spec.rb index 7d6d20e2a..0f49d1c01 100644 --- a/spec/workers/export_content_worker_spec.rb +++ b/spec/workers/export_content_worker_spec.rb @@ -15,12 +15,12 @@ RSpec.describe ExportContentWorker, type: :worker do end it "calls the service" do - worker.perform(user.id) + worker.perform(user.id, user.email) expect(exporter_service).to have_received(:export).once end it "doesn't call the service if non existent user ID is given" do - worker.perform(9999) + worker.perform(9999, user.email) expect(exporter_service).not_to have_received(:export) end end
ID