Merge pull request #3907 from opf/improve-code

Improve according to rubocop

.codeclimate.yml

@@ -27,3 +27,4 @@ exclude_paths:
   - .bundle/**/*
   - lib/plugins/rfpdf/**/*
   - spec_legacy/**/*
+  - lib/redcloth3.rb

app/controllers/repositories_controller.rb

@@ -98,7 +98,11 @@ class RepositoriesController < ApplicationController
     @users.sort!
     if request.post? && params[:committers].is_a?(Hash)
       # Build a hash with repository usernames as keys and corresponding user ids as values
-      @repository.committer_ids = params[:committers].values.inject({}) { |h, c| h[c.first] = c.last; h }
+      @repository.committer_ids = params[:committers].values
+        .inject({}) { |h, c|
+          h[c.first] = c.last
+          h
+        }
       flash[:notice] = l(:notice_successful_update)
       redirect_to action: 'committers', project_id: @project
     end
@@ -146,10 +150,17 @@ class RepositoriesController < ApplicationController
 
   def changes
     @entry = @repository.entry(@path, @rev)
-    (show_error_not_found; return) unless @entry
-    @changesets = @repository.latest_changesets(@path, @rev, Setting.repository_log_display_limit.to_i)
+
+    unless @entry
+      show_error_not_found
+      return
+    end
+
+    @changesets = @repository.latest_changesets(@path,
+                                                @rev,
+                                                Setting.repository_log_display_limit.to_i)
     @properties = @repository.properties(@path, @rev)
-    @changeset = @repository.find_changeset_by_name(@rev)
+    @changeset  = @repository.find_changeset_by_name(@rev)
   end
 
   def revisions
@@ -159,14 +170,21 @@ class RepositoriesController < ApplicationController
                   .per_page(per_page_param)
 
     respond_to do |format|
-      format.html do render layout: false if request.xhr? end
-      format.atom do render_feed(@changesets, title: "#{@project.name}: #{l(:label_revision_plural)}") end
+      format.html do
+        render layout: false if request.xhr?
+      end
+      format.atom do
+        render_feed(@changesets, title: "#{@project.name}: #{l(:label_revision_plural)}")
+      end
     end
   end
 
   def entry
     @entry = @repository.entry(@path, @rev)
-    (show_error_not_found; return) unless @entry
+    unless @entry
+      show_error_not_found
+      return
+    end
 
     # If the entry is a dir, show the browser
     if @entry.dir?
@@ -175,7 +193,12 @@ class RepositoriesController < ApplicationController
     end
 
     @content = @repository.cat(@path, @rev)
-    (show_error_not_found; return) unless @content
+
+    unless @content
+      show_error_not_found
+      return
+    end
+
     if 'raw' == params[:format] ||
        (@content.size && @content.size > Setting.file_max_size_displayed.to_i.kilobyte) ||
        !is_entry_text_data?(@content, @path)
@@ -203,7 +226,8 @@ class RepositoriesController < ApplicationController
     # http://apidock.com/ruby/v1_8_6_287/String/is_binary_data%3F
     if ent.respond_to?('is_binary_data?') && ent.is_binary_data? # Ruby 1.8.x and <1.9.2
       return false
-    elsif ent.respond_to?(:force_encoding) && (ent.dup.force_encoding('UTF-8') != ent.dup.force_encoding('BINARY')) # Ruby 1.9.2
+    elsif ent.respond_to?(:force_encoding) &&
+          (ent.dup.force_encoding('UTF-8') != ent.dup.force_encoding('BINARY')) # Ruby 1.9.2
       # TODO: need to handle edge cases of non-binary content that isn't UTF-8
       return false
     end
@@ -214,9 +238,13 @@ class RepositoriesController < ApplicationController
 
   def annotate
     @entry = @repository.entry(@path, @rev)
-    (show_error_not_found; return) unless @entry
 
-    @annotate = @repository.scm.annotate(@path, @rev)
+    unless @entry
+      show_error_not_found
+      return
+    end
+
+    @annotate  = @repository.scm.annotate(@path, @rev)
     @changeset = @repository.find_changeset_by_name(@rev)
   end
 
@@ -236,7 +264,12 @@ class RepositoriesController < ApplicationController
   def diff
     if params[:format] == 'diff'
       @diff = @repository.diff(@path, @rev, @rev_to)
-      (show_error_not_found; return) unless @diff
+
+      unless @diff
+        show_error_not_found
+        return
+      end
+
       filename = "changeset_r#{@rev}"
       filename << "_r#{@rev_to}" if @rev_to
       send_data @diff.join,
@@ -253,7 +286,9 @@ class RepositoriesController < ApplicationController
         User.current.preference.save
       end
 
-      @cache_key = "repositories/diff/#{@repository.id}/" + Digest::MD5.hexdigest("#{@path}-#{@rev}-#{@rev_to}-#{@diff_type}")
+      @cache_key = "repositories/diff/#{@repository.id}/" +
+                   Digest::MD5.hexdigest("#{@path}-#{@rev}-#{@rev_to}-#{@diff_type}")
+
       unless read_fragment(@cache_key)
         @diff = @repository.diff(@path, @rev, @rev_to)
         show_error_not_found unless @diff
@@ -306,7 +341,11 @@ class RepositoriesController < ApplicationController
 
   def find_repository
     @repository = @project.repository
-    (render_404; return false) unless @repository
+
+    unless @repository
+      render_404
+      return false
+    end
 
     # Prepare checkout instructions
     # available on all pages (even empty!)
@@ -344,20 +383,30 @@ class RepositoriesController < ApplicationController
     @date_to = Date.today
     @date_from = @date_to << 11
     @date_from = Date.civil(@date_from.year, @date_from.month, 1)
-    commits_by_day = Changeset.where(['repository_id = ? AND commit_date BETWEEN ? AND ?', repository.id, @date_from, @date_to]).group(:commit_date).size
+    commits_by_day = Changeset.where(
+      ['repository_id = ? AND commit_date BETWEEN ? AND ?', repository.id, @date_from, @date_to]
+    ).group(:commit_date).size
     commits_by_month = [0] * 12
