diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index d127af2ba..bc7ff0c08 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -35,12 +35,9 @@ module Admin def show @user = User.find(params[:id]) - - if FeatureFlag.enabled?(:admin_member_view) - set_current_tab(params[:tab]) - set_feedback_messages - set_related_reactions - end + set_current_tab(params[:tab]) + set_feedback_messages + set_related_reactions set_user_details end @@ -286,19 +283,18 @@ module Admin end def credit_params - return user_params unless FeatureFlag.enabled?(:admin_member_view) - credit_params = {} - if user_params[:credit_action] == "Add" + + case user_params[:credit_action] + when "Add" credit_params[:add_credits] = user_params[:credit_amount] flash[:success] = "Credits have been added!" - end - - if user_params[:credit_action] == "Remove" + when "Remove" credit_params[:remove_credits] = user_params[:credit_amount] flash[:success] = "Credits have been removed." + else + return user_params end - credit_params end diff --git a/app/views/admin/feedback_messages/_latest_articles.html.erb b/app/views/admin/feedback_messages/_latest_articles.html.erb index ae57a499b..c370fd2e1 100644 --- a/app/views/admin/feedback_messages/_latest_articles.html.erb +++ b/app/views/admin/feedback_messages/_latest_articles.html.erb @@ -15,10 +15,10 @@
Mod - admin + Admin Profile - <%= link_to "admin User", admin_user_path(article.user_id), class: "btn btn-secondary btn-sm" %> - <%= link_to "Destructive Actions", edit_admin_user_path(article.user_id), class: "btn btn-danger btn-sm" %> + <%= link_to "Admin User", admin_user_path(article.user_id), class: "btn btn-secondary btn-sm" %> + <%= link_to "Destructive Actions", admin_user_path(article.user_id), class: "btn btn-danger btn-sm" %>
<% end %> diff --git a/app/views/admin/feedback_messages/_potential_spam_users.html.erb b/app/views/admin/feedback_messages/_potential_spam_users.html.erb index 3530e7d3a..f0961a5c6 100644 --- a/app/views/admin/feedback_messages/_potential_spam_users.html.erb +++ b/app/views/admin/feedback_messages/_potential_spam_users.html.erb @@ -12,8 +12,8 @@
@<%= user.username %> - <%= user.name %>
- <%= link_to "admin User", admin_user_path(user.id), class: "btn btn-secondary btn-sm" %> - <%= link_to "Destructive Actions", edit_admin_user_path(user.id), class: "btn btn-danger btn-sm" %> + <%= link_to "Admin User", admin_user_path(user.id), class: "btn btn-secondary btn-sm" %> + <%= link_to "Destructive Actions", admin_user_path(user.id), class: "btn btn-danger btn-sm" %>
<% end %> diff --git a/app/views/admin/listings/index.html.erb b/app/views/admin/listings/index.html.erb index 77f5f8f02..e8b7b6e29 100644 --- a/app/views/admin/listings/index.html.erb +++ b/app/views/admin/listings/index.html.erb @@ -51,7 +51,7 @@ <% @listings.each do |listing| %> <%= link_to listing.title, edit_admin_listing_path(listing.id), rel: "noopener" %> - <%= link_to listing.user.username, edit_admin_user_path(listing.user.id) %> + <%= link_to listing.user.username, admin_user_path(listing.user.id) %> <%= link_to listing.organization.name, admin_organization_path(listing.organization_id) if listing.organization_id.present? %> <%= listing.category %> <%= listing.cached_tag_list %> diff --git a/app/views/admin/users/_email_tools.html.erb b/app/views/admin/users/_email_tools.html.erb deleted file mode 100644 index f116d7fd4..000000000 --- a/app/views/admin/users/_email_tools.html.erb +++ /dev/null @@ -1,37 +0,0 @@ -
-

Email Tools

