codeforamerica/ohana-api

View on GitHub

Showing 170 of 170 total issues

SubdomainConstraints#matches? calls 'request.subdomain' 3 times
Open

      request.subdomain == @subdomain
    else
      request.subdomain.blank? || request.subdomain == 'www'
Severity: Minor
Found in lib/subdomain_constraints.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.

ServicePresenter#to_service calls 'row.except(:taxonomy_ids)' 2 times
Open

    service.attributes = row.except(:taxonomy_ids)
    assign_categories_to(service, row[:taxonomy_ids])
    assign_parents_for(service, row.except(:taxonomy_ids))
Severity: Minor
Found in lib/service_presenter.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.

Similar blocks of code found in 2 locations. Consider refactoring.
Open

    def update
      @contact = Contact.find(params[:id])
      @location = Location.find(params[:location_id])

      authorize @location
Severity: Minor
Found in app/controllers/admin/contacts_controller.rb and 1 other location - About 30 mins to fix
app/controllers/admin/organization_contacts_controller.rb on lines 13..24

Duplicated Code

Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).

Tuning

This issue has a mass of 33.

We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.

The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.

If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.

See codeclimate-duplication's documentation for more information about tuning the mass threshold in your .codeclimate.yml.

Refactorings

Further Reading

Similar blocks of code found in 2 locations. Consider refactoring.
Open

    def update
      @contact = Contact.find(params[:id])
      @organization = Organization.find(params[:organization_id])

      authorize @contact
Severity: Minor
Found in app/controllers/admin/organization_contacts_controller.rb and 1 other location - About 30 mins to fix
app/controllers/admin/contacts_controller.rb on lines 13..24

Duplicated Code

Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).

Tuning

This issue has a mass of 33.

We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.

The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.

If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.

See codeclimate-duplication's documentation for more information about tuning the mass threshold in your .codeclimate.yml.

Refactorings

Further Reading

Possible SQL injection
Open

      order_query = Arel.sql("#{rank_for(query)} DESC, locations.updated_at DESC")
Severity: Minor
Found in app/models/concerns/search.rb by brakeman

Injection is #1 on the 2013 OWASP Top Ten web security risks. SQL injection is when a user is able to manipulate a value which is used unsafely inside a SQL query. This can lead to data leaks, data loss, elevation of privilege, and other unpleasant outcomes.

Brakeman focuses on ActiveRecord methods dealing with building SQL statements.

A basic (Rails 2.x) example looks like this:

User.first(:conditions => "username = '#{params[:username]}'")

Brakeman would produce a warning like this:

Possible SQL injection near line 30: User.first(:conditions => ("username = '#{params[:username]}'"))

The safe way to do this query is to use a parameterized query:

User.first(:conditions => ["username = ?", params[:username]])

Brakeman also understands the new Rails 3.x way of doing things (and local variables and concatenation):

username = params[:user][:name].downcase
password = params[:user][:password]

User.first.where("username = '" + username + "' AND password = '" + password + "'")

This results in this kind of warning:

Possible SQL injection near line 37:
User.first.where((((("username = '" + params[:user][:name].downcase) + "' AND password = '") + params[:user][:password]) + "'"))

See the Ruby Security Guide for more information and Rails-SQLi.org for many examples of SQL injection in Rails.

Similar blocks of code found in 2 locations. Consider refactoring.
Open

    def create
      @location = Location.find(params[:location_id])
      @contact = @location.contacts.new(contact_params)

      authorize @location
Severity: Minor
Found in app/controllers/admin/contacts_controller.rb and 1 other location - About 30 mins to fix
app/controllers/admin/organization_contacts_controller.rb on lines 35..46

Duplicated Code

Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).

Tuning

This issue has a mass of 32.

We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.

The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.

If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.

See codeclimate-duplication's documentation for more information about tuning the mass threshold in your .codeclimate.yml.

Refactorings

Further Reading

Similar blocks of code found in 2 locations. Consider refactoring.
Open

    def create
      @organization = Organization.find(params[:organization_id])
      @contact = @organization.contacts.new(contact_params)

      authorize @contact
Severity: Minor
Found in app/controllers/admin/organization_contacts_controller.rb and 1 other location - About 30 mins to fix
app/controllers/admin/contacts_controller.rb on lines 35..46

Duplicated Code

Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).

Tuning

This issue has a mass of 32.

We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.

The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.

If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.

See codeclimate-duplication's documentation for more information about tuning the mass threshold in your .codeclimate.yml.

Refactorings

Further Reading

Api::V1::ErrorsController has missing safe method 'raise_not_found!'
Open

      def raise_not_found!

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.

ParentAssigner#assign_parents_for performs a nil-check
Open

      next if row[key].nil?
Severity: Minor
Found in lib/parent_assigner.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)

Search::ClassMethods#rank_for doesn't depend on instance state (maybe move it to another class?)
Open

    def rank_for(query)
Severity: Minor
Found in app/models/concerns/search.rb by reek

A Utility Function is any instance method that has no dependency on the state of the instance.

ArrayValidator::Helper#validate performs a nil-check
Open

        next if value.nil? && validator.options[:allow_nil]
Severity: Minor
Found in app/validators/array_validator.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)

WeekdayValidator#wday_name_to_int doesn't depend on instance state (maybe move it to another class?)
Open

  def wday_name_to_int(value)
Severity: Minor
Found in app/validators/weekday_validator.rb by reek

A Utility Function is any instance method that has no dependency on the state of the instance.

CustomMailer#template_path doesn't depend on instance state (maybe move it to another class?)
Open

  def template_path(resource)
Severity: Minor
Found in app/mailers/custom_mailer.rb by reek

A Utility Function is any instance method that has no dependency on the state of the instance.

ParentAssigner#foreign_keys_in doesn't depend on instance state (maybe move it to another class?)
Open

  def foreign_keys_in(row)
Severity: Minor
Found in lib/parent_assigner.rb by reek

A Utility Function is any instance method that has no dependency on the state of the instance.

PaginationHeaders#pages doesn't depend on instance state (maybe move it to another class?)
Open

  def pages(coll)

A Utility Function is any instance method that has no dependency on the state of the instance.

Api::V1::SearchController#cache_key doesn't depend on instance state (maybe move it to another class?)
Open

      def cache_key(scope)

A Utility Function is any instance method that has no dependency on the state of the instance.

Taggable#shift_and_split_params doesn't depend on instance state (maybe move it to another class?)
Open

  def shift_and_split_params(params, *fields)
Severity: Minor
Found in app/controllers/concerns/taggable.rb by reek

A Utility Function is any instance method that has no dependency on the state of the instance.

Search::ClassMethods#allowed_params doesn't depend on instance state (maybe move it to another class?)
Open

    def allowed_params(params)
Severity: Minor
Found in app/models/concerns/search.rb by reek

A Utility Function is any instance method that has no dependency on the state of the instance.

WeekdayValidator#valid_string_num? doesn't depend on instance state (maybe move it to another class?)
Open

  def valid_string_num?(value)
Severity: Minor
Found in app/validators/weekday_validator.rb by reek

A Utility Function is any instance method that has no dependency on the state of the instance.

Api::V1::StatusController#search_okay? doesn't depend on instance state (maybe move it to another class?)
Open

      def search_okay?

A Utility Function is any instance method that has no dependency on the state of the instance.

Severity
Category
Status
Source
Language