ece517-p3/expertiza

View on GitHub
app/models/team.rb

Summary

Maintainability
B
6 hrs
Test Coverage

Mass assignment is not restricted using attr_accessible
Open

class Team < ActiveRecord::Base
Severity: Critical
Found in app/models/team.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.

Assignment Branch Condition size for randomize_all_by_parent is too high. [44.11/15]
Open

  def self.randomize_all_by_parent(parent, team_type, min_team_size)
    participants = Participant.where(parent_id: parent.id, type: parent.class.to_s + "Participant")
    participants = participants.sort { rand(-1..1) }
    users = participants.map {|p| User.find(p.user_id) }.to_a
    # find teams still need team members and users who are not in any team
Severity: Minor
Found in app/models/team.rb by rubocop

This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric

Assignment Branch Condition size for import is too high. [44.99/15]
Open

  def self.import(row_hash, id, options, teamtype)

    raise ArgumentError, "Not enough fields on this line." if row_hash.empty? || (row_hash[:teammembers].length < 2 && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last")) || (row_hash[:teammembers].empty? && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"))
    if options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"
      name = row_hash[:teamname].to_s
Severity: Minor
Found in app/models/team.rb by rubocop

This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric

Perceived complexity for import is too high. [17/7]
Open

  def self.import(row_hash, id, options, teamtype)

    raise ArgumentError, "Not enough fields on this line." if row_hash.empty? || (row_hash[:teammembers].length < 2 && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last")) || (row_hash[:teammembers].empty? && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"))
    if options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"
      name = row_hash[:teamname].to_s
Severity: Minor
Found in app/models/team.rb by rubocop

This cop tries to produce a complexity score that's a measure of the complexity the reader experiences when looking at a method. For that reason it considers when nodes as something that doesn't add as much complexity as an if or a &&. Except if it's one of those special case/when constructs where there's no expression after case. Then the cop treats it as an if/elsif/elsif... and lets all the when nodes count. In contrast to the CyclomaticComplexity cop, this cop considers else nodes as adding complexity.

Example:

def my_method                   # 1
  if cond                       # 1
    case var                    # 2 (0.8 + 4 * 0.2, rounded)
    when 1 then func_one
    when 2 then func_two
    when 3 then func_three
    when 4..10 then func_other
    end
  else                          # 1
    do_something until a && b   # 2
  end                           # ===
end                             # 7 complexity points

Cyclomatic complexity for import is too high. [15/6]
Open

  def self.import(row_hash, id, options, teamtype)

    raise ArgumentError, "Not enough fields on this line." if row_hash.empty? || (row_hash[:teammembers].length < 2 && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last")) || (row_hash[:teammembers].empty? && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"))
    if options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"
      name = row_hash[:teamname].to_s
Severity: Minor
Found in app/models/team.rb by rubocop

This cop checks that the cyclomatic complexity of methods is not higher than the configured maximum. The cyclomatic complexity is the number of linearly independent paths through a method. The algorithm counts decision points and adds one.

An if statement (or unless or ?:) increases the complexity by one. An else branch does not, since it doesn't add a decision point. The && operator (or keyword and) can be converted to a nested if statement, and ||/or is shorthand for a sequence of ifs, so they also add one. Loops can be said to have an exit condition, so they add one.

Assignment Branch Condition size for create_team_from_single_users is too high. [20.35/15]
Open

  def self.create_team_from_single_users(min_team_size, parent, team_type, users)
    num_of_teams = users.length.fdiv(min_team_size).ceil
    next_team_member_index = 0
    for i in (1..num_of_teams).to_a
      team = Object.const_get(team_type + 'Team').create(name: 'Team_' + i.to_s, parent_id: parent.id)
Severity: Minor
Found in app/models/team.rb by rubocop

This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric

Assignment Branch Condition size for add_member is too high. [19.52/15]
Open

  def add_member(user, _assignment_id = nil)
    raise "The user #{user.name} is already a member of the team #{self.name}" if user?(user)
    can_add_member = false
    unless full?
      can_add_member = true
Severity: Minor
Found in app/models/team.rb by rubocop

This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric

Perceived complexity for handle_duplicate is too high. [9/7]
Open

  def self.handle_duplicate(team, name, id, handle_dups, teamtype)
    return name if team.nil? # no duplicate
    return nil if handle_dups == "ignore" # ignore: do not create the new team
    if handle_dups == "rename" # rename: rename new team
      if teamtype.is_a?(CourseTeam)
Severity: Minor
Found in app/models/team.rb by rubocop

This cop tries to produce a complexity score that's a measure of the complexity the reader experiences when looking at a method. For that reason it considers when nodes as something that doesn't add as much complexity as an if or a &&. Except if it's one of those special case/when constructs where there's no expression after case. Then the cop treats it as an if/elsif/elsif... and lets all the when nodes count. In contrast to the CyclomaticComplexity cop, this cop considers else nodes as adding complexity.