- -
- <%= form_with url: verify_email_ownership_admin_user_path(@user), local: true do |f| %> - Email last verified at: <%= @last_email_verification_date || "Never" %> - <%= f.submit "Verify Email Ownership", class: "btn btn-primary float-right" %> - <% end %> -
- -
-

Email <%= @user.name %>

- <%= form_with url: send_email_admin_user_path(@user), local: true do |f| %> - <%= f.hidden_field(:user_id, value: @user.id) %> -

Subject:

- <%= f.text_field :email_subject, class: "form-control my-1" %> -

Body:

- <%= f.text_area :email_body, class: "form-control my-1", rows: "10" %> - <%= f.submit "Send Email", class: "btn btn-primary float-right" %> - <% end %> -
- -
-

Recent Emails

-
- <% @user.email_messages.order(sent_at: :desc).limit(50).each do |email| %> - - <%= email.subject %> - - <%= email.sent_at&.strftime("%b %e '%y") %> - - <% end %> -
-
-
diff --git a/app/views/admin/users/show.html.erb b/app/views/admin/users/show.html.erb index 103248942..bd0309f35 100644 --- a/app/views/admin/users/show.html.erb +++ b/app/views/admin/users/show.html.erb @@ -1,110 +1,57 @@ -<% if FeatureFlag.enabled?(:admin_member_view) %> - <%= render "admin/users/show/profile" %> - <%= render "admin/users/show/tabs" %> +<%= render "admin/users/show/profile" %> +<%= render "admin/users/show/tabs" %> - <% if @current_tab == "overview" %> -
- <%= render "admin/users/show/overview/stats" if @user.registered %> +<% if @current_tab == "overview" %> +
+ <%= render "admin/users/show/overview/stats" if @user.registered %> -
-
-
- <%= render "admin/users/show/overview/roles" %> -
+
+
+
+ <%= render "admin/users/show/overview/roles" %> +
-
- <%= render "admin/users/show/overview/tag_moderation" %> -
-
+
+ <%= render "admin/users/show/overview/tag_moderation" %> +
+
-
-
- <%= render "admin/users/show/overview/organizations" %> -
+
+
+ <%= render "admin/users/show/overview/organizations" %> +
-
- <%= render "admin/users/show/overview/credits" %> -
-
+
+ <%= render "admin/users/show/overview/credits" %> +
- <% elsif @current_tab == "notes" %> -
-
- <%= render "admin/users/show/notes/form" %> -
-
- <%= render "admin/users/show/notes/index" %> -
-
- <% elsif @current_tab == "emails" %> - <%= render "admin/users/show/emails/verification" %> -
-
- <%= render "admin/users/show/emails/form" %> -
-
- <%= render "admin/users/show/emails/index" %> -
-
- <% elsif @current_tab == "reports" %> -
- <%= render "admin/users/show/reports/index" %> -
- <% elsif @current_tab == "flags" %> -
- <%= render "admin/users/show/flags/index" %> -
- <% end %> -<% else %> -
-
-

- <% if @user.registered? %> - <%= @user.name %><%= link_to "@#{@user.username}", @user.path, class: "ml-2", target: "_blank", rel: "noopener" %> - <% else %> - @<%= @user.username %> has not accepted their invitation yet. - <% end %> -

- <% if @user.registered? %> - Manage User -

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

- <% end %> - -
- -

General Info

