Implement style guide (publiclab#2124)

* Lock paperclip to required version

* Exclude critical files

* Disable magic comment

* Safe navigate on params

* Move Style/FrozenStringLiteralComment to .rubocop.yml

* Rubocop autofix

.rubocop.yml

@@ -15,4 +15,11 @@ AllCops:
     - 'log/*'
     - 'db/**/*'
     - 'Gemfile'
-  TargetRubyVersion: '2.4'
+    - 'config/**/*'
+    - 'script/**/*'
+    - 'lib/**/*'
+    - 'test/**/*'
+  TargetRubyVersion: '2.4'
+
+Style/FrozenStringLiteralComment:
+  Enabled: false

Gemfile

@@ -77,7 +77,7 @@ gem 'geokit-rails'
 
 gem 'rails_autolink'
 gem 'rb-readline'
-gem "paperclip", "~> 4.2.2"
+gem "paperclip", "~> 4.2.0"
 gem "ruby-openid", :require => "openid"
 gem "rack-openid"
 gem "authlogic", "~> 3.5.0"

Gemfile.lock

@@ -401,7 +401,7 @@ DEPENDENCIES
   mustermann (~> 0.4)
   mysql2 (~> 0.3.20)
   nifty-generators
-  paperclip (< 4.2.0)
+  paperclip (~> 4.2.0)
   passenger
   php-serialize
   progress_bar

app/api/srch/search.rb

@@ -118,9 +118,9 @@ class Search < Grape::API
       # Request URL should be /api/srch/locations?srchString=QRY[&seq=KEYCOUNT&showCount=NUM_ROWS&pageNum=PAGE_NUM]
       # Note: Query(QRY as above) must have latitude and longitude as srchString=lat,lon
       desc 'Perform a search of documents having nearby latitude and longitude tag values',
-           hidden: false,
-           is_array: false,
-           nickname: 'srchGetLocations'
+        hidden: false,
+        is_array: false,
+        nickname: 'srchGetLocations'
       params do
         requires :srchString, type: String, documentation: { example: 'Spec' }
         optional :seq, type: Integer, documentation: { example: 995 }

app/controllers/admin_controller.rb

@@ -1,5 +1,5 @@
 class AdminController < ApplicationController
-  before_filter :require_user, only: %i[spam spam_revisions]
+  before_filter :require_user, only: %i(spam spam_revisions)
 
   def promote_admin
     @user = User.find params[:id]
@@ -79,7 +79,7 @@ def spam
                    .order('nid DESC')
       @nodes = if params[:type] == 'wiki'
                  @nodes.where(type: 'page', status: 1)
-               else
+      else
                  @nodes.where(status: 0)
                end
     else

app/controllers/answers_controller.rb

@@ -65,6 +65,7 @@ def accept
         else
           @answer.accepted = true
           @answer.save
+          @answer.node.add_tag('answered', @answer.author)
           AnswerMailer.notify_answer_accept(@answer.author, @answer).deliver_now
         end
         @answer.reload

app/controllers/application_controller.rb

@@ -26,19 +26,19 @@ def set_sidebar(type = :generic, data = :all, args = {})
     else # type is generic
       # remove "classroom" postings; also switch to an EXCEPT operator in sql, see https://github.com/publiclab/plots2/issues/375
       hidden_nids = Node.joins(:node_tag)
-                        .joins('LEFT OUTER JOIN term_data ON term_data.tid = community_tags.tid')
-                        .select('node.*, term_data.*, community_tags.*')
-                        .where(type: 'note', status: 1)
-                        .where('term_data.name = (?)', 'hidden:response')
-                        .collect(&:nid)
+        .joins('LEFT OUTER JOIN term_data ON term_data.tid = community_tags.tid')
+        .select('node.*, term_data.*, community_tags.*')
+        .where(type: 'note', status: 1)
+        .where('term_data.name = (?)', 'hidden:response')
+        .collect(&:nid)
       @notes = if params[:controller] == 'questions'
                  Node.questions
-                     .joins(:revision)
-               else
+                   .joins(:revision)
+      else
                  Node.research_notes
-                     .joins(:revision)
-                     .order('node.nid DESC')
-                     .paginate(page: params[:page])
+                   .joins(:revision)
+                   .order('node.nid DESC')
+                   .paginate(page: params[:page])
                end
       @notes = @notes.where('node.nid != (?)', @node.nid) if @node
       @notes = @notes.where('node_revisions.status = 1 AND node.nid NOT IN (?)', hidden_nids) unless hidden_nids.empty?
@@ -52,11 +52,11 @@ def set_sidebar(type = :generic, data = :all, args = {})
       end
 
       @wikis = Node.order('changed DESC')
-                   .joins(:revision)
-                   .where('node_revisions.status = 1 AND node.status = 1 AND type = "page"')
-                   .limit(10)
-                   .group('node_revisions.nid')
-                   .order('node_revisions.timestamp DESC')
+        .joins(:revision)
+        .where('node_revisions.status = 1 AND node.status = 1 AND type = "page"')
+        .limit(10)
+        .group('node_revisions.nid')
+        .order('node_revisions.timestamp DESC')
     end
   end
 
@@ -73,7 +73,7 @@ def current_user_session
 
   def current_user
     unless defined?(@current_user)
-      @current_user = current_user_session && current_user_session.record
+      @current_user = current_user_session&.record
     end
     # if banned or moderated:
     if @current_user.try(:drupal_user).try(:status) == 0
@@ -162,15 +162,15 @@ def comments_node_and_path
     @node = if @comment.aid == 0
               # finding node for node comments
               @comment.node
-            else
+    else
               # finding node for answer comments
               @comment.answer.node
             end
 
     @path = if params[:type] && params[:type] == 'question'
               # questions path
               @node.path(:question)
-            else
+    else
               # notes path
               @node.path
             end

app/controllers/comment_controller.rb

@@ -2,11 +2,11 @@ class CommentController < ApplicationController
   include CommentHelper
 
   respond_to :html, :xml, :json
-  before_filter :require_user, only: %i[create update make_answer delete]
+  before_filter :require_user, only: %i(create update make_answer delete)
 
   def index
     @comments = Comment.paginate(page: params[:page], per_page: 30)
-                       .order('timestamp DESC')
+      .order('timestamp DESC')
     render template: 'comments/index'
   end
 
@@ -148,8 +148,8 @@ def make_answer
     comments_node_and_path
 
     if @comment.uid == current_user.uid ||
-        current_user.role == 'admin' ||
-        current_user.role == 'moderator'
+       current_user.role == 'admin' ||
+       current_user.role == 'moderator'
 
       @answer = Answer.new(
           nid: @comment.nid,

app/controllers/editor_controller.rb

@@ -1,5 +1,5 @@
 class EditorController < ApplicationController
-  before_filter :require_user, only: %i[post rich legacy editor]
+  before_filter :require_user, only: %i(post rich legacy editor)
 
   # main image via URL passed as GET param
   def legacy
@@ -14,7 +14,7 @@ def legacy
       node = Node.find(params[:n])
       params[:body] = node.body if node
     end
-    if params[:tags] && params[:tags].include?('question:')
+    if params[:tags]&.include?('question:')
       redirect_to "/questions/new?#{request.env['QUERY_STRING']}"
     else
       render template: 'editor/post'
@@ -26,7 +26,7 @@ def editor
   end
 
   def post
-    if params[:tags] && params[:tags].include?('question:')
+    if params[:tags]&.include?('question:')
       redirect_to "/questions/new?#{request.env['QUERY_STRING']}"
     elsif params[:legacy] || params[:template] == 'event'
       legacy

app/controllers/features_controller.rb

@@ -3,7 +3,7 @@ class FeaturesController < ApplicationController
 
   def index
     @features = Node.where(type: 'feature')
-                    .paginate(page: params[:page])
+      .paginate(page: params[:page])
   end
 
   def embed

app/controllers/home_controller.rb

@@ -1,5 +1,5 @@
 class HomeController < ApplicationController
-  before_filter :require_user, only: %i[subscriptions nearby]
+  before_filter :require_user, only: %i(subscriptions nearby)
 
   # caches_action :index, :cache_path => proc { |c|
   #  node = Node.find :last #c.params[:id]
@@ -44,12 +44,12 @@ def front
   end
 
   def dashboard
-    @note_count = Node.select(%i[created type status])
-                      .where(type: 'note', status: 1, created: Time.now.to_i - 1.weeks.to_i..Time.now.to_i)
-                      .count(:all)
+    @note_count = Node.select(%i(created type status))
+      .where(type: 'note', status: 1, created: Time.now.to_i - 1.weeks.to_i..Time.now.to_i)
+      .count(:all)
     @wiki_count = Revision.select(:timestamp)
-                          .where(timestamp: Time.now.to_i - 1.weeks.to_i..Time.now.to_i)
-                          .count
+      .where(timestamp: Time.now.to_i - 1.weeks.to_i..Time.now.to_i)
+      .count
     @user_note_count = Node.where(type: 'note', status: 1, uid: current_user.uid).count if current_user
     @activity, @blog, @notes, @wikis, @revisions, @comments, @answer_comments = self.activity
     render template: 'dashboard/dashboard'
@@ -73,15 +73,15 @@ def activity
     blog = Tag.find_nodes_by_type('blog', 'note', 1).first
     # remove "classroom" postings; also switch to an EXCEPT operator in sql, see https://github.com/publiclab/plots2/issues/375
     hidden_nids = Node.joins(:node_tag)
-                      .joins('LEFT OUTER JOIN term_data ON term_data.tid = community_tags.tid')
-                      .select('node.*, term_data.*, community_tags.*')
-                      .where(type: 'note', status: 1)
-                      .where('term_data.name = (?)', 'hidden:response')
-                      .collect(&:nid)
+      .joins('LEFT OUTER JOIN term_data ON term_data.tid = community_tags.tid')
+      .select('node.*, term_data.*, community_tags.*')
+      .where(type: 'note', status: 1)
+      .where('term_data.name = (?)', 'hidden:response')
+      .collect(&:nid)
     notes = Node.where(type: 'note')
-                .where('node.nid NOT IN (?)', hidden_nids + [0]) # in case hidden_nids is empty
-                .order('nid DESC')
-                .page(params[:page])
+      .where('node.nid NOT IN (?)', hidden_nids + [0]) # in case hidden_nids is empty
+      .order('nid DESC')
+      .page(params[:page])
     notes = notes.where('nid != (?)', blog.nid) if blog
 
     if current_user && (current_user.role == 'moderator' || current_user.role == 'admin')
@@ -95,34 +95,34 @@ def activity
 
     # include revisions, then mix with new pages:
     wikis = Node.where(type: 'page', status: 1)
-                .order('nid DESC')
-                .limit(10)
+      .order('nid DESC')
+      .limit(10)
     revisions = Revision.joins(:node)
-                        .order('timestamp DESC')
-                        .where('type = (?)', 'page')
-                        .where('node.status = 1')
-                        .where('node_revisions.status = 1')
-                        .where('timestamp - node.created > ?', 300) # don't report edits within 5 mins of page creation
-                        .limit(10)
-                        .group('node.title')
+      .order('timestamp DESC')
+      .where('type = (?)', 'page')
+      .where('node.status = 1')
+      .where('node_revisions.status = 1')
+      .where('timestamp - node.created > ?', 300) # don't report edits within 5 mins of page creation
+      .limit(10)
+      .group('node.title')
     # group by day: http://stackoverflow.com/questions/5970938/group-by-day-from-timestamp
     revisions = revisions.group('DATE(FROM_UNIXTIME(timestamp))') if Rails.env == 'production'
     revisions = revisions.to_a # ensure it can be serialized for caching
     wikis += revisions
     wikis = wikis.sort_by(&:created_at).reverse
     comments = Comment.joins(:node, :drupal_user)
-                      .order('timestamp DESC')
-                      .where('timestamp - node.created > ?', 86_400) # don't report edits within 1 day of page creation
-                      .page(params[:page])
-                      .group('title') # group by day: http://stackoverflow.com/questions/5970938/group-by-day-from-timestamp
+      .order('timestamp DESC')
+      .where('timestamp - node.created > ?', 86_400) # don't report edits within 1 day of page creation
+      .page(params[:page])
+      .group('title') # group by day: http://stackoverflow.com/questions/5970938/group-by-day-from-timestamp
     # group by day: http://stackoverflow.com/questions/5970938/group-by-day-from-timestamp
     comments = comments.group('DATE(FROM_UNIXTIME(timestamp))') if Rails.env == 'production'
     comments = comments.to_a # ensure it can be serialized for caching
     answer_comments = Comment.joins(:answer, :drupal_user)
-                             .order('timestamp DESC')
-                             .where('timestamp - answers.created_at > ?', 86_400)
-                             .limit(20)
-                             .group('answers.id')
+      .order('timestamp DESC')
+      .where('timestamp - answers.created_at > ?', 86_400)
+      .limit(20)
+      .group('answers.id')
     answer_comments = answer_comments.group('DATE(FROM_UNIXTIME(timestamp))') if Rails.env == 'production'
     answer_comments = answer_comments.to_a # ensure it can be serialized for caching
     activity = (notes + wikis + comments + answer_comments).sort_by(&:created_at).reverse

app/controllers/images_controller.rb

@@ -2,7 +2,7 @@
 
 class ImagesController < ApplicationController
   respond_to :html, :xml, :json
-  before_filter :require_user, only: %i[create new update delete]
+  before_filter :require_user, only: %i(create new update delete)
 
   def create
     if params[:i]

app/controllers/like_controller.rb

@@ -1,6 +1,6 @@
 class LikeController < ApplicationController
   respond_to :html, :xml, :json
-  before_filter :require_user, only: %i[create delete]
+  before_filter :require_user, only: %i(create delete)
 
   #list all recent likes
   def index
@@ -19,7 +19,7 @@ def liked?
     result = NodeSelection.find_by_user_id_and_nid(current_user.uid, params[:id])
     result = if result.nil?
                false
-             else
+    else
                result.liking
              end
     render json: result

app/controllers/map_controller.rb

@@ -2,13 +2,13 @@ class MapController < ApplicationController
   def index
     @title = 'Maps'
     @nodes = Node.paginate(page: params[:page], per_page: 32)
-                 .order('nid DESC')
-                 .where(type: 'map', status: 1)
+      .order('nid DESC')
+      .where(type: 'map', status: 1)
 
     # I'm not sure if this is actually eager loading the tags...
     @maps = Node.joins(:tag)
-                .where('type = "map" AND status = 1 AND (term_data.name LIKE ? OR term_data.name LIKE ?)', 'lat:%', 'lon:%')
-                .uniq
+      .where('type = "map" AND status = 1 AND (term_data.name LIKE ? OR term_data.name LIKE ?)', 'lat:%', 'lon:%')
+      .uniq
 
     # This is supposed to eager load the url_aliases, and seems to run, but doesn't actually eager load them...?
     # @maps = Node.select("node.*,url_alias.dst AS dst").joins(:tag).where('type = "map" AND status = 1 AND (term_data.name LIKE ? OR term_data.name LIKE ?)', 'lat:%', 'lon:%').joins("INNER JOIN url_alias ON url_alias.src = CONCAT('node/',node.nid)")
@@ -56,10 +56,8 @@ def update
       @revision.title = params[:title]
       @revision.body = params[:body]
 
-      if params[:tags]
-        params[:tags].split(',').each do |tagname|
+      params[:tags]&.split(',').each do |tagname|
           @node.add_tag(tagname, current_user)
-        end
       end
 
       # save main image
@@ -133,10 +131,8 @@ def create
                                               main_image: params[:main_image])
 
       if saved
-        if params[:tags]
-          params[:tags].split(',').each do |tagname|
+        params[:tags]&.split(',').each do |tagname|
             @node.add_tag(tagname, current_user)
-          end
         end
 
         # save main image
@@ -205,8 +201,8 @@ def tag
     @tagnames = params[:id].split(',')
     nids = Tag.find_nodes_by_type(params[:id], 'map', 20).collect(&:nid)
     @notes = Node.paginate(page: params[:page])
-                 .where('nid in (?)', nids)
-                 .order('nid DESC')
+      .where('nid in (?)', nids)
+      .order('nid DESC')
 
     @title = @tagnames.join(', ') if @tagnames
     @unpaginated = true