ece517-p3/expertiza

View on GitHub
app/models/sign_up_topic.rb

Summary

Maintainability
A
1 hr
Test Coverage

Mass assignment is not restricted using attr_accessible
Open

class SignUpTopic < ActiveRecord::Base
Severity: Critical
Found in app/models/sign_up_topic.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 reassign_topic is too high. [29.48/15]
Open

  def self.reassign_topic(session_user_id, assignment_id, topic_id)
    # find whether assignment is team assignment
    assignment = Assignment.find(assignment_id)

    # making sure that the drop date deadline hasn't passed
Severity: Minor
Found in app/models/sign_up_topic.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 reassign_topic is too high. [8/7]
Open

  def self.reassign_topic(session_user_id, assignment_id, topic_id)
    # find whether assignment is team assignment
    assignment = Assignment.find(assignment_id)

    # making sure that the drop date deadline hasn't passed
Severity: Minor
Found in app/models/sign_up_topic.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 reassign_topic is too high. [7/6]
Open

  def self.reassign_topic(session_user_id, assignment_id, topic_id)
    # find whether assignment is team assignment
    assignment = Assignment.find(assignment_id)

    # making sure that the drop date deadline hasn't passed
Severity: Minor
Found in app/models/sign_up_topic.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.

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

  def self.reassign_topic(session_user_id, assignment_id, topic_id)
    # find whether assignment is team assignment
    assignment = Assignment.find(assignment_id)

    # making sure that the drop date deadline hasn't passed
Severity: Minor
Found in app/models/sign_up_topic.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

Avoid more than 3 levels of block nesting.
Open

          unless first_waitlisted_user.nil?
            # As this user is going to be allocated a confirmed topic, all of his waitlisted topic signups should be purged
            ### Bad policy!  Should be changed! (once users are allowed to specify waitlist priorities) -efg
            first_waitlisted_user.is_waitlisted = false
            first_waitlisted_user.save
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

This cop checks for excessive nesting of conditional and looping constructs.

You can configure if blocks are considered using the CountBlocks option. When set to false (the default) blocks are not counted towards the nesting level. Set to true to count blocks as well.

The maximum level of nesting allowed is configurable.

Specify an :inverse_of option.
Open

  has_many :bids, foreign_key: 'topic_id', dependent: :destroy
Severity: Minor
Found in app/models/sign_up_topic.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

Use find_by instead of where.first.
Open

    dropDate = AssignmentDueDate.where(parent_id: assignment.id, deadline_type_id: '6').first
Severity: Minor
Found in app/models/sign_up_topic.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')

Use find_by instead of where.first.
Open

          first_waitlisted_user = SignedUpTeam.where(topic_id: topic_id, is_waitlisted: true).first
Severity: Minor
Found in app/models/sign_up_topic.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_many :signed_up_teams, foreign_key: 'topic_id', dependent: :destroy
Severity: Minor
Found in app/models/sign_up_topic.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

Specify an :inverse_of option.
Open

  has_many :due_dates, class_name: 'TopicDueDate', foreign_key: 'parent_id', dependent: :destroy
Severity: Minor
Found in app/models/sign_up_topic.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

Use find_by instead of where.first.
Open

    topic = SignUpTopic.where(topic_name: row_hash[:topic_name], assignment_id: session[:assignment_id]).first
Severity: Minor
Found in app/models/sign_up_topic.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')

Use find_by instead of where.first.
Open

      next_wait_listed_team = SignedUpTeam.where(topic_id: self.id, is_waitlisted: true).first
Severity: Minor
Found in app/models/sign_up_topic.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')

Use find_by instead of where.first.
Open

      signup_record = SignedUpTeam.where(topic_id: topic_id, team_id:  users_team[0].t_id).first
Severity: Minor
Found in app/models/sign_up_topic.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')

Line is too long. [245/160]
Open

    # SignUpTopic.find_by_sql("SELECT topic_id as topic_id, COUNT(t.max_choosers) as count FROM sign_up_topics t JOIN signed_up_teams u ON t.id = u.topic_id WHERE t.assignment_id =" + assignment_id+  " and u.is_waitlisted = false GROUP BY t.id")
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

Line is too long. [213/160]
Open

    SignedUpTeam.find_by_sql(["SELECT u.id FROM sign_up_topics t, signed_up_teams u WHERE t.id = u.topic_id and u.is_waitlisted = true and t.assignment_id = ? and u.team_id = ?", assignment_id.to_s, team_id.to_s])
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

Do not use Time.now without zone. Use one of Time.zone.now, Time.current, Time.now.in_time_zone, Time.now.utc, Time.now.getlocal, Time.now.iso8601, Time.now.jisx0301, Time.now.rfc3339, Time.now.to_i, Time.now.to_f instead.
Open

    if !dropDate.nil? && dropDate.due_at < Time.now
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

This cop checks for the use of Time methods without zone.

