Refactor audit log code

* Rename `LogFinder` to `AuditLogFinder`
* Rename audit helper methods to `audit_` prefix
* Remove use of audit helper in unrelated view
* More labels with internalization via `_('Text')`
* Make audit dropdowns readable with multiline code
* Add specs to `AuditLogsHelper`
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

ee/app/controllers/admin/audit_logs_controller.rb

@@ -5,7 +5,7 @@ class Admin::AuditLogsController < Admin::ApplicationController
   PER_PAGE = 25
 
   def index
-    @events = LogFinder.new(audit_logs_params).execute.page(params[:page]).per(PER_PAGE)
+    @events = AuditLogFinder.new(audit_logs_params).execute.page(params[:page]).per(PER_PAGE)
     @entity = case audit_logs_params[:entity_type]
               when 'User'
                 User.find_by_id(audit_logs_params[:entity_id])

ee/app/controllers/groups/audit_events_controller.rb

@@ -7,6 +7,6 @@ class Groups::AuditEventsController < Groups::ApplicationController
   layout 'group_settings'
 
   def index
-    @events = LogFinder.new(entity_type: group.class.name, entity_id: group.id).execute.page(params[:page])
+    @events = AuditLogFinder.new(entity_type: group.class.name, entity_id: group.id).execute.page(params[:page])
   end
 end

ee/app/controllers/projects/audit_events_controller.rb

@@ -9,7 +9,7 @@ class Projects::AuditEventsController < Projects::ApplicationController
   layout 'project_settings'
 
   def index
-    @events = LogFinder.new(entity_type: project.class.name, entity_id: project.id).execute.page(params[:page])
+    @events = AuditLogFinder.new(entity_type: project.class.name, entity_id: project.id).execute.page(params[:page])
   end
 
   def check_audit_events_available!

ee/app/finders/log_finder.rb → ee/app/finders/audit_log_finder.rb

 # frozen_string_literal: true
 
-class LogFinder
+class AuditLogFinder
   include CreatedAtFilter
   VALID_ENTITY_TYPES = %w[Project User Group].freeze
 

ee/app/helpers/audit_logs_helper.rb

 # frozen_string_literal: true
 
 module AuditLogsHelper
-  def entity_type_options
+  def audit_entity_type_options
     [
-      { id: '', text: 'All Events' },
+      { id: 'All', text: 'All Events' },
       { id: 'Group', text: 'Group Events' },
       { id: 'Project', text: 'Project Events' },
       { id: 'User', text: 'User Events' }
     ]
   end
 
-  def admin_user_dropdown_label(default_label)
-    if @entity
-      @entity.name
-    else
-      default_label
-    end
-  end
-
-  def admin_project_dropdown_label(default_label)
-    if @entity
-      @entity.full_name
-    else
-      default_label
-    end
-  end
+  def audit_entity_type_label(selected)
+    selected = 'All' unless selected.present?
 
-  def admin_namespace_dropdown_label(default_label)
-    if @entity
-      @entity.full_path
-    else
-      default_label
-    end
+    audit_entity_type_options.find { |type| type[:id] == selected }[:text]
   end
 end

ee/app/views/admin/application_settings/_templates.html.haml

@@ -18,5 +18,5 @@
                 Select a
                 = link_to 'template repository', help_page_path("user/admin_area/settings/instance_template_repository")
             = project_select_tag('application_setting[file_template_project_id]', class: 'project-item-select hidden-filter-value', toggle_class: 'js-project-search js-project-filter js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-project js-filter-submit',
-              placeholder: admin_project_dropdown_label('Search projects'), idAttribute: 'id', data: { order_by: 'last_activity_at', idattribute: 'id', all_projects: 'true', simple_filter: true, allow_clear: true }, value: @application_setting.file_template_project_id)
+              placeholder: _('Search projects'), idAttribute: 'id', data: { order_by: 'last_activity_at', idattribute: 'id', all_projects: 'true', simple_filter: true, allow_clear: true }, value: @application_setting.file_template_project_id)
         = f.submit 'Save changes', class: "btn btn-success"

ee/app/views/admin/audit_logs/index.html.haml

 - page_title 'Audit Log'
+- entity_type = params[:entity_type]
 
 .todos-filters
   .row-content-block.second-block
     = form_tag admin_audit_logs_path, method: :get, class: 'filter-form' do
       .filter-item.inline
-        - if params[:entity_type].present?
-          = hidden_field_tag(:entity_type, params[:entity_type])
-        - entity_type = params[:entity_type].presence || 'All'
-        = dropdown_tag("#{entity_type} Events", options: { toggle_class: 'js-type-search js-filter-submit js-type-filter', dropdown_class: 'dropdown-menu-type dropdown-menu-selectable dropdown-menu-action js-filter-submit',
-          placeholder: 'Search types', data: { field_name: 'entity_type', data: entity_type_options, default_label: 'All Events' } })
+        - if entity_type.present?
+          = hidden_field_tag(:entity_type, entity_type)
+
+        = dropdown_tag(audit_entity_type_label(entity_type),
+          options: { toggle_class: 'js-type-search js-filter-submit js-type-filter',
+          dropdown_class: 'dropdown-menu-type dropdown-menu-selectable dropdown-menu-action js-filter-submit',
+          placeholder: 'Search types',
+          data: { field_name: 'entity_type', data: audit_entity_type_options, default_label: 'All Events' } })
+
       - if params[:entity_type] == 'User'
         .filter-item.inline
           - if params[:entity_id].present?
             = hidden_field_tag(:entity_id, params[:entity_id], class:'hidden-filter-value')
