lujanfernaud/prevy

View on GitHub
app/controllers/groups/topics_controller.rb

Summary

Maintainability
A
2 hrs
Test Coverage

Class has too many lines. [125/100]
Open

class Groups::TopicsController < ApplicationController
  include Groups::AuthorizationRedirecter

  before_action :find_group
  after_action  :verify_authorized, except: [:index, :show]

This cop checks if the length a class exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.

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

class Groups::TopicsController < ApplicationController
  include Groups::AuthorizationRedirecter

  before_action :find_group
  after_action  :verify_authorized, except: [:index, :show]
Severity: Minor
Found in app/controllers/groups/topics_controller.rb - About 2 hrs to fix

    Groups::TopicsController#update has approx 6 statements
    Open

      def update

    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.)

    Groups::TopicsController#create has approx 6 statements
    Open

      def create

    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.)

    Groups::TopicsController has at least 22 methods
    Open

    class Groups::TopicsController < ApplicationController

    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)

    Groups::TopicsController has at least 5 instance variables
    Open

    class Groups::TopicsController < ApplicationController

    Too Many Instance Variables is a special case of LargeClass.

    Example

    Given this configuration

    TooManyInstanceVariables:
      max_instance_variables: 3

    and this code:

    class TooManyInstanceVariables
      def initialize
        @arg_1 = :dummy
        @arg_2 = :dummy
        @arg_3 = :dummy
        @arg_4 = :dummy
      end
    end

    Reek would emit the following warning:

    test.rb -- 5 warnings:
      [1]:TooManyInstanceVariables has at least 4 instance variables (TooManyInstanceVariables)

    Groups::TopicsController assumes too much for instance variable '@group'
    Open

    class Groups::TopicsController < ApplicationController

    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.

    Groups::TopicsController has no descriptive comment
    Open

    class Groups::TopicsController < ApplicationController

    Classes and modules are the units of reuse and release. It is therefore considered good practice to annotate every class and module with a brief comment outlining its responsibilities.

    Example

    Given

    class Dummy
      # Do things...
    end

    Reek would emit the following warning:

    test.rb -- 1 warning:
      [1]:Dummy has no descriptive comment (IrresponsibleModule)

    Fixing this is simple - just an explaining comment:

    # The Dummy class is responsible for ...
    class Dummy
      # Do things...
    end

    Groups::TopicsController assumes too much for instance variable '@topic'
    Open

    class Groups::TopicsController < ApplicationController

    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.

    Inconsistent indentation detected.
    Open

        def topic_params_with_user
          topic_params.merge(user: current_user)
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Inconsistent indentation detected.
    Open

        def set_to_pinned_topic?
          return unless @topic.saved_changes.include?("type")
    
          @topic.pinned?
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Inconsistent indentation detected.
    Open

        def find_comments
          @topic.comments.includes(:edited_by)
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Inconsistent indentation detected.
    Open

        def topic_params
          params.require(:topic)
                .permit(:title, :body, :type)
                .merge(edited_by: current_user)
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Use the return of the conditional for variable assignment and comparison.
    Open

          if @topic.announcement?
            flash[:success] = "New announcement topic created."
          elsif @topic.pinned?
            flash[:success] = "New pinned topic created."
          else

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

        flash[:success] = "Topic deleted."

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

          return unless @topic.saved_changes.include?("type")

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Use %i or %I for an array of symbols.
    Open

      after_action  :verify_authorized, except: [:index, :show]

    This cop can check for array literals made up of symbols that are not using the %i() syntax.

    Alternatively, it checks for symbol arrays using the %i() syntax on projects which do not want to use that syntax.

    Configuration option: MinSize If set, arrays with fewer elements than this value will not trigger the cop. For example, a MinSize of3` will not enforce a style on an array of 2 or fewer elements.

    Example: EnforcedStyle: percent (default)

    # good
    %i[foo bar baz]
    
    # bad
    [:foo, :bar, :baz]

    Example: EnforcedStyle: brackets

    # good
    [:foo, :bar, :baz]
    
    # bad
    %i[foo bar baz]

    Inconsistent indentation detected.
    Open

        def flash_save
          if @topic.announcement?
            flash[:success] = "New announcement topic created."
          elsif @topic.pinned?
            flash[:success] = "New pinned topic created."

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Inconsistent indentation detected.
    Open

        def flash_update
          if set_to_normal_topic?
            flash[:success] = "Topic set to normal topic."
          elsif set_to_pinned_topic?
            flash[:success] = "Topic set to pinned topic."

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Use nested module/class definitions instead of compact style.
    Open

    class Groups::TopicsController < ApplicationController

    This cop checks the style of children definitions at classes and modules. Basically there are two different styles:

    Example: EnforcedStyle: nested (default)

    # good
    # have each child on its own line
    class Foo
      class Bar
      end
    end

    Example: EnforcedStyle: compact

    # good
    # combine definitions as much as possible
    class Foo::Bar
    end

    The compact style is only forced for classes/modules with one child.

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

          add_breadcrumb "New topic"

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Inconsistent indentation detected.
    Open

        def add_breadcrumbs_for_index
          add_breadcrumb @group.name, group_path(@group)
          add_breadcrumb "Topics"
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Inconsistent indentation detected.
    Open

        def find_group
          @group = GroupDecorator.new(Group.find(params[:group_id]))
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

            flash[:success] = "New pinned topic created."

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

          add_breadcrumb "Topics", group_topics_path(@group)

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Missing top-level class documentation comment.
    Open

    class Groups::TopicsController < ApplicationController

    This cop checks for missing top-level documentation of classes and modules. Classes with no body are exempt from the check and so are namespace modules - modules that have nothing in their bodies except classes, other modules, or constant definitions.

    The documentation requirement is annulled if the class or module has a "#:nodoc:" comment next to it. Likewise, "#:nodoc: all" does the same for all its children.

    Example:

    # bad
    class Person
      # ...
    end
    
    # good
    # Description/Explanation of Person class
    class Person
      # ...
    end

    Inconsistent indentation detected.
    Open

        def find_all_topics
          @group.topics_prioritized.paginate(
            page:     params[:page],
            per_page: Topic::TOPICS_PER_PAGE
          )

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Inconsistent indentation detected.
    Open

        def add_breadcrumbs_for_topic_edition
          add_breadcrumb @group.name, group_path(@group)
          add_breadcrumb "Topics", group_topics_path(@group)
          add_breadcrumb @topic.title, group_topic_path(@group, @topic)
          add_breadcrumb "Edit topic"

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

            flash[:success] = "Topic set to pinned topic."

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

          add_breadcrumb "Topics", group_topics_path(@group)

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

            flash[:success] = "New topic created."

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

          add_breadcrumb "Topics"

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

            flash[:success] = "Topic updated."

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

          add_breadcrumb "Topics", group_topics_path(@group)

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Inconsistent indentation detected.
    Open

        def add_breadcrumbs_for_show
          add_breadcrumb @group.name, group_path(@group)
          add_breadcrumb "Topics", group_topics_path(@group)
          add_breadcrumb @topic.title
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Inconsistent indentation detected.
    Open

        def find_topic
          Topic.find(params[:id])
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Inconsistent indentation detected.
    Open

        def create_topic
          @group.topics.new(topic_params_with_user)
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Inconsistent indentation detected.
    Open

        def add_breadcrumbs_for_topic_creation
          add_breadcrumb @group.name, group_path(@group)
          add_breadcrumb "Topics", group_topics_path(@group)
          add_breadcrumb "New topic"
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Use the return of the conditional for variable assignment and comparison.
    Open

          if set_to_normal_topic?
            flash[:success] = "Topic set to normal topic."
          elsif set_to_pinned_topic?
            flash[:success] = "Topic set to pinned topic."
          else

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

          add_breadcrumb "Edit topic"

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

          return unless @topic.saved_changes.include?("type")

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

            flash[:success] = "New announcement topic created."

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    Inconsistent indentation detected.
    Open

        def set_to_normal_topic?
          return unless @topic.saved_changes.include?("type")
    
          @topic.normal?
        end

    This cops checks for inconsistent indentation.

    Example:

    class A
      def test
        puts 'hello'
         puts 'world'
      end
    end

    Prefer single-quoted strings when you don't need string interpolation or special symbols.
    Open

            flash[:success] = "Topic set to normal topic."

    Checks if uses of quotes match the configured preference.

    Example: EnforcedStyle: single_quotes (default)

    # bad
    "No special symbols"
    "No string interpolation"
    "Just text"
    
    # good
    'No special symbols'
    'No string interpolation'
    'Just text'
    "Wait! What's #{this}!"

    Example: EnforcedStyle: double_quotes

    # bad
    'Just some text'
    'No special chars or interpolation'
    
    # good
    "Just some text"
    "No special chars or interpolation"
    "Every string in #{project} uses double_quotes"

    There are no issues that match your filters.

    Category
    Status