Built on top of Ruby on Rails style guide (https://github.com/bbatsov/rails-style-guide#time) and the article http://danilenko.org/2012/7/6/rails_timezones/ .

Two styles are supported for this cop. When EnforcedStyle is 'strict' then only use of Time.zone is allowed.

When EnforcedStyle is 'flexible' then it's also allowed to use Time.intimezone.

Example:

# always offense
Time.now
Time.parse('2015-03-02 19:05:37')

# no offense
Time.zone.now
Time.zone.parse('2015-03-02 19:05:37')

# no offense only if style is 'flexible'
Time.current
DateTime.strptime(str, "%Y-%m-%d %H:%M %Z").in_time_zone
Time.at(timestamp).in_time_zone

end at 117, 12 is not aligned with unless at 108, 10.
Open

            end
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

This cop checks whether the end keywords are aligned properly.

Three modes are supported through the EnforcedStyleAlignWith configuration parameter:

If it's set to keyword (which is the default), the end shall be aligned with the start of the keyword (if, class, etc.).

If it's set to variable the end shall be aligned with the left-hand-side of the variable assignment, if there is one.

If it's set to start_of_line, the end shall be aligned with the start of the line where the matching keyword appears.

Example: EnforcedStyleAlignWith: keyword (default)

# bad

variable = if true
    end

# good

variable = if true
           end

Example: EnforcedStyleAlignWith: variable

# bad

variable = if true
    end

# good

variable = if true
end

Example: EnforcedStyleAlignWith: startofline

# bad

variable = if true
    end

# good

puts(if true
end)

Line is too long. [245/160]
Open

    # SignUpTopic.find_by_sql("SELECT topic_id as topic_id, COUNT(t.max_choosers) as count FROM sign_up_topics t JOIN signed_up_teams u ON t.id = u.topic_id WHERE t.assignment_id =" + assignment_id +  " and u.is_waitlisted = true GROUP BY t.id")
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

Line is too long. [185/160]
Open

      raise ArgumentError, "The CSV File expects the format: Topic identifier, Topic name, Max choosers, Topic Category (optional), Topic Description (Optional), Topic Link (optional)."
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

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

    if !no_of_students_who_selected_the_topic.nil?
Severity: Minor
Found in app/models/sign_up_topic.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

Line is too long. [240/160]
Open

    SignUpTopic.find_by_sql(["SELECT topic_id as topic_id, COUNT(t.max_choosers) as count FROM sign_up_topics t JOIN signed_up_teams u ON t.id = u.topic_id WHERE t.assignment_id = ? and u.is_waitlisted = true GROUP BY t.id", assignment_id])
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

Do not place comments on the same line as the end keyword.
Open

    end # end condition for 'drop deadline' check
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

This cop checks for comments put on the same line as some keywords. These keywords are: begin, class, def, end, module.

Note that some comments (such as :nodoc: and rubocop:disable) are allowed.

Example:

# bad
if condition
  statement
end # end if

# bad
class X # comment
  statement
end

# bad
def x; end # comment

# good
if condition
  statement
end

# good
class X # :nodoc:
  y
end

Rename has_suggested_topic? to suggested_topic?.
Open

  def self.has_suggested_topic?(assignment_id)
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

This cop makes sure that predicates are named properly.

Example:

# bad
def is_even?(value)
end

# good
def even?(value)
end

# bad
def has_value?
end

# good
def value?
end

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

      if topic.max_choosers > no_of_students_who_selected_the_topic.size
Severity: Minor
Found in app/models/sign_up_topic.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

Use snake_case for method names.
Open

  def self.slotAvailable?(topic_id)
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

This cop makes sure that all methods use the configured style, snake_case or camelCase, for their names.

Example: EnforcedStyle: snake_case (default)

# bad
def fooBar; end

# good
def foo_bar; end

Example: EnforcedStyle: camelCase

# bad
def foo_bar; end

# good
def fooBar; end

Use snake_case for variable names.
Open

    dropDate = AssignmentDueDate.where(parent_id: assignment.id, deadline_type_id: '6').first
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

This cop makes sure that all variables use the configured style, snake_case or camelCase, for their names.

Example: EnforcedStyle: snake_case (default)

# bad
fooBar = 1

# good
foo_bar = 1

Example: EnforcedStyle: camelCase

# bad
foo_bar = 1

# good
fooBar = 1

Line is too long. [241/160]
Open

    SignUpTopic.find_by_sql(["SELECT topic_id as topic_id, COUNT(t.max_choosers) as count FROM sign_up_topics t JOIN signed_up_teams u ON t.id = u.topic_id WHERE t.assignment_id = ? and u.is_waitlisted = false GROUP BY t.id", assignment_id])
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

Line is too long. [218/160]
Open

    # SignedUpTeam.find_by_sql("SELECT u.id FROM sign_up_topics t, signed_up_teams u WHERE t.id = u.topic_id and u.is_waitlisted = true and t.assignment_id = " + assignment_id.to_s + " and u.team_id = " + team_id.to_s)
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

Line is too long. [187/160]
Open

  #     raise ArgumentError, "The CSV File expects the format: Topic identifier, Topic name, Max choosers, Topic Category (optional), Topic Description (Optional), Topic Link (optional)."
Severity: Minor
Found in app/models/sign_up_topic.rb by rubocop

There are no issues that match your filters.

Category
Status