-    commits_by_day.each do |c| commits_by_month[(@date_to.month - c.first.to_date.month) % 12] += c.last end
+    commits_by_day.each do |c|
+      commits_by_month[(@date_to.month - c.first.to_date.month) % 12] += c.last
+    end
 
     changes_by_day = Change.includes(:changeset)
-                     .where(["#{Changeset.table_name}.repository_id = ? AND #{Changeset.table_name}.commit_date BETWEEN ? AND ?", repository.id, @date_from, @date_to])
+                     .where(["#{Changeset.table_name}.repository_id = ? "\
+                             "AND #{Changeset.table_name}.commit_date BETWEEN ? AND ?",
+                             repository.id, @date_from, @date_to])
                      .references(:changesets)
                      .group(:commit_date)
                      .size
     changes_by_month = [0] * 12
-    changes_by_day.each do |c| changes_by_month[(@date_to.month - c.first.to_date.month) % 12] += c.last end
+    changes_by_day.each do |c|
+      changes_by_month[(@date_to.month - c.first.to_date.month) % 12] += c.last
+    end
 
     fields = []
-    12.times do |m| fields << month_name(((Date.today.month - 1 - m) % 12) + 1) end
+    12.times do |m|
+      fields << month_name(((Date.today.month - 1 - m) % 12) + 1)
+    end
 
     graph = SVG::Graph::Bar.new(
       height: 300,
@@ -386,14 +435,19 @@ class RepositoriesController < ApplicationController
 
   def graph_commits_per_author(repository)
     commits_by_author = Changeset.where(['repository_id = ?', repository.id]).group(:committer).size
-    commits_by_author.to_a.sort! do |x, y| x.last <=> y.last end
+    commits_by_author.to_a.sort! do |x, y|
+      x.last <=> y.last
+    end
 
     changes_by_author = Change.includes(:changeset)
                         .where(["#{Changeset.table_name}.repository_id = ?", repository.id])
                         .references(:changesets)
                         .group(:committer)
                         .size
-    h = changes_by_author.inject({}) { |o, i| o[i.first] = i.last; o }
+    h = changes_by_author.inject({}) { |o, i|
+      o[i.first] = i.last
+      o
+    }
 
     fields = commits_by_author.map(&:first)
     commits_data = commits_by_author.map(&:last)

app/controllers/users_controller.rb

@@ -40,7 +40,8 @@ class UsersController < ApplicationController
                                    :destroy_membership,
                                    :destroy,
                                    :deletion_info]
-  before_filter :require_login, only: [:deletion_info] # should also contain destroy but post data can not be redirected
+  # should also contain destroy but post data can not be redirected
+  before_filter :require_login, only: [:deletion_info]
   before_filter :authorize_for_user, only: [:destroy]
   before_filter :check_if_deletion_allowed, only: [:deletion_info,
                                                    :destroy]
@@ -73,7 +74,8 @@ class UsersController < ApplicationController
 
     unless params[:name].blank?
       name = "%#{params[:name].strip.downcase}%"
-      c << ['LOWER(login) LIKE ? OR LOWER(firstname) LIKE ? OR LOWER(lastname) LIKE ? OR LOWER(mail) LIKE ?', name, name, name, name]
+      c << ['LOWER(login) LIKE ? OR LOWER(firstname) LIKE ? OR '\
+            'LOWER(lastname) LIKE ? OR LOWER(mail) LIKE ?', name, name, name, name]
     end
 
     @users = scope.order(sort_clause)
@@ -97,7 +99,9 @@ class UsersController < ApplicationController
     @events_by_day = events.group_by { |e| e.event_datetime.to_date }
 
     unless User.current.admin?
-      if !(@user.active? || @user.registered?) || (@user != User.current && @memberships.empty? && events.empty?)
+      if !(@user.active? ||
+         @user.registered?) ||
+         (@user != User.current && @memberships.empty? && events.empty?)
         render_404
         return
       end
@@ -109,13 +113,15 @@ class UsersController < ApplicationController
   end
 
   def new
-    @user = User.new(language: Setting.default_language, mail_notification: Setting.default_notification_option)
+    @user = User.new(language: Setting.default_language,
+                     mail_notification: Setting.default_notification_option)
     @auth_sources = AuthSource.all
   end
 
   verify method: :post, only: :create, render: { nothing: true, status: :method_not_allowed }
   def create
-    @user = User.new(language: Setting.default_language, mail_notification: Setting.default_notification_option)
+    @user = User.new(language: Setting.default_language,
+                     mail_notification: Setting.default_notification_option)
     @user.attributes = permitted_params.user_create_as_admin(false, @user.change_password_allowed?)
     @user.admin = params[:user][:admin] || false
     @user.login = params[:user][:login] || @user.mail
@@ -124,10 +130,7 @@ class UsersController < ApplicationController
       respond_to do |format|
         format.html do
           flash[:notice] = l(:notice_successful_create)
-          redirect_to(params[:continue] ?
-            new_user_path :
-            edit_user_path(@user)
-                     )
+          redirect_to(params[:continue] ? new_user_path : edit_user_path(@user))
         end
       end
     else
@@ -170,7 +173,6 @@ class UsersController < ApplicationController
                                 self_notified: params[:self_notified] == '1',
                                 notified_project_ids: params[:notified_project_ids])
 
-
       @user.pref.attributes = pref_params
       @user.pref.save
 
@@ -248,11 +250,16 @@ class UsersController < ApplicationController
     @membership.save if request.post?
     respond_to do |format|
       if @membership.valid?
-        format.html do redirect_to controller: '/users', action: 'edit', id: @user, tab: 'memberships' end
+        format.html do
+          redirect_to controller: '/users', action: 'edit', id: @user, tab: 'memberships'
+        end
+
         format.js do
           render(:update) {|page|
             page.replace_html 'tab-content-memberships', partial: 'users/memberships'
-            page.insert_html :top, 'tab-content-memberships', partial: 'members/common_notice', locals: { message: l(:notice_successful_update) }
+            page.insert_html :top, 'tab-content-memberships',
+                             partial: 'members/common_notice',
+                             locals: { message: l(:notice_successful_update) }
             page.visual_effect(:highlight, "member-#{@membership.id}")
           }
         end
