Merge branch 'remove-unnecessary-self-from-user-model' into 'master'

Remove unnecessary self from user model

self keyword nod needed. Code is more clean and understandable. https://github.com/bbatsov/ruby-style-guide#no-self-unless-required

See merge request !7551

app/models/user.rb

@@ -229,19 +229,19 @@ class User < ActiveRecord::Base
     def filter(filter_name)
       case filter_name
       when 'admins'
-        self.admins
+        admins
       when 'blocked'
-        self.blocked
+        blocked
       when 'two_factor_disabled'
-        self.without_two_factor
+        without_two_factor
       when 'two_factor_enabled'
-        self.with_two_factor
+        with_two_factor
       when 'wop'
-        self.without_projects
+        without_projects
       when 'external'
-        self.external
+        external
       else
-        self.active
+        active
       end
     end
 
@@ -339,7 +339,7 @@ class User < ActiveRecord::Base
   end
 
   def generate_password
-    if self.force_random_password
+    if force_random_password
       self.password = self.password_confirmation = Devise.friendly_token.first(Devise.password_length.min)
     end
   end
@@ -380,56 +380,55 @@ class User < ActiveRecord::Base
   end
 
   def two_factor_otp_enabled?
-    self.otp_required_for_login?
+    otp_required_for_login?
   end
 
   def two_factor_u2f_enabled?
-    self.u2f_registrations.exists?
+    u2f_registrations.exists?
   end
 
   def namespace_uniq
     # Return early if username already failed the first uniqueness validation
-    return if self.errors.key?(:username) &&
-      self.errors[:username].include?('has already been taken')
+    return if errors.key?(:username) &&
+      errors[:username].include?('has already been taken')
 
-    namespace_name = self.username
-    existing_namespace = Namespace.by_path(namespace_name)
-    if existing_namespace && existing_namespace != self.namespace
-      self.errors.add(:username, 'has already been taken')
+    existing_namespace = Namespace.by_path(username)
+    if existing_namespace && existing_namespace != namespace
+      errors.add(:username, 'has already been taken')
     end
   end
 
   def avatar_type
-    unless self.avatar.image?
-      self.errors.add :avatar, "only images allowed"
+    unless avatar.image?
+      errors.add :avatar, "only images allowed"
     end
   end
 
   def unique_email
-    if !self.emails.exists?(email: self.email) && Email.exists?(email: self.email)
-      self.errors.add(:email, 'has already been taken')
+    if !emails.exists?(email: email) && Email.exists?(email: email)
+      errors.add(:email, 'has already been taken')
     end
   end
 
   def owns_notification_email
-    return if self.temp_oauth_email?
+    return if temp_oauth_email?
 
-    self.errors.add(:notification_email, "is not an email you own") unless self.all_emails.include?(self.notification_email)
+    errors.add(:notification_email, "is not an email you own") unless all_emails.include?(notification_email)
   end
 
   def owns_public_email
-    return if self.public_email.blank?
+    return if public_email.blank?
 
-    self.errors.add(:public_email, "is not an email you own") unless self.all_emails.include?(self.public_email)
+    errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email)
   end
 
   def update_emails_with_primary_email
-    primary_email_record = self.emails.find_by(email: self.email)
+    primary_email_record = emails.find_by(email: email)
     if primary_email_record
       primary_email_record.destroy
-      self.emails.create(email: self.email_was)
+      emails.create(email: email_was)
 
-      self.update_secondary_emails!
+      update_secondary_emails!
     end
   end
 
@@ -617,7 +616,7 @@ class User < ActiveRecord::Base
   end
 
   def project_deploy_keys
-    DeployKey.unscoped.in_projects(self.authorized_projects.pluck(:id)).distinct(:id)
+    DeployKey.unscoped.in_projects(authorized_projects.pluck(:id)).distinct(:id)
   end
 
   def accessible_deploy_keys
@@ -633,38 +632,38 @@ class User < ActiveRecord::Base
   end
 
   def sanitize_attrs
-    %w(name username skype linkedin twitter).each do |attr|
-      value = self.send(attr)
-      self.send("#{attr}=", Sanitize.clean(value)) if value.present?
+    %w[name username skype linkedin twitter].each do |attr|
+      value = public_send(attr)
+      public_send("#{attr}=", Sanitize.clean(value)) if value.present?
     end
   end
 
   def set_notification_email
-    if self.notification_email.blank? || !self.all_emails.include?(self.notification_email)
-      self.notification_email = self.email
+    if notification_email.blank? || !all_emails.include?(notification_email)
+      self.notification_email = email
     end
   end
 
   def set_public_email
