ece517-p3/expertiza

View on GitHub
app/controllers/suggestion_controller.rb

Summary

Maintainability
A
1 hr
Test Coverage

Unprotected mass assignment
Open

    @suggestion = Suggestion.new(suggestion_params)

Mass assignment is a feature of Rails which allows an application to create a record from the values of a hash.

Example:

User.new(params[:user])

Unfortunately, if there is a user field called admin which controls administrator access, now any user can make themselves an administrator.

attr_accessible and attr_protected can be used to limit mass assignment. However, Brakeman will warn unless attr_accessible is used, or mass assignment is completely disabled.

There are two different mass assignment warnings which can arise. The first is when mass assignment actually occurs, such as the example above. This results in a warning like

Unprotected mass assignment near line 61: User.new(params[:user])

The other warning is raised whenever a model is found which does not use attr_accessible. This produces generic warnings like

Mass assignment is not restricted using attr_accessible

with a list of affected models.

In Rails 3.1 and newer, mass assignment can easily be disabled:

config.active_record.whitelist_attributes = true

Unfortunately, it can also easily be bypassed:

User.new(params[:user], :without_protection => true)

Brakeman will warn on uses of without_protection.

Assignment Branch Condition size for approve is too high. [25.67/15]
Open

  def approve
    @suggestion = Suggestion.find(params[:id])
    @user_id = User.find_by(name: @suggestion.unityID).try(:id)
    if @user_id
      @team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)

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 create is too high. [23.43/15]
Open

  def create
    @suggestion = Suggestion.new(suggestion_params)
    @suggestion.assignment_id = session[:assignment_id]
    @assignment = Assignment.find(session[:assignment_id])
    @suggestion.status = 'Initiated'

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_comment is too high. [23.62/15]
Open

  def add_comment
    @suggestioncomment = SuggestionComment.new(vote: params[:suggestion_comment][:vote], comments: params[:suggestion_comment][:comments])
    @suggestioncomment.suggestion_id = params[:id]
    @suggestioncomment.commenter = session[:user].name
    if @suggestioncomment.save

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 update_suggestion is too high. [18/15]
Open

  def update_suggestion
    Suggestion.find(params[:id]).update_attributes(title: params[:suggestion][:title],
                                                   description: params[:suggestion][:description],
                                                   signup_preference: params[:suggestion][:signup_preference])
    redirect_to action: 'new', id: Suggestion.find(params[:id]).assignment_id

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 send_email is too high. [16.4/15]
Open

  def send_email
    proposer = User.find_by(id: @user_id)
    if proposer
      teams_users = TeamsUser.where(team_id: @team_id)
      cc_mail_list = []

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 create_new_team is too high. [16.28/15]
Open

  def create_new_team
    new_team = AssignmentTeam.create(name: 'Team_' + rand(10_000).to_s,
                                     parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
    t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
    SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)

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 submit is too high. [15.3/15]
Open

  def submit
    if !params[:add_comment].nil?
      add_comment
    elsif !params[:approve_suggestion].nil?
      approve_suggestion

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 create has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.
Open

  def create
    @suggestion = Suggestion.new(suggestion_params)
    @suggestion.assignment_id = session[:assignment_id]
    @assignment = Assignment.find(session[:assignment_id])
    @suggestion.status = 'Initiated'
Severity: Minor
Found in app/controllers/suggestion_controller.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

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

  def notification
    #--zhewei-----06/22/2015--------------------------------------------------------------------------------------
    # If you want to create a new team with topic and team members on view, you have to
    # 1. create new Team
    # 2. create new TeamsUser
Severity: Minor
Found in app/controllers/suggestion_controller.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

Avoid using update_attribute because it skips validations.
Open

    if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')

This cop checks for the use of methods which skip validations which are listed in http://guides.rubyonrails.org/active_record_validations.html#skipping-validations

Example:

# bad
Article.first.decrement!(:view_count)
DiscussionBoard.decrement_counter(:post_count, 5)
Article.first.increment!(:view_count)
DiscussionBoard.increment_counter(:post_count, 5)
person.toggle :active
product.touch
Billing.update_all("category = 'authorized', author = 'David'")
user.update_attribute(website: 'example.com')
user.update_columns(last_request_at: Time.current)
Post.update_counters 5, comment_count: -1, action_count: 1

# good
user.update_attributes(website: 'example.com')
FileUtils.touch('file')

Avoid using update_attribute because it skips validations.
Open

    if @suggestion.update_attribute('status', 'Rejected')

This cop checks for the use of methods which skip validations which are listed in http://guides.rubyonrails.org/active_record_validations.html#skipping-validations

Example:

# bad
Article.first.decrement!(:view_count)
DiscussionBoard.decrement_counter(:post_count, 5)
Article.first.increment!(:view_count)
DiscussionBoard.increment_counter(:post_count, 5)
person.toggle :active
product.touch
Billing.update_all("category = 'authorized', author = 'David'")
user.update_attribute(website: 'example.com')
user.update_columns(last_request_at: Time.current)
Post.update_counters 5, comment_count: -1, action_count: 1

# good
user.update_attributes(website: 'example.com')
FileUtils.touch('file')

Convert if nested inside else to elsif.
Open

        if @topic_id.nil?

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

end at 80, 4 is not aligned with if at 76, 26.
Open

    end

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)

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

    if proposer

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

There are no issues that match your filters.

Category
Status