Skip to content

Commit

Permalink
Code clean-up
Browse files Browse the repository at this point in the history
  • Loading branch information
nanego committed Aug 28, 2023
1 parent faa703d commit ae1da42
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 62 deletions.
4 changes: 2 additions & 2 deletions app/helpers/journal_settings_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def add_index_to_journal_for_history(journals)
position = @journal_count.to_i - (per_page * (page - 1))
journals.each do |journal|
journal.indice = position
position -=1
position -= 1
end
end

Expand Down Expand Up @@ -133,7 +133,7 @@ def journal_setting_to_strings(journal)
case type
when :boolean
strings << show_boolean_details(value[0], value[1][1], value[1][0])
#when ,if we want to treat another type
# when , if we want to treat another type
else
strings << show_details_by_default(value)
end
Expand Down
2 changes: 1 addition & 1 deletion config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,4 @@ en:
text_journal_create_user_journal_entry: "This user has been created %{label}."
label_manual_creation: manually
label_auto_creation: automatically
text_setting_auto_create_user_journal_entry: "User <i>%{user}</i> has been created automatically."
text_setting_auto_create_user_journal_entry: "User <i>%{user}</i> has been created automatically."
2 changes: 1 addition & 1 deletion config/locales/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,4 @@ fr:
text_journal_create_user_journal_entry: "Cet utilisateur a été créé %{label}."
label_manual_creation: manuellement
label_auto_creation: automatiquement
text_setting_auto_create_user_journal_entry: "L'utilisateur <i>%{user}</i> a été créé automatiquement."
text_setting_auto_create_user_journal_entry: "L'utilisateur <i>%{user}</i> a été créé automatiquement."
46 changes: 31 additions & 15 deletions lib/redmine_admin_activity/controllers/concerns/journalizable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,18 @@ def add_member_journal_entry(project:, value: nil, old_value: nil)
value = value.to_json if value
old_value = old_value.to_json if old_value
add_journal_entry project, JournalDetail.new(
property: 'members',
prop_key: prop_key,
value: value,
old_value: old_value)
property: 'members',
prop_key: prop_key,
value: value,
old_value: old_value)
end

def add_journal_entry_for_user(user:, property:, prop_key:, value: nil, old_value: nil, author: User.current)
add_journal_entry user, JournalDetail.new(
property: property,
prop_key: prop_key,
value: value,
old_value: old_value), author: author
property: property,
prop_key: prop_key,
value: value,
old_value: old_value), author: author
end

