(WIP)Move some controller code to models (#519)

* Remove trailing white spaces and unnecessary blank lines

* Remove basic rubocop offenses

* add few maps  tests

* refactor maps controller

* fix 'line too long' rubocop offense

app/controllers/annotations_controller.rb

@@ -5,56 +5,53 @@ class AnnotationsController < ApplicationController
   before_filter :find_map
 
   def index
-    render :file => 'annotations/index.json.erb', :content_type => 'application/json'
+    render file: 'annotations/index.json.erb', content_type: 'application/json'
   end
 
   def create
     geojson = params[:annotation]
 
     respond_to do |format|
-      format.json { 
+      format.json {
         @annotation = @map.annotations.create(
-          :annotation_type => geojson[:properties][:annotation_type],
-          :coordinates => geojson[:geometry][:coordinates],
-          :text => geojson[:properties][:textContent],
-          :style => geojson[:properties][:style],
+          annotation_type: geojson[:properties][:annotation_type],
+          coordinates: geojson[:geometry][:coordinates],
+          text: geojson[:properties][:textContent],
+          style: geojson[:properties][:style]
         )
         @annotation.user_id = current_user.id if logged_in?
-        if @annotation.save
-          redirect_to map_annotation_url(@map, @annotation)
-        end
+        redirect_to map_annotation_url(@map, @annotation) if @annotation.save
       }
     end
   end
 
   def show
     @annotation = Annotation.find params[:id]
-    render :file => 'annotations/show.json.erb', :content_type => 'application/json'
+    render file: 'annotations/show.json.erb', content_type: 'application/json'
   end
 
   def update
     @annotation = Annotation.find params[:id]
     geojson = params[:annotation]
     if @annotation.user_id.nil? || current_user.can_edit?(@annotation)
       Annotation.update(@annotation.id,
-        :coordinates => geojson[:geometry][:coordinates],
-        :text => geojson[:properties][:textContent],
-        :style => geojson[:properties][:style]
-      )
-      render :file => 'annotations/update.json.erb', :content_type => 'application/json'
+                        coordinates: geojson[:geometry][:coordinates],
+                        text: geojson[:properties][:textContent],
+                        style: geojson[:properties][:style])
+      render file: 'annotations/update.json.erb',
+             content_type: 'application/json'
     end
-  end  
+  end
 
   def destroy
     @annotation = Annotation.find params[:id]
     # if current_user.can_delete?(@annotation)
-      @annotation.delete 
-      head :ok
+    @annotation.delete
+    head :ok
     # end
   end
 
   def find_map
     @map = Map.find params[:map_id]
   end
-
 end

app/controllers/application_controller.rb

@@ -1,5 +1,5 @@
 class ApplicationController < ActionController::Base
-  #include OpenIdAuthentication # shouldn't be necessary!!
+  # include OpenIdAuthentication # shouldn't be necessary!!
   # Prevent CSRF attacks by raising an exception.
   # For APIs, you may want to use :null_session instead.
   protect_from_forgery with: :exception
@@ -10,7 +10,7 @@ class ApplicationController < ActionController::Base
   helper_method :logged_in?
 
   def current_user
-    user_id = session[:user_id] 
+    user_id = session[:user_id]
     if user_id
       begin
         @user = User.find(user_id)
@@ -24,26 +24,21 @@ def current_user
 
   private
 
-    def require_login
-      unless logged_in?
-        path_info = request.env['PATH_INFO']
-        flash[:warning] = "You must be logged in to access this section"
-        redirect_to '/login?back_to=' + path_info.to_param # halts request cycle
-      end
+  def require_login
+    unless logged_in?
+      path_info = request.env['PATH_INFO']
+      flash[:warning] = 'You must be logged in to access this section'
+      redirect_to '/login?back_to=' + path_info.to_param # halts request cycle
     end
+  end
 
-    def logged_in?
-      user_id = session[:user_id]
+  def logged_in?
+    user_id = session[:user_id]
 
-      begin
-        if user_id and User.find(user_id)
-          return true
-        else
-          return false
-        end
-      rescue
-        return false
-      end
+    begin
+      user_id && User.find(user_id) ? true : false
+    rescue
+      return false
     end
-
+  end
 end

app/controllers/comments_controller.rb

@@ -1,55 +1,52 @@
 class CommentsController < ApplicationController
-
   def create
     if logged_in?
       @map = Map.find params[:map_id]
 
       @comment = @map.comments.new(
-        :user_id => current_user.id,
-        :body => params[:comment][:body]
+        user_id: current_user.id,
+        body: params[:comment][:body]
       )
       if @comment.save!
         users = @map.comments.collect(&:user)
         users += [@map.user] unless @map.user.nil?
         users.uniq.each do |user|
           unless user.id == current_user.id
-            CommentMailer.notify(user,@comment).deliver
+            CommentMailer.notify(user, @comment).deliver
           end
         end
       end
 
       respond_to do |format|
-        #format.html { redirect_to "/maps/" + params[:map_id] }
-        format.html   { render :partial => 'comments/comment', :locals => {:comment => @comment} }
-        #format.js   { render :partial => 'comments/comment', :locals => {:comment => @comment} }
+        format.html { render partial: 'comments/comment', locals: { comment: @comment } }
         format.json { render json: @comment, status: :created }
       end
-     
+
     else
       # we intercept this message in /app/assets/javascripts/maps.js
-      render :text => "Login required."
+      render text: 'Login required.'
     end
   end
 
   def update
     @comment = Comment.find params[:id]
     if logged_in? && current_user.can_edit?(@comment)
       @comment.update_attribute(:body, params[:comment][:body])
-      redirect_to "/maps/" + params[:map_id]
+      redirect_to '/maps/' + params[:map_id]
     else
-      flash[:error] = "You do not have permissions to update that comment."
-      redirect_to "/login"            
+      flash[:error] = 'You do not have permissions to update that comment.'
+      redirect_to '/login'
     end
   end
 
   def destroy
     @comment = Comment.find(params[:id])
 
     if logged_in? && current_user.can_delete?(@comment)
-      @comment.delete 
-      flash[:notice] = "Comment deleted."
+      @comment.delete
+      flash[:notice] = 'Comment deleted.'
     else
-      flash[:error] = "You do not have permission to delete that comment."
+      flash[:error] = 'You do not have permission to delete that comment.'
     end
     redirect_to "/maps/#{params[:map_id]}"
   end

app/controllers/export_controller.rb

@@ -1,15 +1,15 @@
 class ExportController < ApplicationController
-  protect_from_forgery :except => [:formats]
+  protect_from_forgery except: :formats
 
   def index
-    @exports = Export.where('status NOT IN (?)',['failed','complete','none'])
-      .order('updated_at DESC')
-    @day = Export.where(status:'complete')
-      .where('updated_at > (?)',(Time.now-1.day).to_s(:db))
-      .count
-    @week = Export.where(status:'complete')
-      .where('updated_at > (?)',(Time.now-1.week).to_s(:db))
-      .count
+    @exports = Export.where('status NOT IN (?)', %w[failed complete none])
+                     .order('updated_at DESC')
+    @day = Export.where(status: 'complete')
+                 .where('updated_at > (?)', (Time.now - 1.day).to_s(:db))
+                 .count
+    @week = Export.where(status: 'complete')
+                  .where('updated_at > (?)', (Time.now - 1.week).to_s(:db))
+                  .count
   end
 
   # override logger to suppress huge amounts of inane /export/progress logging
@@ -23,34 +23,35 @@ def logger
 
   # https://mapknitter.org/warps/yale-farm/yale-farm.jpg
   def jpg
-    send_file 'public/warps/'+params[:id]+'/'+params[:id]+'.jpg'
+    send_file 'public/warps/' + params[:id] + '/' + params[:id] + '.jpg'
   end
 
   # https://mapknitter.org/warps/yale-farm/yale-farm-geo.tif
   def geotiff
-    send_file 'public/warps/'+params[:id]+'/'+params[:id]+'-geo.tif'
+    send_file 'public/warps/' + params[:id] + '/' + params[:id] + '-geo.tif'
   end
 
   def cancel
-    @map = Map.find params[:id] 
+    @map = Map.find params[:id]
     if @map.anonymous? || logged_in?
       export = @map.export
       export.status = 'none'
       export.save
       if params[:exports]
-        flash[:notice] = "Export cancelled."
-        redirect_to "/exports"
+        flash[:notice] = 'Export cancelled.'
+        redirect_to '/exports'
       else
-        render :text => 'cancelled'
+        render text: 'cancelled'
       end
     else
-      render :text => 'You must be logged in to export, unless the map is anonymous.'
+      render text: 'You must be logged in to export, unless the map is anonymous.'
     end
   end
 
   def progress
-    map = Map.find params[:id] 
-    if export = map.export
+    map = Map.find params[:id]
+    export = map.export
+    if export.present?
       if  export.status == 'complete'
         output = 'complete'
       elsif export.status == 'none'
@@ -63,15 +64,15 @@ def progress
     else
       output = 'export has not been run'
     end
-    render :text => output, :layout => false 
+    render text: output, layout: false
   end
 
   def status
     map = Map.find(params[:id])
     if export = map.export
       render json: export.to_json
-    else      
+    else
       render json: { status: 'export has not been run' }
-    end    
+    end
   end
 end

app/controllers/feeds_controller.rb

@@ -1,56 +1,56 @@
 class FeedsController < ApplicationController
-
   def all
-    #(Warpable.all + Map.all).sort_by(&:created_at)
-    @maps = Map.find(:all, 
-      :order => "id DESC",:limit => 20, 
-      :conditions => {:archived => false, :password => ''},
-      :joins => [:user, :warpables], 
-      :group => "maps.id")
-    render :layout => false, :template => "feeds/all"
-    response.headers["Content-Type"] = "application/xml; charset=utf-8"
+    # (Warpable.all + Map.all).sort_by(&:created_at)
+    @maps = Map.find(:all, order: 'id DESC', limit: 20,
+                           conditions: { archived: false, password: '' },
+                           joins: %i[user warpables],
+                           group: 'maps.id')
+    render layout: false, template: 'feeds/all'
+    response.headers['Content-Type'] = 'application/xml; charset=utf-8'
   end
 
   def clean
     @maps = Map.order(id: :desc)
-      .limit(20)
-      .where(archived: false, password: '')
-      .joins(:warpables)
-      .group("maps.id")
-    render layout: false, template: "feeds/clean"
-    response.headers["Content-Type"] = "application/xml; charset=utf-8"
+               .limit(20)
+               .where(archived: false, password: '')
+               .joins(:warpables)
+               .group('maps.id')
+    render layout: false, template: 'feeds/clean'
+    response.headers['Content-Type'] = 'application/xml; charset=utf-8'
   end
 
   def license
-     @maps = Map.order(id: :desc)
-    .limit(20)
-    .where(archived: false, password: '', license: params[:id])
-    .joins(:warpables)
-    .group("maps.id")
-    render layout: false, template: "feeds/license"
-    response.headers["Content-Type"] = "application/xml; charset=utf-8"
+    @maps = Map.order(id: :desc)
+               .limit(20)
+               .where(archived: false, password: '', license: params[:id])
+               .joins(:warpables)
+               .group('maps.id')
+    render layout: false, template: 'feeds/license'
+    response.headers['Content-Type'] = 'application/xml; charset=utf-8'
   end
 
   def author
-    @maps = Map.find_all_by_author(params[:id],:order => "id DESC", :conditions => {:archived => false, :password => ''},:joins => :warpables, :group => "maps.id")
+    @maps = Map.find_all_by_author(params[:id],
+                                   order: 'id DESC',
+                                   conditions: { archived: false, password: '' },
+                                   joins: :warpables, group: 'maps.id')
     images = []
     @maps.each do |map|
-      images = images + map.warpables
+      images += map.warpables
     end
     @feed = (@maps + images).sort_by(&:created_at)
-    render :layout => false, :template => "feeds/author"
-    response.headers["Content-Type"] = "application/xml; charset=utf-8"
+    render layout: false, template: 'feeds/author'
+    response.headers['Content-Type'] = 'application/xml; charset=utf-8'
   end
 
   def tag
     @tag = Tag.find_by_name params[:id]
     if @tag
-      @maps = @tag.maps.paginate(:page => params[:page], :per_page => 24)
-      render :layout => false, :template => "feeds/tag"
-      response.headers["Content-Type"] = "application/xml; charset=utf-8"
+      @maps = @tag.maps.paginate(page: params[:page], per_page: 24)
+      render layout: false, template: 'feeds/tag'
+      response.headers['Content-Type'] = 'application/xml; charset=utf-8'
     else
       render text: "No maps with tag #{params[:id]}"
     end
   end
-
 end