Example:

def my_method                   # 1
  if cond                       # 1
    case var                    # 2 (0.8 + 4 * 0.2, rounded)
    when 1 then func_one
    when 2 then func_two
    when 3 then func_three
    when 4..10 then func_other
    end
  else                          # 1
    do_something until a && b   # 2
  end                           # ===
end                             # 7 complexity points

Cyclomatic complexity for handle_duplicate is too high. [7/6]
Open

  def self.handle_duplicate(team, name, id, handle_dups, teamtype)
    return name if team.nil? # no duplicate
    return nil if handle_dups == "ignore" # ignore: do not create the new team
    if handle_dups == "rename" # rename: rename new team
      if teamtype.is_a?(CourseTeam)
Severity: Minor
Found in app/models/team.rb by rubocop

This cop checks that the cyclomatic complexity of methods is not higher than the configured maximum. The cyclomatic complexity is the number of linearly independent paths through a method. The algorithm counts decision points and adds one.

An if statement (or unless or ?:) increases the complexity by one. An else branch does not, since it doesn't add a decision point. The && operator (or keyword and) can be converted to a nested if statement, and ||/or is shorthand for a sequence of ifs, so they also add one. Loops can be said to have an exit condition, so they add one.

Assignment Branch Condition size for export is too high. [15.81/15]
Open

  def self.export(csv, parent_id, options, teamtype)
    if teamtype.is_a?(CourseTeam)
      teams = CourseTeam.where(parent_id: parent_id)
    elsif teamtype.is_a?(AssignmentTeam)
      teams = AssignmentTeam.where(parent_id: parent_id)
Severity: Minor
Found in app/models/team.rb by rubocop

This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric

Method import has a Cognitive Complexity of 13 (exceeds 5 allowed). Consider refactoring.
Open

  def self.import(row_hash, id, options, teamtype)

    raise ArgumentError, "Not enough fields on this line." if row_hash.empty? || (row_hash[:teammembers].length < 2 && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last")) || (row_hash[:teammembers].empty? && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"))
    if options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"
      name = row_hash[:teamname].to_s
Severity: Minor
Found in app/models/team.rb - About 1 hr 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

Consider simplifying this complex logical expression.
Open

    raise ArgumentError, "Not enough fields on this line." if row_hash.empty? || (row_hash[:teammembers].length < 2 && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last")) || (row_hash[:teammembers].empty? && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"))