@@ -260,7 +267,9 @@ class UsersController < ApplicationController
         format.js do
           render(:update) {|page|
             page.replace_html 'tab-content-memberships', partial: 'users/memberships'
-            page.insert_html :top, 'tab-content-memberships', partial: 'members/member_errors', locals: { member: @membership }
+            page.insert_html :top, 'tab-content-memberships',
+                             partial: 'members/member_errors',
+                             locals: { member: @membership }
           }
         end
       end
@@ -284,15 +293,22 @@ class UsersController < ApplicationController
 
   def destroy_membership
     @membership = Member.find(params.delete(:membership_id))
+
     if request.post? && @membership.deletable?
       @membership.destroy && @membership = nil
     end
+
     respond_to do |format|
-      format.html do redirect_to controller: '/users', action: 'edit', id: @user, tab: 'memberships' end
+      format.html do
+        redirect_to controller: '/users', action: 'edit', id: @user, tab: 'memberships'
+      end
+
       format.js do
         render(:update) { |page|
           page.replace_html 'tab-content-memberships', partial: 'users/memberships'
-          page.insert_html :top, 'tab-content-memberships', partial: 'members/common_notice', locals: { message: l(:notice_successful_delete) }
+          page.insert_html :top, 'tab-content-memberships',
+                           partial: 'members/common_notice',
+                           locals: { message: l(:notice_successful_delete) }
         }
       end
     end

app/helpers/application_helper.rb

@@ -109,7 +109,8 @@ module ApplicationHelper
     html_options.symbolize_keys!
     tag(:input, html_options.merge(
                   type: 'image', src: image_path(name),
-                  onclick: (html_options[:onclick] ? "#{html_options[:onclick]}; " : '') + "#{function};"
+                  onclick: (html_options[:onclick] ? "#{html_options[:onclick]}; " : '') +
+                            "#{function};"
     ))
   end
 
@@ -127,7 +128,9 @@ module ApplicationHelper
   end
 
   def format_activity_description(text)
-    html_escape_once(truncate(text.to_s, length: 120).gsub(%r{[\r\n]*<(pre|code)>.*$}m, '...')).gsub(/[\r\n]+/, '<br />').html_safe
+    html_escape_once(truncate(text.to_s, length: 120).gsub(%r{[\r\n]*<(pre|code)>.*$}m, '...'))
+      .gsub(/[\r\n]+/, '<br />')
+      .html_safe
   end
 
   def format_version_name(version)
@@ -136,7 +139,8 @@ module ApplicationHelper
 
   def due_date_distance_in_words(date)
     if date
-      l((date < Date.today ? :label_roadmap_overdue : :label_roadmap_due_in), distance_of_date_in_words(Date.today, date))
+      label = date < Date.today ? :label_roadmap_overdue : :label_roadmap_due_in
+      l(label, distance_of_date_in_words(Date.today, date))
     end
   end
 
@@ -146,8 +150,11 @@ module ApplicationHelper
     content_tag :ul, class: 'pages-hierarchy' do
       pages[node].map { |page|
         content_tag :li do
+          title = if options[:timestamp] && page.updated_on
+                    l(:label_updated_time, distance_of_time_in_words(Time.now, page.updated_on))
+                  end
           concat link_to(page.pretty_title, project_wiki_path(page.project, page),
-                         title: (options[:timestamp] && page.updated_on ? l(:label_updated_time, distance_of_time_in_words(Time.now, page.updated_on)) : nil))
+                         title: title)
           concat render_page_hierarchy(pages, page.id, options) if pages[page.id]
         end
       }.join.html_safe
@@ -160,8 +167,9 @@ module ApplicationHelper
     error_messages = objects.map { |o| o.errors.full_messages }.flatten
 
     unless error_messages.empty?
-      render partial: 'common/validation_error', locals: { error_messages: error_messages,
-                                                           object_name: options[:object_name].to_s.gsub('_', '') }
+      render partial: 'common/validation_error',
+             locals: { error_messages: error_messages,
+                       object_name: options[:object_name].to_s.gsub('_', '') }
     end
   end
 
@@ -204,11 +212,19 @@ module ApplicationHelper
 
     content_tag :div, html_options do
       if User.current.impaired?
-        concat(content_tag('a', join_flash_messages(message), href: 'javascript:;', class: 'impaired--empty-link'))
-        concat(content_tag(:i, '', class: 'icon-close close-handler', tabindex: '0', role: 'button', aria: { label: ::I18n.t('js.close_popup_title') }))
+        concat(content_tag('a', join_flash_messages(message),
+                           href: 'javascript:;',
+                           class: 'impaired--empty-link'))
+        concat(content_tag(:i, '', class: 'icon-close close-handler',
+                                   tabindex: '0',
+                                   role: 'button',
+                                   aria: { label: ::I18n.t('js.close_popup_title') }))
       else
         concat(join_flash_messages(message))
-        concat(content_tag(:i, '', class: 'icon-close close-handler', tabindex: '0', role: 'button', aria: { label: ::I18n.t('js.close_popup_title') }))
+        concat(content_tag(:i, '', class: 'icon-close close-handler',
+                                   tabindex: '0',
+                                   role: 'button',
+                                   aria: { label: ::I18n.t('js.close_popup_title') }))
       end
     end
   end
@@ -299,10 +315,7 @@ module ApplicationHelper
       id = name.gsub(/[\[\]]+/, '_') + object.id.to_s
 
       object_options = options.inject({}) { |h, (k, v)|
-        h[k] = v.is_a?(Symbol) ?
-                 send(v, object) :
-                 v
-
+        h[k] = v.is_a?(Symbol) ? send(v, object) : v
         h
       }
 
@@ -317,17 +330,24 @@ module ApplicationHelper
   end
 
   def html_hours(text)
