From ced626484650146d2cbac8f0692ea4ddf139a02d Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 28 May 2019 16:30:50 +0100 Subject: [PATCH] Fix rubocop issues in enterprise model --- .rubocop_manual_todo.yml | 1 - .rubocop_todo.yml | 64 +++++++++++-------------- app/models/enterprise.rb | 101 +++++++++++++++++++++++++++------------ 3 files changed, 99 insertions(+), 67 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 1facd4bad5..670c97fe88 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -78,7 +78,6 @@ Metrics/LineLength: - app/models/concerns/variant_stock.rb - app/models/content_configuration.rb - app/models/customer.rb - - app/models/enterprise.rb - app/models/enterprise_fee.rb - app/models/enterprise_group.rb - app/models/enterprise_role.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c19c41981b..7acbe5179d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config --exclude-limit 1400` -# on 2019-05-24 11:45:11 +0100 using RuboCop version 0.57.2. +# on 2019-05-28 16:29:07 +0100 using RuboCop version 0.57.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -25,6 +25,22 @@ Layout/EndAlignment: - 'app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb' - 'app/serializers/api/admin/order_cycle_serializer.rb' +# Offense count: 2 +# Cop supports --auto-correct. +# Configuration parameters: IndentationWidth. +# SupportedStyles: special_inside_parentheses, consistent, align_braces +Layout/IndentHash: + EnforcedStyle: consistent + +# Offense count: 7 +# Cop supports --auto-correct. +# Configuration parameters: EnforcedStyle, IndentationWidth. +# SupportedStyles: aligned, indented, indented_relative_to_receiver +Layout/MultilineMethodCallIndentation: + Exclude: + - 'app/models/spree/line_item_decorator.rb' + - 'app/models/spree/product_decorator.rb' + # Offense count: 4 Lint/AmbiguousOperator: Exclude: @@ -75,12 +91,6 @@ Lint/UselessAccessModifier: - 'lib/open_food_network/reports/bulk_coop_report.rb' - 'spec/lib/open_food_network/reports/report_spec.rb' -# Offense count: 1 -Lint/UselessAssignment: - Exclude: - - 'spec/**/*' - - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb' - # Offense count: 91 # Configuration parameters: CheckForMethodsWithNoSideEffects. Lint/Void: @@ -104,12 +114,6 @@ Lint/Void: Metrics/BlockLength: Max: 774 -# Offense count: 1 -# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. -# URISchemes: http, https -Metrics/LineLength: - Max: 109 - # Offense count: 7 Naming/AccessorMethodName: Exclude: @@ -256,33 +260,13 @@ Rails/OutputSafety: - 'lib/spree/money_decorator.rb' - 'spec/features/admin/orders_spec.rb' -# Offense count: 42 -# Configuration parameters: Include. -# Include: app/models/**/*.rb -Rails/ScopeArgs: - Exclude: - - 'app/models/enterprise.rb' - - 'app/models/enterprise_group.rb' - - 'app/models/enterprise_relationship.rb' - - 'app/models/enterprise_role.rb' - - 'app/models/exchange.rb' - - 'app/models/inventory_item.rb' - - 'app/models/order_cycle.rb' - - 'app/models/spree/adjustment_decorator.rb' - - 'app/models/spree/line_item_decorator.rb' - - 'app/models/spree/payment_method_decorator.rb' - - 'app/models/spree/product_decorator.rb' - - 'app/models/spree/shipping_method_decorator.rb' - - 'app/models/spree/variant_decorator.rb' - -# Offense count: 16 +# Offense count: 15 # Configuration parameters: EnforcedStyle. # SupportedStyles: strict, flexible Rails/TimeZone: Exclude: - 'app/controllers/api/statuses_controller.rb' - 'app/jobs/heartbeat_job.rb' - - 'app/models/enterprise_relationship.rb' - 'lib/open_food_network/rack_request_blocker.rb' - 'lib/open_food_network/users_and_enterprises_report.rb' - 'spec/controllers/api/statuses_controller_spec.rb' @@ -298,6 +282,14 @@ Rails/UnknownEnv: Exclude: - 'app/models/spree/app_configuration_decorator.rb' +# Offense count: 1 +# Cop supports --auto-correct. +# Configuration parameters: EnforcedStyle. +# SupportedStyles: braces, no_braces, context_dependent +Style/BracesAroundHashParameters: + Exclude: + - 'spec/spec_helper.rb' + # Offense count: 2 Style/CaseEquality: Exclude: @@ -409,7 +401,7 @@ Style/FormatStringToken: - 'lib/open_food_network/sales_tax_report.rb' - 'spec/models/enterprise_spec.rb' -# Offense count: 70 +# Offense count: 69 # Configuration parameters: MinBodyLength. Style/GuardClause: Exclude: @@ -509,7 +501,7 @@ Style/RegexpLiteral: - 'spec/mailers/subscription_mailer_spec.rb' - 'spec/models/content_configuration_spec.rb' -# Offense count: 244 +# Offense count: 243 Style/Send: Exclude: - 'app/models/spree/shipping_method_decorator.rb' diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 56581c88a1..a85ffb00e7 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -19,19 +19,29 @@ class Enterprise < ActiveRecord::Base acts_as_gmappable process_geocoding: false - has_many :relationships_as_parent, class_name: 'EnterpriseRelationship', foreign_key: 'parent_id', dependent: :destroy - has_many :relationships_as_child, class_name: 'EnterpriseRelationship', foreign_key: 'child_id', dependent: :destroy + has_many :relationships_as_parent, class_name: 'EnterpriseRelationship', + foreign_key: 'parent_id', + dependent: :destroy + has_many :relationships_as_child, class_name: 'EnterpriseRelationship', + foreign_key: 'child_id', + dependent: :destroy has_and_belongs_to_many :groups, class_name: 'EnterpriseGroup' has_many :producer_properties, foreign_key: 'producer_id' has_many :properties, through: :producer_properties - has_many :supplied_products, class_name: 'Spree::Product', foreign_key: 'supplier_id', dependent: :destroy + has_many :supplied_products, class_name: 'Spree::Product', + foreign_key: 'supplier_id', + dependent: :destroy has_many :distributed_orders, class_name: 'Spree::Order', foreign_key: 'distributor_id' belongs_to :address, class_name: 'Spree::Address' has_many :enterprise_fees has_many :enterprise_roles, dependent: :destroy has_many :users, through: :enterprise_roles - belongs_to :owner, class_name: 'Spree::User', foreign_key: :owner_id, inverse_of: :owned_enterprises - has_and_belongs_to_many :payment_methods, join_table: 'distributors_payment_methods', class_name: 'Spree::PaymentMethod', foreign_key: 'distributor_id' + belongs_to :owner, class_name: 'Spree::User', + foreign_key: :owner_id, + inverse_of: :owned_enterprises + has_and_belongs_to_many :payment_methods, join_table: 'distributors_payment_methods', + class_name: 'Spree::PaymentMethod', + foreign_key: 'distributor_id' has_many :distributor_shipping_methods, foreign_key: :distributor_id has_many :shipping_methods, through: :distributor_shipping_methods has_many :customers @@ -42,8 +52,14 @@ class Enterprise < ActiveRecord::Base delegate :latitude, :longitude, :city, :state_name, to: :address accepts_nested_attributes_for :address - accepts_nested_attributes_for :producer_properties, allow_destroy: true, reject_if: lambda { |pp| pp[:property_name].blank? } - accepts_nested_attributes_for :tag_rules, allow_destroy: true, reject_if: lambda { |tag_rule| tag_rule[:preferred_customer_tags].blank? } + accepts_nested_attributes_for :producer_properties, allow_destroy: true, + reject_if: lambda { |pp| + pp[:property_name].blank? + } + accepts_nested_attributes_for :tag_rules, allow_destroy: true, + reject_if: lambda { |tag_rule| + tag_rule[:preferred_customer_tags].blank? + } has_attached_file :logo, styles: { medium: "300x300>", small: "180x180>", thumb: "100x100>" }, @@ -51,7 +67,11 @@ class Enterprise < ActiveRecord::Base path: 'public/images/enterprises/logos/:id/:style/:basename.:extension' has_attached_file :promo_image, - styles: { large: ["1200x260#", :jpg], medium: ["720x156#", :jpg], thumb: ["100x100>", :jpg] }, + styles: { + large: ["1200x260#", :jpg], + medium: ["720x156#", :jpg], + thumb: ["100x100>", :jpg] + }, url: '/images/enterprises/promo_images/:id/:style/:basename.:extension', path: 'public/images/enterprises/promo_images/:id/:style/:basename.:extension' @@ -112,22 +132,32 @@ class Enterprise < ActiveRecord::Base select('DISTINCT enterprises.*') } - scope :with_order_cycles_as_supplier_outer, - -> { joins("LEFT OUTER JOIN exchanges ON (exchanges.sender_id = enterprises.id AND exchanges.incoming = 't')"). - joins('LEFT OUTER JOIN order_cycles ON (order_cycles.id = exchanges.order_cycle_id)') } + scope :with_order_cycles_as_supplier_outer, -> { + joins(" + LEFT OUTER JOIN exchanges + ON (exchanges.sender_id = enterprises.id AND exchanges.incoming = 't')"). + joins("LEFT OUTER JOIN order_cycles ON (order_cycles.id = exchanges.order_cycle_id)") + } - scope :with_order_cycles_as_distributor_outer, - -> { joins("LEFT OUTER JOIN exchanges ON (exchanges.receiver_id = enterprises.id AND exchanges.incoming = 'f')"). - joins('LEFT OUTER JOIN order_cycles ON (order_cycles.id = exchanges.order_cycle_id)') } + scope :with_order_cycles_as_distributor_outer, -> { + joins(" + LEFT OUTER JOIN exchanges + ON (exchanges.receiver_id = enterprises.id AND exchanges.incoming = 'f')"). + joins("LEFT OUTER JOIN order_cycles ON (order_cycles.id = exchanges.order_cycle_id)") + } - scope :with_order_cycles_outer, - -> { joins("LEFT OUTER JOIN exchanges ON (exchanges.receiver_id = enterprises.id OR exchanges.sender_id = enterprises.id)"). - joins('LEFT OUTER JOIN order_cycles ON (order_cycles.id = exchanges.order_cycle_id)') } + scope :with_order_cycles_outer, -> { + joins(" + LEFT OUTER JOIN exchanges + ON (exchanges.receiver_id = enterprises.id OR exchanges.sender_id = enterprises.id)"). + joins("LEFT OUTER JOIN order_cycles ON (order_cycles.id = exchanges.order_cycle_id)") + } - scope :with_order_cycles_and_exchange_variants_outer, - -> { with_order_cycles_as_distributor_outer. - joins('LEFT OUTER JOIN exchange_variants ON (exchange_variants.exchange_id = exchanges.id)'). - joins('LEFT OUTER JOIN spree_variants ON (spree_variants.id = exchange_variants.variant_id)') } + scope :with_order_cycles_and_exchange_variants_outer, -> { + with_order_cycles_as_distributor_outer. + joins("LEFT OUTER JOIN exchange_variants ON (exchange_variants.exchange_id = exchanges.id)"). + joins("LEFT OUTER JOIN spree_variants ON (spree_variants.id = exchange_variants.variant_id)") + } scope :distributors_with_active_order_cycles, lambda { with_order_cycles_as_distributor_outer. @@ -185,8 +215,12 @@ class Enterprise < ActiveRecord::Base def set_producer_property(property_name, property_value) transaction do - property = Spree::Property.where(name: property_name).first_or_create!(presentation: property_name) - producer_property = ProducerProperty.where(producer_id: id, property_id: property.id).first_or_initialize + property = Spree::Property. + where(name: property_name). + first_or_create!(presentation: property_name) + producer_property = ProducerProperty. + where(producer_id: id, property_id: property.id). + first_or_initialize producer_property.value = property_value producer_property.save! end @@ -243,7 +277,10 @@ class Enterprise < ActiveRecord::Base end def distributed_variants - Spree::Variant.joins(:product).merge(Spree::Product.in_distributor(self)).select('spree_variants.*') + Spree::Variant. + joins(:product). + merge(Spree::Product.in_distributor(self)). + select('spree_variants.*') end def is_distributor @@ -271,7 +308,7 @@ class Enterprise < ActiveRecord::Base when "non_producer_sells_any" :hub # Hub selling others products in order cycles. when "non_producer_sells_own" - :hub # Wholesaler selling through own shopfront? Does this need a separate name? Should it exist? + :hub # Wholesaler selling through own shopfront? Does this need a separate name or even exist? when "non_producer_sells_none" :hub_profile # Hub selling outside the system. end @@ -300,7 +337,12 @@ class Enterprise < ActiveRecord::Base def self.find_available_permalink(test_permalink) test_permalink = test_permalink.parameterize test_permalink = "my-enterprise" if test_permalink.blank? - existing = Enterprise.select(:permalink).order(:permalink).where("permalink LIKE ?", "#{test_permalink}%").map(&:permalink) + existing = Enterprise. + select(:permalink). + order(:permalink). + where("permalink LIKE ?", "#{test_permalink}%"). + map(&:permalink) + if existing.include?(test_permalink) used_indices = existing.map do |p| p.slice!(/^#{test_permalink}/) @@ -329,9 +371,7 @@ class Enterprise < ActiveRecord::Base dups = Enterprise.where(name: name) dups = dups.where('id != ?', id) unless new_record? - if dups.any? - errors.add :name, I18n.t(:enterprise_name_error, email: dups.first.owner.email) - end + errors.add :name, I18n.t(:enterprise_name_error, email: dups.first.owner.email) if dups.any? end def send_welcome_email @@ -356,7 +396,8 @@ class Enterprise < ActiveRecord::Base def enforce_ownership_limit unless owner.can_own_more_enterprises? - errors.add(:owner, I18n.t(:enterprise_owner_error, email: owner.email, enterprise_limit: owner.enterprise_limit )) + errors.add(:owner, I18n.t(:enterprise_owner_error, email: owner.email, + enterprise_limit: owner.enterprise_limit )) end end