Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AO3-6467 Record Fannish Next of Kin changes in user history #4582

Merged
merged 32 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
b0b195b
Adding
ceithir Jul 17, 2023
8c9f826
Remove
ceithir Jul 18, 2023
dff308c
Replace many ifelse with case
ceithir Jul 18, 2023
e0d0e67
Display logs on admin page
ceithir Jul 18, 2023
78c03d9
Forgot to log admin identity
ceithir Jul 18, 2023
26a3f0d
Also show logs on the FNOK side
ceithir Jul 18, 2023
cb63e18
Test for display
ceithir Jul 18, 2023
7ca5241
Add Alterity
ceithir Jul 18, 2023
e4986ea
Forgot to commit previously
ceithir Jul 18, 2023
8a7edd9
Hound
ceithir Jul 18, 2023
23b89f3
Hound (bis)
ceithir Jul 18, 2023
aef77f6
Hound (ter)
ceithir Jul 18, 2023
ed3111a
Add i18n keys to exceptions
ceithir Jul 18, 2023
77231d6
Remove Alterity actually
ceithir Aug 8, 2023
3b80613
Merge remote-tracking branch 'upstream/master' into AO3-6467
ceithir Aug 8, 2023
b892b72
Add pt-online-schema-change syntax for prod
ceithir Aug 8, 2023
f8c67b7
Remove explicit foreign key
ceithir Aug 8, 2023
972432b
Implicit NULL default value
ceithir Aug 8, 2023
237508e
Merge remote-tracking branch 'upstream/master' into AO3-6467
ceithir Aug 21, 2023
082f483
Test id too
ceithir Aug 21, 2023
857acba
Add user destroy controller test
ceithir Aug 21, 2023
365896b
Log automatic removal of fnok on deletion
ceithir Aug 21, 2023
7bb875c
Hound
ceithir Aug 21, 2023
b8ead04
Use classical I18n syntax
ceithir Aug 21, 2023
2ec994a
Simplify fnok name action
ceithir Aug 21, 2023
c47cc76
Clean other I18n keys
ceithir Aug 21, 2023
e5ad52e
Hound
ceithir Aug 21, 2023
f394e18
reviewdog
ceithir Aug 21, 2023
270272c
Normalize i18n
ceithir Aug 21, 2023
1f6faff
Syntax more compliant with i18n-tasks
ceithir Aug 22, 2023
3e327e7
Wordings
ceithir Aug 24, 2023
af8114b
Explicit over succinct
ceithir Aug 24, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ gem 'unicorn', '~> 5.5', require: false
gem 'god', '~> 0.13.7'

group :staging, :production do
gem "alterity"
ceithir marked this conversation as resolved.
Show resolved Hide resolved
# Place the New Relic gem as low in the list as possible, allowing the
# frameworks above it to be instrumented when the gem initializes.
gem "newrelic_rpm"
Expand Down
4 changes: 4 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ GEM
activerecord (>= 4.2)
activesupport
akismetor (1.0.0)
alterity (1.4.2)
mysql2 (>= 0.3)
rails (>= 6.1)
ast (2.4.2)
audited (4.10.0)
activerecord (>= 4.2, < 6.2)
Expand Down Expand Up @@ -608,6 +611,7 @@ DEPENDENCIES
addressable
after_commit_everywhere
akismetor
alterity
audited (~> 4.4)
awesome_print
aws-sdk-s3
Expand Down
20 changes: 18 additions & 2 deletions app/controllers/admin/admin_users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def show
@user = authorize User.find_by!(login: params[:id])
@hide_dashboard = true
@page_subtitle = t(".page_title", login: @user.login)
@log_items = @user.log_items.sort_by(&:created_at).reverse
log_items
end

# POST admin/users/update
Expand Down Expand Up @@ -75,6 +75,12 @@ def update_next_of_kin
if kin.blank? && kin_email.blank?
if fnok.present?
fnok.destroy
@user.create_log_item({
action: ArchiveConfig.ACTION_REMOVE_FNOK,
fnok_user_id: fnok.kin.id,
admin_id: current_admin.id,
note: "Change made by #{current_admin.login}"
})
flash[:notice] = ts("Fannish next of kin was removed.")
end
redirect_to admin_user_path(@user)
Expand All @@ -84,11 +90,17 @@ def update_next_of_kin
fnok = @user.build_fannish_next_of_kin if fnok.blank?
fnok.assign_attributes(kin: kin, kin_email: kin_email)
if fnok.save
@user.create_log_item({
action: ArchiveConfig.ACTION_ADD_FNOK,
fnok_user_id: fnok.kin.id,
admin_id: current_admin.id,
note: "Change made by #{current_admin.login}"
})
flash[:notice] = ts("Fannish next of kin was updated.")
redirect_to admin_user_path(@user)
else
@hide_dashboard = true
@log_items = @user.log_items.sort_by(&:created_at).reverse
log_items
render :show
end
end
Expand Down Expand Up @@ -168,4 +180,8 @@ def activate
redirect_to action: :show
end
end