-    text.gsub(%r{(\d+)\.(\d+)}, '<span class="hours hours-int">\1</span><span class="hours hours-dec">.\2</span>').html_safe
+    text.gsub(%r{(\d+)\.(\d+)},
+              '<span class="hours hours-int">\1</span><span class="hours hours-dec">.\2</span>')
+      .html_safe
   end
 
   def authoring(created, author, options = {})
-    l(options[:label] || :label_added_time_by, author: link_to_user(author), age: time_tag(created)).html_safe
+    label = options[:label] || :label_added_time_by
+    l(label, author: link_to_user(author), age: time_tag(created)).html_safe
   end
 
   def time_tag(time)
     text = distance_of_time_in_words(Time.now, time)
     if @project and @project.module_enabled?('activity')
-      link_to(text, { controller: '/activities', action: 'index', project_id: @project, from: time.to_date }, title: format_time(time))
+      link_to(text, { controller: '/activities',
+                      action: 'index',
+                      project_id: @project,
+                      from: time.to_date },
+              title: format_time(time))
     else
       datetime = time.acts_like?(:time) ? time.xmlschema : time.iso8601
       content_tag(:time, text, datetime: datetime,
@@ -399,11 +419,15 @@ module ApplicationHelper
       if ancestors.any?
         root = ancestors.shift
         b << link_to_project(root, { jump: current_menu_item }, class: 'root')
+
         if ancestors.size > 2
           b << '&#8230;'
           ancestors = ancestors[-2, 2]
         end
-        b += ancestors.map { |p| link_to_project(p, { jump: current_menu_item }, class: 'ancestor') }
+
+        b += ancestors.map { |p|
+          link_to_project(p, { jump: current_menu_item }, class: 'ancestor')
+        }
       end
       b << h(@project)
       b.join(' &#187; ')
@@ -455,13 +479,23 @@ module ApplicationHelper
              [['(auto)', '']]
            else
              []
-          end
-    auto + valid_languages.map { |lang| [ll(lang.to_s, :general_lang_name), lang.to_s] }.sort { |x, y| x.last <=> y.last }
+           end
+
+    mapped_languages = valid_languages.map { |lang|
+      [ll(lang.to_s, :general_lang_name), lang.to_s]
+    }
+
+    auto + mapped_languages.sort { |x, y| x.last <=> y.last }
   end
 
   def all_lang_options_for_select(blank = true)
-    (blank ? [['(auto)', '']] : []) +
-      all_languages.map { |lang| [ll(lang.to_s, :general_lang_name), lang.to_s] }.sort { |x, y| x.last <=> y.last }
+    initial_lang_options = blank ? [['(auto)', '']] : []
+
+    mapped_languages = all_languages.map { |lang|
+      [ll(lang.to_s, :general_lang_name), lang.to_s]
+    }
+
+    initial_lang_options + mapped_languages.sort { |x, y| x.last <=> y.last }
   end
 
   def labelled_tabular_form_for(record, options = {}, &block)
@@ -511,10 +545,11 @@ module ApplicationHelper
     legend = options[:legend] || ''
 
     content_tag :span do
-      content_tag :span, class: 'progress-bar', style: "width: #{width}" do
-        content_tag(:span, '', class: 'inner-progress closed', style: "width: #{closed}%") +
-          content_tag(:span, '', class: 'inner-progress done',   style: "width: #{done}%")
-      end.<<(content_tag :span, "#{legend}% #{l(:total_progress)}", class: 'progress-bar-legend')
+      concat content_tag :span, class: 'progress-bar', style: "width: #{width}" do
+        concat content_tag(:span, '', class: 'inner-progress closed', style: "width: #{closed}%")
+        concat content_tag(:span, '', class: 'inner-progress done',   style: "width: #{done}%")
+      end
+      concat content_tag(:span, "#{legend}% #{l(:total_progress)}", class: 'progress-bar-legend')
     end
   end
 
@@ -671,7 +706,8 @@ module ApplicationHelper
   end
 
   def icon_wrapper(icon_class, label)
-    content =  content_tag(:span, '', class: icon_class)
+    content  = content_tag(:span, '', class: icon_class)
     content += content_tag(:span, label, class: 'hidden-for-sighted')
+    content
   end
 end

app/models/permitted_params.rb

@@ -50,7 +50,9 @@ class PermittedParams
   end
 
   def self.permit(key, *params)
-    raise(ArgumentError, "no permitted params are configured for #{key}") unless permitted_attributes.has_key?(key)
+    unless permitted_attributes.has_key?(key)
+      raise(ArgumentError, "no permitted params are configured for #{key}")
+    end
 
     permitted_attributes[key].concat(params)
   end
@@ -114,11 +116,13 @@ class PermittedParams
   end
 
   def planning_element_type
-    params.require(:planning_element_type).permit(*self.class.permitted_attributes[:planning_element_type])
+    params.require(:planning_element_type)
+      .permit(*self.class.permitted_attributes[:planning_element_type])
   end
 
   def planning_element_type_move
-    params.require(:planning_element_type).permit(*self.class.permitted_attributes[:move_to])
+    params.require(:planning_element_type)
+      .permit(*self.class.permitted_attributes[:move_to])
   end
 
   def planning_element(args = {})
@@ -149,7 +153,8 @@ class PermittedParams
     # the sort_criteria hash itself (again with content) in the same hash.
     # Here we try to circumvent this
     p = params.require(:query).permit(*self.class.permitted_attributes[:query])
-    p[:sort_criteria] = params.require(:query).permit(sort_criteria: { '0' => [], '1' => [], '2' => [] })
+    p[:sort_criteria] = params.require(:query)
+                        .permit(sort_criteria: { '0' => [], '1' => [], '2' => [] })
     p[:sort_criteria].delete :sort_criteria
     p
   end
@@ -190,7 +195,9 @@ class PermittedParams
     user_create_as_admin(external_authentication, change_password_allowed, [group_ids: []])
   end
 
-  def user_create_as_admin(external_authentication, change_password_allowed, additional_params = [])
+  def user_create_as_admin(external_authentication,
+                           change_password_allowed,
+                           additional_params = [])
     if current_user.admin?
       additional_params << :auth_source_id unless external_authentication
       additional_params << :force_password_change if change_password_allowed
@@ -249,17 +256,6 @@ class PermittedParams
   end
 
   def timeline
-    acceptable_options_params = ["exist", "zoom_factor", "initial_outline_expansion", "timeframe_start",
-    "timeframe_end", "columns", "project_sort", "compare_to_relative", "compare_to_relative_unit",
-    "compare_to_absolute", "vertical_planning_elements", "exclude_own_planning_elements",
-    "planning_element_status", "planning_element_types", "planning_element_responsibles",
-    "planning_element_assignee", "exclude_reporters", "exclude_empty", "project_types",
-    "project_status", "project_responsibles", "parents", "planning_element_time_types",
-    "planning_element_time_absolute_one", "planning_element_time_absolute_two",
-    "planning_element_time_relative_one", "planning_element_time_relative_one_unit",
-    "planning_element_time_relative_two", "planning_element_time_relative_two_unit",
-    "grouping_one_enabled", "grouping_one_selection", "grouping_one_sort", "hide_other_group"]
-
     # Options here will be empty. This is just initializing it.
     whitelist = params.require(:timeline).permit(:name, options: {})
 
@@ -273,9 +269,8 @@ class PermittedParams
   end
 
   def pref
-    params.require(:pref).permit(:hide_mail, :time_zone, :impaired,
-                                 :comments_sorting, :warn_on_leaving_unsaved,
-                                 :theme)
+    params.require(:pref).permit(:hide_mail, :time_zone, :impaired, :theme,
+                                 :comments_sorting, :warn_on_leaving_unsaved)
   end
 
   def project(instance = nil)
@@ -290,8 +285,8 @@ class PermittedParams
                                                 type_ids: [],
                                                 enabled_module_names: [])
 