-
-
ID:
-
<%= @user.id %>
-
Username:
-
<%= @user.username %>
-
Name:
-
<%= @user.name %>
-
Email:
-
<%= @user.email %>
-
Access Locked:
-
<%= @user.access_locked? %>
-
Twitter:
- <% if @user.twitter_username %> -
<%= link_to @user.twitter_username, "https://twitter.com/#{@user.twitter_username}" %>
- <% else %> -
N/A
- <% end %> -
GitHub:
- <% if @user.github_username %> -
<%= link_to @user.github_username, "https://github.com/#{@user.github_username}" %>
- <% else %> -
N/A
- <% end %> -
+
+<% elsif @current_tab == "notes" %> +
+
+ <%= render "admin/users/show/notes/form" %>
- - <% if @user.registered? %> - <%= render "activity" %> - <%= render "credits" %> - <%= render "notes" %> - <%= render "user_organizations" %> - <%= render "email_tools" %> - <% end %> +
+ <%= render "admin/users/show/notes/index" %> +
+
+<% elsif @current_tab == "emails" %> + <%= render "admin/users/show/emails/verification" %> +
+
+ <%= render "admin/users/show/emails/form" %> +
+
+ <%= render "admin/users/show/emails/index" %> +
+
+<% elsif @current_tab == "reports" %> +
+ <%= render "admin/users/show/reports/index" %> +
+<% else @current_tab == "flags" %> +
+ <%= render "admin/users/show/flags/index" %> +
<% end %> diff --git a/config/routes/admin.rb b/config/routes/admin.rb index c2f16a37d..526a120bb 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -43,7 +43,7 @@ namespace :admin do resources :gdpr_delete_requests, only: %i[index destroy] end - resources :users, only: %i[index show edit update destroy] do + resources :users, only: %i[index show update destroy] do resources :email_messages, only: :show member do diff --git a/lib/data_update_scripts/20220309194124_remove_admin_member_view_feature_flag.rb b/lib/data_update_scripts/20220309194124_remove_admin_member_view_feature_flag.rb new file mode 100644 index 000000000..a9452bae4 --- /dev/null +++ b/lib/data_update_scripts/20220309194124_remove_admin_member_view_feature_flag.rb @@ -0,0 +1,8 @@ +module DataUpdateScripts + class RemoveAdminMemberViewFeatureFlag + def run + FeatureFlag.disable(:admin_member_view) + FeatureFlag.remove(:admin_member_view) + end + end +end diff --git a/spec/lib/data_update_scripts/remove_admin_member_view_feature_flag_spec.rb b/spec/lib/data_update_scripts/remove_admin_member_view_feature_flag_spec.rb new file mode 100644 index 000000000..9337d5869 --- /dev/null +++ b/spec/lib/data_update_scripts/remove_admin_member_view_feature_flag_spec.rb @@ -0,0 +1,28 @@ +require "rails_helper" +require Rails.root.join( + "lib/data_update_scripts/20220309194124_remove_admin_member_view_feature_flag.rb", +) + +describe DataUpdateScripts::RemoveAdminMemberViewFeatureFlag do + it "disables the :admin_member_view feature flag" do + FeatureFlag.enable(:admin_member_view) + + described_class.new.run + + expect(FeatureFlag.enabled?(:admin_member_view)).to be(false) + end + + it "removes the :admin_member_view feature flag" do + FeatureFlag.enable(:admin_member_view) + + described_class.new.run + + expect(FeatureFlag.exist?(:admin_member_view)).to be(false) + end + + it "works if the flag is not available" do + described_class.new.run + + expect(FeatureFlag.exist?(:admin_member_view)).to be(false) + end +end diff --git a/spec/requests/admin/users_spec.rb b/spec/requests/admin/users_spec.rb index e8fc2214f..fb2f62ed3 100644 --- a/spec/requests/admin/users_spec.rb +++ b/spec/requests/admin/users_spec.rb @@ -33,95 +33,66 @@ RSpec.describe "/admin/users", type: :request do end describe "GET /admin/users/:id" do - it "renders to appropriate page", :aggregate_failures do + it "renders to appropriate page" do get admin_user_path(user) expect(response.body).to include(user.username) - expect(response.body).not_to include("Go back to All members") end - context "when a user is unregistered" do - it "renders a message stating that the user isn't registered" do - user.update_columns(registered: false) - get admin_user_path(user.id) - expect(response.body).to include("@#{user.username} has not accepted their invitation yet.") - end - - it "only displays limited information about the user" do - user.update_columns(registered: false) - get admin_user_path(user.id) - expect(response.body).not_to include("Activity") - end - end - - context "when a user is registered" do - it "renders the Admin User profile as expected" do - get admin_user_path(user.id) - expect(response.body).to include("Activity") - end - end - - context "when a user has been sent an email" do - it "renders a link to the user email preview" do - email = create(:email_message, user: user, to: user.email) - get admin_user_path(user.id) - - preview_path = admin_user_email_message_path(user, email) - expect(response.body).to include(preview_path) - end - end - end - - describe "GET /admin/users/:id/edit" do it "redirects from /username/moderate" do get "/#{user.username}/moderate" expect(response).to redirect_to(admin_user_path(user.id)) end it "shows banish button for new users" do - get edit_admin_user_path(user.id) - expect(response.body).to include("Banish User for Spam!") + get admin_user_path(user.id) + expect(response.body).to include("Banish user") end it "does not show banish button for non-admins" do sign_out(admin) - expect { get edit_admin_user_path(user.id) }.to raise_error(Pundit::NotAuthorizedError) + expect { get admin_user_path(user.id) }.to raise_error(Pundit::NotAuthorizedError) end - it "displays the 'Current Roles' section" do - get edit_admin_user_path(user.id) - expect(response.body).to include("Current Roles") + it "displays a user's current roles in the 'Overview' tab" do + get admin_user_path(user.id) + expect(response.body).to include("Roles") end - it "displays the 'Recent Reactions' section" do - get edit_admin_user_path(user.id) - expect(response.body).to include("Recent Reactions") + it "displays a user's current roles in the 'Emails' tab" do + get "/admin/users/#{user.id}?tab=emails" + expect(response.body).to include("Previous emails") + end + + it "displays a user's current flags in the 'Flags' tab" do + get "/admin/users/#{user.id}?tab=flags" + expect(response.body).to include("Flags received") end it "displays a message when there are no related vomit reactions for a user" do - get edit_admin_user_path(user.id) - expect(response.body).to include("Nothing negative to see here! 👀") + get "/admin/users/#{user.id}?tab=flags" + expect(response.body).to include("No flags received against") end it "displays a list of recent related vomit reactions for a user if any exist" do vomit = build(:reaction, category: "vomit", user_id: user.id, reactable_type: "Article", status: "valid") - get edit_admin_user_path(user.id) + get admin_user_path(user.id) expect(response.body).to include(vomit.reactable_type) end - it "displays the 'Recent Reports' section" do - get edit_admin_user_path(user.id) - expect(response.body).to include("Recent Reports") + it "displays a user's current reports in the 'Reports' tab" do + get "/admin/users/#{user.id}?tab=reports" + expect(response.body).to include("Reports submitted by") end - it "displays a message when there are no related reports for a user" do - get edit_admin_user_path(user.id) - expect(response.body).to include("Nothing to report here! 👀") + it "displays a message when there are no current reports for a user" do + get "/admin/users/#{user.id}?tab=reports" + expect(response.body).to include("No comment or post has been reported yet.") end - it "displays a list of recent reports for a user if any exist" do + it "displays a list of current reports for a user if any exist" do report = build(:feedback_message, category: "spam", affected_id: user.id, feedback_type: "spam", status: "Open") - get edit_admin_user_path(user.id) + get admin_user_path(user.id) expect(response.body).to include(report.feedback_type) end end diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index 5237c051a..d0a2df6fb 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -68,11 +68,6 @@ admin_user = User.find_by(email: "admin@forem.local") ############################################################################## -# Enable Admin Member View feature flag for tests -FeatureFlag.enable(:admin_member_view) - -############################################################################## - seeder.create_if_doesnt_exist(User, "email", "trusted-user-1@forem.local") do user = User.create!( name: "Trusted User 1 \\:/", diff --git a/spec/system/admin/admin_bans_or_warns_user_spec.rb b/spec/system/admin/admin_bans_or_warns_user_spec.rb index 54b97a3cc..9794d929b 100644 --- a/spec/system/admin/admin_bans_or_warns_user_spec.rb +++ b/spec/system/admin/admin_bans_or_warns_user_spec.rb @@ -6,22 +6,22 @@ RSpec.describe "Admin bans user", type: :system do before do sign_in admin - visit edit_admin_user_path(user.id) + visit admin_user_path(user.id) end def suspend_user - visit edit_admin_user_path(user.id) + visit admin_user_path(user.id) select("Suspend", from: "user_user_status") fill_in("user_note_for_current_role", with: "something") - click_button("Update User Status") + click_button("Add") expect(page).to have_content("User has been updated") end def warn_user - visit edit_admin_user_path(user.id) + visit admin_user_path(user.id) select("Warn", from: "user_user_status") fill_in("user_note_for_current_role", with: "something") - click_button("Update User Status") + click_button("Add") expect(page).to have_content("User has been updated") end @@ -31,10 +31,10 @@ RSpec.describe "Admin bans user", type: :system do end def unsuspend_user - visit edit_admin_user_path(user.id) + visit admin_user_path(user.id) select("Regular Member", from: "user_user_status") fill_in("user_note_for_current_role", with: "good user") - click_button("Update User Status") + click_button("Add") expect(page).to have_content("User has been updated") end diff --git a/spec/system/admin/admin_deletes_user_spec.rb b/spec/system/admin/admin_deletes_user_spec.rb index e6f478326..0accf907e 100644 --- a/spec/system/admin/admin_deletes_user_spec.rb +++ b/spec/system/admin/admin_deletes_user_spec.rb @@ -6,12 +6,12 @@ RSpec.describe "Admin deletes user", type: :system do before do sign_in admin - visit edit_admin_user_path(user.id) + visit admin_user_path(user.id) end it "enqueues a job for deleting the user" do sidekiq_assert_enqueued_jobs(1, only: Users::DeleteWorker) do - click_button "☠️ Fully Delete User & All Activity ☠️" + click_button "Delete now" end message = "@#{user.username} (email: #{user.email}, user_id: #{user.id}) has been fully deleted." @@ -23,7 +23,7 @@ RSpec.describe "Admin deletes user", type: :system do user.update(email: nil) sidekiq_perform_enqueued_jobs do - click_button "☠️ Fully Delete User & All Activity ☠️" + click_button "Delete now" end message = "@#{user.username} (email: no email, user_id: #{user.id}) has been fully deleted." diff --git a/spec/system/admin/user_email_tools_spec.rb b/spec/system/admin/user_email_tools_spec.rb deleted file mode 100644 index 2188419fc..000000000 --- a/spec/system/admin/user_email_tools_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -require "rails_helper" - -RSpec.describe "Admin Email Tools within User management", type: :system do - let(:admin) { create(:user, :super_admin) } - let(:user) { create(:user) } - let(:email_subject) { "Email subject" } - let(:email_body) { "Email body" } - - context "when looking at a user with recorded emails in the admin panel" do - before do - allow(ForemInstance).to receive(:smtp_enabled?).and_return(true) - sign_in admin - visit admin_user_path(user) - - # Send the test user an email using the built in form - within(:css, "#user-email-tools") do - fill_in("email_subject", with: email_subject) - fill_in("email_body", with: email_body) - click_on("Send Email") - end - - visit admin_user_path(user) - end - - it "shows the email sent to the user" do - email_tools_section = find("#user-email-tools") - expect(email_tools_section).to have_text(email_subject) - end - - it "redirects and displays the email details" do - within(:css, "#user-email-tools") do - all("a.list-group-item-action").first.click - end - - expect(page).to have_content(email_subject) - expect(page).to have_content(email_body) - expect(page).to have_content("NotifyMailer#user_contact_email") - expect(page).to have_content(user.email) - end - end -end