def add_member_creation_to_journal(member, role_ids, function_ids = nil)
Expand All @@ -58,17 +58,33 @@ def add_member_deletion_to_journal(member, previous_role_ids, previous_function_
add_journal_entry_for_user(user: member.user, property: 'associations', prop_key: 'projects', old_value: member.project.id)
end

def journalize_user_auto_creation(user)
# Global admin journal
JournalSetting.create(
:user_id => user.id,
:value_changes => user.previous_changes,
:journalized => user,
:journalized_entry_type => "auto_creation",
)
# User journal
add_journal_entry_for_user(user: user,
property: 'creation',
prop_key: 'creation',
value: User::USER_AUTO_CREATION,
author: user)
end

def value_hash(member, role_ids, function_ids)
value = {name: member.principal.to_s, roles: Role.where(id: role_ids).pluck(:name)}
value.merge!({functions: Function.where(id: function_ids).pluck(:name)}) if limited_visibility_plugin_installed?
value = { name: member.principal.to_s, roles: Role.where(id: role_ids).pluck(:name) }
value.merge!({ functions: Function.where(id: function_ids).pluck(:name) }) if limited_visibility_plugin_installed?
value
end

def get_previous_has_and_belongs_to_many(obj)
previous_has_and_belongs_to_many = {}
obj.class.reflect_on_all_associations(:has_and_belongs_to_many).each do |reflect|
reflect_ids = obj.send reflect.name
previous_has_and_belongs_to_many[reflect.name ] = [reflect_ids.map(&:id)]
previous_has_and_belongs_to_many[reflect.name] = [reflect_ids.map(&:id)]
end
previous_has_and_belongs_to_many
end
Expand Down Expand Up @@ -107,22 +123,22 @@ def get_has_many_ids_changes(obj, previous_h_m_ids)
def get_custom_field_enumerations_changes(obj, previous_h_m_ids)
changes = {}

reflect = obj.class.reflect_on_all_associations(:has_many).select{ |ref| ref.name == :enumerations }
reflect = obj.class.reflect_on_all_associations(:has_many).select { |ref| ref.name == :enumerations }
previous_enumerations = obj.send reflect[0].name
previous_enumerations_ids = previous_enumerations.where(active: true).map(&:id).sort
# reload the object for the deleting case
obj.reload

reflect = obj.class.reflect_on_all_associations(:has_many).select{ |ref| ref.name == :enumerations }
reflect = obj.class.reflect_on_all_associations(:has_many).select { |ref| ref.name == :enumerations }
new_enumerations = obj.send reflect[0].name
new_enumerations_ids = new_enumerations.where(active: true).map(&:id).sort

# Don't save if the relation m_to_m not changed case adding / deleting of active enumerations)
if previous_h_m_ids.sort != new_enumerations_ids
changes[reflect[0].name] = [previous_h_m_ids, new_enumerations.select { |i| i.active }.map(&:id)]
# (case of activation/ inactivation don't save if the activation m_to_m not changed) or (deleting of inactive enumerations)
# (case of activation/ inactivation don't save if the activation m_to_m not changed) or (deleting of inactive enumerations)
elsif new_enumerations.map(&:active) != previous_enumerations.map(&:active) && previous_enumerations_ids != new_enumerations_ids
changes[reflect[0].name] = [previous_enumerations_ids , new_enumerations_ids]
changes[reflect[0].name] = [previous_enumerations_ids, new_enumerations_ids]
end
changes
end
Expand Down
29 changes: 14 additions & 15 deletions lib/redmine_admin_activity/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,21 @@

class User < Principal

has_many :JournalSetting, :dependent => :nullify
has_many :journal_settings, :dependent => :nullify
has_many :journals, :as => :journalized, :dependent => :destroy, :inverse_of => :journalized

has_many :journals, :as => :journalized, :dependent => :destroy, :inverse_of => :journalized
attr_reader :current_journal

attr_reader :current_journal

after_save :create_journal
after_save :create_journal

USER_MANUAL_CREATION = 'manual'
USER_AUTO_CREATION = 'auto'

def init_journal(user)
@current_journal ||= Journal.new(:journalized => self, :user => user)
end
def init_journal(user)
@current_journal ||= Journal.new(:journalized => self, :user => user)
end

# Returns the current journal or nil if it's not initialized
# Returns the current journal or nil if it's not initialized
def current_journal
@current_journal
end
Expand All @@ -31,11 +30,11 @@ def journalized_attribute_names
names
end

def create_journal
if current_journal
current_journal.save
end
end
def create_journal
if current_journal
current_journal.save
end
end

def notified_users
[]
Expand All @@ -46,7 +45,7 @@ def notified_watchers
end

def self.representative_columns
return "firstname" , "lastname"
return "firstname", "lastname"
end

def self.representative_link_path(obj)
Expand Down
20 changes: 10 additions & 10 deletions spec/controllers/projects_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
render_views

fixtures :projects, :users, :roles, :members, :member_roles, :issues, :issue_statuses, :versions,
:trackers, :projects_trackers, :issue_categories, :enabled_modules, :enumerations, :attachments,
:workflows, :custom_fields, :custom_values, :custom_fields_projects, :custom_fields_trackers,
:time_entries, :journals, :journal_details, :queries, :repositories, :changesets
:trackers, :projects_trackers, :issue_categories, :enabled_modules, :enumerations, :attachments,
:workflows, :custom_fields, :custom_values, :custom_fields_projects, :custom_fields_trackers,
:time_entries, :journals, :journal_details, :queries, :repositories, :changesets

fixtures :issue_template_projects, :issue_templates if Redmine::Plugin.installed?(:redmine_templates)
include Redmine::I18n
Expand All @@ -17,7 +17,7 @@
@request = ActionDispatch::TestRequest.create
@response = ActionDispatch::TestResponse.new
User.current = nil
@request.session[:user_id] = 2 #permissions are hard
@request.session[:user_id] = 2 # permissions are hard
end

describe "POST modules" do
Expand Down Expand Up @@ -227,7 +227,7 @@
expect do
patch :update, params: { id: 1, project: { issue_template_ids: ["1", "3"], tab: "issue_templates" } }
end.to change { IssueTemplateProject.count }.by(2)
.and change { JournalDetail.count }.by(2)
.and change { JournalDetail.count }.by(2)
expect(JournalDetail.last(2)[0].prop_key).to eq('enabled_template')
expect(JournalDetail.last(2)[0].property).to eq('templates')
expect(JournalDetail.last(2)[0].value).to eq(IssueTemplate.find(1).template_title)
Expand All @@ -238,7 +238,7 @@
expect do
patch :update, params: { id: 2, project: { issue_template_ids: ["1", "2", "3", "4", "5"], tab: "issue_templates" } }
end.to change { IssueTemplateProject.count }.by(-1)
.and change { JournalDetail.count }.by(1)
.and change { JournalDetail.count }.by(1)
expect(JournalDetail.last.prop_key).to eq('enabled_template')
expect(JournalDetail.last.property).to eq('templates')
expect(JournalDetail.last.old_value).to eq(IssueTemplate.find(6).template_title)
Expand All @@ -254,21 +254,21 @@
it "check the number of elements by page" do
# Generating 5 Journals Settings
5.times do |index|
patch :update, :params => { :id => "ecookbook" , :project => { issue_template_ids: [], :name => "Test changed name #{index}" }}
patch :update, :params => { :id => "ecookbook", :project => { issue_template_ids: [], :name => "Test changed name #{index}" } }
end

# Get all journals of the first page
get :settings, :params => { :id => Project.find(1).id, :tab => "admin_activity", page: 1}
get :settings, :params => { :id => Project.find(1).id, :tab => "admin_activity", page: 1 }
first_page = assigns(:journals)

# Get all journals of the second page
get :settings, :params => { :id => Project.find(1).id, :tab => "admin_activity", page: 2}
get :settings, :params => { :id => Project.find(1).id, :tab => "admin_activity", page: 2 }
second_page = assigns(:journals)

# Tests
expect(first_page.count).to eq(3)
expect(second_page.count).to eq(2)
expect(first_page.first.id).to be > second_page.first.id
expect(first_page.first.id).to be > second_page.first.id
end
end
end
36 changes: 18 additions & 18 deletions spec/controllers/users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
@request = ActionDispatch::TestRequest.create
@response = ActionDispatch::TestResponse.new
User.current = nil
@request.session[:user_id] = 1 #permissions admin
@request.session[:user_id] = 1 # permissions admin
end

describe "POST create" do
Expand All @@ -27,28 +27,28 @@
:generate_password => '1',
}
}
user = User.last
new_user = User.last