-
-    if instance && (instance.new_record? || current_user.allowed_to?(:select_project_modules, instance))
+    if instance &&
+       (instance.new_record? || current_user.allowed_to?(:select_project_modules, instance))
       whitelist.permit(enabled_module_names: [])
     end
 
@@ -322,15 +317,15 @@ class PermittedParams
     # now it is less work to do it this way than have the plugin override this
     # method. We hopefully will change this in the future.
     params.fetch(:version, {}).permit(:name,
-                                    :description,
-                                    :effective_date,
-                                    :due_date,
-                                    :start_date,
-                                    :wiki_page_title,
-                                    :status,
-                                    :sharing,
-                                    :custom_field_value,
-                                    version_settings_attributes: [:id, :display, :project_id])
+                                      :description,
+                                      :effective_date,
+                                      :due_date,
+                                      :start_date,
+                                      :wiki_page_title,
+                                      :status,
+                                      :sharing,
+                                      :custom_field_value,
+                                      version_settings_attributes: [:id, :display, :project_id])
   end
 
   def comment
@@ -353,15 +348,15 @@ class PermittedParams
   end
 
   def enumerations
-    acceptable_params = [:active, :is_default, :move_to, :name, :reassign_to_i, :parent_id,
-                         :custom_field_values, :reassign_to_id]
+    acceptable_params = [:active, :is_default, :move_to, :name, :reassign_to_i,
+                         :parent_id, :custom_field_values, :reassign_to_id]
 
     whitelist = ActionController::Parameters.new
 
     # Sometimes we receive one enumeration, sometimes many in params, hence
     # the following branching.
     if params[:enumerations].present?
-      params[:enumerations].each do |enum, value|
+      params[:enumerations].each do |enum, _value|
         enum.tap do
           whitelist[enum] = {}
           acceptable_params.each do |param|
@@ -394,7 +389,9 @@ class PermittedParams
   end
 
   def reporting
-    params.fetch(:reporting, {}).permit(:reporting_to_project_id, :reported_project_status_id, :reported_project_status_comment)
+    params.fetch(:reporting, {}).permit(:reporting_to_project_id,
+                                        :reported_project_status_id,
+                                        :reported_project_status_comment)
   end
 
   def membership
@@ -413,9 +410,7 @@ class PermittedParams
     # 'id as string' => 'value as string'
     values.reject! do |k, v| k.to_i < 1 || !v.is_a?(String) end
 
-    values.empty? ?
-      {} :
-      { 'custom_field_values' => values }
+    values.empty? ? {} : { 'custom_field_values' => values }
   end
 
   def permitted_attributes(key, additions = {})
@@ -635,8 +630,6 @@ class PermittedParams
     end
   end
 
-  private
-
   ## Add attributes as permitted attributes (only to be used by the plugins plugin)
   #
   # attributes should be given as a Hash in the form

app/models/project.rb

