3scale/porta

View on GitHub
app/models/contract.rb

Summary

Maintainability
A
2 hrs
Test Coverage

attr_accessible is recommended over attr_protected
Open

  attr_protected :plan_id, :state, :provider_public_key, :paid_until, :trial_period_expires_at, :setup_fee, :type, :variable_cost_paid_until, :application_id, :user_key, :user_account_id, :tenant_id, :audit_ids
Severity: Minor
Found in app/models/contract.rb by brakeman

This warning comes up if a model does not limit what attributes can be set through mass assignment.

In particular, this check looks for attr_accessible inside model definitions. If it is not found, this warning will be issued.

Brakeman also warns on use of attr_protected - especially since it was found to be vulnerable to bypass. Warnings for mass assignment on models using attr_protected will be reported, but at a lower confidence level.

Note that disabling mass assignment globally will suppress these warnings.

Class Contract has 22 methods (exceeds 20 allowed). Consider refactoring.
Open

class Contract < ApplicationRecord
  # Need to define table_name before audited because of
  # https://github.com/collectiveidea/audited/blob/f03c5b5d1717f2ebec64032d269316dc74476056/lib/audited/auditor.rb#L305-L311
  self.table_name = 'cinstances'

Severity: Minor
Found in app/models/contract.rb - About 2 hrs to fix

    Contract#update_counter_cache? is controlled by argument 'association_name'
    Open

        case association_name
    Severity: Minor
    Found in app/models/contract.rb by reek

    Control Parameter is a special case of Control Couple

    Example

    A simple example would be the "quoted" parameter in the following method:

    def write(quoted)
      if quoted
        write_quoted @value
      else
        write_unquoted @value
      end
    end

    Fixing those problems is out of the scope of this document but an easy solution could be to remove the "write" method alltogether and to move the calls to "writequoted" / "writeunquoted" in the initial caller of "write".

    Contract#customize_plan! has approx 8 statements
    Open

      def customize_plan!(attrs = {})
    Severity: Minor
    Found in app/models/contract.rb by reek

    A method with Too Many Statements is any method that has a large number of lines.

    Too Many Statements warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements counts +1 for every simple statement in a method and +1 for every statement within a control structure (if, else, case, when, for, while, until, begin, rescue) but it doesn't count the control structure itself.

    So the following method would score +6 in Reek's statement-counting algorithm:

    def parse(arg, argv, &error)
      if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
        return nil, block, nil                                         # +1
      end
      opt = (val = parse_arg(val, &error))[1]                          # +2
      val = conv_arg(*val)                                             # +3
      if opt and !arg
        argv.shift                                                     # +4
      else
        val[0] = nil                                                   # +5
      end
      val                                                              # +6
    end

    (You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)

    Contract#change_plan_internal has approx 10 statements
    Open

      def change_plan_internal(new_plan, &block)
    Severity: Minor
    Found in app/models/contract.rb by reek

    A method with Too Many Statements is any method that has a large number of lines.

    Too Many Statements warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements counts +1 for every simple statement in a method and +1 for every statement within a control structure (if, else, case, when, for, while, until, begin, rescue) but it doesn't count the control structure itself.

    So the following method would score +6 in Reek's statement-counting algorithm:

    def parse(arg, argv, &error)
      if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
        return nil, block, nil                                         # +1
      end
      opt = (val = parse_arg(val, &error))[1]                          # +2
      val = conv_arg(*val)                                             # +3
      if opt and !arg
        argv.shift                                                     # +4
      else
        val[0] = nil                                                   # +5
      end
      val                                                              # +6
    end

    (You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)

    Contract#self.by_plan_type has approx 9 statements
    Open

      def self.by_plan_type(type)
    Severity: Minor
    Found in app/models/contract.rb by reek

    A method with Too Many Statements is any method that has a large number of lines.

    Too Many Statements warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements counts +1 for every simple statement in a method and +1 for every statement within a control structure (if, else, case, when, for, while, until, begin, rescue) but it doesn't count the control structure itself.

    So the following method would score +6 in Reek's statement-counting algorithm:

    def parse(arg, argv, &error)
      if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
        return nil, block, nil                                         # +1
      end
      opt = (val = parse_arg(val, &error))[1]                          # +2
      val = conv_arg(*val)                                             # +3
      if opt and !arg
        argv.shift                                                     # +4
      else
        val[0] = nil                                                   # +5
      end
      val                                                              # +6
    end

    (You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)

    Contract#bill_for has approx 8 statements
    Open

      def bill_for(period, invoice, plan = self.plan)
    Severity: Minor
    Found in app/models/contract.rb by reek

    A method with Too Many Statements is any method that has a large number of lines.

    Too Many Statements warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements counts +1 for every simple statement in a method and +1 for every statement within a control structure (if, else, case, when, for, while, until, begin, rescue) but it doesn't count the control structure itself.

    So the following method would score +6 in Reek's statement-counting algorithm:

    def parse(arg, argv, &error)
      if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
        return nil, block, nil                                         # +1
      end
      opt = (val = parse_arg(val, &error))[1]                          # +2
      val = conv_arg(*val)                                             # +3
      if opt and !arg
        argv.shift                                                     # +4
      else
        val[0] = nil                                                   # +5
      end
      val                                                              # +6
    end

    (You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)

    Contract has at least 19 methods
    Open

    class Contract < ApplicationRecord
    Severity: Minor
    Found in app/models/contract.rb by reek

    Too Many Methods is a special case of LargeClass.

    Example

    Given this configuration

    TooManyMethods:
      max_methods: 3

    and this code:

    class TooManyMethods
      def one; end
      def two; end
      def three; end
      def four; end
    end

    Reek would emit the following warning:

    test.rb -- 1 warning:
      [1]:TooManyMethods has at least 4 methods (TooManyMethods)

    Method change_plan_internal has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.
    Open

      def change_plan_internal(new_plan, &block)
        return if self.plan == new_plan || new_plan.nil?
        raise 'change_plan_internal must be called with a block' unless block_given?
    
        transaction do
    Severity: Minor
    Found in app/models/contract.rb - About 35 mins to fix

    Cognitive Complexity

    Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

    A method's cognitive complexity is based on a few simple rules:

    • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
    • Code is considered more complex for each "break in the linear flow of the code"
    • Code is considered more complex when "flow breaking structures are nested"

    Further reading

    Contract#self.by_plan_type calls 'plans.where' 2 times
    Open

                      plans.where { (cost_per_month == 0) & (setup_fee == 0) & (pricing_rules.id == nil) } # rubocop:disable Style/NumericPredicate,Style/NilComparison
                    when 'paid'
                      plans.where { (cost_per_month != 0) | (setup_fee != 0) | (pricing_rules.id != nil) } # rubocop:disable Style/NumericPredicate,Style/NonNilCheck
    Severity: Minor
    Found in app/models/contract.rb by reek

    Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

    Reek implements a check for Duplicate Method Call.

    Example

    Here's a very much simplified and contrived example. The following method will report a warning:

    def double_thing()
      @other.thing + @other.thing
    end

    One quick approach to silence Reek would be to refactor the code thus:

    def double_thing()
      thing = @other.thing
      thing + thing
    end

    A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

    class Other
      def double_thing()
        thing + thing
      end
    end

    The approach you take will depend on balancing other factors in your code.

    Contract assumes too much for instance variable '@old_plan'
    Open

    class Contract < ApplicationRecord
    Severity: Minor
    Found in app/models/contract.rb by reek

    Classes should not assume that instance variables are set or present outside of the current class definition.

    Good:

    class Foo
      def initialize
        @bar = :foo
      end
    
      def foo?
        @bar == :foo
      end
    end

    Good as well:

    class Foo
      def foo?
        bar == :foo
      end
    
      def bar
        @bar ||= :foo
      end
    end

    Bad:

    class Foo
      def go_foo!
        @bar = :foo
      end
    
      def foo?
        @bar == :foo
      end
    end

    Example

    Running Reek on:

    class Dummy
      def test
        @ivar
      end
    end

    would report:

    [1]:InstanceVariableAssumption: Dummy assumes too much for instance variable @ivar

    Note that this example would trigger this smell warning as well:

    class Parent
      def initialize(omg)
        @omg = omg
      end
    end
    
    class Child < Parent
      def foo
        @omg
      end
    end

    The way to address the smell warning is that you should create an attr_reader to use @omg in the subclass and not access @omg directly like this:

    class Parent
      attr_reader :omg
    
      def initialize(omg)
        @omg = omg
      end
    end
    
    class Child < Parent
      def foo
        omg
      end
    end

    Directly accessing instance variables is considered a smell because it breaks encapsulation and makes it harder to reason about code.

    If you don't want to expose those methods as public API just make them private like this:

    class Parent
      def initialize(omg)
        @omg = omg
      end
    
      private
      attr_reader :omg
    end
    
    class Child < Parent
      def foo
        omg
      end
    end

    Current Support in Reek

    An instance variable must:

    • be set in the constructor
    • or be accessed through a method with lazy initialization / memoization.

    If not, Instance Variable Assumption will be reported.

    Contract#self.by_plan_type calls 'pricing_rules.id' 2 times
    Open

                      plans.where { (cost_per_month == 0) & (setup_fee == 0) & (pricing_rules.id == nil) } # rubocop:disable Style/NumericPredicate,Style/NilComparison
                    when 'paid'
                      plans.where { (cost_per_month != 0) | (setup_fee != 0) | (pricing_rules.id != nil) } # rubocop:disable Style/NumericPredicate,Style/NonNilCheck
    Severity: Minor
    Found in app/models/contract.rb by reek

    Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

    Reek implements a check for Duplicate Method Call.

    Example

    Here's a very much simplified and contrived example. The following method will report a warning:

    def double_thing()
      @other.thing + @other.thing
    end

    One quick approach to silence Reek would be to refactor the code thus:

    def double_thing()
      thing = @other.thing
      thing + thing
    end

    A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

    class Other
      def double_thing()
        thing + thing
      end
    end

    The approach you take will depend on balancing other factors in your code.

    Contract#bill_for calls 'invoice.used?' 2 times
    Open

          self.save(:validate => false) if invoice.used?
        end
    
        invoice.used?
    Severity: Minor
    Found in app/models/contract.rb by reek

    Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

    Reek implements a check for Duplicate Method Call.

    Example

    Here's a very much simplified and contrived example. The following method will report a warning:

    def double_thing()
      @other.thing + @other.thing
    end

    One quick approach to silence Reek would be to refactor the code thus:

    def double_thing()
      thing = @other.thing
      thing + thing
    end

    A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

    class Other
      def double_thing()
        thing + thing
      end
    end

    The approach you take will depend on balancing other factors in your code.

    Contract#intersect_with_unpaid_period calls 'period.begin' 2 times
    Open

          period = period.begin..(period.end - 1.second)
        end
    
        from = [ period.begin.to_date, paid_end ].max.to_date
    Severity: Minor
    Found in app/models/contract.rb by reek

    Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

    Reek implements a check for Duplicate Method Call.

    Example

    Here's a very much simplified and contrived example. The following method will report a warning:

    def double_thing()
      @other.thing + @other.thing
    end

    One quick approach to silence Reek would be to refactor the code thus:

    def double_thing()
      thing = @other.thing
      thing + thing
    end

    A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

    class Other
      def double_thing()
        thing + thing
      end
    end

    The approach you take will depend on balancing other factors in your code.

    Contract#intersect_with_unpaid_period calls 'period.end' 2 times
    Open

          period = period.begin..(period.end - 1.second)
        end
    
        from = [ period.begin.to_date, paid_end ].max.to_date
        to = [ period.end.to_date, from ].max.to_date
    Severity: Minor
    Found in app/models/contract.rb by reek

    Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

    Reek implements a check for Duplicate Method Call.

    Example

    Here's a very much simplified and contrived example. The following method will report a warning:

    def double_thing()
      @other.thing + @other.thing
    end

    One quick approach to silence Reek would be to refactor the code thus:

    def double_thing()
      thing = @other.thing
      thing + thing
    end

    A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

    class Other
      def double_thing()
        thing + thing
      end
    end

    The approach you take will depend on balancing other factors in your code.

    Contract#change_plan_internal calls 'self.plan' 2 times
    Open

        return if self.plan == new_plan || new_plan.nil?
        raise 'change_plan_internal must be called with a block' unless block_given?
    
        transaction do
    
    
    Severity: Minor
    Found in app/models/contract.rb by reek

    Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

    Reek implements a check for Duplicate Method Call.

    Example

    Here's a very much simplified and contrived example. The following method will report a warning:

    def double_thing()
      @other.thing + @other.thing
    end

    One quick approach to silence Reek would be to refactor the code thus:

    def double_thing()
      thing = @other.thing
      thing + thing
    end

    A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

    class Other
      def double_thing()
        thing + thing
      end
    end

    The approach you take will depend on balancing other factors in your code.

    Contract#bill_for calls 'period.end' 2 times
    Open

          if paid_until.to_date < period.end.to_date
    
            period = intersect_with_unpaid_period(period, paid_until)
    
            bill_fixed_fee_for(period, invoice, plan)
    Severity: Minor
    Found in app/models/contract.rb by reek

    Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

    Reek implements a check for Duplicate Method Call.

    Example

    Here's a very much simplified and contrived example. The following method will report a warning:

    def double_thing()
      @other.thing + @other.thing
    end

    One quick approach to silence Reek would be to refactor the code thus:

    def double_thing()
      thing = @other.thing
      thing + thing
    end

    A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

    class Other
      def double_thing()
        thing + thing
      end
    end

    The approach you take will depend on balancing other factors in your code.

    Contract has missing safe method 'customize_plan!'
    Open

      def customize_plan!(attrs = {})
    Severity: Minor
    Found in app/models/contract.rb by reek

    A candidate method for the Missing Safe Method smell are methods whose names end with an exclamation mark.

    An exclamation mark in method names means (the explanation below is taken from here ):

    The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name. So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth.

    Such a method is called Missing Safe Method if and only if her non-bang version does not exist and this method is reported as a smell.

    Example

    Given

    class C
      def foo; end
      def foo!; end
      def bar!; end
    end

    Reek would report bar! as Missing Safe Method smell but not foo!.

    Reek reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this:

    class Parent
      def foo; end
    end
    
    module Dangerous
      def foo!; end
    end
    
    class Son < Parent
      include Dangerous
    end
    
    class Daughter < Parent
    end

    In this example, Reek would not report the Missing Safe Method smell for the method foo of the Dangerous module.

    Contract has missing safe method 'decustomize_plan!'
    Open

      def decustomize_plan!
    Severity: Minor
    Found in app/models/contract.rb by reek

    A candidate method for the Missing Safe Method smell are methods whose names end with an exclamation mark.

    An exclamation mark in method names means (the explanation below is taken from here ):

    The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name. So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth.

    Such a method is called Missing Safe Method if and only if her non-bang version does not exist and this method is reported as a smell.

    Example

    Given

    class C
      def foo; end
      def foo!; end
      def bar!; end
    end

    Reek would report bar! as Missing Safe Method smell but not foo!.

    Reek reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this:

    class Parent
      def foo; end
    end
    
    module Dangerous
      def foo!; end
    end
    
    class Son < Parent
      include Dangerous
    end
    
    class Daughter < Parent
    end

    In this example, Reek would not report the Missing Safe Method smell for the method foo of the Dangerous module.

    Contract#self.by_plan_type performs a nil-check
    Open

                      plans.where { (cost_per_month == 0) & (setup_fee == 0) & (pricing_rules.id == nil) } # rubocop:disable Style/NumericPredicate,Style/NilComparison
    Severity: Minor
    Found in app/models/contract.rb by reek

    A NilCheck is a type check. Failures of NilCheck violate the "tell, don't ask" principle.

    Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.

    Example

    Given

    class Klass
      def nil_checker(argument)
        if argument.nil?
          puts "argument isn't nil!"
        end
      end
    end

    Reek would emit the following warning:

    test.rb -- 1 warning:
      [3]:Klass#nil_checker performs a nil-check. (NilCheck)

    Contract#change_plan_internal performs a nil-check
    Open

        return if self.plan == new_plan || new_plan.nil?
    Severity: Minor
    Found in app/models/contract.rb by reek

    A NilCheck is a type check. Failures of NilCheck violate the "tell, don't ask" principle.

    Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.

    Example

    Given

    class Klass
      def nil_checker(argument)
        if argument.nil?
          puts "argument isn't nil!"
        end
      end
    end

    Reek would emit the following warning:

    test.rb -- 1 warning:
      [3]:Klass#nil_checker performs a nil-check. (NilCheck)

    There are no issues that match your filters.

    Category
    Status