-          = dropdown_tag(admin_user_dropdown_label('User'), options: { toggle_class: 'js-user-search js-filter-submit', filter: true, dropdown_class: 'dropdown-menu-user dropdown-menu-selectable',
-            placeholder: 'Search users', data: { first_user: (current_user.username if current_user), null_user: true, current_user: true, field_name: 'entity_id' } })
+
+          = dropdown_tag(@entity&.name || _('Search users'),
+            options: { toggle_class: 'js-user-search js-filter-submit', filter: true,
+            dropdown_class: 'dropdown-menu-user dropdown-menu-selectable',
+            placeholder: _('Search users'),
+            data: { first_user: (current_user.username if current_user), null_user: true, current_user: true, field_name: 'entity_id' } })
+
       - elsif params[:entity_type] == 'Project'
         .filter-item.inline
-          = project_select_tag(:entity_id, { class: 'project-item-select hidden-filter-value', toggle_class: 'js-project-search js-project-filter js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-project js-filter-submit',
-            placeholder: admin_project_dropdown_label('Search projects'), idAttribute: 'id', data: { order_by: 'last_activity_at', idattribute: 'id', all_projects: 'true', simple_filter: true } })
+          = project_select_tag(:entity_id, { class: 'project-item-select hidden-filter-value',
+            toggle_class: 'js-project-search js-project-filter js-filter-submit',
+            dropdown_class: 'dropdown-menu-selectable dropdown-menu-project js-filter-submit',
+            placeholder: @entity&.full_name || _('Search projects'), idAttribute: 'id',
+            data: { order_by: 'last_activity_at', idattribute: 'id', all_projects: 'true', simple_filter: true } })
+
       - elsif params[:entity_type] == 'Group'
         .filter-item.inline
-          = groups_select_tag(:entity_id, { required: true, class: 'group-item-select project-item-select hidden-filter-value', toggle_class: 'js-group-search js-group-filter js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-group js-filter-submit',
-            placeholder: admin_namespace_dropdown_label('Search groups'), idAttribute: 'id', data: { order_by: 'last_activity_at', idattribute: 'id', all_available: true } })
+          = groups_select_tag(:entity_id, { required: true, class: 'group-item-select project-item-select hidden-filter-value',
+            toggle_class: 'js-group-search js-group-filter js-filter-submit',
+            dropdown_class: 'dropdown-menu-selectable dropdown-menu-group js-filter-submit',
+            placeholder: @entity&.full_path || _('Search groups'), idAttribute: 'id',
+            data: { order_by: 'last_activity_at', idattribute: 'id', all_available: true } })
 
 - if @events.present?
   %table#events-table.table

ee/lib/api/audit_events.rb

@@ -14,7 +14,7 @@ module API
         success EE::API::Entities::AuditEvent
       end
       params do
-        optional :entity_type, type: String, desc: 'Return list of audit events for the specified entity type', values: LogFinder::VALID_ENTITY_TYPES
+        optional :entity_type, type: String, desc: 'Return list of audit events for the specified entity type', values: AuditLogFinder::VALID_ENTITY_TYPES
         optional :entity_id, type: Integer
         given :entity_id do
           requires :entity_type, type: String
@@ -25,7 +25,7 @@ module API
         use :pagination
       end
       get do
-        audit_events = LogFinder.new(params).execute
+        audit_events = AuditLogFinder.new(params).execute
 
         present paginate(audit_events), with: EE::API::Entities::AuditEvent
       end

ee/spec/controllers/groups/audit_events_controller_spec.rb

@@ -24,7 +24,7 @@ describe Groups::AuditEventsController do
         end
 
         it 'renders index with 200 status code' do
-          expect(LogFinder).to receive(:new).with(entity_type: ::Group.name, entity_id: group.id).and_call_original
+          expect(AuditLogFinder).to receive(:new).with(entity_type: ::Group.name, entity_id: group.id).and_call_original
 
           request
 

ee/spec/controllers/projects/audit_events_controller_spec.rb

@@ -24,7 +24,7 @@ describe Projects::AuditEventsController do
         end
 
         it 'renders index with 200 status code' do
-          expect(LogFinder).to receive(:new).with(entity_type: ::Project.name, entity_id: project.id).and_call_original
+          expect(AuditLogFinder).to receive(:new).with(entity_type: ::Project.name, entity_id: project.id).and_call_original
 
           request
 

ee/spec/features/admin/admin_audit_logs_spec.rb

@@ -47,7 +47,7 @@ describe 'Admin::AuditLogs', :js do
       it 'filters by user' do
         filter_by_type('User Events')
 
-        click_button 'User'
+        click_button 'Search users'
         wait_for_requests
 
         within '.dropdown-menu-user' do
@@ -109,7 +109,8 @@ describe 'Admin::AuditLogs', :js do
   end
 
   def filter_by_type(type)
-    click_button 'Events'
+    click_button 'All Events'
+
     within '.dropdown-menu-type' do
       click_link type
     end

ee/spec/finders/log_finder_spec.rb → ee/spec/finders/audit_log_finder_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe LogFinder do
+describe AuditLogFinder do
   describe '#execute' do
     set(:user_audit_event) { create(:user_audit_event, created_at: 3.days.ago) }
     set(:project_audit_event) { create(:project_audit_event, created_at: 2.days.ago) }

ee/spec/helpers/audit_logs_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AuditLogsHelper do
+  using RSpec::Parameterized::TableSyntax
+
+  describe '#audit_entity_type_label' do
+    where(:label, :result) do
+      nil       | 'All Events'
+      'All'     | 'All Events'
+      'User'    | 'User Events'
+      'Group'   | 'Group Events'
+      'Project' | 'Project Events'
+    end
+
+    with_them do
+      it { expect(audit_entity_type_label(label)).to eq(result) }
+    end
+  end
+end