Severity: Major
Found in app/models/team.rb - About 1 hr to fix

    Method assign_single_users_to_teams has a Cognitive Complexity of 9 (exceeds 5 allowed). Consider refactoring.
    Open

      def self.assign_single_users_to_teams(min_team_size, parent, teams, users)
        teams.each do |team|
          curr_team_size = Team.size(team.id)
          member_num_difference = min_team_size - curr_team_size
          while member_num_difference > 0
    Severity: Minor
    Found in app/models/team.rb - About 55 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

    Method handle_duplicate has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
    Open

      def self.handle_duplicate(team, name, id, handle_dups, teamtype)
        return name if team.nil? # no duplicate
        return nil if handle_dups == "ignore" # ignore: do not create the new team
        if handle_dups == "rename" # rename: rename new team
          if teamtype.is_a?(CourseTeam)
    Severity: Minor
    Found in app/models/team.rb - About 45 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

    Method handle_duplicate has 5 arguments (exceeds 4 allowed). Consider refactoring.
    Open

      def self.handle_duplicate(team, name, id, handle_dups, teamtype)
    Severity: Minor
    Found in app/models/team.rb - About 35 mins to fix

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

        def self.randomize_all_by_parent(parent, team_type, min_team_size)
          participants = Participant.where(parent_id: parent.id, type: parent.class.to_s + "Participant")
          participants = participants.sort { rand(-1..1) }
          users = participants.map {|p| User.find(p.user_id) }.to_a
          # find teams still need team members and users who are not in any team
      Severity: Minor
      Found in app/models/team.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

      Avoid too many return statements within this method.
      Open

            return name
      Severity: Major
      Found in app/models/team.rb - About 30 mins to fix

        Avoid too many return statements within this method.
        Open

              return nil
        Severity: Major
        Found in app/models/team.rb - About 30 mins to fix

          Method import_team_members has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
          Open

            def import_team_members(starting_index = 0, row_hash)
              starting_index
              index = 0
              row_hash[:teammembers].each do |teammember|
                next if index < starting_index # not sure this will work, hash is not ordered like array
          Severity: Minor
          Found in app/models/team.rb - About 25 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

          Use find_by instead of where.first.
          Open

                team = where(["name =? && parent_id =?", name, id]).first
          Severity: Minor
          Found in app/models/team.rb by rubocop

          This cop is used to identify usages of where.first and change them to use find_by instead.

          Example:

          # bad
          User.where(name: 'Bruce').first
          User.where(name: 'Bruce').take
          
          # good
          User.find_by(name: 'Bruce')

          Specify an :inverse_of option.
          Open

            has_one :team_node, foreign_key: :node_object_id, dependent: :destroy
          Severity: Minor
          Found in app/models/team.rb by rubocop

          This cop looks for has(one|many) and belongsto associations where ActiveRecord can't automatically determine the inverse association because of a scope or the options used. This can result in unnecessary queries in some circumstances. :inverse_of must be manually specified for associations to work in both ways, or set to false to opt-out.

          Example:

          # good
          class Blog < ApplicationRecord
            has_many :posts
          end
          
          class Post < ApplicationRecord
            belongs_to :blog
          end

          Example:

          # bad
          class Blog < ApplicationRecord
            has_many :posts, -> { order(published_at: :desc) }
          end
          
          class Post < ApplicationRecord
            belongs_to :blog
          end
          
          # good
          class Blog < ApplicationRecord
            has_many(:posts,
              -> { order(published_at: :desc) },
              inverse_of: :blog
            )
          end
          
          class Post < ApplicationRecord
            belongs_to :blog
          end
          
          # good
          class Blog < ApplicationRecord
            with_options inverse_of: :blog do
              has_many :posts, -> { order(published_at: :desc) }
            end
          end
          
          class Post < ApplicationRecord
            belongs_to :blog
          end

          Example:

          # bad
          class Picture < ApplicationRecord
            belongs_to :imageable, polymorphic: true
          end
          
          class Employee < ApplicationRecord
            has_many :pictures, as: :imageable
          end
          
          class Product < ApplicationRecord
            has_many :pictures, as: :imageable
          end
          
          # good
          class Picture < ApplicationRecord
            belongs_to :imageable, polymorphic: true
          end
          
          class Employee < ApplicationRecord
            has_many :pictures, as: :imageable, inverse_of: :imageable
          end
          
          class Product < ApplicationRecord
            has_many :pictures, as: :imageable, inverse_of: :imageable
          end

          Example:

          # bad
          # However, RuboCop can not detect this pattern...
          class Physician < ApplicationRecord
            has_many :appointments
            has_many :patients, through: :appointments
          end
          
          class Appointment < ApplicationRecord
            belongs_to :physician
            belongs_to :patient
          end
          
          class Patient < ApplicationRecord
            has_many :appointments
            has_many :physicians, through: :appointments
          end
          
          # good
          class Physician < ApplicationRecord
            has_many :appointments
            has_many :patients, through: :appointments
          end
          
          class Appointment < ApplicationRecord
            belongs_to :physician, inverse_of: :appointments
            belongs_to :patient, inverse_of: :appointments
          end
          
          class Patient < ApplicationRecord
            has_many :appointments
            has_many :physicians, through: :appointments
          end

          @see http://guides.rubyonrails.org/association_basics.html#bi-directional-associations @see http://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html#module-ActiveRecord::Associations::ClassMethods-label-Setting+Inverses

          Extra empty line detected at method body beginning.
          Open

          
              raise ArgumentError, "Not enough fields on this line." if row_hash.empty? || (row_hash[:teammembers].length < 2 && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last")) || (row_hash[:teammembers].empty? && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"))
          Severity: Minor
          Found in app/models/team.rb by rubocop

          This cops checks if empty lines exist around the bodies of methods.

          Example:

          # good
          
          def foo
            # ...
          end
          
          # bad
          
          def bar
          
            # ...
          
          end

          Unused method argument - team_name_prefix. If it's necessary, use _ or _team_name_prefix as an argument name to indicate that it won't be used. You can also write as generate_team_name(*) if you want the method to accept any arguments but don't care about them.
          Open

            def self.generate_team_name(team_name_prefix = '')
          Severity: Minor
          Found in app/models/team.rb by rubocop

          This cop checks for unused method arguments.

          Example:

          # bad
          
          def some_method(used, unused, _unused_but_allowed)
            puts used
          end

          Example:

          # good
          
          def some_method(used, _unused, _unused_but_allowed)
            puts used
          end

          Line is too long. [321/160]
          Open

              raise ArgumentError, "Not enough fields on this line." if row_hash.empty? || (row_hash[:teammembers].length < 2 && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last")) || (row_hash[:teammembers].empty? && (options[:has_teamname] == "true_first" || options[:has_teamname] == "true_last"))
          Severity: Minor
          Found in app/models/team.rb by rubocop

          Use a guard clause instead of wrapping the code inside a conditional expression.
          Open

                if teamtype.is_a?(CourseTeam)
          Severity: Minor
          Found in app/models/team.rb by rubocop

          Use a guard clause instead of wrapping the code inside a conditional expression

          Example:

          # bad
          def test
            if something
              work
            end
          end
          
          # good
          def test
            return unless something
            work
          end
          
          # also good
          def test
            work if something
          end
          
          # bad
          if something
            raise 'exception'
          else
            ok
          end
          
          # good
          raise 'exception' if something
          ok

          Convert if nested inside else to elsif.
          Open

                if teamtype.is_a?(CourseTeam)
          Severity: Minor
          Found in app/models/team.rb by rubocop

          If the else branch of a conditional consists solely of an if node, it can be combined with the else to become an elsif. This helps to keep the nesting level from getting too deep.

          Example:

          # bad
          if condition_a
            action_a
          else
            if condition_b
              action_b
            else
              action_c
            end
          end
          
          # good
          if condition_a
            action_a
          elsif condition_b
            action_b
          else
            action_c
          end

          Variable starting_index used in void context.
          Open

              starting_index
          Severity: Minor
          Found in app/models/team.rb by rubocop

          This cop checks for operators, variables and literals used in void context.

          Example:

          # bad
          
          def some_method
            some_num * 10
            do_something
          end

          Example:

          # bad
          
          def some_method(some_var)
            some_var
            do_something
          end

          Example:

          # good
          
          def some_method
            do_something
            some_num * 10
          end

          Example:

          # good
          
          def some_method(some_var)
            do_something
            some_var
          end

          Prefer each over for.
          Open

              for i in (1..num_of_teams).to_a
          Severity: Minor
          Found in app/models/team.rb by rubocop

          This cop looks for uses of the for keyword, or each method. The preferred alternative is set in the EnforcedStyle configuration parameter. An each call with a block on a single line is always allowed, however.

          Line is too long. [170/160]
          Open

              parent = parent_model id # current_task will be either a course object or an assignment object. # current_task will be either a course object or an assignment object.
          Severity: Minor
          Found in app/models/team.rb by rubocop

          Use a guard clause instead of wrapping the code inside a conditional expression.
          Open

              if handle_dups == "replace" # replace: delete old team
          Severity: Minor
          Found in app/models/team.rb by rubocop

          Use a guard clause instead of wrapping the code inside a conditional expression

          Example:

          # bad
          def test
            if something
              work
            end
          end
          
          # good
          def test
            return unless something
            work
          end
          
          # also good
          def test
            work if something
          end
          
          # bad
          if something
            raise 'exception'
          else
            ok
          end
          
          # good
          raise 'exception' if something
          ok

          Redundant use of Object#to_s in interpolation.
          Open

                  raise ImportError, "The user '#{teammember.to_s}' was not found. <a href='/users/new'>Create</a> this user?"
          Severity: Minor
          Found in app/models/team.rb by rubocop

          This cop checks for string conversion in string interpolation, which is redundant.

          Example:

          # bad
          
          "result is #{something.to_s}"

          Example:

          # good
          
          "result is #{something}"

          Convert if nested inside else to elsif.
          Open

                  add_member(user) if TeamsUser.find_by(team_id: id, user_id: user.id).nil?
          Severity: Minor
          Found in app/models/team.rb by rubocop

          If the else branch of a conditional consists solely of an if node, it can be combined with the else to become an elsif. This helps to keep the nesting level from getting too deep.

          Example:

          # bad
          if condition_a
            action_a
          else
            if condition_b
              action_b
            else
              action_c
            end
          end
          
          # good
          if condition_a
            action_a
          elsif condition_b
            action_b
          else
            action_c
          end

          Use a guard clause instead of wrapping the code inside a conditional expression.
          Open

                if user.nil?
          Severity: Minor
          Found in app/models/team.rb by rubocop

          Use a guard clause instead of wrapping the code inside a conditional expression

          Example:

          # bad
          def test
            if something
              work
            end
          end
          
          # good
          def test
            return unless something
            work
          end
          
          # also good
          def test
            work if something
          end
          
          # bad
          if something
            raise 'exception'
          else
            ok
          end
          
          # good
          raise 'exception' if something
          ok

          Optional arguments should appear at the end of the argument list.
          Open

            def import_team_members(starting_index = 0, row_hash)
          Severity: Minor
          Found in app/models/team.rb by rubocop

          This cop checks for optional arguments to methods that do not come at the end of the argument list

          Example:

          # bad
          def foo(a = 1, b, c)
          end
          
          # good
          def baz(a, b, c = 1)
          end
          
          def foobar(a = 1, b = 2, c = 3)
          end

          There are no issues that match your filters.

          Category
          Status