chore: rubocop

chore: fix api test conventions

chore: rubocop -A spec/

chore: more rubocop -A

fix failing test

rubocop fixes

removes helper methods that are in my opinion dead code

more rubocop fixes

rubocop -a --auto-gen-config

app/models/article.rb

@@ -42,7 +42,7 @@ class Article < ApplicationRecord
   belongs_to :supplier
   # @!attribute article_prices
   #   @return [Array<ArticlePrice>] Price history (current price first).
-  has_many :article_prices, -> { order("created_at DESC") }
+  has_many :article_prices, -> { order('created_at DESC') }
   # @!attribute order_articles
   #   @return [Array<OrderArticle>] Order articles for this article.
   has_many :order_articles
@@ -60,16 +60,16 @@ class Article < ApplicationRecord
   scope :not_in_stock, -> { where(type: nil) }
 
   # Validations
-  validates_presence_of :name, :unit, :price, :tax, :deposit, :unit_quantity, :supplier_id, :article_category
-  validates_length_of :name, :in => 4..60
-  validates_length_of :unit, :in => 1..15
-  validates_length_of :note, :maximum => 255
-  validates_length_of :origin, :maximum => 255
-  validates_length_of :manufacturer, :maximum => 255
-  validates_length_of :order_number, :maximum => 255
-  validates_numericality_of :price, :greater_than_or_equal_to => 0
-  validates_numericality_of :unit_quantity, :greater_than => 0
-  validates_numericality_of :deposit, :tax
+  validates :name, :unit, :price, :tax, :deposit, :unit_quantity, :supplier_id, :article_category, presence: true
+  validates :name, length: { in: 4..60 }
+  validates :unit, length: { in: 1..15 }
+  validates :note, length: { maximum: 255 }
+  validates :origin, length: { maximum: 255 }
+  validates :manufacturer, length: { maximum: 255 }
+  validates :order_number, length: { maximum: 255 }
+  validates :price, numericality: { greater_than_or_equal_to: 0 }
+  validates :unit_quantity, numericality: { greater_than: 0 }
+  validates :deposit, :tax, numericality: true
   # validates_uniqueness_of :name, :scope => [:supplier_id, :deleted_at, :type], if: Proc.new {|a| a.supplier.shared_sync_method.blank? or a.supplier.shared_sync_method == 'import' }
   # validates_uniqueness_of :name, :scope => [:supplier_id, :deleted_at, :type, :unit, :unit_quantity]
   validate :uniqueness_of_name
@@ -78,12 +78,12 @@ class Article < ApplicationRecord
   before_save :update_price_history
   before_destroy :check_article_in_use
 
-  def self.ransackable_attributes(auth_object = nil)
-    %w(id name supplier_id article_category_id unit note manufacturer origin unit_quantity order_number)
+  def self.ransackable_attributes(_auth_object = nil)
+    %w[id name supplier_id article_category_id unit note manufacturer origin unit_quantity order_number]
   end
 
-  def self.ransackable_associations(auth_object = nil)
-    %w(article_category supplier order_articles orders)
+  def self.ransackable_associations(_auth_object = nil)
+    %w[article_category supplier order_articles orders]
   end
 
   # Returns true if article has been updated at least 2 days ago
@@ -96,7 +96,7 @@ class Article < ApplicationRecord
     @in_open_order ||= begin
       order_articles = OrderArticle.where(order_id: Order.open.collect(&:id))
       order_article = order_articles.detect { |oa| oa.article_id == id }
-      order_article ? order_article.order : nil
+      order_article&.order
     end
   end
 
@@ -112,15 +112,15 @@ class Article < ApplicationRecord
   def shared_article_changed?(supplier = self.supplier)
     # skip early if the timestamp hasn't changed
     shared_article = self.shared_article(supplier)
-    unless shared_article.nil? || self.shared_updated_on == shared_article.updated_on
-      attrs = unequal_attributes(shared_article)
-      if attrs.empty?
-        # when attributes not changed, update timestamp of article
-        self.update_attribute(:shared_updated_on, shared_article.updated_on)
-        false
-      else
-        attrs
-      end
+    return if shared_article.nil? || shared_updated_on == shared_article.updated_on
+
+    attrs = unequal_attributes(shared_article)
+    if attrs.empty?
+      # when attributes not changed, update timestamp of article
+      update_attribute(:shared_updated_on, shared_article.updated_on)
+      false
+    else
+      attrs
     end
   end
 
@@ -131,30 +131,31 @@ class Article < ApplicationRecord
   def unequal_attributes(new_article, options = {})
     # try to convert different units when desired
     if options[:convert_units] == false
-      new_price, new_unit_quantity = nil, nil
+      new_price = nil
+      new_unit_quantity = nil
     else
       new_price, new_unit_quantity = convert_units(new_article)
     end
     if new_price && new_unit_quantity
-      new_unit = self.unit
+      new_unit = unit
     else
       new_price = new_article.price
       new_unit_quantity = new_article.unit_quantity
       new_unit = new_article.unit
     end
 