def log_items
@log_items ||= (@user.log_items + LogItem.where(fnok_user_id: @user.id)).sort_by(&:created_at).reverse
end
end
85 changes: 60 additions & 25 deletions app/helpers/users_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,31 +119,66 @@ def authored_items(pseud, work_counts = {}, rec_counts = {})
items.html_safe
end

def log_item_action_name(action)
if action == ArchiveConfig.ACTION_ACTIVATE
t('users_helper.log_validated', default: 'Account Validated')
elsif action == ArchiveConfig.ACTION_ADD_ROLE
t('users_helper.log_role_added', default: 'Role Added: ')
elsif action == ArchiveConfig.ACTION_REMOVE_ROLE
t('users_helper.log_role_removed', default: 'Role Removed: ')
elsif action == ArchiveConfig.ACTION_SUSPEND
t('users_helper.log_suspended', default: 'Suspended until ')
elsif action == ArchiveConfig.ACTION_UNSUSPEND
t('users_helper.log_lift_suspension', default: 'Suspension Lifted')
elsif action == ArchiveConfig.ACTION_BAN
t('users_helper.log_ban', default: 'Suspended Permanently')
elsif action == ArchiveConfig.ACTION_WARN
t('users_helper.log_warn', default: 'Warned')
elsif action == ArchiveConfig.ACTION_RENAME
t('users_helper.log_rename', default: 'Username Changed')
elsif action == ArchiveConfig.ACTION_PASSWORD_RESET
t('users_helper.log_password_change', default: 'Password Changed')
elsif action == ArchiveConfig.ACTION_NEW_EMAIL
t('users_helper.log_email_change', default: 'Email Changed')
elsif action == ArchiveConfig.ACTION_TROUBLESHOOT
t('users_helper.log_troubleshot', default: 'Account Troubleshot')
elsif action == ArchiveConfig.ACTION_NOTE
t('users_helper.log_note', default: 'Note Added')
def log_item_action_name(item, user)
action = item.action

if [ArchiveConfig.ACTION_ADD_FNOK, ArchiveConfig.ACTION_REMOVE_FNOK].include?(action)
if item.fnok_user_id == user.id
if action == ArchiveConfig.ACTION_REMOVE_FNOK
return t(
"users_helper.log_removed_as_fnok",
default: "Removed as Fannish Next of Kin for: %{user_id}",
user_id: item.user_id
)
end

return t(
"users_helper.log_added_as_fnok",
default: "Added as Fannish Next of Kin for: %{user_id}",
user_id: item.user_id
)
end

if action == ArchiveConfig.ACTION_REMOVE_FNOK
return t(
"users_helper.log_remove_fnok",
default: "Fannish Next of Kin Removed: %{user_id}",
user_id: item.fnok_user_id
)
end

return t(
"users_helper.log_add_fnok",
default: "Fannish Next of Kin Added: %{user_id}",
user_id: item.fnok_user_id
)
end
ceithir marked this conversation as resolved.
Show resolved Hide resolved

case action
ceithir marked this conversation as resolved.
Show resolved Hide resolved
when ArchiveConfig.ACTION_ACTIVATE
t("users_helper.log_validated", default: "Account Validated")
when ArchiveConfig.ACTION_ADD_ROLE
t("users_helper.log_role_added", default: "Role Added: ")
when ArchiveConfig.ACTION_REMOVE_ROLE
t("users_helper.log_role_removed", default: "Role Removed: ")
when ArchiveConfig.ACTION_SUSPEND
t("users_helper.log_suspended", default: "Suspended until ")
when ArchiveConfig.ACTION_UNSUSPEND
t("users_helper.log_lift_suspension", default: "Suspension Lifted")
when ArchiveConfig.ACTION_BAN
t("users_helper.log_ban", default: "Suspended Permanently")
when ArchiveConfig.ACTION_WARN
t("users_helper.log_warn", default: "Warned")
when ArchiveConfig.ACTION_RENAME
t("users_helper.log_rename", default: "Username Changed")
when ArchiveConfig.ACTION_PASSWORD_RESET
t("users_helper.log_password_change", default: "Password Changed")
when ArchiveConfig.ACTION_NEW_EMAIL
t("users_helper.log_email_change", default: "Email Changed")
when ArchiveConfig.ACTION_TROUBLESHOOT
t("users_helper.log_troubleshot", default: "Account Troubleshot")
when ArchiveConfig.ACTION_NOTE
t("users_helper.log_note", default: "Note Added")
end
end

Expand Down
2 changes: 2 additions & 0 deletions app/models/log_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ class LogItem < ApplicationRecord

belongs_to :role

belongs_to :fnok_user, class_name: "User"

