3scale/porta

View on GitHub
app/models/cms/section.rb

Summary

Maintainability
A
2 hrs
Test Coverage

Mass assignment is not restricted using attr_accessible
Open

class CMS::Section < ApplicationRecord
Severity: Critical
Found in app/models/cms/section.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.

Class Section has 21 methods (exceeds 20 allowed). Consider refactoring.
Open

class CMS::Section < ApplicationRecord
  include ThreeScale::Search::Scopes
  include CMS::Filtering
  extend System::Database::Scopes::IdOrSystemName
  include NormalizePathAttribute
Severity: Minor
Found in app/models/cms/section.rb - About 2 hrs to fix

    CMS::Section has at least 17 methods
    Open

    class CMS::Section < ApplicationRecord
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    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)

    CMS::Section#add_remove_by_ids has approx 12 statements
    Open

      def add_remove_by_ids( model_type, inside_ids)
    Severity: Minor
    Found in app/models/cms/section.rb by reek

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

    CMS::Section#destroy has approx 14 statements
    Open

      def destroy
    Severity: Minor
    Found in app/models/cms/section.rb by reek

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

    CMS::Section tests 'root?' at least 3 times
    Wontfix

        unless root?
          CMS::Section.transaction do
            builtins.each{|p| p.section = parent; p.save!}
            pages.each   {|p| p.section = parent; p.save!}
            files.each   {|f| f.section = parent; f.save!}
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    Repeated Conditional is a special case of Simulated Polymorphism. Basically it means you are checking the same value throughout a single class and take decisions based on this.

    Example

    Given

    class RepeatedConditionals
      attr_accessor :switch
    
      def repeat_1
        puts "Repeat 1!" if switch
      end
    
      def repeat_2
        puts "Repeat 2!" if switch
      end
    
      def repeat_3
        puts "Repeat 3!" if switch
      end
    end

    Reek would emit the following warning:

    test.rb -- 4 warnings:
      [5, 9, 13]:RepeatedConditionals tests switch at least 3 times (RepeatedConditional)

    If you get this warning then you are probably not using the right abstraction or even more probable, missing an additional abstraction.

    CMS::Section#destroy calls 'p.section = parent' 2 times
    Open

            builtins.each{|p| p.section = parent; p.save!}
            pages.each   {|p| p.section = parent; p.save!}
    Severity: Minor
    Found in app/models/cms/section.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.

    CMS::Section#dirty? calls 'p.dirty?' 2 times
    Open

          self.pages.any? { |p| p.dirty? } ||
          self.builtins.any? { |p| p.dirty? }
    Severity: Minor
    Found in app/models/cms/section.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.

    CMS::Section#destroy calls 'p.save!' 2 times
    Open

            builtins.each{|p| p.section = parent; p.save!}
            pages.each   {|p| p.section = parent; p.save!}
    Severity: Minor
    Found in app/models/cms/section.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.

    CMS::Section#accessible_by? calls 'parent.accessible_by?(buyer)' 2 times
    Open

          parent.accessible_by?(buyer)
        else # protected. buyer has to be an Account
          buyer && buyer.accessible_sections.include?(self) && parent.accessible_by?(buyer)
    Severity: Minor
    Found in app/models/cms/section.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.

    CMS::Section#add_remove_by_ids calls 'all.find(file_id)' 2 times
    Open

          if all.exists?(file_id) &&  all.find(file_id).valid?
            inside << all.find(file_id)
    Severity: Minor
    Found in app/models/cms/section.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.

    CMS::Section#add_remove_by_ids calls 'model_type.to_s' 2 times
    Open

        all = provider.send(model_type.to_s.pluralize)
        inside = self.send(model_type.to_s.pluralize)
    Severity: Minor
    Found in app/models/cms/section.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.

    CMS::Section#add_remove_by_ids calls 'model_type.to_s.pluralize' 2 times
    Open

        all = provider.send(model_type.to_s.pluralize)
        inside = self.send(model_type.to_s.pluralize)
    Severity: Minor
    Found in app/models/cms/section.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.

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

      def add_remove_by_ids( model_type, inside_ids)
        all = provider.send(model_type.to_s.pluralize)
        inside = self.send(model_type.to_s.pluralize)
    
        inside_ids = (inside_ids || []).uniq
    Severity: Minor
    Found in app/models/cms/section.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

    CMS::Section#set_provider performs a nil-check
    Open

        self.provider = parent.provider if self.parent && self.provider.nil?
    Severity: Minor
    Found in app/models/cms/section.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)

    CMS::Section#dirty? has the variable name 'c'
    Open

        self.children.any? { |c| c.dirty? } ||
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

    Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

    CMS::Section#destroy has the variable name 'p'
    Open

            builtins.each{|p| p.section = parent; p.save!}
            pages.each   {|p| p.section = parent; p.save!}
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

    Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

    CMS::Section#destroy has the variable name 'f'
    Open

            files.each   {|f| f.section = parent; f.save!}
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

    Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

    CMS::Section#dirty? has the variable name 'f'
    Open

          self.files.any? { |f| f.dirty? } ||
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

    Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

    CMS::Section#destroy has the variable name 's'
    Open

            children.each{|s| s.parent = parent; s.save!}
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

    Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

    CMS::Section#add_remove_by_ids has the variable name 'k'
    Open

        to_add = inside_ids - to_keep.map{ |k| k.id.to_s}
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

    Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

    CMS::Section#add_remove_by_ids has the variable name 'p'
    Open

        to_keep, to_delete = inside.partition { |p| inside_ids.include?(p.id.to_s) }
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

    Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

    CMS::Section#dirty? has the variable name 'p'
    Open

          self.pages.any? { |p| p.dirty? } ||
          self.builtins.any? { |p| p.dirty? }
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

    Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

    CMS::Section#add_remove_by_ids has the variable name 'a'
    Open

        to_delete.each{|a| a.section = provider.sections.root; a.save} unless self.root?
    Severity: Minor
    Found in app/models/cms/section.rb by reek

    An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

    Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

    There are no issues that match your filters.

    Category
    Status