@@ -58,14 +58,14 @@ class Project < ActiveRecord::Base
   }, class_name: 'Member'
   # Read only
   has_many :possible_assignees, -> {
-      # Have to reference it again although possible_assignee_members does already specify it
-      # to be able to use the Project.possible_principles_condition there
-      includes(members: :roles)
+    # Have to reference it again although possible_assignee_members does already specify it
+    # to be able to use the Project.possible_principles_condition there
+    includes(members: :roles)
       .references(:roles)
       .merge(Principal.order_by_name)
-    },
-    through: :possible_assignee_members,
-    source: :principal
+  },
+           through: :possible_assignee_members,
+           source: :principal
   has_many :possible_responsible_members, -> {
     includes(:principal, :roles)
       .where(Project.possible_principles_condition)
@@ -73,14 +73,14 @@ class Project < ActiveRecord::Base
   }, class_name: 'Member'
   # Read only
   has_many :possible_responsibles, -> {
-      # Have to reference it again although possible_assignee_members does already specify it
-      # to be able to use the Project.possible_principles_condition there
-      includes(members: :roles)
+    # Have to reference it again although possible_assignee_members does already specify it
+    # to be able to use the Project.possible_principles_condition there
+    includes(members: :roles)
       .references(:roles)
       .merge(Principal.order_by_name)
-    },
-    through: :possible_responsible_members,
-    source: :principal
+  },
+           through: :possible_responsible_members,
+           source: :principal
   has_many :memberships, class_name: 'Member'
   has_many :member_principals, -> {
     includes(:principal)

app/models/work_package/pdf_exporter.rb

@@ -36,8 +36,7 @@ module WorkPackage::PdfExporter
 
   # Returns a PDF string of a list of work_packages
   def pdf(work_packages, project, query, results, options = {})
-    if  current_language.to_s.downcase == 'ko' || current_language.to_s.downcase == 'ja' || current_language.to_s.downcase == 'zh' || current_language.to_s.downcase == 'zh-tw' ||
-        current_language.to_s.downcase == 'th'
+    if ['ko', 'ja', 'zh', 'zh-tw', 'th'].include?(current_language.to_s.downcase)
       pdf = IFPDF.new(current_language)
     else
       pdf = ITCPDF.new(current_language)
@@ -62,7 +61,13 @@ module WorkPackage::PdfExporter
     col_width = []
     unless query.columns.empty?
       col_width = query.columns.map { |c|
-        (c.name == :subject || (c.is_a?(QueryCustomFieldColumn) && ['string', 'text'].include?(c.custom_field.field_format))) ? 4.0 : 1.0
+        if c.name == :subject ||
+           (c.is_a?(QueryCustomFieldColumn) &&
+            ['string', 'text'].include?(c.custom_field.field_format))
+          4.0
+        else
+          1.0
+        end
       }
       ratio = table_width / col_width.reduce(:+)
       col_width = col_width.map { |w| w * ratio }
@@ -88,9 +93,14 @@ module WorkPackage::PdfExporter
     work_packages.each do |work_package|
       if query.grouped? && (group = query.group_by_column.value(work_package)) != previous_group
         pdf.SetFontStyle('B', 9)
-        pdf.RDMCell(277, row_height,
-                    (group.blank? ? 'None' : group.to_s) + " (#{results.work_package_count_for(group)})",
-                    1, 1, 'L')
+        pdf.RDMCell(
+          277,
+          row_height,
+          (group.blank? ? 'None' : group.to_s) + " (#{results.work_package_count_for(group)})",
+          1,
+          1,
+          'L'
+        )
         pdf.SetFontStyle('', 8)
         previous_group = group
       end
@@ -98,7 +108,9 @@ module WorkPackage::PdfExporter
       # fetch all the row values
       col_values = query.columns.map { |column|
         s = if column.is_a?(QueryCustomFieldColumn)
-              cv = work_package.custom_values.detect { |v| v.custom_field_id == column.custom_field.id }
+              cv = work_package.custom_values.detect { |v|
+                v.custom_field_id == column.custom_field.id
+              }
               show_value(cv)
             else
               value = work_package.send(column.name)
@@ -166,8 +178,7 @@ module WorkPackage::PdfExporter
 
   # Returns a PDF string of a single work_package
   def work_package_to_pdf(work_package)
-    if  current_language.to_s.downcase == 'ko' || current_language.to_s.downcase == 'ja' || current_language.to_s.downcase == 'zh' || current_language.to_s.downcase == 'zh-tw' ||
-        current_language.to_s.downcase == 'th'
+    if ['ko', 'ja', 'zh', 'zh-tw', 'th'].include?(current_language.to_s.downcase)
       pdf = IFPDF.new(current_language)
     else
       pdf = ITCPDF.new(current_language)
@@ -178,7 +189,11 @@ module WorkPackage::PdfExporter
     pdf.AddPage
 
     pdf.SetFontStyle('B', 11)
-    pdf.RDMMultiCell(190, 5, "#{work_package.project} - #{work_package.type} # #{work_package.id}: #{work_package.subject}")
+    pdf.RDMMultiCell(
+      190,
+      5,
+      "#{work_package.project} - #{work_package.type} # #{work_package.id}: #{work_package.subject}"
+    )
     pdf.Ln
 
     y0 = pdf.GetY
@@ -223,7 +238,7 @@ module WorkPackage::PdfExporter
     pdf.RDMCell(60, 5, format_date(work_package.due_date), 'RB')
     pdf.Ln
 
-    for custom_value in work_package.custom_field_values
+    work_package.custom_field_values.each do |custom_value|
       pdf.SetFontStyle('B', 9)
       pdf.RDMCell(35, 5, custom_value.custom_field.name + ':', 'L')
       pdf.SetFontStyle('', 9)
@@ -239,11 +254,13 @@ module WorkPackage::PdfExporter
     pdf.Line(pdf.GetX, pdf.GetY, pdf.GetX + 190, pdf.GetY)
     pdf.Ln
 
-    if work_package.changesets.any? && User.current.allowed_to?(:view_changesets, work_package.project)
+    if work_package.changesets.any? &&
+       User.current.allowed_to?(:view_changesets, work_package.project)
       pdf.SetFontStyle('B', 9)
       pdf.RDMCell(190, 5, l(:label_associated_revisions), 'B')
       pdf.Ln
-      for changeset in work_package.changesets
+
+      work_package.changesets.each do |changeset|
         pdf.SetFontStyle('B', 8)
         pdf.RDMCell(190, 5, format_time(changeset.committed_on) + ' - ' + changeset.author.to_s)
         pdf.Ln
@@ -258,14 +275,21 @@ module WorkPackage::PdfExporter
     pdf.SetFontStyle('B', 9)
     pdf.RDMCell(190, 5, l(:label_history), 'B')
     pdf.Ln
-    for journal in work_package.journals.includes(:user).order("#{Journal.table_name}.created_at ASC")
+    journals = work_package.journals.includes(:user).order("#{Journal.table_name}.created_at ASC")
+    journals.each do |journal|
       next if journal.initial?
+
       pdf.SetFontStyle('B', 8)
       pdf.RDMCell(190, 5, format_time(journal.created_at) + ' - ' + journal.user.name)
       pdf.Ln
       pdf.SetFontStyle('I', 8)
-      for detail in journal.details
-        pdf.RDMMultiCell(190, 5, '- ' + journal.render_detail(detail, no_html: true, only_path: false))
+
+      journal.details.each do |detail|
+        pdf.RDMMultiCell(
+          190,
+          5,
+          '- ' + journal.render_detail(detail, no_html: true, only_path: false)
+        )
         pdf.Ln
       end
       if journal.notes?
@@ -280,7 +304,8 @@ module WorkPackage::PdfExporter
       pdf.SetFontStyle('B', 9)
       pdf.RDMCell(190, 5, l(:label_attachment_plural), 'B')
       pdf.Ln
-      for attachment in work_package.attachments
+
+      work_package.attachments.each do |attachment|
         pdf.SetFontStyle('', 8)
         pdf.RDMCell(80, 5, attachment.filename)
         pdf.RDMCell(20, 5, number_to_human_size(attachment.filesize), 0, 0, 'R')
@@ -289,6 +314,7 @@ module WorkPackage::PdfExporter
         pdf.Ln
       end
     end
+
     pdf.Output
   end
 
@@ -418,7 +444,9 @@ module WorkPackage::PdfExporter
         # 0x5c char handling
         txtar = txt.split('\\')
         txtar << '' if txt[-1] == ?\\
-        txtar.map { |x| x.encode(l(:general_pdf_encoding), 'UTF-8') }.join('\\').gsub(/\\/, '\\\\\\\\')
+        txtar.map { |x|
+          x.encode(l(:general_pdf_encoding), 'UTF-8')
+        }.join('\\').gsub(/\\/, '\\\\\\\\')
       rescue
         txt
       end || ''

features/support/paths.rb

@@ -321,9 +321,7 @@ module NavigationHelpers
       project_identifier = project.identifier.gsub(' ', '%20')
       timeline = project.timelines.detect { |t| t.name == timeline_name }
 
-      timeline_id = timeline ?
-                      "/#{timeline.id}" :
-                      ''
+      timeline_id = timeline ? "/#{timeline.id}" : ''
 
       "/projects/#{project_identifier}/timelines#{timeline_id}"
 

spec/models/permitted_params_spec.rb

@@ -85,28 +85,28 @@ describe PermittedParams, type: :model do
 
   describe '#timeline' do
     it 'should permit all acceptable options params and one name params' do
-      acceptable_options_params = ["exist", "zoom_factor", "initial_outline_expansion", "timeframe_start",
-        "timeframe_end", "columns", "project_sort", "compare_to_relative", "compare_to_relative_unit",
-        "compare_to_absolute", "vertical_planning_elements", "exclude_own_planning_elements",
-        "planning_element_status", "planning_element_types", "planning_element_responsibles",
-        "planning_element_assignee", "exclude_reporters", "exclude_empty", "project_types",
-        "project_status", "project_responsibles", "parents", "planning_element_time_types",
-        "planning_element_time_absolute_one", "planning_element_time_absolute_two",
-        "planning_element_time_relative_one", "planning_element_time_relative_one_unit",
-        "planning_element_time_relative_two", "planning_element_time_relative_two_unit",
-        "grouping_one_enabled", "grouping_one_selection", "grouping_one_sort", "hide_other_group"]
+      acceptable_options_params = ['exist', 'zoom_factor', 'initial_outline_expansion', 'timeframe_start',
+                                   'timeframe_end', 'columns', 'project_sort', 'compare_to_relative', 'compare_to_relative_unit',
+                                   'compare_to_absolute', 'vertical_planning_elements', 'exclude_own_planning_elements',
+                                   'planning_element_status', 'planning_element_types', 'planning_element_responsibles',
+                                   'planning_element_assignee', 'exclude_reporters', 'exclude_empty', 'project_types',
+                                   'project_status', 'project_responsibles', 'parents', 'planning_element_time_types',
+                                   'planning_element_time_absolute_one', 'planning_element_time_absolute_two',
+                                   'planning_element_time_relative_one', 'planning_element_time_relative_one_unit',
+                                   'planning_element_time_relative_two', 'planning_element_time_relative_two_unit',
+                                   'grouping_one_enabled', 'grouping_one_selection', 'grouping_one_sort', 'hide_other_group']
 
-      acceptable_options_params_with_data = HashWithIndifferentAccess[acceptable_options_params.map {|x| [x, 'value']}]
+      acceptable_options_params_with_data = HashWithIndifferentAccess[acceptable_options_params.map { |x| [x, 'value'] }]
 
-      params = ActionController::Parameters.new(timeline: {'name' => 'my name', 'options' => acceptable_options_params_with_data})
+      params = ActionController::Parameters.new(timeline: { 'name' => 'my name', 'options' => acceptable_options_params_with_data })
 
-      expect(PermittedParams.new(params, user).timeline).to eq({'name' => 'my name', 'options' => acceptable_options_params_with_data})
+      expect(PermittedParams.new(params, user).timeline).to eq({ 'name' => 'my name', 'options' => acceptable_options_params_with_data })
     end
 
     it 'should accept with no options' do
-      params = ActionController::Parameters.new(timeline: {'name' => 'my name'})
+      params = ActionController::Parameters.new(timeline: { 'name' => 'my name' })
 
-      expect(PermittedParams.new(params, user).timeline).to eq({'name' => 'my name'})
+      expect(PermittedParams.new(params, user).timeline).to eq({ 'name' => 'my name' })
     end
   end
 
@@ -116,7 +116,7 @@ describe PermittedParams, type: :model do
                            :comments_sorting, :warn_on_leaving_unsaved,
                            :theme]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
       params = ActionController::Parameters.new(pref: acceptable_params_with_data)
 
@@ -127,15 +127,15 @@ describe PermittedParams, type: :model do
   describe '#time_entry' do
     it 'should permit its whitelisted params' do
       acceptable_params = [:hours, :comments, :work_package_id,
-                            :activity_id, :spent_on]
+                           :activity_id, :spent_on]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
       acceptable_params_with_data.merge!(custom_field_values: {
-        '1' => 'foo',
-        '2' => 'bar',
-        '3' => 'baz'
-      })
+                                           '1' => 'foo',
+                                           '2' => 'bar',
+                                           '3' => 'baz'
+                                         })
 
       params = ActionController::Parameters.new(time_entry: acceptable_params_with_data)
 
