Remove admin_member_view Feature Flag-Related Conditionals (#16764)

* Removes admin_member_view feature flags and refactors  conditionals

* Removes unused admin/users/_email_tools partial and user_email_tools_spec

* Actually remove the admin/user_email_tools_spec this time

* Removes the edit route from admin.rb users

* Adds a DUS (and spec) to disable and remove admin_member_view flag

* Fixes admin/users_spec.rb failures by updating spec with Member Detail changes

* Fixes spec failures within admin_bans_or_warns_user_spec.rb

* Refactors Admin::UsersController#credit_params due to failing specs

* Removes last traces of edit_admin_user_path and fixes buttons, specs, etc.
This commit is contained in:
Julianna Tetreault 2022-03-10 08:29:05 -07:00 committed by GitHub
parent dbe1fb6ff3
commit aa44786ebd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 136 additions and 269 deletions

View file

@ -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

View file

@ -15,10 +15,10 @@
<div class="ml-auto shrink-0">
<a href="<%= article.path %>/mod" class="btn btn-secondary btn-sm">Mod</a>
<a href="<%= article.path %>/moderate" class="btn btn-secondary btn-sm">admin</a>
<a href="<%= article.path %>/moderate" class="btn btn-secondary btn-sm">Admin</a>
<a href="<%= article.user.path %>" class="btn btn-secondary btn-sm">Profile</a>
<%= 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" %>
</div>
</div>
<% end %>

View file

@ -12,8 +12,8 @@
<div class="py-2 flex">
<a href="<%= user.path %>">@<%= user.username %></a> - <%= user.name %>
<div class="ml-auto shrink-0">
<%= 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" %>
</div>
</div>
<% end %>

View file

@ -51,7 +51,7 @@
<% @listings.each do |listing| %>
<tr>
<td><%= link_to listing.title, edit_admin_listing_path(listing.id), rel: "noopener" %></td>
<td><%= link_to listing.user.username, edit_admin_user_path(listing.user.id) %></td>
<td><%= link_to listing.user.username, admin_user_path(listing.user.id) %></td>
<td><%= link_to listing.organization.name, admin_organization_path(listing.organization_id) if listing.organization_id.present? %></td>
<td><%= listing.category %></td>
<td><%= listing.cached_tag_list %></td>

View file

@ -1,37 +0,0 @@
<div class="crayons-card p-6 grid gap-6" id="user-email-tools">
<h2>Email Tools</h2>
<div>
<%= 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 %>
</div>
<div>
<h3>Email <%= @user.name %></h3>
<%= form_with url: send_email_admin_user_path(@user), local: true do |f| %>
<%= f.hidden_field(:user_id, value: @user.id) %>
<h4>Subject:</h4>
<%= f.text_field :email_subject, class: "form-control my-1" %>
<h4>Body:</h4>
<%= f.text_area :email_body, class: "form-control my-1", rows: "10" %>
<%= f.submit "Send Email", class: "btn btn-primary float-right" %>
<% end %>
</div>
<div>
<h3>Recent Emails</h3>
<div class="list-group-flush">
<% @user.email_messages.order(sent_at: :desc).limit(50).each do |email| %>
<a href="<%= admin_user_email_message_path(@user, email) %>"
class="list-group-item list-group-item-action"
rel="noopener noreferer"
target="_blank">
<%= email.subject %>
<em> - <%= email.sent_at&.strftime("%b %e '%y") %></em>
</a>
<% end %>
</div>
</div>
</div>

View file

@ -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" %>
<div class="crayons-card p-3 s:p-4 m:p-7">
<%= render "admin/users/show/overview/stats" if @user.registered %>
<% if @current_tab == "overview" %>
<div class="crayons-card p-3 s:p-4 m:p-7">
<%= render "admin/users/show/overview/stats" if @user.registered %>
<div class="l:grid gap-8 grid-cols-2">
<div>
<section class="mb-7">
<%= render "admin/users/show/overview/roles" %>
</section>
<div class="l:grid gap-8 grid-cols-2">
<div>
<section class="mb-7">
<%= render "admin/users/show/overview/roles" %>
</section>
<section class="mb-7">
<%= render "admin/users/show/overview/tag_moderation" %>
</section>
</div>
<section class="mb-7">
<%= render "admin/users/show/overview/tag_moderation" %>
</section>
</div>
<div>
<section class="mb-7">
<%= render "admin/users/show/overview/organizations" %>
</section>
<div>
<section class="mb-7">
<%= render "admin/users/show/overview/organizations" %>
</section>
<section class="mb-7">
<%= render "admin/users/show/overview/credits" %>
</section>
</div>
<section class="mb-7">
<%= render "admin/users/show/overview/credits" %>
</section>
</div>
</div>
<% elsif @current_tab == "notes" %>
<section class="flex flex-col m:flex-row gap-4">
<div class="crayons-card p-3 s:p-4 m:p-7 flex-1">
<%= render "admin/users/show/notes/form" %>
</div>
<div class="crayons-card p-3 s:p-4 m:p-7 flex-1">
<%= render "admin/users/show/notes/index" %>
</div>
</section>
<% elsif @current_tab == "emails" %>
<%= render "admin/users/show/emails/verification" %>
<section class="flex flex-col m:flex-row gap-4">
<div class="crayons-card p-3 s:p-4 m:p-7 flex-1">
<%= render "admin/users/show/emails/form" %>
</div>
<div class="crayons-card p-3 s:p-4 m:p-7 flex-1">
<%= render "admin/users/show/emails/index" %>
</div>
</section>
<% elsif @current_tab == "reports" %>
<section class="crayons-card p-3 s:p-4 m:p-7">
<%= render "admin/users/show/reports/index" %>
</section>
<% elsif @current_tab == "flags" %>
<section class="crayons-card p-3 s:p-4 m:p-7">
<%= render "admin/users/show/flags/index" %>
</section>
<% end %>
<% else %>
<div class="grid gap-6">
<div class="crayons-card p-6">
<h2 class="d-inline">
<% 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 %>
</h2>
<% if @user.registered? %>
<a href="<%= edit_admin_user_path(@user) %>" class="btn btn-primary float-right">Manage User</a>
<p class="font-italic">Member since <%= @user.created_at.strftime("%b %e '%y") %></p>
<% end %>
<hr>
<h2>General Info</h2>
<dl>
<dt>ID:</dt>
<dd><%= @user.id %></dd>
<dt>Username:</dt>
<dd><%= @user.username %></dd>
<dt>Name:</dt>
<dd><%= @user.name %></dd>
<dt>Email:</dt>
<dd><%= @user.email %></dd>
<dt>Access Locked:</dt>
<dd><%= @user.access_locked? %></dd>
<dt>Twitter:</dt>
<% if @user.twitter_username %>
<dd><%= link_to @user.twitter_username, "https://twitter.com/#{@user.twitter_username}" %></dd>
<% else %>
<dd>N/A</dd>
<% end %>
<dt>GitHub:</dt>
<% if @user.github_username %>
<dd><%= link_to @user.github_username, "https://github.com/#{@user.github_username}" %></dd>
<% else %>
<dd>N/A</dd>
<% end %>
</dl>
</div>
<% elsif @current_tab == "notes" %>
<section class="flex flex-col m:flex-row gap-4">
<div class="crayons-card p-3 s:p-4 m:p-7 flex-1">
<%= render "admin/users/show/notes/form" %>
</div>
<% if @user.registered? %>
<%= render "activity" %>
<%= render "credits" %>
<%= render "notes" %>
<%= render "user_organizations" %>
<%= render "email_tools" %>
<% end %>
<div class="crayons-card p-3 s:p-4 m:p-7 flex-1">
<%= render "admin/users/show/notes/index" %>
</div>
</section>
<% elsif @current_tab == "emails" %>
<%= render "admin/users/show/emails/verification" %>
<section class="flex flex-col m:flex-row gap-4">
<div class="crayons-card p-3 s:p-4 m:p-7 flex-1">
<%= render "admin/users/show/emails/form" %>
</div>
<div class="crayons-card p-3 s:p-4 m:p-7 flex-1">
<%= render "admin/users/show/emails/index" %>
</div>
</section>
<% elsif @current_tab == "reports" %>
<section class="crayons-card p-3 s:p-4 m:p-7">
<%= render "admin/users/show/reports/index" %>
</section>
<% else @current_tab == "flags" %>
<section class="crayons-card p-3 s:p-4 m:p-7">
<%= render "admin/users/show/flags/index" %>
</section>
<% end %>

View file

@ -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

View file

@ -0,0 +1,8 @@
module DataUpdateScripts
class RemoveAdminMemberViewFeatureFlag
def run
FeatureFlag.disable(:admin_member_view)
FeatureFlag.remove(:admin_member_view)
end
end
end

View file

@ -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

View file

@ -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

View file

@ -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 \\:/",

View file

@ -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

View file

@ -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."

View file

@ -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