Rubocop (#617)

* update rubocop

* run rubocop

* fix tests

* set metrics/ABCsize back to 17

* make rubocop happy

* Fix rubocop offences

---------

Co-authored-by: Yanick Minder <[email protected]>
Co-authored-by: Robin Steiner <[email protected]>

.rubocop.yml

@@ -1,5 +1,6 @@
 require: rubocop-rails
 AllCops:
+  NewCops: enable
   DisplayCopNames: true
   Exclude:
     - Rakefile
@@ -45,6 +46,9 @@ Metrics/ParameterLists:
 Rails:
   Enabled: true
 
+Rails/I18nLocaleTexts:
+  Enabled: false
+
 # Keep for now, easier with superclass definitions
 Style/ClassAndModuleChildren:
   Enabled: false

Gemfile.lock

@@ -386,7 +386,7 @@ GEM
       rspec-mocks (~> 3.12)
       rspec-support (~> 3.12)
     rspec-support (3.13.1)
-    rubocop (1.60.2)
+    rubocop (1.61.0)
       json (~> 2.3)
       language_server-protocol (>= 3.17.0)
       parallel (~> 1.10)
@@ -397,8 +397,8 @@ GEM
       rubocop-ast (>= 1.30.0, < 2.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 2.4.0, < 3.0)
-    rubocop-ast (1.30.0)
-      parser (>= 3.2.1.0)
+    rubocop-ast (1.31.1)
+      parser (>= 3.3.0.4)
     rubocop-rails (2.23.1)
       activesupport (>= 4.2.0)
       rack (>= 1.1)

app/controllers/api/crud_controller.rb

@@ -71,7 +71,7 @@ def render_errors
   def entry_url
     send("#{self.class.name.underscore
          .gsub(/_controller$/, '')
-         .gsub(/\//, '_').singularize}_url", entry)
+         .gsub('/', '_').singularize}_url", entry)
   end
 
   # Only allow a trusted parameter "white list" through.
@@ -101,9 +101,7 @@ def relationship_ids(model_data, name)
     return model_data[:data][:id] unless model_data[:data].is_a?(Array)
 
     if permitted_relationship?(name)
-      model_data[:data].collect do |e|
-        e[:id]
-      end
+      model_data[:data].pluck(:id)
     end
   end
 

app/controllers/api/env_settings_controller.rb

@@ -10,18 +10,18 @@ def index
 
   def values
     {
-      sentry: ENV['SENTRY_DSN_FRONTEND'],
-      helplink: ENV['HELPLINK'],
+      sentry: ENV.fetch('SENTRY_DSN_FRONTEND', nil),
+      helplink: ENV.fetch('HELPLINK', nil),
       keycloak: keycloak_info
     }
   end
 
   def keycloak_info
     {
       disable: Rails.application.keycloak_disabled? ? 'true' : nil,
-      url: ENV['EMBER_KEYCLOAK_SERVER_URL'],
-      realm: ENV['EMBER_KEYCLOAK_REALM_NAME'],
-      clientId: ENV['EMBER_KEYCLOAK_CLIENT_ID']
+      url: ENV.fetch('EMBER_KEYCLOAK_SERVER_URL', nil),
+      realm: ENV.fetch('EMBER_KEYCLOAK_REALM_NAME', nil),
+      clientId: ENV.fetch('EMBER_KEYCLOAK_CLIENT_ID', nil)
     }
   end
 

app/controllers/api/people/picture_controller.rb

@@ -5,15 +5,16 @@ class PictureController < Api::CrudController
 
     self.permitted_attrs = %i[picture]
 
+    def show
+      picture_url = person.picture.file.nil? ? default_avatar_path : person.picture.url
+      send_file(picture_url, disposition: 'inline')
+    end
+
     def update
       person.update!(picture: params[:picture])
       render json: { data: { picture_path: picture_api_person_path(params[:id]) } }
     end
 
-    def show
-      picture_url = person.picture.file.nil? ? default_avatar_path : person.picture.url
-      send_file(picture_url, disposition: 'inline')
-    end
 
     private
 

app/controllers/api/skills_controller.rb

@@ -23,7 +23,7 @@ def index
   def unrated_by_person
     if params[:person_id].present?
       entries = Skill.default_set.where
-                     .not(id: PeopleSkill.where(person_id: params[:person_id]).pluck(:skill_id))
+                     .not(id: PeopleSkill.where(person_id: params[:person_id]).select(:skill_id))
     else
       entries = Skill.list
     end

app/controllers/api/static_assets_controller.rb

@@ -6,7 +6,7 @@ class Api::StaticAssetsController < Api::ApplicationController
   protect_from_forgery with: :exception
 
   def index
-    render file: Rails.root.join('public/index.html'), formats: [:html]
+    render file: Rails.public_path.join('index.html'), formats: [:html]
   end
 
 end

app/controllers/people/picture_controller.rb

@@ -5,16 +5,16 @@ class PictureController < CrudController
 
     self.permitted_attrs = %i[picture]
 
-    def update
-      person.update!(picture: params[:picture])
-      render json: { data: { picture_path: picture_person_path(params[:id]) } }
-    end
-
     def show
       picture_url = person.picture.file.nil? ? default_avatar_path : person.picture.url
       send_file(picture_url, disposition: 'inline')
     end
 
+    def update
+      person.update!(picture: params[:picture])
+      render json: { data: { picture_path: picture_person_path(params[:id]) } }
+    end
+
     private
 
     def model_class

app/controllers/people_controller.rb

@@ -10,13 +10,6 @@ class PeopleController < CrudController
                           { person_roles_attributes: [:role_id, :person_role_level_id,
                                                       :percent, :id, :_destroy] }]
 
-  def update
-    if false?(params['has_nationality2']['checked'])
-      params['person']['nationality2'] = nil
-    end
-    super
-  end
-
   def show
     if format_odt?
       export
@@ -27,6 +20,13 @@ def show
     super
   end
 
+  def update
+    if false?(params['has_nationality2']['checked'])
+      params['person']['nationality2'] = nil
+    end
+    super
+  end
+
   private
 
   def fetch_entries

app/controllers/skills_controller.rb

@@ -17,7 +17,7 @@ def create
   def unrated_by_person
     if params[:person_id].present?
       entries = Skill.default_set.where
-                     .not(id: PeopleSkill.where(person_id: params[:person_id]).pluck(:skill_id))
+                     .not(id: PeopleSkill.where(person_id: params[:person_id]).select(:skill_id))
     else
       entries = Skill.list
     end

app/domain/people_skills_filter.rb

@@ -106,6 +106,6 @@ def persons_with_required_skill(skills_per_person)
   end
 
   def skill_ids
-    levels_and_interests_for_skills.map { |ls| ls[:skill] }
+    levels_and_interests_for_skills.pluck(:skill)
   end
 end

app/exporters/odt/cv.rb

@@ -56,10 +56,9 @@ def location
       BranchAdress.find(@params[:location])
     end
 
-    # rubocop:disable Metrics/AbcSize
     attr_accessor :person
 
-    def insert_general_sections(report)
+    def insert_general_sections(report) # rubocop:disable Metrics/AbcSize
       report.add_field(:client, 'mg')
       report.add_field(:project, 'pcv')
       report.add_field(:section, 'dev1')
@@ -78,7 +77,7 @@ def insert_locations(report)
       report.add_field(:niederlassung, location.adress_information)
     end
 
-    def insert_personalien(report)
+    def insert_personalien(report) # rubocop:disable Metrics/AbcSize
       report.add_field(:title, person.title)
       unless anon?
         report.add_field(:birthdate, Date.parse(person.birthdate.to_s)
@@ -90,8 +89,6 @@ def insert_personalien(report)
       report.add_field(:languages, languages)
     end
 
-    # rubocop:enable Metrics/AbcSize
-
     def insert_competences(report)
       insert_level_skills(report) if include_skills_by_level?
       insert_core_competences(report)
@@ -162,9 +159,8 @@ def competence_notes_list
       end
     end
 
-    # rubocop:disable Metrics/AbcSize
     # rubocop:disable Metrics/MethodLength
-    def insert_educations(report)
+    def insert_educations(report) # rubocop:disable Metrics/AbcSize
       educations_list = person.educations.list.collect do |e|
         { year_from: formatted_year(e.year_from),
           month_from: formatted_month(e.month_from),
@@ -182,7 +178,7 @@ def insert_educations(report)
       end
     end
 
-    def insert_advanced_trainings(report)
+    def insert_advanced_trainings(report) # rubocop:disable Metrics/AbcSize
       advanced_trainings_list = person.advanced_trainings.list.collect do |at|
         { year_from: formatted_year(at.year_from),
           month_from: formatted_month(at.month_from),
@@ -200,7 +196,7 @@ def insert_advanced_trainings(report)
       end
     end
 
-    def insert_activities(report)
+    def insert_activities(report) # rubocop:disable Metrics/AbcSize
       activities_list = person.activities.list.collect do |a|
         { year_from: formatted_year(a.year_from),
           month_from: formatted_month(a.month_from),
@@ -218,7 +214,7 @@ def insert_activities(report)
       end
     end
 
-    def insert_projects(report)
+    def insert_projects(report) # rubocop:disable Metrics/AbcSize
       projects_list = person.projects.list.collect do |p|
         { year_from: formatted_year(p.year_from),
           month_from: formatted_month(p.month_from),
@@ -241,7 +237,6 @@ def insert_projects(report)
         t.add_column(:project_technology, :project_technology)
       end
     end
-    # rubocop:enable Metrics/AbcSize
     # rubocop:enable Metrics/MethodLength
 
     def formatted_month(month)

app/models/activity.rb

@@ -26,7 +26,7 @@ class Activity < ApplicationRecord
 
   belongs_to :person, touch: true
 
-  validates :person_id, :role, presence: true
+  validates :role, presence: true
   validates :description, length: { maximum: 5000 }
   validates :role, length: { maximum: 500 }
 

app/models/advanced_training.rb

@@ -25,7 +25,7 @@ class AdvancedTraining < ApplicationRecord
 
   belongs_to :person, touch: true
 
-  validates :person_id, :description, presence: true
+  validates :description, presence: true
   validates :description, length: { maximum: 5000 }
 
   private

app/models/education.rb

@@ -25,7 +25,7 @@ class Education < ApplicationRecord
 
   belongs_to :person, touch: true
 
-  validates :person_id, :title, :location, presence: true
+  validates :title, :location, presence: true
   validates :location, :title, length: { maximum: 500 }
 
   private

app/models/people_skill.rb

@@ -28,8 +28,8 @@ class PeopleSkill < ApplicationRecord
                                                allow_nil: true }
 
   validates :person_id, uniqueness: { scope: :skill_id,
-                                      message: 'Pro Person kann ein Skill nicht'\
-                                               ' mehrmals ausgewählt werden' }
+                                      message: 'Pro Person kann ein Skill nicht ' \
+                                               'mehrmals ausgewählt werden' }
 
   scope :list, -> { order(:person_id, :skill_id) }
 end

app/models/person_role.rb

@@ -16,7 +16,6 @@ class PersonRole < ApplicationRecord
   belongs_to :role
   belongs_to :person_role_level
 
-  validates :person_id, :role_id, presence: true
 
   validate :percent_must_be_a_number
 

app/models/project.rb

@@ -30,7 +30,7 @@ class Project < ApplicationRecord
 
   has_many :project_technologies, dependent: :destroy
 
-  validates :person_id, :role, :title, presence: true
+  validates :role, :title, presence: true
   validates :description, :technology, :role, length: { maximum: 5000 }
   validates :title, length: { maximum: 500 }
 

app/serializers/people_search_skill_serializer.rb

@@ -4,9 +4,8 @@ class PeopleSearchSkillSerializer
   # write custom serializer to serialize object that doesn't exist as a model
   # serialize data into JSON API format
   # rubocop:disable Metrics/MethodLength
-  # rubocop:disable Metrics/AbcSize
   # rubocop:disable Metrics/BlockLength
-  def self.serialize(data)
+  def self.serialize(data) # rubocop:disable Metrics/AbcSize
     {
       data: data.map do |entry|
         {
@@ -70,6 +69,5 @@ def self.serialize(data)
     }
   end
   # rubocop:enable Metrics/BlockLength
-  # rubocop:enable Metrics/AbcSize
   # rubocop:enable Metrics/MethodLength
 end

lib/auth_config.rb

@@ -32,7 +32,7 @@ def get_var_from_environment(key, required: true)
       else
         raise("Environment variable not set: '#{key}'") if required && ENV[key.to_s.upcase].nil?
 
-        ENV[key.to_s.upcase]
+        ENV.fetch(key.to_s.upcase)
       end
     end
 

lib/ldap_tools.rb

@@ -42,11 +42,11 @@ def connect_ldap
     end
 
     def basename
-      @@basename ||= ENV['LDAP_BASENAME']
+      @@basename ||= ENV.fetch('LDAP_BASENAME', nil)
     end
 
     def hostname
-      @@hostname ||= ENV['LDAP_HOSTNAME']
+      @@hostname ||= ENV.fetch('LDAP_HOSTNAME', nil)
     end
 
     def port

spec/models/activity_spec.rb

@@ -24,9 +24,8 @@
     it 'checks whether required attribute values are present' do
       activity = Activity.new
       activity.valid?
-
       expect(activity.errors[:year_from].first).to eq('muss ausgefüllt werden')
-      expect(activity.errors[:person_id].first).to eq('muss ausgefüllt werden')
+      expect(activity.errors[:person].first).to eq('muss ausgefüllt werden')
       expect(activity.errors[:role].first).to eq('muss ausgefüllt werden')
     end
 

spec/models/advanced_training_spec.rb

@@ -25,7 +25,7 @@
       advanced_training.valid?
 
       expect(advanced_training.errors[:year_from].first).to eq('muss ausgefüllt werden')
-      expect(advanced_training.errors[:person_id].first).to eq('muss ausgefüllt werden')
+      expect(advanced_training.errors[:person].first).to eq('muss ausgefüllt werden')
     end
 
     it 'checks validation maximum length for attribute' do

spec/models/education_spec.rb

@@ -25,7 +25,7 @@
       education.valid?
 
       expect(education.errors[:year_from].first).to eq('muss ausgefüllt werden')
-      expect(education.errors[:person_id].first).to eq('muss ausgefüllt werden')
+      expect(education.errors[:person].first).to eq('muss ausgefüllt werden')
       expect(education.errors[:title].first).to eq('muss ausgefüllt werden')
       expect(education.errors[:location].first).to eq('muss ausgefüllt werden')
     end

spec/models/people_role_spec.rb

@@ -21,10 +21,8 @@
 
       expect(person_role.errors[:person].first).to eq('muss ausgefüllt werden')
       expect(person_role.errors[:role].first).to eq('muss ausgefüllt werden')
-      expect(person_role.errors[:person_id].first).to eq('muss ausgefüllt werden')
-      expect(person_role.errors[:role_id].first).to eq('muss ausgefüllt werden')
     end
-    
+
     it 'percent must be a number between 0 and 200' do
       person_role = person_roles(:'bob_software_engineer')
       person_role.percent = 300