-    if self.public_email.blank? || !self.all_emails.include?(self.public_email)
+    if public_email.blank? || !all_emails.include?(public_email)
       self.public_email = ''
     end
   end
 
   def update_secondary_emails!
-    self.set_notification_email
-    self.set_public_email
-    self.save if self.notification_email_changed? || self.public_email_changed?
+    set_notification_email
+    set_public_email
+    save if notification_email_changed? || public_email_changed?
   end
 
   def set_projects_limit
     # `User.select(:id)` raises
     # `ActiveModel::MissingAttributeError: missing attribute: projects_limit`
     # without this safeguard!
-    return unless self.has_attribute?(:projects_limit)
+    return unless has_attribute?(:projects_limit)
 
     connection_default_value_defined = new_record? && !projects_limit_changed?
-    return unless self.projects_limit.nil? || connection_default_value_defined
+    return unless projects_limit.nil? || connection_default_value_defined
 
     self.projects_limit = current_application_settings.default_projects_limit
   end
@@ -694,7 +693,7 @@ class User < ActiveRecord::Base
 
   def with_defaults
     User.defaults.each do |k, v|
-      self.send("#{k}=", v)
+      public_send("#{k}=", v)
     end
 
     self
@@ -714,7 +713,7 @@ class User < ActiveRecord::Base
   # Thus it will automatically generate a new fragment
   # when the event is updated because the key changes.
   def reset_events_cache
-    Event.where(author_id: self.id).
+    Event.where(author_id: id).
       order('id DESC').limit(1000).
       update_all(updated_at: Time.now)
   end
@@ -747,8 +746,8 @@ class User < ActiveRecord::Base
 
   def all_emails
     all_emails = []
-    all_emails << self.email unless self.temp_oauth_email?
-    all_emails.concat(self.emails.map(&:email))
+    all_emails << email unless temp_oauth_email?
+    all_emails.concat(emails.map(&:email))
     all_emails
   end
 
@@ -762,21 +761,21 @@ class User < ActiveRecord::Base
 
   def ensure_namespace_correct
     # Ensure user has namespace
-    self.create_namespace!(path: self.username, name: self.username) unless self.namespace
+    create_namespace!(path: username, name: username) unless namespace
 
-    if self.username_changed?
-      self.namespace.update_attributes(path: self.username, name: self.username)
+    if username_changed?
+      namespace.update_attributes(path: username, name: username)
     end
   end
 
   def post_create_hook
-    log_info("User \"#{self.name}\" (#{self.email}) was created")
-    notification_service.new_user(self, @reset_token) if self.created_by_id
+    log_info("User \"#{name}\" (#{email}) was created")
+    notification_service.new_user(self, @reset_token) if created_by_id
     system_hook_service.execute_hooks_for(self, :create)
   end
 
   def post_destroy_hook
-    log_info("User \"#{self.name}\" (#{self.email})  was removed")
+    log_info("User \"#{name}\" (#{email})  was removed")
     system_hook_service.execute_hooks_for(self, :destroy)
   end
 
@@ -820,7 +819,7 @@ class User < ActiveRecord::Base
   end
 
   def oauth_authorized_tokens
-    Doorkeeper::AccessToken.where(resource_owner_id: self.id, revoked_at: nil)
+    Doorkeeper::AccessToken.where(resource_owner_id: id, revoked_at: nil)
   end
 
   # Returns the projects a user contributed to in the last year.
@@ -951,7 +950,7 @@ class User < ActiveRecord::Base
   end
 
   def ensure_external_user_rights
-    return unless self.external?
+    return unless external?
 
     self.can_create_group   = false
     self.projects_limit     = 0
@@ -963,7 +962,7 @@ class User < ActiveRecord::Base
 
     if current_application_settings.domain_blacklist_enabled?
       blocked_domains = current_application_settings.domain_blacklist
-      if domain_matches?(blocked_domains, self.email)
+      if domain_matches?(blocked_domains, email)
         error = 'is not from an allowed domain.'
         valid = false
       end
@@ -971,7 +970,7 @@ class User < ActiveRecord::Base
 
     allowed_domains = current_application_settings.domain_whitelist
     unless allowed_domains.blank?
-      if domain_matches?(allowed_domains, self.email)
+      if domain_matches?(allowed_domains, email)
         valid = true
       else
         error = "domain is not authorized for sign-up"
@@ -979,7 +978,7 @@ class User < ActiveRecord::Base
       end
     end
 
-    self.errors.add(:email, error) unless valid
+    errors.add(:email, error) unless valid
 
     valid
   end

changelogs/unreleased/remove-unnecessary-self-from-user-model.yml

+---
+title: Remove unnecessary self from user model
+merge_request: 7551
+author: Semyon Pupkov