-    return Article.compare_attributes(
+    Article.compare_attributes(
       {
-        :name => [self.name, new_article.name],
-        :manufacturer => [self.manufacturer, new_article.manufacturer.to_s],
-        :origin => [self.origin, new_article.origin],
-        :unit => [self.unit, new_unit],
-        :price => [self.price.to_f.round(2), new_price.to_f.round(2)],
-        :tax => [self.tax, new_article.tax],
-        :deposit => [self.deposit.to_f.round(2), new_article.deposit.to_f.round(2)],
+        name: [name, new_article.name],
+        manufacturer: [manufacturer, new_article.manufacturer.to_s],
+        origin: [origin, new_article.origin],
+        unit: [unit, new_unit],
+        price: [price.to_f.round(2), new_price.to_f.round(2)],
+        tax: [tax, new_article.tax],
+        deposit: [deposit.to_f.round(2), new_article.deposit.to_f.round(2)],
         # take care of different num-objects.
-        :unit_quantity => [self.unit_quantity.to_s.to_f, new_unit_quantity.to_s.to_f],
-        :note => [self.note.to_s, new_article.note.to_s]
+        unit_quantity: [unit_quantity.to_s.to_f, new_unit_quantity.to_s.to_f],
+        note: [note.to_s, new_article.note.to_s]
       }
     )
   end
@@ -165,14 +166,20 @@ class Article < ApplicationRecord
   # @param attributes [Hash<Symbol, Array>] Attributes with old and new values
   # @return [Hash<Symbol, Object>] Changed attributes with new values
   def self.compare_attributes(attributes)
-    unequal_attributes = attributes.select { |name, values| values[0] != values[1] && !(values[0].blank? && values[1].blank?) }
-    Hash[unequal_attributes.to_a.map { |a| [a[0], a[1].last] }]
+    unequal_attributes = attributes.select do |_name, values|
+      values[0] != values[1] && !(values[0].blank? && values[1].blank?)
+    end
+    unequal_attributes.to_a.map { |a| [a[0], a[1].last] }.to_h
   end
 
   # to get the correspondent shared article
   def shared_article(supplier = self.supplier)
-    self.order_number.blank? and return nil
-    @shared_article ||= supplier.shared_supplier.find_article_by_number(self.order_number) rescue nil
+    order_number.blank? and return nil
+    @shared_article ||= begin
+      supplier.shared_supplier.find_article_by_number(order_number)
+    rescue StandardError
+      nil
+    end
   end
 
   # convert units in foodcoop-size
@@ -181,31 +188,37 @@ class Article < ApplicationRecord
   # returns false if units aren't foodsoft-compatible
   # returns nil if units are eqal
   def convert_units(new_article = shared_article)
-    if unit != new_article.unit
-      # legacy, used by foodcoops in Germany
-      if new_article.unit == "KI" && unit == "ST" # 'KI' means a box, with a different amount of items in it
-        # try to match the size out of its name, e.g. "banana 10-12 St" => 10
-        new_unit_quantity = /[0-9\-\s]+(St)/.match(new_article.name).to_s.to_i
-        if new_unit_quantity && new_unit_quantity > 0
-          new_price = (new_article.price / new_unit_quantity.to_f).round(2)
-          [new_price, new_unit_quantity]
-        else
-          false
-        end
-      else # use ruby-units to convert
-        fc_unit = (::Unit.new(unit) rescue nil)
-        supplier_unit = (::Unit.new(new_article.unit) rescue nil)
-        if fc_unit && supplier_unit && fc_unit =~ supplier_unit
-          conversion_factor = (supplier_unit / fc_unit).to_base.to_r
-          new_price = new_article.price / conversion_factor
-          new_unit_quantity = new_article.unit_quantity * conversion_factor
-          [new_price, new_unit_quantity]
-        else
-          false
-        end
+    return unless unit != new_article.unit
+
+    # legacy, used by foodcoops in Germany
+    if new_article.unit == 'KI' && unit == 'ST' # 'KI' means a box, with a different amount of items in it
+      # try to match the size out of its name, e.g. "banana 10-12 St" => 10
+      new_unit_quantity = /[0-9\-\s]+(St)/.match(new_article.name).to_s.to_i
+      if new_unit_quantity && new_unit_quantity > 0
+        new_price = (new_article.price / new_unit_quantity.to_f).round(2)
+        [new_price, new_unit_quantity]
+      else
+        false
+      end
+    else # use ruby-units to convert
+      fc_unit = begin
+        ::Unit.new(unit)
+      rescue StandardError
+        nil
+      end
+      supplier_unit = begin
+        ::Unit.new(new_article.unit)
+      rescue StandardError
+        nil
+      end
+      if fc_unit && supplier_unit && fc_unit =~ supplier_unit
+        conversion_factor = (supplier_unit / fc_unit).to_base.to_r
+        new_price = new_article.price / conversion_factor
+        new_unit_quantity = new_article.unit_quantity * conversion_factor
+        [new_price, new_unit_quantity]
+      else
+        false
       end
-    else
-      nil
     end
   end
 
@@ -222,19 +235,19 @@ class Article < ApplicationRecord
 
   # Checks if the article is in use before it will deleted
   def check_article_in_use
-    raise I18n.t('articles.model.error_in_use', :article => self.name.to_s) if self.in_open_order
+    raise I18n.t('articles.model.error_in_use', article: name.to_s) if in_open_order
   end
 
   # Create an ArticlePrice, when the price-attr are changed.
   def update_price_history
-    if price_changed?
-      article_prices.build(
-        :price => price,
-        :tax => tax,
-        :deposit => deposit,
-        :unit_quantity => unit_quantity
-      )
-    end
+    return unless price_changed?
+
+    article_prices.build(
+      price: price,
+      tax: tax,
+      deposit: deposit,
+      unit_quantity: unit_quantity
+    )
   end
 
   def price_changed?
@@ -250,8 +263,8 @@ class Article < ApplicationRecord
     # supplier should always be there - except, perhaps, on initialization (on seeding)
     if supplier && (supplier.shared_sync_method.blank? || supplier.shared_sync_method == 'import')
       errors.add :name, :taken if matches.any?
-    else
-      errors.add :name, :taken_with_unit if matches.where(unit: unit, unit_quantity: unit_quantity).any?
+    elsif matches.where(unit: unit, unit_quantity: unit_quantity).any?
+      errors.add :name, :taken_with_unit
     end
   end
 end

app/models/article_category.rb

@@ -17,16 +17,16 @@ class ArticleCategory < ApplicationRecord
 
   normalize_attributes :name, :description
 
-  validates :name, :presence => true, :uniqueness => true, :length => { :minimum => 2 }
+  validates :name, presence: true, uniqueness: true, length: { minimum: 2 }
 
   before_destroy :check_for_associated_articles
 
-  def self.ransackable_attributes(auth_object = nil)
-    %w(id name)
+  def self.ransackable_attributes(_auth_object = nil)
+    %w[id name]
   end
 
-  def self.ransackable_associations(auth_object = nil)
-    %w(articles order_articles orders)
+  def self.ransackable_associations(_auth_object = nil)
+    %w[articles order_articles orders]
   end
 
   # Find a category that matches a category name; may return nil.
@@ -40,7 +40,11 @@ class ArticleCategory < ApplicationRecord
     # case-insensitive substring match (take the closest match = shortest)
     c = ArticleCategory.where('name LIKE ?', "%#{category}%") unless c && c.any?
     # case-insensitive phrase present in category description
-    c = ArticleCategory.where('description LIKE ?', "%#{category}%").select { |s| s.description.match /(^|,)\s*#{category}\s*(,|$)/i } unless c && c.any?
+    unless c && c.any?
+      c = ArticleCategory.where('description LIKE ?', "%#{category}%").select do |s|
+        s.description.match(/(^|,)\s*#{category}\s*(,|$)/i)
+      end
+    end
     # return closest match if there are multiple
     c = c.sort_by { |s| s.name.length }.first if c.respond_to? :sort_by
     c
@@ -50,6 +54,9 @@ class ArticleCategory < ApplicationRecord
 
   # Deny deleting the category when there are associated articles.
   def check_for_associated_articles
-    raise I18n.t('activerecord.errors.has_many_left', collection: Article.model_name.human) if articles.undeleted.exists?
+    return unless articles.undeleted.exists?
+
+    raise I18n.t('activerecord.errors.has_many_left',
+                 collection: Article.model_name.human)
   end
 end

app/models/article_price.rb

@@ -24,8 +24,8 @@ class ArticlePrice < ApplicationRecord
 
   localize_input_of :price, :tax, :deposit
 
-  validates_presence_of :price, :tax, :deposit, :unit_quantity
-  validates_numericality_of :price, :greater_than_or_equal_to => 0
-  validates_numericality_of :unit_quantity, :greater_than => 0
-  validates_numericality_of :deposit, :tax
+  validates :price, :tax, :deposit, :unit_quantity, presence: true
+  validates :price, numericality: { greater_than_or_equal_to: 0 }
+  validates :unit_quantity, numericality: { greater_than: 0 }
+  validates :deposit, :tax, numericality: true
 end

app/models/bank_account.rb

@@ -5,10 +5,10 @@ class BankAccount < ApplicationRecord
 
   normalize_attributes :name, :iban, :description
 
-  validates :name, :presence => true, :uniqueness => true, :length => { :minimum => 2 }
-  validates :iban, :presence => true, :uniqueness => true
-  validates_format_of :iban, :with => /\A[A-Z]{2}[0-9]{2}[0-9A-Z]{,30}\z/
-  validates_numericality_of :balance, :message => I18n.t('bank_account.model.invalid_balance')
+  validates :name, presence: true, uniqueness: true, length: { minimum: 2 }
+  validates :iban, presence: true, uniqueness: true
+  validates :iban, format: { with: /\A[A-Z]{2}[0-9]{2}[0-9A-Z]{,30}\z/ }
+  validates :balance, numericality: { message: I18n.t('bank_account.model.invalid_balance') }
 
   # @return [Function] Method wich can be called to import transaction from a bank or nil if unsupported
   def find_connector
@@ -18,10 +18,8 @@ class BankAccount < ApplicationRecord
 
   def assign_unlinked_transactions
     count = 0
-    bank_transactions.without_financial_link.includes(:supplier, :user).each do |t|
-      if t.assign_to_ordergroup || t.assign_to_invoice
-        count += 1
-      end
+    bank_transactions.without_financial_link.includes(:supplier, :user).find_each do |t|
+      count += 1 if t.assign_to_ordergroup || t.assign_to_invoice
     end
     count
   end

app/models/bank_gateway.rb

@@ -4,5 +4,5 @@ class BankGateway < ApplicationRecord
 
   scope :with_unattended_support, -> { where.not(unattended_user: nil) }
 
-  validates_presence_of :name, :url
+  validates :name, :url, presence: true
 end

app/models/bank_transaction.rb

@@ -22,8 +22,8 @@ class BankTransaction < ApplicationRecord
   belongs_to :supplier, optional: true, foreign_key: 'iban', primary_key: 'iban'
   belongs_to :user, optional: true, foreign_key: 'iban', primary_key: 'iban'
 
-  validates_presence_of :date, :amount, :bank_account_id
-  validates_numericality_of :amount
+  validates :date, :amount, :bank_account_id, presence: true
+  validates :amount, numericality: true
 
   scope :without_financial_link, -> { where(financial_link: nil) }
 
@@ -31,13 +31,13 @@ class BankTransaction < ApplicationRecord
   localize_input_of :amount
 
   def image_url
-    'data:image/png;base64,' + Base64.encode64(self.image)
+    'data:image/png;base64,' + Base64.encode64(image)
   end
 
   def assign_to_invoice
     return false unless supplier
 
-    content = text || ""
+    content = text || ''
     content += "\n" + reference if reference.present?
     invoices = supplier.invoices.unpaid.select { |i| content.include? i.number }
     invoices_sum = invoices.map(&:amount).sum
@@ -49,7 +49,7 @@ class BankTransaction < ApplicationRecord
       update_attribute :financial_link, link
     end
 
-    return true
+    true
   end
 
   def assign_to_ordergroup
@@ -78,6 +78,6 @@ class BankTransaction < ApplicationRecord
       update_attribute :financial_link, link
     end
 
-    return true
+    true
   end
 end

app/models/concerns/custom_fields.rb

@@ -10,7 +10,7 @@ module CustomFields
     end
 
     after_save do
-      self.settings.custom_fields = custom_fields if custom_fields
+      settings.custom_fields = custom_fields if custom_fields
     end
   end
 end

app/models/concerns/find_each_with_order.rb

@@ -3,9 +3,9 @@ module FindEachWithOrder
   extend ActiveSupport::Concern
 
   class_methods do
-    def find_each_with_order(options = {})
+    def find_each_with_order(options = {}, &block)
       find_in_batches_with_order(options) do |records|
-        records.each { |record| yield record }
+        records.each(&block)
       end
     end
 

app/models/concerns/localize_input.rb

@@ -5,12 +5,12 @@ module LocalizeInput
     return input unless input.is_a? String
 
     Rails.logger.debug { "Input: #{input.inspect}" }
-    separator = I18n.t("separator", scope: "number.format")
-    delimiter = I18n.t("delimiter", scope: "number.format")
-    input.gsub!(delimiter, "") if input.match(/\d+#{Regexp.escape(delimiter)}+\d+#{Regexp.escape(separator)}+\d+/) # Remove delimiter
-    input.gsub!(separator, ".") # Replace separator with db compatible character
+    separator = I18n.t('separator', scope: 'number.format')
+    delimiter = I18n.t('delimiter', scope: 'number.format')
+    input.gsub!(delimiter, '') if input.match(/\d+#{Regexp.escape(delimiter)}+\d+#{Regexp.escape(separator)}+\d+/) # Remove delimiter
+    input.gsub!(separator, '.') # Replace separator with db compatible character
     input
-  rescue
+  rescue StandardError
     Rails.logger.warn "Can't localize input: #{input}"
     input
   end

app/models/concerns/mark_as_deleted_with_name.rb

@@ -3,7 +3,7 @@ module MarkAsDeletedWithName
 
   def mark_as_deleted
     # get maximum length of name
-    max_length = 100000
+    max_length = 100_000
     if lenval = self.class.validators_on(:name).detect { |v| v.is_a?(ActiveModel::Validations::LengthValidator) }
       max_length = lenval.options[:maximum]
     end

app/models/concerns/price_calculation.rb

@@ -15,6 +15,6 @@ module PriceCalculation
   private
 
   def add_percent(value, percent)
-    (value * (percent * 0.01 + 1)).round(2)
+    (value * ((percent * 0.01) + 1)).round(2)
   end
 end

app/models/delivery.rb

@@ -4,10 +4,10 @@ class Delivery < StockEvent
 
   scope :recent, -> { order('created_at DESC').limit(10) }
 
-  validates_presence_of :supplier_id
+  validates :supplier_id, presence: true
   validate :stock_articles_must_be_unique
 
-  accepts_nested_attributes_for :stock_changes, :allow_destroy => :true
+  accepts_nested_attributes_for :stock_changes, allow_destroy: :true
 
   def new_stock_changes=(stock_change_attributes)
     for attributes in stock_change_attributes
@@ -16,7 +16,7 @@ class Delivery < StockEvent
   end
 
   def includes_article?(article)
-    self.stock_changes.map { |stock_change| stock_change.stock_article.id }.include? article.id
+    stock_changes.map { |stock_change| stock_change.stock_article.id }.include? article.id
   end
 
   def sum(type = :gross)
@@ -39,8 +39,8 @@ class Delivery < StockEvent
   protected
 
   def stock_articles_must_be_unique
-    unless stock_changes.reject { |sc| sc.marked_for_destruction? }.map { |sc| sc.stock_article.id }.uniq!.nil?
-      errors.add(:base, I18n.t('model.delivery.each_stock_article_must_be_unique'))
-    end
+    return if stock_changes.reject { |sc| sc.marked_for_destruction? }.map { |sc| sc.stock_article.id }.uniq!.nil?
+
+    errors.add(:base, I18n.t('model.delivery.each_stock_article_must_be_unique'))
   end
 end

app/models/financial_link.rb

@@ -4,13 +4,13 @@ class FinancialLink < ApplicationRecord
   has_many :invoices
 
   scope :incomplete, -> { with_full_sum.where.not('full_sums.full_sum' => 0) }
-  scope :unused, -> {
+  scope :unused, lambda {
     includes(:bank_transactions, :financial_transactions, :invoices)
       .where(bank_transactions: { financial_link_id: nil })
       .where(financial_transactions: { financial_link_id: nil })
       .where(invoices: { financial_link_id: nil })
   }
-  scope :with_full_sum, -> {
+  scope :with_full_sum, lambda {
     select(:id, :note, :full_sum).joins(<<-SQL)
       LEFT JOIN (
         SELECT id, COALESCE(bt_sum, 0) - COALESCE(ft_sum, 0) + COALESCE(i_sum, 0) AS full_sum

app/models/financial_transaction.rb

@@ -8,14 +8,16 @@ class FinancialTransaction < ApplicationRecord
   belongs_to :financial_link, optional: true
   belongs_to :financial_transaction_type
   belongs_to :group_order, optional: true
-  belongs_to :reverts, optional: true, class_name: 'FinancialTransaction', foreign_key: 'reverts_id'
+  belongs_to :reverts, optional: true, class_name: 'FinancialTransaction'
   has_one :reverted_by, class_name: 'FinancialTransaction', foreign_key: 'reverts_id'
 
-  validates_presence_of :amount, :note, :user_id
-  validates_numericality_of :amount, greater_then: -100_000,
-                                     less_than: 100_000
+  validates :amount, :note, :user_id, presence: true
+  validates :amount, numericality: { greater_then: -100_000,
+                                     less_than: 100_000 }
 
-  scope :visible, -> { joins('LEFT JOIN financial_transactions r ON financial_transactions.id = r.reverts_id').where('r.id IS NULL').where(reverts: nil) }
+  scope :visible, lambda {
+                    joins('LEFT JOIN financial_transactions r ON financial_transactions.id = r.reverts_id').where('r.id IS NULL').where(reverts: nil)
+                  }
   scope :without_financial_link, -> { where(financial_link: nil) }
   scope :with_ordergroup, -> { where.not(ordergroup: nil) }
 
@@ -28,12 +30,12 @@ class FinancialTransaction < ApplicationRecord
   # @todo remove alias (and rename created_on to created_at below) after #575
   ransack_alias :created_at, :created_on
 
-  def self.ransackable_attributes(auth_object = nil)
-    %w(id amount note created_on user_id)
+  def self.ransackable_attributes(_auth_object = nil)
+    %w[id amount note created_on user_id]
   end
 
-  def self.ransackable_associations(auth_object = nil)
-    %w() # none, and certainly not user until we've secured that more
+  def self.ransackable_associations(_auth_object = nil)
+    %w[] # none, and certainly not user until we've secured that more
   end
 
   # Use this save method instead of simple save and after callback

app/models/financial_transaction_class.rb

@@ -5,7 +5,7 @@ class FinancialTransactionClass < ApplicationRecord
   has_many :ordergroups, -> { distinct }, through: :financial_transactions
 
   validates :name, presence: true
-  validates_uniqueness_of :name
+  validates :name, uniqueness: true
 
   after_save :update_balance_of_ordergroups
 

app/models/financial_transaction_type.rb

@@ -5,13 +5,13 @@ class FinancialTransactionType < ApplicationRecord
   has_many :ordergroups, -> { distinct }, through: :financial_transactions
 
   validates :name, presence: true
-  validates_uniqueness_of :name
-  validates_uniqueness_of :name_short, allow_blank: true, allow_nil: true
-  validates_format_of :name_short, :with => /\A[A-Za-z]*\z/
+  validates :name, uniqueness: true
+  validates :name_short, uniqueness: { allow_blank: true }
+  validates :name_short, format: { with: /\A[A-Za-z]*\z/ }
   validates :financial_transaction_class, presence: true
 
-  after_save :update_balance_of_ordergroups
   before_destroy :restrict_deleting_last_financial_transaction_type
+  after_save :update_balance_of_ordergroups
 
   scope :with_name_short, -> { where.not(name_short: [nil, '']) }
 
@@ -20,7 +20,7 @@ class FinancialTransactionType < ApplicationRecord
   end
 
   def self.has_multiple_types
-    self.count > 1
+    count > 1
   end
 
   protected

app/models/group.rb

@@ -7,8 +7,8 @@ class Group < ApplicationRecord
   has_many :memberships, dependent: :destroy
   has_many :users, -> { where(deleted_at: nil) }, through: :memberships
 
-  validates :name, :presence => true, :length => { :in => 1..25 }
-  validates_uniqueness_of :name
+  validates :name, presence: true, length: { in: 1..25 }
+  validates :name, uniqueness: true
 
   attr_reader :user_tokens
 
@@ -25,7 +25,7 @@ class Group < ApplicationRecord
   end
 
   def user_tokens=(ids)
-    self.user_ids = ids.split(",")
+    self.user_ids = ids.split(',')
   end
 
   def deleted?

app/models/group_order.rb

@@ -6,14 +6,14 @@ class GroupOrder < ApplicationRecord
 
   belongs_to :order
   belongs_to :ordergroup, optional: true
-  has_many :group_order_articles, :dependent => :destroy
-  has_many :order_articles, :through => :group_order_articles
+  has_many :group_order_articles, dependent: :destroy
+  has_many :order_articles, through: :group_order_articles
   has_one :financial_transaction
   belongs_to :updated_by, optional: true, class_name: 'User', foreign_key: 'updated_by_user_id'
 
-  validates_presence_of :order_id
-  validates_numericality_of :price
-  validates_uniqueness_of :ordergroup_id, :scope => :order_id # order groups can only order once per order
+  validates :order_id, presence: true
+  validates :price, numericality: true
+  validates :ordergroup_id, uniqueness: { scope: :order_id } # order groups can only order once per order
 
   scope :in_open_orders, -> { joins(:order).merge(Order.open) }
   scope :in_finished_orders, -> { joins(:order).merge(Order.finished_not_closed) }
@@ -21,12 +21,12 @@ class GroupOrder < ApplicationRecord
 
   scope :ordered, -> { includes(:ordergroup).order('groups.name') }
 
-  def self.ransackable_attributes(auth_object = nil)
-    %w(id price)
+  def self.ransackable_attributes(_auth_object = nil)
+    %w[id price]
   end
 
-  def self.ransackable_associations(auth_object = nil)
-    %w(order group_order_articles)
+  def self.ransackable_associations(_auth_object = nil)
+    %w[order group_order_articles]
   end
 
   # Generate some data for the javascript methods in ordering view
@@ -37,24 +37,24 @@ class GroupOrder < ApplicationRecord
 
     # load prices and other stuff....
     data[:order_articles] = {}
-    order.articles_grouped_by_category.each do |article_category, order_articles|
+    order.articles_grouped_by_category.each do |_article_category, order_articles|
       order_articles.each do |order_article|
         # Get the result of last time ordering, if possible
         goa = group_order_articles.detect { |goa| goa.order_article_id == order_article.id }
 
         # Build hash with relevant data
         data[:order_articles][order_article.id] = {
-          :price => order_article.article.fc_price,
-          :unit => order_article.article.unit_quantity,
-          :quantity => (goa ? goa.quantity : 0),
-          :others_quantity => order_article.quantity - (goa ? goa.quantity : 0),
-          :used_quantity => (goa ? goa.result(:quantity) : 0),
-          :tolerance => (goa ? goa.tolerance : 0),
-          :others_tolerance => order_article.tolerance - (goa ? goa.tolerance : 0),
-          :used_tolerance => (goa ? goa.result(:tolerance) : 0),
-          :total_price => (goa ? goa.total_price : 0),
-          :missing_units => order_article.missing_units,
-          :quantity_available => (order.stockit? ? order_article.article.quantity_available : 0)
+          price: order_article.article.fc_price,
+          unit: order_article.article.unit_quantity,
+          quantity: (goa ? goa.quantity : 0),
+          others_quantity: order_article.quantity - (goa ? goa.quantity : 0),
+          used_quantity: (goa ? goa.result(:quantity) : 0),
+          tolerance: (goa ? goa.tolerance : 0),
+          others_tolerance: order_article.tolerance - (goa ? goa.tolerance : 0),
+          used_tolerance: (goa ? goa.result(:tolerance) : 0),
+          total_price: (goa ? goa.total_price : 0),
+          missing_units: order_article.missing_units,
+          quantity_available: (order.stockit? ? order_article.article.quantity_available : 0)
         }
       end
     end
@@ -69,12 +69,12 @@ class GroupOrder < ApplicationRecord
 
       # Get ordered quantities and update group_order_articles/_quantities...
       if group_order_articles_attributes
-        quantities = group_order_articles_attributes.fetch(order_article.id.to_s, { :quantity => 0, :tolerance => 0 })
+        quantities = group_order_articles_attributes.fetch(order_article.id.to_s, { quantity: 0, tolerance: 0 })
         group_order_article.update_quantities(quantities[:quantity].to_i, quantities[:tolerance].to_i)
       end
 
       # Also update results for the order_article
-      logger.debug "[save_group_order_articles] update order_article.results!"
+      logger.debug '[save_group_order_articles] update order_article.results!'
       order_article.update_results!
     end
 
@@ -83,7 +83,7 @@ class GroupOrder < ApplicationRecord
 
   # Updates the "price" attribute.
   def update_price!
-    total = group_order_articles.includes(:order_article => [:article, :article_price]).to_a.sum(&:total_price)
+    total = group_order_articles.includes(order_article: %i[article article_price]).to_a.sum(&:total_price)
     update_attribute(:price, total)
   end
 
@@ -97,7 +97,12 @@ class GroupOrder < ApplicationRecord
   end
 
   def ordergroup_name
-    ordergroup ? ordergroup.name : I18n.t('model.group_order.stock_ordergroup_name', :user => updated_by.try(:name) || '?')
+    if ordergroup
+      ordergroup.name
+    else
+      I18n.t('model.group_order.stock_ordergroup_name',
+             user: updated_by.try(:name) || '?')
+    end
   end
 
   def total

app/models/group_order_article.rb

@@ -8,21 +8,21 @@ class GroupOrderArticle < ApplicationRecord
   belongs_to :order_article
   has_many   :group_order_article_quantities, dependent: :destroy
 
-  validates_presence_of :group_order, :order_article
-  validates_uniqueness_of :order_article_id, :scope => :group_order_id # just once an article per group order
+  validates :group_order, :order_article, presence: true
+  validates :order_article_id, uniqueness: { scope: :group_order_id } # just once an article per group order
   validate :check_order_not_closed # don't allow changes to closed (aka settled) orders
   validates :quantity, :tolerance, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
 
-  scope :ordered, -> { includes(:group_order => :ordergroup).order('groups.name') }
+  scope :ordered, -> { includes(group_order: :ordergroup).order('groups.name') }
 
   localize_input_of :result
 
-  def self.ransackable_attributes(auth_object = nil)
-    %w(id quantity tolerance result)
+  def self.ransackable_attributes(_auth_object = nil)
+    %w[id quantity tolerance result]
   end
 
-  def self.ransackable_associations(auth_object = nil)
-    %w(order_article group_order)
+  def self.ransackable_associations(_auth_object = nil)
+    %w[order_article group_order]
   end
 
   # Setter used in group_order_article#new
@@ -32,7 +32,7 @@ class GroupOrderArticle < ApplicationRecord
   end
 
   def ordergroup_id
-    group_order.try!(:ordergroup_id)
+    group_order&.ordergroup_id
   end
 
   # Updates the quantity/tolerance for this GroupOrderArticle by updating both GroupOrderArticle properties
@@ -45,7 +45,7 @@ class GroupOrderArticle < ApplicationRecord
 
     # When quantity and tolerance are zero, we don't serve any purpose
     if quantity == 0 && tolerance == 0
-      logger.debug("Self-destructing since requested quantity and tolerance are zero")
+      logger.debug('Self-destructing since requested quantity and tolerance are zero')
       destroy!
       return
     end
@@ -54,26 +54,28 @@ class GroupOrderArticle < ApplicationRecord
     quantities = group_order_article_quantities.order('created_on DESC').to_a
     logger.debug("GroupOrderArticleQuantity items found: #{quantities.size}")
 
-    if (quantities.size == 0)
+    if quantities.size == 0
       # There is no GroupOrderArticleQuantity item yet, just insert with desired quantities...
-      logger.debug("No quantities entry at all, inserting a new one with the desired quantities")
-      quantities.push(GroupOrderArticleQuantity.new(:group_order_article => self, :quantity => quantity, :tolerance => tolerance))
-      self.quantity, self.tolerance = quantity, tolerance
+      logger.debug('No quantities entry at all, inserting a new one with the desired quantities')
+      quantities.push(GroupOrderArticleQuantity.new(group_order_article: self, quantity: quantity,
+                                                    tolerance: tolerance))
+      self.quantity = quantity
+      self.tolerance = tolerance
     else
       # Decrease quantity/tolerance if necessary by going through the existing items and decreasing their values...
       i = 0
-      while (i < quantities.size && (quantity < self.quantity || tolerance < self.tolerance))
+      while i < quantities.size && (quantity < self.quantity || tolerance < self.tolerance)
         logger.debug("Need to decrease quantities for GroupOrderArticleQuantity[#{quantities[i].id}]")
-        if (quantity < self.quantity && quantities[i].quantity > 0)
+        if quantity < self.quantity && quantities[i].quantity > 0
           delta = self.quantity - quantity
-          delta = (delta > quantities[i].quantity ? quantities[i].quantity : delta)
+          delta = [delta, quantities[i].quantity].min
           logger.debug("Decreasing quantity by #{delta}")
           quantities[i].quantity -= delta
           self.quantity -= delta
         end
-        if (tolerance < self.tolerance && quantities[i].tolerance > 0)
+        if tolerance < self.tolerance && quantities[i].tolerance > 0
           delta = self.tolerance - tolerance
-          delta = (delta > quantities[i].tolerance ? quantities[i].tolerance : delta)
+          delta = [delta, quantities[i].tolerance].min
           logger.debug("Decreasing tolerance by #{delta}")
           quantities[i].tolerance -= delta
           self.tolerance -= delta
@@ -81,12 +83,12 @@ class GroupOrderArticle < ApplicationRecord
         i += 1
       end
       # If there is at least one increased value: insert a new GroupOrderArticleQuantity object
-      if (quantity > self.quantity || tolerance > self.tolerance)
-        logger.debug("Inserting a new GroupOrderArticleQuantity")
+      if quantity > self.quantity || tolerance > self.tolerance
+        logger.debug('Inserting a new GroupOrderArticleQuantity')
         quantities.insert(0, GroupOrderArticleQuantity.new(
-                               :group_order_article => self,
-                               :quantity => (quantity > self.quantity ? quantity - self.quantity : 0),
-                               :tolerance => (tolerance > self.tolerance ? tolerance - self.tolerance : 0)
+                               group_order_article: self,
+                               quantity: (quantity > self.quantity ? quantity - self.quantity : 0),
+                               tolerance: (tolerance > self.tolerance ? tolerance - self.tolerance : 0)
                              ))
         # Recalc totals:
         self.quantity += quantities[0].quantity
@@ -95,8 +97,9 @@ class GroupOrderArticle < ApplicationRecord
     end
 
     # Check if something went terribly wrong and quantites have not been adjusted as desired.
-    if (self.quantity != quantity || self.tolerance != tolerance)
-      raise ActiveRecord::RecordNotSaved.new('Unable to update GroupOrderArticle/-Quantities to desired quantities!', self)
+    if self.quantity != quantity || self.tolerance != tolerance
+      raise ActiveRecord::RecordNotSaved.new('Unable to update GroupOrderArticle/-Quantities to desired quantities!',
+                                             self)
     end
 
     # Remove zero-only items.
@@ -121,7 +124,7 @@ class GroupOrderArticle < ApplicationRecord
     quantity = tolerance = total_quantity = 0
 
     # Get total
-    if not total.nil?
+    if !total.nil?
       logger.debug "<#{order_article.article.name}> => #{total} (given)"
     elsif order_article.article.is_a?(StockArticle)
       total = order_article.article.quantity
@@ -145,7 +148,7 @@ class GroupOrderArticle < ApplicationRecord
         q = goaq.quantity
         q = [q, total - total_quantity].min if first_order_first_serve
         total_quantity += q
-        if goaq.group_order_article_id == self.id
+        if goaq.group_order_article_id == id
           logger.debug "increasing quantity by #{q}"
           quantity += q
         end
@@ -154,11 +157,11 @@ class GroupOrderArticle < ApplicationRecord
 
       # Determine tolerance to be ordered...
       if total_quantity < total
-        logger.debug "determining additional items to be ordered from tolerance"
+        logger.debug 'determining additional items to be ordered from tolerance'
         order_quantities.each do |goaq|
           q = [goaq.tolerance, total - total_quantity].min
           total_quantity += q
-          if goaq.group_order_article_id == self.id
+          if goaq.group_order_article_id == id
             logger.debug "increasing tolerance by #{q}"
             tolerance += q
           end
@@ -170,7 +173,7 @@ class GroupOrderArticle < ApplicationRecord
     end
 
     # memoize result unless a total is given
-    r = { :quantity => quantity, :tolerance => tolerance, :total => quantity + tolerance }
+    r = { quantity: quantity, tolerance: tolerance, total: quantity + tolerance }
     @calculate_result = r if total.nil?
     r
   end
@@ -185,8 +188,8 @@ class GroupOrderArticle < ApplicationRecord
   # This is used for automatic distribution, e.g., in order.finish! or when receiving orders
   def save_results!(article_total = nil)
     new_result = calculate_result(article_total)[:total]
-    self.update_attribute(:result_computed, new_result)
-    self.update_attribute(:result, new_result)
+    update_attribute(:result_computed, new_result)
+    update_attribute(:result, new_result)
   end
 
   # Returns total price for this individual article
@@ -213,8 +216,8 @@ class GroupOrderArticle < ApplicationRecord
   private
 
   def check_order_not_closed
-    if order_article.order.closed?
-      errors.add(:order_article, I18n.t('model.group_order_article.order_closed'))
-    end
+    return unless order_article.order.closed?
+
+    errors.add(:order_article, I18n.t('model.group_order_article.order_closed'))
   end
 end

app/models/group_order_article_quantity.rb

@@ -4,5 +4,5 @@
 class GroupOrderArticleQuantity < ApplicationRecord
   belongs_to :group_order_article
 
-  validates_presence_of :group_order_article_id
+  validates :group_order_article_id, presence: true
 end

app/models/invite.rb

@@ -5,12 +5,12 @@ class Invite < ApplicationRecord
   belongs_to :user
   belongs_to :group
 
-  validates_format_of :email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i
-  validates_presence_of :user
-  validates_presence_of :group
-  validates_presence_of :token
-  validates_presence_of :expires_at
-  validate :email_not_already_registered, :on => :create
+  validates :email, format: { with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i }
+  validates :user, presence: true
+  validates :group, presence: true
+  validates :token, presence: true
+  validates :expires_at, presence: true
+  validate :email_not_already_registered, on: :create
 
   before_validation :set_token_and_expires_at
 
@@ -19,15 +19,15 @@ class Invite < ApplicationRecord
   # Before validation, set token and expires_at.
   def set_token_and_expires_at
     self.token = Digest::SHA1.hexdigest(Time.now.to_s + rand(100).to_s)
-    self.expires_at = Time.now.advance(:days => 7)
+    self.expires_at = Time.now.advance(days: 7)
   end
 
   private
 
   # Custom validation: check that email does not already belong to a registered user.
   def email_not_already_registered
-    unless User.find_by_email(self.email).nil?
-      errors.add(:email, I18n.t('invites.errors.already_member'))
-    end
+    return if User.find_by_email(email).nil?
+
+    errors.add(:email, I18n.t('invites.errors.already_member'))
   end
 end

app/models/invoice.rb

@@ -3,13 +3,13 @@ class Invoice < ApplicationRecord
   include LocalizeInput
 
   belongs_to :supplier
-  belongs_to :created_by, :class_name => 'User', :foreign_key => 'created_by_user_id'
+  belongs_to :created_by, class_name: 'User', foreign_key: 'created_by_user_id'
   belongs_to :financial_link, optional: true
   has_many :deliveries, dependent: :nullify
   has_many :orders, dependent: :nullify
 
-  validates_presence_of :supplier_id
-  validates_numericality_of :amount, :deposit, :deposit_credit
+  validates :supplier_id, presence: true
+  validates :amount, :deposit, :deposit_credit, numericality: true
   validate :valid_attachment
 
   scope :unpaid, -> { where(paid_on: nil) }
@@ -23,18 +23,18 @@ class Invoice < ApplicationRecord
   def attachment=(incoming_file)
     self.attachment_data = incoming_file.read
     # allow to soft-fail when FileMagic isn't present and removed from Gemfile (e.g. Heroku)
-    self.attachment_mime = defined?(FileMagic) ? FileMagic.new(FileMagic::MAGIC_MIME).buffer(self.attachment_data) : 'application/octet-stream'
+    self.attachment_mime = defined?(FileMagic) ? FileMagic.new(FileMagic::MAGIC_MIME).buffer(attachment_data) : 'application/octet-stream'
   end
 
   def delete_attachment=(value)
-    if value == '1'
-      self.attachment_data = nil
-      self.attachment_mime = nil
-    end
+    return unless value == '1'
+
+    self.attachment_data = nil
+    self.attachment_mime = nil
   end
 
   def user_can_edit?(user)
-    user.role_finance? || (user.role_invoices? && !self.paid_on && self.created_by.try(:id) == user.id)
+    user.role_finance? || (user.role_invoices? && !paid_on && created_by.try(:id) == user.id)
   end
 
   # Amount without deposit
@@ -45,9 +45,9 @@ class Invoice < ApplicationRecord
   def orders_sum
     orders
       .joins(order_articles: [:article_price])
-      .sum("COALESCE(order_articles.units_received, order_articles.units_billed, order_articles.units_to_order)" \
-        + "* article_prices.unit_quantity" \
-        + "* ROUND((article_prices.price + article_prices.deposit) * (100 + article_prices.tax) / 100, 2)")
+      .sum('COALESCE(order_articles.units_received, order_articles.units_billed, order_articles.units_to_order)' \
+        + '* article_prices.unit_quantity' \
+        + '* ROUND((article_prices.price + article_prices.deposit) * (100 + article_prices.tax) / 100, 2)')
   end
 
   def orders_transport_sum
@@ -63,11 +63,11 @@ class Invoice < ApplicationRecord
   protected
 
   def valid_attachment
-    if attachment_data
-      mime = MIME::Type.simplified(attachment_mime)
-      unless ['application/pdf', 'image/jpeg'].include? mime
-        errors.add :attachment, I18n.t('model.invoice.invalid_mime', :mime => mime)
-      end
-    end
+    return unless attachment_data
+
+    mime = MIME::Type.simplified(attachment_mime)
+    return if ['application/pdf', 'image/jpeg'].include? mime
+
+    errors.add :attachment, I18n.t('model.invoice.invalid_mime', mime: mime)
   end
 end

app/models/membership.rb

@@ -8,6 +8,6 @@ class Membership < ApplicationRecord
 
   # check if this is the last admin-membership and deny
   def check_last_admin
-    raise I18n.t('model.membership.no_admin_delete') if self.group.role_admin? && self.group.memberships.size == 1 && Group.where(role_admin: true).count == 1
+    raise I18n.t('model.membership.no_admin_delete') if group.role_admin? && group.memberships.size == 1 && Group.where(role_admin: true).count == 1
   end
 end

app/models/order.rb

@@ -2,29 +2,29 @@ class Order < ApplicationRecord
   attr_accessor :ignore_warnings, :transport_distribution
 
   # Associations
-  has_many :order_articles, :dependent => :destroy
-  has_many :articles, :through => :order_articles
-  has_many :group_orders, :dependent => :destroy
-  has_many :ordergroups, :through => :group_orders
-  has_many :users_ordered, :through => :ordergroups, :source => :users
-  has_many :comments, -> { order('created_at') }, :class_name => "OrderComment"
+  has_many :order_articles, dependent: :destroy
+  has_many :articles, through: :order_articles
+  has_many :group_orders, dependent: :destroy
+  has_many :ordergroups, through: :group_orders
+  has_many :users_ordered, through: :ordergroups, source: :users
+  has_many :comments, -> { order('created_at') }, class_name: 'OrderComment'
   has_many :stock_changes
   belongs_to :invoice, optional: true
   belongs_to :supplier, optional: true
-  belongs_to :updated_by, :class_name => 'User', :foreign_key => 'updated_by_user_id'
-  belongs_to :created_by, :class_name => 'User', :foreign_key => 'created_by_user_id'
+  belongs_to :updated_by, class_name: 'User', foreign_key: 'updated_by_user_id'
+  belongs_to :created_by, class_name: 'User', foreign_key: 'created_by_user_id'
 
   enum end_action: { no_end_action: 0, auto_close: 1, auto_close_and_send: 2, auto_close_and_send_min_quantity: 3 }
-  enum transport_distribution: [:skip, :ordergroup, :price, :articles]
+  enum transport_distribution: { skip: 0, ordergroup: 1, price: 2, articles: 3 }
 
   # Validations
-  validates_presence_of :starts
+  validates :starts, presence: true
   validate :starts_before_ends, :include_articles
   validate :keep_ordered_articles
 
+  before_validation :distribute_transport
   # Callbacks
   after_save :save_order_articles, :update_price_of_group_orders!
-  before_validation :distribute_transport
 
   # Finders
   scope :started, -> { where('starts <= ?', Time.now) }
@@ -49,12 +49,12 @@ class Order < ApplicationRecord
   include DateTimeAttributeValidate
   date_time_attribute :starts, :boxfill, :ends
 
-  def self.ransackable_attributes(auth_object = nil)
-    %w(id state supplier_id starts boxfill ends pickup)
+  def self.ransackable_attributes(_auth_object = nil)
+    %w[id state supplier_id starts boxfill ends pickup]
   end
 
-  def self.ransackable_associations(auth_object = nil)
-    %w(supplier articles order_articles)
+  def self.ransackable_associations(_auth_object = nil)
+    %w[supplier articles order_articles]
   end
 
   def stockit?
@@ -70,9 +70,9 @@ class Order < ApplicationRecord
       # make sure to include those articles which are no longer available
       # but which have already been ordered in this stock order
       StockArticle.available.includes(:article_category)
-                  .order('article_categories.name', 'articles.name').reject { |a|
+                  .order('article_categories.name', 'articles.name').reject do |a|
         a.quantity_available <= 0 && !a.ordered_in_order?(self)
-      }.group_by { |a| a.article_category.name }
+      end.group_by { |a| a.article_category.name }
     else
       supplier.articles.available.group_by { |a| a.article_category.name }
     end
@@ -87,9 +87,7 @@ class Order < ApplicationRecord
   end
 
   # Save ids, and create/delete order_articles after successfully saved the order
-  def article_ids=(ids)
-    @article_ids = ids
-  end
+  attr_writer :article_ids
 
   def article_ids
     @article_ids ||= order_articles.map { |a| a.article_id.to_s }
@@ -101,19 +99,19 @@ class Order < ApplicationRecord
   end
 
   def open?
-    state == "open"
+    state == 'open'
   end
 
   def finished?
-    state == "finished" || state == "received"
+    state == 'finished' || state == 'received'
   end
 
   def received?
-    state == "received"
+    state == 'received'
   end
 
   def closed?
-    state == "closed"
+    state == 'closed'
   end
 
   def boxfill?
@@ -134,11 +132,18 @@ class Order < ApplicationRecord
     self.starts ||= Time.now
     if FoodsoftConfig[:order_schedule]
       # try to be smart when picking a reference day
-      last = (DateTime.parse(FoodsoftConfig[:order_schedule][:initial]) rescue nil)
+      last = begin
+        DateTime.parse(FoodsoftConfig[:order_schedule][:initial])
+      rescue StandardError
+        nil
+      end
       last ||= Order.finished.reorder(:starts).first.try(:starts)
       last ||= self.starts
       # adjust boxfill and end date
-      self.boxfill ||= FoodsoftDateUtil.next_occurrence last, self.starts, FoodsoftConfig[:order_schedule][:boxfill] if is_boxfill_useful?
+      if is_boxfill_useful?
+        self.boxfill ||= FoodsoftDateUtil.next_occurrence last, self.starts,
+                                                          FoodsoftConfig[:order_schedule][:boxfill]
+      end
       self.ends ||= FoodsoftDateUtil.next_occurrence last, self.starts, FoodsoftConfig[:order_schedule][:ends]
     end
     self
@@ -149,7 +154,7 @@ class Order < ApplicationRecord
   def self.ordergroup_group_orders_map(ordergroup)
     orders = includes(:supplier)
     group_orders = GroupOrder.where(ordergroup_id: ordergroup.id, order_id: orders.map(&:id))
-    group_orders_hash = Hash[group_orders.collect { |go| [go.order_id, go] }]
+    group_orders_hash = group_orders.index_by { |go| go.order_id }
     orders.map do |order|
       {
         order: order,
@@ -160,11 +165,11 @@ class Order < ApplicationRecord
 
   # search GroupOrder of given Ordergroup
   def group_order(ordergroup)
-    group_orders.where(:ordergroup_id => ordergroup.id).first
+    group_orders.where(ordergroup_id: ordergroup.id).first
   end
 
   def stock_group_order
-    group_orders.where(:ordergroup_id => nil).first
+    group_orders.where(ordergroup_id: nil).first
   end
 
   # Returns OrderArticles in a nested Array, grouped by category and ordered by article name.
@@ -172,7 +177,7 @@ class Order < ApplicationRecord
   # e.g: [["drugs",[teethpaste, toiletpaper]], ["fruits" => [apple, banana, lemon]]]
   def articles_grouped_by_category
     @articles_grouped_by_category ||= order_articles
-                                      .includes([:article_price, :group_order_articles, :article => :article_category])
+                                      .includes([:article_price, :group_order_articles, { article: :article_category }])
                                       .order('articles.name')
                                       .group_by { |a| a.article.article_category.name }
                                       .sort { |a, b| a[0] <=> b[0] }
@@ -189,10 +194,10 @@ class Order < ApplicationRecord
   # FIXME: Consider order.foodcoop_result
   def profit(options = {})
     markup = options[:without_markup] || false
-    if invoice
-      groups_sum = markup ? sum(:groups_without_markup) : sum(:groups)
-      groups_sum - invoice.net_amount
-    end
+    return unless invoice
+
+    groups_sum = markup ? sum(:groups_without_markup) : sum(:groups)
+    groups_sum - invoice.net_amount
   end
 
   # Returns the all round price of a finished order
@@ -202,7 +207,7 @@ class Order < ApplicationRecord
   # :fc, guess what...
   def sum(type = :gross)
     total = 0
-    if type == :net || type == :gross || type == :fc
+    if %i[net gross fc].include?(type)
       for oa in order_articles.ordered.includes(:article, :article_price)
         quantity = oa.units * oa.price.unit_quantity
         case type
@@ -214,8 +219,8 @@ class Order < ApplicationRecord
           total += quantity * oa.price.fc_price
         end
       end
-    elsif type == :groups || type == :groups_without_markup
-      for go in group_orders.includes(group_order_articles: { order_article: [:article, :article_price] })
+    elsif %i[groups groups_without_markup].include?(type)
+      for go in group_orders.includes(group_order_articles: { order_article: %i[article article_price] })
         for goa in go.group_order_articles
           case type
           when :groups
@@ -232,36 +237,36 @@ class Order < ApplicationRecord
   # Finishes this order. This will set the order state to "finish" and the end property to the current time.
   # Ignored if the order is already finished.
   def finish!(user)
-    unless finished?
-      Order.transaction do
-        # set new order state (needed by notify_order_finished)
-        update!(state: 'finished', ends: Time.now, updated_by: user)
+    return if finished?
 
-        # Update order_articles. Save the current article_price to keep price consistency
-        # Also save results for each group_order_result
-        # Clean up
-        order_articles.includes(:article).each do |oa|
-          oa.update_attribute(:article_price, oa.article.article_prices.first)
-          oa.group_order_articles.each do |goa|
-            goa.save_results!
-            # Delete no longer required order-history (group_order_article_quantities) and
-            # TODO: Do we need articles, which aren't ordered? (units_to_order == 0 ?)
-            #    A: Yes, we do - for redistributing articles when the number of articles
-            #       delivered changes, and for statistics on popular articles. Records
-            #       with both tolerance and quantity zero can be deleted.
-            # goa.group_order_article_quantities.clear
-          end
+    Order.transaction do
+      # set new order state (needed by notify_order_finished)
+      update!(state: 'finished', ends: Time.now, updated_by: user)
+
+      # Update order_articles. Save the current article_price to keep price consistency
+      # Also save results for each group_order_result
+      # Clean up
+      order_articles.includes(:article).find_each do |oa|
+        oa.update_attribute(:article_price, oa.article.article_prices.first)
+        oa.group_order_articles.each do |goa|
+          goa.save_results!
+          # Delete no longer required order-history (group_order_article_quantities) and
+          # TODO: Do we need articles, which aren't ordered? (units_to_order == 0 ?)
+          #    A: Yes, we do - for redistributing articles when the number of articles
+          #       delivered changes, and for statistics on popular articles. Records
+          #       with both tolerance and quantity zero can be deleted.
+          # goa.group_order_article_quantities.clear
         end
-
-        # Update GroupOrder prices
-        group_orders.each(&:update_price!)
-
-        # Stats
-        ordergroups.each(&:update_stats!)
-
-        # Notifications
-        NotifyFinishedOrderJob.perform_later(self)
       end
+
+      # Update GroupOrder prices
+      group_orders.each(&:update_price!)
+
+      # Stats
+      ordergroups.each(&:update_stats!)
+
+      # Notifications
+      NotifyFinishedOrderJob.perform_later(self)
     end
   end
 
@@ -277,11 +282,11 @@ class Order < ApplicationRecord
       if stockit?                                         # Decreases the quantity of stock_articles
         for oa in order_articles.includes(:article)
           oa.update_results!                              # Update units_to_order of order_article
-          stock_changes.create! :stock_article => oa.article, :quantity => oa.units_to_order * -1
+          stock_changes.create! stock_article: oa.article, quantity: oa.units_to_order * -1
         end
       end
 
-      self.update!(state: 'closed', updated_by: user, foodcoop_result: profit)
+      update!(state: 'closed', updated_by: user, foodcoop_result: profit)
     end
   end
 
@@ -289,7 +294,10 @@ class Order < ApplicationRecord
   def close_direct!(user)
     raise I18n.t('orders.model.error_closed') if closed?
 
-    comments.create(user: user, text: I18n.t('orders.model.close_direct_message')) unless FoodsoftConfig[:charge_members_manually]
+    unless FoodsoftConfig[:charge_members_manually]
+      comments.create(user: user,
+                      text: I18n.t('orders.model.close_direct_message'))
+    end
     update!(state: 'closed', updated_by: user)
   end
 
@@ -313,13 +321,12 @@ class Order < ApplicationRecord
   end
 
   def self.finish_ended!
-    orders = Order.where.not(end_action: Order.end_actions[:no_end_action]).where(state: 'open').where('ends <= ?', DateTime.now)
+    orders = Order.where.not(end_action: Order.end_actions[:no_end_action]).where(state: 'open').where('ends <= ?',
+                                                                                                       DateTime.now)
     orders.each do |order|
-      begin
-        order.do_end_action!
-      rescue => error
-        ExceptionNotifier.notify_exception(error, data: { foodcoop: FoodsoftConfig.scope, order_id: order.id })
-      end
+      order.do_end_action!
+    rescue StandardError => e
+      ExceptionNotifier.notify_exception(e, data: { foodcoop: FoodsoftConfig.scope, order_id: order.id })
     end
   end
 
@@ -329,7 +336,10 @@ class Order < ApplicationRecord
     delta = Rails.env.test? ? 1 : 0 # since Rails 4.2 tests appear to have time differences, with this validation failing
     errors.add(:ends, I18n.t('orders.model.error_starts_before_ends')) if ends && starts && ends <= (starts - delta)
     errors.add(:ends, I18n.t('orders.model.error_boxfill_before_ends')) if ends && boxfill && ends <= (boxfill - delta)
-    errors.add(:boxfill, I18n.t('orders.model.error_starts_before_boxfill')) if boxfill && starts && boxfill <= (starts - delta)
+    return unless boxfill && starts && boxfill <= (starts - delta)
+
+    errors.add(:boxfill,
+               I18n.t('orders.model.error_starts_before_boxfill'))
   end
 
   def include_articles
@@ -340,17 +350,17 @@ class Order < ApplicationRecord
     chosen_order_articles = order_articles.where(article_id: article_ids)
     to_be_removed = order_articles - chosen_order_articles
     to_be_removed_but_ordered = to_be_removed.select { |a| a.quantity > 0 || a.tolerance > 0 }
-    unless to_be_removed_but_ordered.empty? || ignore_warnings
-      errors.add(:articles, I18n.t(stockit? ? 'orders.model.warning_ordered_stock' : 'orders.model.warning_ordered'))
-      @erroneous_article_ids = to_be_removed_but_ordered.map { |a| a.article_id }
-    end
+    return if to_be_removed_but_ordered.empty? || ignore_warnings
+
+    errors.add(:articles, I18n.t(stockit? ? 'orders.model.warning_ordered_stock' : 'orders.model.warning_ordered'))
+    @erroneous_article_ids = to_be_removed_but_ordered.map { |a| a.article_id }
   end
 
   def save_order_articles
     # fetch selected articles
     articles_list = Article.find(article_ids)
     # create new order_articles
-    (articles_list - articles).each { |article| order_articles.create(:article => article) }
+    (articles_list - articles).each { |article| order_articles.create(article: article) }
     # delete old order_articles
     articles.reject { |article| articles_list.include?(article) }.each do |article|
       order_articles.detect { |order_article| order_article.article_id == article.id }.destroy
@@ -363,17 +373,17 @@ class Order < ApplicationRecord
     return unless group_orders.any?
 
     case transport_distribution.try(&:to_i)
-    when Order.transport_distributions[:ordergroup] then
+    when Order.transport_distributions[:ordergroup]
       amount = transport / group_orders.size
       group_orders.each do |go|
         go.transport = amount.ceil(2)
       end
-    when Order.transport_distributions[:price] then
+    when Order.transport_distributions[:price]
       amount = transport / group_orders.sum(:price)
       group_orders.each do |go|
         go.transport = (amount * go.price).ceil(2)
       end
-    when Order.transport_distributions[:articles] then
+    when Order.transport_distributions[:articles]
       amount = transport / group_orders.includes(:group_order_articles).sum(:result)
       group_orders.each do |go|
         go.transport = (amount * go.group_order_articles.sum(:result)).ceil(2)
@@ -389,7 +399,7 @@ class Order < ApplicationRecord
 
   def charge_group_orders!(user, transaction_type = nil)
     note = transaction_note
-    group_orders.includes(:ordergroup).each do |group_order|
+    group_orders.includes(:ordergroup).find_each do |group_order|
       if group_order.ordergroup
         price = group_order.total * -1 # decrease! account balance
         group_order.ordergroup.add_financial_transaction!(price, note, user, transaction_type, nil, group_order)

app/models/order_article.rb

@@ -7,25 +7,27 @@ class OrderArticle < ApplicationRecord
   belongs_to :order
   belongs_to :article
   belongs_to :article_price, optional: true
-  has_many :group_order_articles, :dependent => :destroy
+  has_many :group_order_articles, dependent: :destroy
 
-  validates_presence_of :order_id, :article_id
+  validates :order_id, :article_id, presence: true
   validate :article_and_price_exist
-  validates_uniqueness_of :article_id, scope: :order_id
+  validates :article_id, uniqueness: { scope: :order_id }
 
-  _ordered_sql = "order_articles.units_to_order > 0 OR order_articles.units_billed > 0 OR order_articles.units_received > 0"
+  _ordered_sql = 'order_articles.units_to_order > 0 OR order_articles.units_billed > 0 OR order_articles.units_received > 0'
   scope :ordered, -> { where(_ordered_sql) }
-  scope :ordered_or_member, -> { includes(:group_order_articles).where("#{_ordered_sql} OR order_articles.quantity > 0 OR group_order_articles.result > 0") }
+  scope :ordered_or_member, lambda {
+                              includes(:group_order_articles).where("#{_ordered_sql} OR order_articles.quantity > 0 OR group_order_articles.result > 0")
+                            }
 
   before_create :init_from_balancing
   after_destroy :update_ordergroup_prices
 
-  def self.ransackable_attributes(auth_object = nil)
-    %w(id order_id article_id quantity tolerance units_to_order)
+  def self.ransackable_attributes(_auth_object = nil)
+    %w[id order_id article_id quantity tolerance units_to_order]
   end
 
-  def self.ransackable_associations(auth_object = nil)
-    %w(order article)
+  def self.ransackable_associations(_auth_object = nil)
+    %w[order article]
   end
 
   # This method returns either the ArticlePrice or the Article
@@ -46,7 +48,7 @@ class OrderArticle < ApplicationRecord
   # In balancing this can differ from ordered (by supplier) quantity for this article.
   def group_orders_sum
     quantity = group_order_articles.collect(&:result).sum
-    { :quantity => quantity, :price => quantity * price.fc_price }
+    { quantity: quantity, price: quantity * price.fc_price }
   end
 
   # Update quantity/tolerance/units_to_order from group_order_articles
@@ -97,15 +99,13 @@ class OrderArticle < ApplicationRecord
     units * price.unit_quantity * price.gross_price
   end
 
-  def ordered_quantities_different_from_group_orders?(ordered_mark = "!", billed_mark = "?", received_mark = "?")
-    if not units_received.nil?
-      ((units_received * price.unit_quantity) == group_orders_sum[:quantity]) ? false : received_mark
-    elsif not units_billed.nil?
-      ((units_billed * price.unit_quantity) == group_orders_sum[:quantity]) ? false : billed_mark
-    elsif not units_to_order.nil?
-      ((units_to_order * price.unit_quantity) == group_orders_sum[:quantity]) ? false : ordered_mark
-    else
-      nil # can happen in integration tests
+  def ordered_quantities_different_from_group_orders?(ordered_mark = '!', billed_mark = '?', received_mark = '?')
+    if !units_received.nil?
+      (units_received * price.unit_quantity) == group_orders_sum[:quantity] ? false : received_mark
+    elsif !units_billed.nil?
+      (units_billed * price.unit_quantity) == group_orders_sum[:quantity] ? false : billed_mark
+    elsif !units_to_order.nil?
+      (units_to_order * price.unit_quantity) == group_orders_sum[:quantity] ? false : ordered_mark
     end
   end
 
@@ -124,7 +124,7 @@ class OrderArticle < ApplicationRecord
     if surplus.index(:tolerance).nil?
       qty_for_members = [qty_left, self.quantity].min
     else
-      qty_for_members = [qty_left, self.quantity + self.tolerance].min
+      qty_for_members = [qty_left, self.quantity + tolerance].min
       counts[surplus.index(:tolerance)] = [0, qty_for_members - self.quantity].max
     end
 
@@ -139,9 +139,7 @@ class OrderArticle < ApplicationRecord
       # 2) if not found, create new stock article
       #      avoiding duplicate stock article names
     end
-    if qty_left > 0 && surplus.index(nil)
-      counts[surplus.index(nil)] = qty_left
-    end
+    counts[surplus.index(nil)] = qty_left if qty_left > 0 && surplus.index(nil)
 
     # Update GroupOrder prices & Ordergroup stats
     # TODO only affected group_orders, and once after redistributing all articles
@@ -150,7 +148,7 @@ class OrderArticle < ApplicationRecord
       order.ordergroups.each(&:update_stats!)
     end
 
-    # TODO notifications
+    # TODO: notifications
 
     counts
   end
@@ -159,7 +157,7 @@ class OrderArticle < ApplicationRecord
   def update_article_and_price!(order_article_attributes, article_attributes, price_attributes = nil)
     OrderArticle.transaction do
       # Updates self
-      self.update!(order_article_attributes)
+      update!(order_article_attributes)
 
       # Updates article
       article.update!(article_attributes)
@@ -186,7 +184,7 @@ class OrderArticle < ApplicationRecord
   end
 
   def update_global_price=(value)
-    @update_global_price = (value == true || value == '1') ? true : false
+    @update_global_price = [true, '1'].include?(value) ? true : false
   end
 
   # @return [Number] Units missing for the last +unit_quantity+ of the article.
@@ -210,16 +208,19 @@ class OrderArticle < ApplicationRecord
   private
 
   def article_and_price_exist
-    errors.add(:article, I18n.t('model.order_article.error_price')) if !(article = Article.find(article_id)) || article.fc_price.nil?
-  rescue
+    if !(article = Article.find(article_id)) || article.fc_price.nil?
+      errors.add(:article,
+                 I18n.t('model.order_article.error_price'))
+    end
+  rescue StandardError
     errors.add(:article, I18n.t('model.order_article.error_price'))
   end
 
   # Associate with current article price if created in a finished order
   def init_from_balancing
-    if order.present? && order.finished?
-      self.article_price = article.article_prices.first
-    end
+    return unless order.present? && order.finished?
+
+    self.article_price = article.article_prices.first
   end
 
   def update_ordergroup_prices
@@ -241,7 +242,8 @@ class OrderArticle < ApplicationRecord
     unless (delta_q == 0 && delta_t >= 0) ||
            (delta_mis < 0 && delta_box >= 0 && delta_t >= 0) ||
            (delta_q > 0 && delta_q == -delta_t)
-      raise ActiveRecord::RecordNotSaved.new("Change not acceptable in boxfill phase for '#{article.name}', sorry.", self)
+      raise ActiveRecord::RecordNotSaved.new("Change not acceptable in boxfill phase for '#{article.name}', sorry.",
+                                             self)
     end
   end
 

app/models/order_comment.rb

@@ -2,6 +2,6 @@ class OrderComment < ApplicationRecord
   belongs_to :order
   belongs_to :user
 
-  validates_presence_of :order_id, :user_id, :text
-  validates_length_of :text, :minimum => 3
+  validates :order_id, :user_id, :text, presence: true
+  validates :text, length: { minimum: 3 }
 end

app/models/ordergroup.rb

@@ -15,7 +15,7 @@ class Ordergroup < Group
   has_many :orders, through: :group_orders
   has_many :group_order_articles, through: :group_orders
 
-  validates_numericality_of :account_balance, :message => I18n.t('ordergroups.model.invalid_balance')
+  validates :account_balance, numericality: { message: I18n.t('ordergroups.model.invalid_balance') }
   validate :uniqueness_of_name, :uniqueness_of_members
 
   after_create :update_stats!
@@ -32,7 +32,7 @@ class Ordergroup < Group
 
   def self.include_transaction_class_sum
     columns = ['groups.*']
-    FinancialTransactionClass.all.each do |c|
+    FinancialTransactionClass.all.find_each do |c|
       columns << "sum(CASE financial_transaction_types.financial_transaction_class_id WHEN #{c.id} THEN financial_transactions.amount ELSE 0 END) AS sum_of_class_#{c.id}"
     end
 
@@ -51,9 +51,9 @@ class Ordergroup < Group
 
   def last_user_activity
     last_active_user = users.order('users.last_activity DESC').first
-    if last_active_user
-      last_active_user.last_activity
-    end
+    return unless last_active_user
+
+    last_active_user.last_activity
   end
 
   # the most recent order this ordergroup was participating in
@@ -86,12 +86,14 @@ class Ordergroup < Group
   # Throws an exception if it fails.
   def add_financial_transaction!(amount, note, user, transaction_type, link = nil, group_order = nil)
     transaction do
-      t = FinancialTransaction.new(ordergroup: self, amount: amount, note: note, user: user, financial_transaction_type: transaction_type, financial_link: link, group_order: group_order)
+      t = FinancialTransaction.new(ordergroup: self, amount: amount, note: note, user: user,
+                                   financial_transaction_type: transaction_type, financial_link: link, group_order: group_order)
       t.save!
       update_balance!
       # Notify only when order group had a positive balance before the last transaction:
-      if t.amount < 0 && self.account_balance < 0 && self.account_balance - t.amount >= 0
-        NotifyNegativeBalanceJob.perform_later(self, t)
+      if t.amount < 0 && account_balance < 0 && account_balance - t.amount >= 0
+        NotifyNegativeBalanceJob.perform_later(self,
+                                               t)
       end
       t
     end
@@ -101,10 +103,11 @@ class Ordergroup < Group
     # Get hours for every job of each user in period
     jobs = users.to_a.sum { |u| u.tasks.done.where('updated_on > ?', APPLE_MONTH_AGO.month.ago).sum(:duration) }
     # Get group_order.price for every finished order in this period
-    orders_sum = group_orders.includes(:order).merge(Order.finished).where('orders.ends >= ?', APPLE_MONTH_AGO.month.ago).references(:orders).sum(:price)
+    orders_sum = group_orders.includes(:order).merge(Order.finished).where('orders.ends >= ?',
+                                                                           APPLE_MONTH_AGO.month.ago).references(:orders).sum(:price)
 
     @readonly = false # Dirty hack, avoid getting RecordReadOnly exception when called in task after_save callback. A rails bug?
-    update_attribute(:stats, { :jobs_size => jobs, :orders_sum => orders_sum })
+    update_attribute(:stats, { jobs_size: jobs, orders_sum: orders_sum })
   end
 
   def update_balance!
@@ -116,13 +119,17 @@ class Ordergroup < Group
   end
 
   def avg_jobs_per_euro
-    stats[:jobs_size].to_f / stats[:orders_sum].to_f rescue 0
+    stats[:jobs_size].to_f / stats[:orders_sum].to_f
+  rescue StandardError
+    0
   end
 
   # This is the ordergroup job per euro performance
   # in comparison to the hole foodcoop average
   def apples
-    ((avg_jobs_per_euro / Ordergroup.avg_jobs_per_euro) * 100).to_i rescue 0
+    ((avg_jobs_per_euro / Ordergroup.avg_jobs_per_euro) * 100).to_i
+  rescue StandardError
+    0
   end
 
   # If the the option stop_ordering_under is set, the ordergroup is only allowed to participate in an order,
@@ -141,7 +148,11 @@ class Ordergroup < Group
   # Global average
   def self.avg_jobs_per_euro
     stats = Ordergroup.pluck(:stats)
-    stats.sum { |s| s[:jobs_size].to_f } / stats.sum { |s| s[:orders_sum].to_f } rescue 0
+    begin
+      stats.sum { |s| s[:jobs_size].to_f } / stats.sum { |s| s[:orders_sum].to_f }
+    rescue StandardError
+      0
+    end
   end
 
   def account_updated
@@ -149,22 +160,22 @@ class Ordergroup < Group
   end
 
   def self.sort_by_param(param)
-    param ||= "name"
+    param ||= 'name'
 
     sort_param_map = {
-      "name" => "name",
-      "name_reverse" => "name DESC",
-      "members_count" => "count(users.id)",
-      "members_count_reverse" => "count(users.id) DESC",
-      "last_user_activity" => "max(users.last_activity)",
-      "last_user_activity_reverse" => "max(users.last_activity) DESC",
-      "last_order" => "max(orders.starts)",
-      "last_order_reverse" => "max(orders.starts) DESC"
+      'name' => 'name',
+      'name_reverse' => 'name DESC',
+      'members_count' => 'count(users.id)',
+      'members_count_reverse' => 'count(users.id) DESC',
+      'last_user_activity' => 'max(users.last_activity)',
+      'last_user_activity_reverse' => 'max(users.last_activity) DESC',
+      'last_order' => 'max(orders.starts)',
+      'last_order_reverse' => 'max(orders.starts) DESC'
     }
 
     result = self
-    result = result.left_joins(:users).group("groups.id") if param.starts_with?("members_count", "last_user_activity")
-    result = result.left_joins(:orders).group("groups.id") if param.starts_with?("last_order")
+    result = result.left_joins(:users).group('groups.id') if param.starts_with?('members_count', 'last_user_activity')
+    result = result.left_joins(:orders).group('groups.id') if param.starts_with?('last_order')
 
     # Never pass user input data to Arel.sql() because of SQL Injection vulnerabilities.
     # This case here is okay, as param is mapped to the actual order string.
@@ -176,17 +187,21 @@ class Ordergroup < Group
   # Make sure, that a user can only be in one ordergroup
   def uniqueness_of_members
     users.each do |user|
-      errors.add :user_tokens, I18n.t('ordergroups.model.error_single_group', :user => user.display) if user.groups.where(:type => 'Ordergroup').size > 1
+      next unless user.groups.where(type: 'Ordergroup').size > 1
+
+      errors.add :user_tokens,
+                 I18n.t('ordergroups.model.error_single_group',
+                        user: user.display)
     end
   end
 
   # Make sure, the name is uniq, add usefull message if uniq group is already deleted
   def uniqueness_of_name
     group = Ordergroup.where(name: name)
-    group = group.where.not(id: self.id) unless new_record?
-    if group.exists?
-      message = group.first.deleted? ? :taken_with_deleted : :taken
-      errors.add :name, message
-    end
+    group = group.where.not(id: id) unless new_record?
+    return unless group.exists?
+
+    message = group.first.deleted? ? :taken_with_deleted : :taken
+    errors.add :name, message
   end
 end

app/models/periodic_task_group.rb

@@ -5,7 +5,7 @@ class PeriodicTaskGroup < ApplicationRecord
     return false if tasks.empty?
     return false if tasks.first.due_date.nil?
 
-    return true
+    true
   end
 
   def create_next_task
@@ -18,15 +18,13 @@ class PeriodicTaskGroup < ApplicationRecord
     next_task.save
 
     self.next_task_date += period_days
-    self.save
+    save
   end
 
   def create_tasks_until(create_until)
-    if has_next_task?
-      while next_task_date.nil? || next_task_date < create_until
-        create_next_task
-      end
-    end
+    return unless has_next_task?
+
+    create_next_task while next_task_date.nil? || next_task_date < create_until
   end
 
   def create_tasks_for_upfront_days
@@ -36,7 +34,7 @@ class PeriodicTaskGroup < ApplicationRecord
   end
 
   def exclude_tasks_before(task)
-    tasks.where("due_date < '#{task.due_date}'").each do |t|
+    tasks.where("due_date < '#{task.due_date}'").find_each do |t|
       t.update_attribute(:periodic_task_group, nil)
     end
   end
@@ -53,7 +51,7 @@ class PeriodicTaskGroup < ApplicationRecord
                    due_date: task.due_date + due_date_delta)
     end
     group_tasks.each do |task|
-      task.update_columns(periodic_task_group_id: self.id)
+      task.update_columns(periodic_task_group_id: id)
     end
   end
 

app/models/shared_article.rb

@@ -4,23 +4,23 @@ class SharedArticle < ApplicationRecord
   # set correct table_name in external DB
   self.table_name = 'articles'
 
-  belongs_to :shared_supplier, :foreign_key => :supplier_id
+  belongs_to :shared_supplier, foreign_key: :supplier_id
 
   def build_new_article(supplier)
     supplier.articles.build(
-      :name => name,
-      :unit => unit,
-      :note => note,
-      :manufacturer => manufacturer,
-      :origin => origin,
-      :price => price,
-      :tax => tax,
-      :deposit => deposit,
-      :unit_quantity => unit_quantity,
-      :order_number => number,
-      :article_category => ArticleCategory.find_match(category),
+      name: name,
+      unit: unit,
+      note: note,
+      manufacturer: manufacturer,
+      origin: origin,
+      price: price,
+      tax: tax,
+      deposit: deposit,
+      unit_quantity: unit_quantity,
+      order_number: number,
+      article_category: ArticleCategory.find_match(category),
       # convert to db-compatible-string
-      :shared_updated_on => updated_on.to_formatted_s(:db)
+      shared_updated_on: updated_on.to_fs(:db)
     )
   end
 end

app/models/shared_supplier.rb

@@ -5,10 +5,10 @@ class SharedSupplier < ApplicationRecord
   self.table_name = 'suppliers'
 
   has_many :suppliers, -> { undeleted }
-  has_many :shared_articles, :foreign_key => :supplier_id
+  has_many :shared_articles, foreign_key: :supplier_id
 
   def find_article_by_number(order_number)
-    # note that `shared_articles` uses number instead order_number
+    # NOTE: that `shared_articles` uses number instead order_number
     cached_articles.detect { |a| a.number == order_number }
   end
 
@@ -19,15 +19,18 @@ class SharedSupplier < ApplicationRecord
   # These set of attributes are used to autofill attributes of new supplier,
   # when created by import from shared supplier feature.
   def autofill_attributes
-    whitelist = %w(name address phone fax email url delivery_days note)
+    whitelist = %w[name address phone fax email url delivery_days note]
     attributes.select { |k, _v| whitelist.include?(k) }
   end
 
   # return list of synchronisation methods available for this supplier
   def shared_sync_methods
     methods = []
-    methods += %w(all_available all_unavailable) if shared_articles.count < FoodsoftConfig[:shared_supplier_article_sync_limit]
-    methods += %w(import)
+    if shared_articles.count < FoodsoftConfig[:shared_supplier_article_sync_limit]
+      methods += %w[all_available
+                    all_unavailable]
+    end
+    methods += %w[import]
     methods
   end
 end

app/models/stock_article.rb

@@ -10,11 +10,11 @@ class StockArticle < Article
   ransack_alias :quantity_available, :quantity # in-line with {StockArticleSerializer}
 
   def self.ransackable_attributes(auth_object = nil)
-    super(auth_object) - %w(supplier_id) + %w(quantity)
+    super(auth_object) - %w[supplier_id] + %w[quantity]
   end
 
   def self.ransackable_associations(auth_object = nil)
-    super(auth_object) - %w(supplier)
+    super(auth_object) - %w[supplier]
   end
 
   # Update the quantity of items in stock
@@ -48,7 +48,7 @@ class StockArticle < Article
   protected
 
   def check_quantity
-    raise I18n.t('stockit.check.not_empty', :name => name) unless quantity == 0
+    raise I18n.t('stockit.check.not_empty', name: name) unless quantity == 0
   end
 
   # Overwrite Price history of Article. For StockArticles isn't it necessary.

app/models/stock_change.rb

@@ -4,11 +4,11 @@ class StockChange < ApplicationRecord
   belongs_to :stock_taking, optional: true, foreign_key: 'stock_event_id'
   belongs_to :stock_article
 
-  validates_presence_of :stock_article_id, :quantity
-  validates_numericality_of :quantity
+  validates :stock_article_id, :quantity, presence: true
+  validates :quantity, numericality: true
 
-  after_save :update_article_quantity
   after_destroy :update_article_quantity
+  after_save :update_article_quantity
 
   protected
 

app/models/stock_event.rb

@@ -2,5 +2,5 @@ class StockEvent < ApplicationRecord
   has_many :stock_changes, dependent: :destroy
   has_many :stock_articles, through: :stock_changes
 
-  validates_presence_of :date
+  validates :date, presence: true
 end

app/models/supplier.rb

@@ -2,7 +2,9 @@ class Supplier < ApplicationRecord
   include MarkAsDeletedWithName
   include CustomFields
 
-  has_many :articles, -> { where(:type => nil).includes(:article_category).order('article_categories.name', 'articles.name') }
+  has_many :articles, lambda {
+                        where(type: nil).includes(:article_category).order('article_categories.name', 'articles.name')
+                      }
   has_many :stock_articles, -> { includes(:article_category).order('article_categories.name', 'articles.name') }
   has_many :orders
   has_many :deliveries
@@ -10,24 +12,24 @@ class Supplier < ApplicationRecord
   belongs_to :supplier_category
   belongs_to :shared_supplier, optional: true # for the sharedLists-App
 
-  validates :name, :presence => true, :length => { :in => 4..30 }
-  validates :phone, :presence => true, :length => { :in => 8..25 }
-  validates :address, :presence => true, :length => { :in => 8..50 }
-  validates_format_of :iban, :with => /\A[A-Z]{2}[0-9]{2}[0-9A-Z]{,30}\z/, :allow_blank => true
-  validates_uniqueness_of :iban, :case_sensitive => false, :allow_blank => true
-  validates_length_of :order_howto, :note, maximum: 250
+  validates :name, presence: true, length: { in: 4..30 }
+  validates :phone, presence: true, length: { in: 8..25 }
+  validates :address, presence: true, length: { in: 8..50 }
+  validates :iban, format: { with: /\A[A-Z]{2}[0-9]{2}[0-9A-Z]{,30}\z/, allow_blank: true }
+  validates :iban, uniqueness: { case_sensitive: false, allow_blank: true }
+  validates :order_howto, :note, length: { maximum: 250 }
   validate :valid_shared_sync_method
   validate :uniqueness_of_name
 
   scope :undeleted, -> { where(deleted_at: nil) }
   scope :having_articles, -> { where(id: Article.undeleted.select(:supplier_id).distinct) }
 
-  def self.ransackable_attributes(auth_object = nil)
-    %w(id name)
+  def self.ransackable_attributes(_auth_object = nil)
+    %w[id name]
   end
 
-  def self.ransackable_associations(auth_object = nil)
-    %w(articles stock_articles orders)
+  def self.ransackable_associations(_auth_object = nil)
+    %w[articles stock_articles orders]
   end
 
   # sync all articles with the external database
@@ -35,7 +37,9 @@ class Supplier < ApplicationRecord
   # also returns an array with outlisted_articles, which should be deleted
   # also returns an array with new articles, which should be added (depending on shared_sync_method)
   def sync_all
-    updated_article_pairs, outlisted_articles, new_articles = [], [], []
+    updated_article_pairs = []
+    outlisted_articles = []
+    new_articles = []
     existing_articles = Set.new
     for article in articles.undeleted
       # try to find the associated shared_article
@@ -44,30 +48,28 @@ class Supplier < ApplicationRecord
       if shared_article # article will be updated
         existing_articles.add(shared_article.id)
         unequal_attributes = article.shared_article_changed?(self)
-        unless unequal_attributes.blank? # skip if shared_article has not been changed
+        if unequal_attributes.present? # skip if shared_article has not been changed
           article.attributes = unequal_attributes
           updated_article_pairs << [article, unequal_attributes]
         end
       # Articles with no order number can be used to put non-shared articles
       # in a shared supplier, with sync keeping them.
-      elsif not article.order_number.blank?
+      elsif article.order_number.present?
         # article isn't in external database anymore
         outlisted_articles << article
       end
     end
     # Find any new articles, unless the import is manual
-    if ['all_available', 'all_unavailable'].include?(shared_sync_method)
+    if %w[all_available all_unavailable].include?(shared_sync_method)
       # build new articles
       shared_supplier
         .shared_articles
         .where.not(id: existing_articles.to_a)
         .find_each { |new_shared_article| new_articles << new_shared_article.build_new_article(self) }
       # make them unavailable when desired
-      if shared_sync_method == 'all_unavailable'
-        new_articles.each { |new_article| new_article.availability = false }
-      end
+      new_articles.each { |new_article| new_article.availability = false } if shared_sync_method == 'all_unavailable'
     end
-    return [updated_article_pairs, outlisted_articles, new_articles]
+    [updated_article_pairs, outlisted_articles, new_articles]
   end
 
   # Synchronise articles with spreadsheet.
@@ -78,8 +80,10 @@ class Supplier < ApplicationRecord
   # @option options [Boolean] :convert_units Omit or set to +true+ to keep current units, recomputing unit quantity and price.
   def sync_from_file(file, options = {})
     all_order_numbers = []
-    updated_article_pairs, outlisted_articles, new_articles = [], [], []
-    FoodsoftFile::parse file, options do |status, new_attrs, line|
+    updated_article_pairs = []
+    outlisted_articles = []
+    new_articles = []
+    FoodsoftFile.parse file, options do |status, new_attrs, line|
       article = articles.undeleted.where(order_number: new_attrs[:order_number]).first
       new_attrs[:article_category] = ArticleCategory.find_match(new_attrs[:article_category])
       new_attrs[:tax] ||= FoodsoftConfig[:tax_default]
@@ -101,15 +105,13 @@ class Supplier < ApplicationRecord
       # stop when there is a parsing error
       elsif status.is_a? String
         # @todo move I18n key to model
-        raise I18n.t('articles.model.error_parse', :msg => status, :line => line.to_s)
+        raise I18n.t('articles.model.error_parse', msg: status, line: line.to_s)
       end
 
       all_order_numbers << article.order_number if article
     end
-    if options[:outlist_absent]
-      outlisted_articles += articles.undeleted.where.not(order_number: all_order_numbers + [nil])
-    end
-    return [updated_article_pairs, outlisted_articles, new_articles]
+    outlisted_articles += articles.undeleted.where.not(order_number: all_order_numbers + [nil]) if options[:outlist_absent]
+    [updated_article_pairs, outlisted_articles, new_articles]
   end
 
   # default value
@@ -140,18 +142,18 @@ class Supplier < ApplicationRecord
 
   # make sure the shared_sync_method is allowed for the shared supplier
   def valid_shared_sync_method
-    if shared_supplier && !shared_supplier.shared_sync_methods.include?(shared_sync_method)
-      errors.add :shared_sync_method, :included
-    end
+    return unless shared_supplier && !shared_supplier.shared_sync_methods.include?(shared_sync_method)
+
+    errors.add :shared_sync_method, :included
   end
 
   # Make sure, the name is uniq, add usefull message if uniq group is already deleted
   def uniqueness_of_name
     supplier = Supplier.where(name: name)
-    supplier = supplier.where.not(id: self.id) unless new_record?
-    if supplier.exists?
-      message = supplier.first.deleted? ? :taken_with_deleted : :taken
-      errors.add :name, message
-    end
+    supplier = supplier.where.not(id: id) unless new_record?
+    return unless supplier.exists?
+
+    message = supplier.first.deleted? ? :taken_with_deleted : :taken
+    errors.add :name, message
   end
 end

app/models/task.rb

@@ -1,9 +1,9 @@
 class Task < ApplicationRecord
-  has_many :assignments, :dependent => :destroy
-  has_many :users, :through => :assignments
+  has_many :assignments, dependent: :destroy
+  has_many :users, through: :assignments
   belongs_to :workgroup, optional: true
   belongs_to :periodic_task_group, optional: true
-  belongs_to :created_by, :class_name => 'User', :foreign_key => 'created_by_user_id', optional: true
+  belongs_to :created_by, class_name: 'User', foreign_key: 'created_by_user_id', optional: true
 
   scope :non_group, -> { where(workgroup_id: nil, done: false) }
   scope :done, -> { where(done: true) }
@@ -11,12 +11,12 @@ class Task < ApplicationRecord
 
   attr_accessor :current_user_id
 
-  validates :name, :presence => true, :length => { :minimum => 3 }
-  validates :required_users, :presence => true
-  validates_numericality_of :duration, :required_users, :only_integer => true, :greater_than => 0
-  validates_length_of :description, maximum: 250
+  validates :name, presence: true, length: { minimum: 3 }
+  validates :required_users, presence: true
+  validates :duration, :required_users, numericality: { only_integer: true, greater_than: 0 }
+  validates :description, length: { maximum: 250 }
   validates :done, exclusion: { in: [true] }, if: :periodic?, on: :create
-  validates_presence_of :due_date, if: :periodic?
+  validates :due_date, presence: { if: :periodic? }
 
   before_save :exclude_from_periodic_task_group, if: :changed?, unless: :new_record?
   after_save :update_ordergroup_stats
@@ -35,7 +35,7 @@ class Task < ApplicationRecord
   # find all tasks in the period (or another number of days)
   def self.next_assigned_tasks_for(user, number = FoodsoftConfig[:tasks_period_days].to_i)
     user.tasks.undone.where(assignments: { accepted: true })
-        .where(["tasks.due_date >= ? AND tasks.due_date <= ?", Time.now, number.days.from_now])
+        .where(['tasks.due_date >= ? AND tasks.due_date <= ?', Time.now, number.days.from_now])
   end
 
   # count tasks with not enough responsible people
@@ -49,7 +49,7 @@ class Task < ApplicationRecord
 
   def self.next_unassigned_tasks_for(user, max = 2)
     periodic_task_group_count = {}
-    self.unassigned_tasks_for(user).reject do |item|
+    unassigned_tasks_for(user).reject do |item|
       next false unless item.periodic_task_group
 
       count = periodic_task_group_count[item.periodic_task_group] || 0
@@ -59,19 +59,19 @@ class Task < ApplicationRecord
   end
 
   def periodic?
-    not periodic_task_group.nil?
+    !periodic_task_group.nil?
   end
 
   def is_assigned?(user)
-    self.assignments.detect { |ass| ass.user_id == user.id }
+    assignments.detect { |ass| ass.user_id == user.id }
   end
 
   def is_accepted?(user)
-    self.assignments.detect { |ass| ass.user_id == user.id && ass.accepted }
+    assignments.detect { |ass| ass.user_id == user.id && ass.accepted }
   end
 
   def enough_users_assigned?
-    assignments.to_a.count(&:accepted) >= required_users ? true : false
+    assignments.to_a.count(&:accepted) >= required_users
   end
 
   def still_required_users
@@ -82,39 +82,35 @@ class Task < ApplicationRecord
   # and makes the users responsible for the task
   # TODO: check for maximal number of users
   def user_list=(ids)
-    list = ids.split(",").map(&:to_i)
+    list = ids.split(',').map(&:to_i)
     new_users = (list - users.collect(&:id)).uniq
     old_users = users.reject { |user| list.include?(user.id) }
 
     self.class.transaction do
       # delete old assignments
-      if old_users.any?
-        assignments.where(user_id: old_users.map(&:id)).each(&:destroy)
-      end
+      assignments.where(user_id: old_users.map(&:id)).find_each(&:destroy) if old_users.any?
       # create new assignments
       new_users.each do |id|
         user = User.find(id)
         if user.blank?
           errors.add(:user_list)
+        elsif id == current_user_id.to_i
+          assignments.build user: user, accepted: true
+        # current_user will accept, when he puts himself to the list of users
         else
-          if id == current_user_id.to_i
-            # current_user will accept, when he puts himself to the list of users
-            self.assignments.build :user => user, :accepted => true
-          else
-            # normal assignement
-            self.assignments.build :user => user
-          end
+          # normal assignement
+          assignments.build user: user
         end
       end
     end
   end
 
   def user_list
-    @user_list ||= users.collect(&:id).join(", ")
+    @user_list ||= users.collect(&:id).join(', ')
   end
 
   def update_ordergroup_stats(user_ids = self.user_ids)
-    Ordergroup.joins(:users).where(users: { id: user_ids }).each(&:update_stats!)
+    Ordergroup.joins(:users).where(users: { id: user_ids }).find_each(&:update_stats!)
   end
 
   def exclude_from_periodic_task_group

app/models/user.rb

@@ -4,19 +4,19 @@ class User < ApplicationRecord
   include CustomFields
   # TODO: acts_as_paraniod ??
 
-  has_many :memberships, :dependent => :destroy
-  has_many :groups, :through => :memberships
+  has_many :memberships, dependent: :destroy
+  has_many :groups, through: :memberships
   # has_one :ordergroup, :through => :memberships, :source => :group, :class_name => "Ordergroup"
   def ordergroup
-    Ordergroup.joins(:memberships).where(memberships: { user_id: self.id }).first
+    Ordergroup.joins(:memberships).where(memberships: { user_id: id }).first
   end
 
-  has_many :workgroups, :through => :memberships, :source => :group, :class_name => "Workgroup"
-  has_many :assignments, :dependent => :destroy
-  has_many :tasks, :through => :assignments
-  has_many :send_messages, :class_name => "Message", :foreign_key => "sender_id"
-  has_many :created_orders, :class_name => 'Order', :foreign_key => 'created_by_user_id', :dependent => :nullify
-  has_many :mail_delivery_status, :class_name => 'MailDeliveryStatus', :foreign_key => 'email', :primary_key => 'email'
+  has_many :workgroups, through: :memberships, source: :group, class_name: 'Workgroup'
+  has_many :assignments, dependent: :destroy
+  has_many :tasks, through: :assignments
+  has_many :send_messages, class_name: 'Message', foreign_key: 'sender_id'
+  has_many :created_orders, class_name: 'Order', foreign_key: 'created_by_user_id', dependent: :nullify
+  has_many :mail_delivery_status, class_name: 'MailDeliveryStatus', foreign_key: 'email', primary_key: 'email'
 
   attr_accessor :create_ordergroup, :password, :send_welcome_mail, :settings_attributes
 
@@ -26,22 +26,22 @@ class User < ApplicationRecord
   # makes the current_user (logged-in-user) available in models
   cattr_accessor :current_user
 
-  validates_presence_of :email
-  validates_presence_of :password, :on => :create
-  validates_format_of :email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i
-  validates_uniqueness_of :email, :case_sensitive => false
-  validates_presence_of :first_name # for simple_form validations
-  validates_length_of :first_name, :in => 2..50
-  validates_confirmation_of :password
-  validates_length_of :password, :in => 5..50, :allow_blank => true
+  validates :email, presence: true
+  validates :password, presence: { on: :create }
+  validates :email, format: { with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i }
+  validates :email, uniqueness: { case_sensitive: false }
+  validates :first_name, presence: true # for simple_form validations
+  validates :first_name, length: { in: 2..50 }
+  validates :password, confirmation: true
+  validates :password, length: { in: 5..50, allow_blank: true }
   # allow nick to be nil depending on foodcoop config
   # TODO Rails 4 may have a more beautiful way
   #   http://stackoverflow.com/questions/19845910/conditional-allow-nil-part-of-validation
-  validates_length_of :nick, :in => 2..25, :allow_nil => true, :unless => Proc.new { FoodsoftConfig[:use_nick] }
-  validates_length_of :nick, :in => 2..25, :allow_nil => false, :if => Proc.new { FoodsoftConfig[:use_nick] }
-  validates_uniqueness_of :nick, :case_sensitive => false, :allow_nil => true # allow_nil in length validation
-  validates_format_of :iban, :with => /\A[A-Z]{2}[0-9]{2}[0-9A-Z]{,30}\z/, :allow_blank => true
-  validates_uniqueness_of :iban, :case_sensitive => false, :allow_blank => true
+  validates :nick, length: { in: 2..25, allow_nil: true, unless: proc { FoodsoftConfig[:use_nick] } }
+  validates :nick, length: { in: 2..25, allow_nil: false, if: proc { FoodsoftConfig[:use_nick] } }
+  validates :nick, uniqueness: { case_sensitive: false, allow_nil: true } # allow_nil in length validation
+  validates :iban, format: { with: /\A[A-Z]{2}[0-9]{2}[0-9A-Z]{,30}\z/, allow_blank: true }
+  validates :iban, uniqueness: { case_sensitive: false, allow_blank: true }
 
   before_validation :set_password
   after_initialize do
@@ -58,17 +58,19 @@ class User < ApplicationRecord
   end
 
   after_save do
-    settings_attributes.each do |key, value|
-      value.each do |k, v|
-        case v
-        when '1'
-          value[k] = true
-        when '0'
-          value[k] = false
+    if settings_attributes
+      settings_attributes.each do |key, value|
+        value.each do |k, v|
+          case v
+          when '1'
+            value[k] = true
+          when '0'
+            value[k] = false
+          end
         end
+        settings.merge!(key, value)
       end
-      self.settings.merge!(key, value)
-    end if settings_attributes
+    end
 
     if ActiveModel::Type::Boolean.new.cast(create_ordergroup)
       og = Ordergroup.new({ name: name })
@@ -103,7 +105,7 @@ class User < ApplicationRecord
     match_name = q.split.map do |a|
       users[:first_name].matches("%#{a}%").or users[:last_name].matches("%#{a}%")
     end.reduce(:and)
-    User.where(match_nick.or match_name)
+    User.where(match_nick.or(match_name))
   end
 
   def locale
@@ -111,7 +113,7 @@ class User < ApplicationRecord
   end
 
   def name
-    [first_name, last_name].join(" ")
+    [first_name, last_name].join(' ')
   end
 
   def receive_email?
@@ -120,22 +122,24 @@ class User < ApplicationRecord
 
   # Sets the user's password. It will be stored encrypted along with a random salt.
   def set_password
-    unless password.blank?
-      salt = [Array.new(6) { rand(256).chr }.join].pack("m").chomp
-      self.password_hash, self.password_salt = Digest::SHA1.hexdigest(password + salt), salt
-    end
+    return if password.blank?
+
+    salt = [Array.new(6) { rand(256).chr }.join].pack('m').chomp
+    self.password_hash = Digest::SHA1.hexdigest(password + salt)
+    self.password_salt = salt
   end
 
   # Returns true if the password argument matches the user's password.
   def has_password(password)
-    Digest::SHA1.hexdigest(password + self.password_salt) == self.password_hash
+    Digest::SHA1.hexdigest(password + password_salt) == password_hash
   end
 
   # Returns a random password.
   def new_random_password(size = 6)
-    c = %w(b c d f g h j k l m n p qu r s t v w x z ch cr fr nd ng nk nt ph pr rd sh sl sp st th tr)
-    v = %w(a e i o u y)
-    f, r = true, ''
+    c = %w[b c d f g h j k l m n p qu r s t v w x z ch cr fr nd ng nk nt ph pr rd sh sl sp st th tr]
+    v = %w[a e i o u y]
+    f = true
+    r = ''
     (size * 2).times do
       r << (f ? c[rand * c.size] : v[rand * v.size])
       f = !f
@@ -198,12 +202,12 @@ class User < ApplicationRecord
 
   # returns true if user is a member of a given group
   def member_of?(group)
-    group.users.exists?(self.id)
+    group.users.exists?(id)
   end
 
   # Returns an array with the users groups (but without the Ordergroups -> because tpye=>"")
-  def member_of_groups()
-    self.groups.where(type: '')
+  def member_of_groups
+    groups.where(type: '')
   end
 
   def deleted?
@@ -220,11 +224,9 @@ class User < ApplicationRecord
 
   def self.authenticate(login, password)
     user = find_by_nick(login) || find_by_email(login)
-    if user && password && user.has_password(password)
-      user
-    else
-      nil
-    end
+    return unless user && password && user.has_password(password)
+
+    user
   end
 
   def self.custom_fields
@@ -248,29 +250,29 @@ class User < ApplicationRecord
   def token_attributes
     # would be sensible to match ApplicationController#show_user
     #   this should not be part of the model anyway
-    { :id => id, :name => "#{display} (#{ordergroup.try(:name)})" }
+    { id: id, name: "#{display} (#{ordergroup.try(:name)})" }
   end
 
   def self.sort_by_param(param)
-    param ||= "name"
+    param ||= 'name'
 
     sort_param_map = {
-      "nick" => "nick",
-      "nick_reverse" => "nick DESC",
-      "name" => "first_name, last_name",
-      "name_reverse" => "first_name DESC, last_name DESC",
-      "email" => "users.email",
-      "email_reverse" => "users.email DESC",
-      "phone" => "phone",
-      "phone_reverse" => "phone DESC",
-      "last_activity" => "last_activity",
-      "last_activity_reverse" => "last_activity DESC",
-      "ordergroup" => "IFNULL(groups.type, '') <> 'Ordergroup', groups.name",
-      "ordergroup_reverse" => "IFNULL(groups.type, '') <> 'Ordergroup', groups.name DESC"
+      'nick' => 'nick',
+      'nick_reverse' => 'nick DESC',
+      'name' => 'first_name, last_name',
+      'name_reverse' => 'first_name DESC, last_name DESC',
+      'email' => 'users.email',
+      'email_reverse' => 'users.email DESC',
+      'phone' => 'phone',
+      'phone_reverse' => 'phone DESC',
+      'last_activity' => 'last_activity',
+      'last_activity_reverse' => 'last_activity DESC',
+      'ordergroup' => "IFNULL(groups.type, '') <> 'Ordergroup', groups.name",
+      'ordergroup_reverse' => "IFNULL(groups.type, '') <> 'Ordergroup', groups.name DESC"
     }
 
     # Never pass user input data to Arel.sql() because of SQL Injection vulnerabilities.
     # This case here is okay, as param is mapped to the actual order string.
-    self.eager_load(:groups).order(Arel.sql(sort_param_map[param])) # eager_load is like left_join but without duplicates
+    eager_load(:groups).order(Arel.sql(sort_param_map[param])) # eager_load is like left_join but without duplicates
   end
 end

app/models/workgroup.rb

@@ -3,26 +3,26 @@ class Workgroup < Group
 
   has_many :tasks
   # returns all non-finished tasks
-  has_many :open_tasks, -> { where(:done => false).order('due_date', 'name') }, :class_name => 'Task'
+  has_many :open_tasks, -> { where(done: false).order('due_date', 'name') }, class_name: 'Task'
 
-  validates_uniqueness_of :name
-  validate :last_admin_on_earth, :on => :update
+  validates :name, uniqueness: true
+  validate :last_admin_on_earth, on: :update
   before_destroy :check_last_admin_group
 
   protected
 
   # Check before destroy a group, if this is the last group with admin role
   def check_last_admin_group
-    if role_admin && Workgroup.where(role_admin: true).size == 1
-      raise I18n.t('workgroups.error_last_admin_group')
-    end
+    return unless role_admin && Workgroup.where(role_admin: true).size == 1
+
+    raise I18n.t('workgroups.error_last_admin_group')
   end
 
   # add validation check on update
   # Return an error if this is the last group with admin role and role_admin should set to false
   def last_admin_on_earth
-    if !role_admin && !Workgroup.where(role_admin: true).where.not(id: id).exists?
-      errors.add(:role_admin, I18n.t('workgroups.error_last_admin_role'))
-    end
+    return unless !role_admin && !Workgroup.where(role_admin: true).where.not(id: id).exists?
+
+    errors.add(:role_admin, I18n.t('workgroups.error_last_admin_role'))
   end
 end