@@ -153,7 +153,7 @@ describe PermittedParams, type: :model do
     it 'should permit its whitelisted params' do
       acceptable_params = [:title, :summary, :description]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
       params = ActionController::Parameters.new(news: acceptable_params_with_data)
 
@@ -165,7 +165,7 @@ describe PermittedParams, type: :model do
     it 'should permit its whitelisted params' do
       acceptable_params = [:commented, :author, :comments]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
       params = ActionController::Parameters.new(comment: acceptable_params_with_data)
 
@@ -177,7 +177,7 @@ describe PermittedParams, type: :model do
     it 'should permit its whitelisted params' do
       acceptable_params = [:watchable, :user, :user_id]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
       params = ActionController::Parameters.new(watcher: acceptable_params_with_data)
 
@@ -189,7 +189,7 @@ describe PermittedParams, type: :model do
     it 'should permit its whitelisted params' do
       acceptable_params = [:content, :subject]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
       params = ActionController::Parameters.new(reply: acceptable_params_with_data)
 
@@ -201,7 +201,7 @@ describe PermittedParams, type: :model do
     it 'should permit its whitelisted params' do
       acceptable_params = [:start_page]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
       params = ActionController::Parameters.new(wiki: acceptable_params_with_data)
 
@@ -213,7 +213,7 @@ describe PermittedParams, type: :model do
     it 'should permit its whitelisted params' do
       acceptable_params = [:reporting_to_project_id, :reported_project_status_id, :reported_project_status_comment]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
       params = ActionController::Parameters.new(reporting: acceptable_params_with_data)
 