expect(response).to redirect_to("/users/#{user.id}/edit")
expect(response).to redirect_to("/users/#{new_user.id}/edit")

expect(JournalSetting.all).to_not be_empty

journal_setting_last = JournalSetting.last
last_journal_setting = JournalSetting.last

expect(journal_setting_last.value_changes).to include("id" => [nil, user.id])
expect(journal_setting_last.value_changes).to include("login" => ["", "newuser"])
expect(journal_setting_last.value_changes).to include("firstname" => ["", "new"])
expect(journal_setting_last.value_changes).to include("lastname" => ["", "user"])
expect(journal_setting_last.value_changes).to include("type" => [nil, "User"])
expect(journal_setting_last).to have_attributes(:journalized_type => "Principal")
expect(journal_setting_last).to have_attributes(:journalized_entry_type => "create")
expect(last_journal_setting.value_changes).to include("id" => [nil, new_user.id])
expect(last_journal_setting.value_changes).to include("login" => ["", "newuser"])
expect(last_journal_setting.value_changes).to include("firstname" => ["", "new"])
expect(last_journal_setting.value_changes).to include("lastname" => ["", "user"])
expect(last_journal_setting.value_changes).to include("type" => [nil, "User"])
expect(last_journal_setting).to have_attributes(:journalized_type => "Principal")
expect(last_journal_setting).to have_attributes(:journalized_entry_type => "create")

journal_detail_last = JournalDetail.last
last_journal_detail = JournalDetail.last

expect(journal_detail_last.property).to eq('creation')
expect(journal_detail_last.prop_key).to eq('creation')
expect(journal_detail_last.value).to eq(User::USER_MANUAL_CREATION)
expect(Journal.last.journalized_id).to eq(user.id)
expect(last_journal_detail.property).to eq('creation')
expect(last_journal_detail.prop_key).to eq('creation')
expect(last_journal_detail.value).to eq(User::USER_MANUAL_CREATION)
expect(Journal.last.journalized_id).to eq(new_user.id)
end
end

Expand Down Expand Up @@ -187,11 +187,11 @@
patch :update, :params => { :id => user.id, :user => { :mail => "test#{index}@example.net" } }
end
# Get all journals of the first page
get :history, :params => { :id => user.id, page: 1}
get :history, :params => { :id => user.id, page: 1 }
first_page = assigns(:journals)

# Get all journals of the second page
get :history, :params => { :id => user.id, page: 2}
get :history, :params => { :id => user.id, page: 2 }
second_page = assigns(:journals)

# Tests
Expand Down

0 comments on commit ae1da42

Please sign in to comment.