validates_presence_of :note
validates_presence_of :action

Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/admin_users/_user_history.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
@log_items.each do |item| %>
<tr>
<td><%= item.created_at %></td>
<td><%= log_item_action_name(item.action) %><%= item.role.name if item.role %><%= item.enddate if item.enddate %></td>
<td><%= log_item_action_name(item, @user) %><%= item.role.name if item.role %><%= item.enddate if item.enddate %></td>
<td><%= item.note %></td>
</tr>
<% end %>
Expand Down
2 changes: 2 additions & 0 deletions config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,8 @@ ACTION_PASSWORD_RESET: 8
ACTION_NEW_EMAIL: 9
ACTION_TROUBLESHOOT: 10
ACTION_NOTE: 11
ACTION_ADD_FNOK: 12
ACTION_REMOVE_FNOK: 13


# Elasticsearch index prefix
Expand Down
4 changes: 4 additions & 0 deletions config/i18n-tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ ignore_missing:
- users_helper.log_troubleshot
- users_helper.log_validated
- users_helper.log_warn
- users_helper.log_add_fnok
- users_helper.log_remove_fnok
- users_helper.log_added_as_fnok
- users_helper.log_removed_as_fnok
ceithir marked this conversation as resolved.
Show resolved Hide resolved
# Files: app/views/admin/_admin_nav.html.erb and app/views/admin_posts/show.html.erb
- admin.admin_nav.delete
# File: app/views/admin/admin_invitations/find.html.erb
Expand Down
11 changes: 11 additions & 0 deletions db/migrate/20230717161221_add_fnok_user_id_to_log_items.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class AddFnokUserIdToLogItems < ActiveRecord::Migration[6.1]
def up
add_column :log_items, :fnok_user_id, :integer, nullable: true, default: nil
add_index :log_items, :fnok_user_id
add_foreign_key :log_items, :users, column: :fnok_user_id
end

def down
remove_column :log_items, :fnok_user_id
end
end
9 changes: 9 additions & 0 deletions factories/fannish_next_of_kin.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
require "faker"

FactoryBot.define do
factory :fannish_next_of_kin do
user { create(:user) }
kin { create(:user) }
kin_email { |u| u.kin.email }
end
end
7 changes: 7 additions & 0 deletions features/admins/users/admin_fnok.feature
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Feature: Admin Fannish Next Of Kind actions
And I fill in "Fannish next of kin's email" with "[email protected]"
And I press "Update Fannish Next of Kin"
Then I should see "Fannish next of kin was updated."
Then I should see "Fannish Next of Kin Added"
ceithir marked this conversation as resolved.
Show resolved Hide resolved

When I go to the manage users page
And I fill in "Name" with "harrykim"
Expand All @@ -24,6 +25,9 @@ Feature: Admin Fannish Next Of Kind actions
When I follow "libby"
Then I should be on libby's user page

When I go to the user administration page for "libby"
Then I should see "Added as Fannish Next of Kin"

Scenario: An invalid Fannish Next of Kin username is added
Given the fannish next of kin "libby" for the user "harrykim"
And I am logged in as a "support" admin
Expand Down Expand Up @@ -66,6 +70,9 @@ Feature: Admin Fannish Next Of Kind actions
And I fill in "Fannish next of kin's email" with ""
And I press "Update Fannish Next of Kin"
Then I should see "Fannish next of kin was removed."
And I should see "Fannish Next of Kin Removed"
When I go to the user administration page for "libby"
Then I should see "Removed as Fannish Next of Kin"

Scenario: A Fannish Next of Kin updates when the next of kin user changes their username
Given the fannish next of kin "libby" for the user "harrykim"
Expand Down
33 changes: 33 additions & 0 deletions spec/controllers/admin/admin_users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,39 @@
it_behaves_like "authorized admin can add next of kin"
end
end

it "logs adding a fannish next of kin" do
admin = create(:support_admin)
fake_login_admin(admin)

post :update_next_of_kin, params: {
user_login: user.login, next_of_kin_name: kin.login, next_of_kin_email: kin.email
}
user.reload
expect(user.fannish_next_of_kin.kin).to eq(kin)
log_item = user.log_items.last
expect(log_item.action).to eq(ArchiveConfig.ACTION_ADD_FNOK)
expect(log_item.fnok_user.id).to eq(kin.id)
expect(log_item.admin_id).to eq(admin.id)
expect(log_item.note).to eq("Change made by #{admin.login}")
end

it "logs removing a fannish next of kin" do
admin = create(:support_admin)
fake_login_admin(admin)
kin_user_id = create(:fannish_next_of_kin, user: user).kin_id

post :update_next_of_kin, params: {
user_login: user.login
}
user.reload
expect(user.fannish_next_of_kin).to be_nil
log_item = user.log_items.last
expect(log_item.action).to eq(ArchiveConfig.ACTION_REMOVE_FNOK)
expect(log_item.fnok_user.id).to eq(kin_user_id)
expect(log_item.admin_id).to eq(admin.id)
expect(log_item.note).to eq("Change made by #{admin.login}")
end
end

describe "POST #update_status" do
Expand Down