@@ -228,7 +228,7 @@ describe PermittedParams, type: :model do
 
   describe '#membership' do
     it 'should permit its whitelisted params' do
-      acceptable_params_with_data = HashWithIndifferentAccess[project_id: '1', role_ids: [1,2,4]]
+      acceptable_params_with_data = HashWithIndifferentAccess[project_id: '1', role_ids: [1, 2, 4]]
 
       params = ActionController::Parameters.new(membership: acceptable_params_with_data)
 
@@ -240,7 +240,7 @@ describe PermittedParams, type: :model do
     it 'should permit its whitelisted params' do
       acceptable_params = [:name, :assigned_to_id]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
       params = ActionController::Parameters.new(category: acceptable_params_with_data)
 
@@ -254,11 +254,11 @@ describe PermittedParams, type: :model do
                            :start_date, :wiki_page_title, :status, :sharing,
                            :custom_field_value]
 
-      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+      acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
-      acceptable_params_with_data.merge!(version_settings_attributes: {id: '1',
-                                                                       display: '2',
-                                                                       project_id: '3'})
+      acceptable_params_with_data.merge!(version_settings_attributes: { id: '1',
+                                                                        display: '2',
+                                                                        project_id: '3' })
 
       params = ActionController::Parameters.new(version: acceptable_params_with_data)
 
@@ -276,7 +276,7 @@ describe PermittedParams, type: :model do
       it 'should permit its whitelisted params' do
         acceptable_params = [:subject, :content, :board_id]
 
-        acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+        acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
         # Sticky and evil should not make it.
         params = ActionController::Parameters.new(message: acceptable_params_with_data.merge(evil: true, sticky: true))
@@ -297,7 +297,7 @@ describe PermittedParams, type: :model do
 
         acceptable_params = [:locked, :sticky]
 
-        acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map {|x| [x, 'value']}]
+        acceptable_params_with_data = HashWithIndifferentAccess[acceptable_params.map { |x| [x, 'value'] }]
 
         params = ActionController::Parameters.new(message: acceptable_params_with_data)
 
@@ -309,7 +309,7 @@ describe PermittedParams, type: :model do
   describe '#attachments' do
     it 'should permit its whitelisted params' do
       acceptable_params_with_data = HashWithIndifferentAccess[file: 'myfile',
-                                     description: 'mydescription']
+                                                              description: 'mydescription']
 
       params = ActionController::Parameters.new(attachments: acceptable_params_with_data)
 
@@ -950,20 +950,20 @@ describe PermittedParams, type: :model do
   end
 
   shared_examples_for 'allows enumeration move params' do
-    let(:hash) { {"2" =>{ 'move_to' => 'lower' }} }
+    let(:hash) { { '2' => { 'move_to' => 'lower' } } }
 
     it_behaves_like 'allows params'
   end
 
   shared_examples_for 'allows move params' do
-    let(:hash) { { 'move_to' => 'lower' }}
+    let(:hash) { { 'move_to' => 'lower' } }
 
     it_behaves_like 'allows params'
   end
 
   shared_examples_for 'allows custom fields' do
     describe 'valid custom fields' do
-      let(:hash) { {"1" => { 'custom_field_values' => { '1' => '5' } } }}
+      let(:hash) { { '1' => { 'custom_field_values' => { '1' => '5' } } } }
 
       it_behaves_like 'allows params'
     end
@@ -1011,25 +1011,25 @@ describe PermittedParams, type: :model do
     let (:attribute) { :enumerations }
 
     describe 'name' do
-      let(:hash) { {"1" => { 'name' => 'blubs' } }}
+      let(:hash) { { '1' => { 'name' => 'blubs' } } }
 
       it_behaves_like 'allows params'
     end
 
     describe 'active' do
-      let(:hash) { {"1" => { 'active' => 'true' } }}
+      let(:hash) { { '1' => { 'active' => 'true' } } }
 
       it_behaves_like 'allows params'
     end
 
     describe 'is_default' do
-      let(:hash) { {"1" => { 'is_default' => 'true' } }}
+      let(:hash) { { '1' => { 'is_default' => 'true' } } }
 
       it_behaves_like 'allows params'
     end
 
     describe 'reassign_to_id' do
-      let(:hash) { {"1" => { 'reassign_to_id' => '1' } }}
+      let(:hash) { { '1' => { 'reassign_to_id' => '1' } } }
 
       it_behaves_like 'allows params'
     end