Backend search constraints (length & whitespace) (#630)

* refactor search * delete commented lines * resolve code climate * add flash notice * use request.referrer to reference last page * fix rubocop * refactorings * fix rubocop * more refactorings * fix more rubocop * squeeze center whitespace * strip leading and trailing whitespace * refactor map.rb to fix rubocop class over 250 lines * fix offences * fix cops * offence fix * fix last robocop * fix cop * cop * fix redundant self * remove redunant self pt2 * undo new asset path * tag fix
publiclab · May 30, 2019 · 6c79669 · 6c79669
1 parent 275c2a3
commit 6c79669
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 91 deletions.
diff --git a/app/controllers/maps_controller.rb b/app/controllers/maps_controller.rb
@@ -173,26 +173,27 @@ def license
 
   def featured
     @title = 'Featured maps'
-    @maps = Map.joins(:warpables)
-               .select('maps.*, count(maps.id) as image_count')
-               .group('warpables.map_id')
-               .order('image_count DESC')
-               .paginate(page: params[:page], per_page: 24)
+    @maps = Map.featured.paginate(page: params[:page], per_page: 24)
     render 'maps/index', layout: 'application'
   end
 
-  def search
-    params[:id] ||= params[:q]
-    @maps = Map.select('archived, author, created_at, description, id, lat, license, location, name, slug,
-                        tile_layer, tile_url, tiles, updated_at, user_id, version, zoom')
-               .where('archived = ? AND (author LIKE ? OR name LIKE ? OR location LIKE ? OR description LIKE ?)',
-                      false, '%' + params[:id] + '%', '%' + params[:id] + '%', '%' + params[:id] + '%', '%' + params[:id] + '%')
-               .paginate(page: params[:page], per_page: 24)
-    @title = "Search results for '#{params[:id]}'"
+  def search    
+    data = params[:q]
+    query = params[:q].gsub(/\s+/, '')
+
     respond_to do |format|
-      format.html { render 'maps/index', layout: 'application' }
-      format.json { render json: @maps }
-    end
+      if query.length < 3
+        flash.now[:notice] = 'Invalid Query: non white-space character count is less than 3'
+        @title = 'Featured maps'
+        @maps = Map.featured.paginate(page: params[:page], per_page: 24)
+        format.html { render 'maps/index', layout: 'application' }
+      else
+        @title = "Search results for '#{data}'"
+        @maps = Map.search(data).paginate(page: params[:page], per_page: 24)
+        format.html { render 'maps/index', layout: 'application' }
+        format.json { render json: @maps }
+      end
+    end    
   end
 
   private

diff --git a/app/models/map.rb b/app/models/map.rb
@@ -1,29 +1,30 @@
-require 'open3'
-
-class NotAtOriginValidator < ActiveModel::Validator
-  def validate(record)
-    if record.lat == 0 || record.lon == 0
-      record.errors[:base] << "Your location at 0,0 is unlikely."
-    end
-  end
-end
-
 class Map < ActiveRecord::Base
+  include ActiveModel::Validations
   extend FriendlyId
   friendly_id :name, :use => [:slugged, :static]
 
-  attr_accessible :author, :name, :slug, :lat, :lon, :location, :description, :zoom, :license
+  attr_accessible :author, :name, :slug, :lat, :lon, 
+                  :location, :description, :zoom, :license
   attr_accessor :image_urls
 
-  validates_presence_of :name, :slug, :author, :lat, :lon
-  validates_uniqueness_of :slug
-  validates_presence_of :location, :message => ' cannot be found. Try entering a latitude and longitude if this problem persists.'
-  validates_format_of   :slug,
-                        :with => /^[\w-]*$/,
-                        :message => " must not include spaces and must be alphanumeric, as it'll be used in the URL of your map, like: https://mapknitter.org/maps/your-map-name. You may use dashes and underscores.",
-                        :on => :create
-#  validates_format_of :tile_url, :with => /^(http|https):\/\/[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,5}(:[0-9]{1,5})?(\/.*)?$/ix
-  validates_with NotAtOriginValidator
+  validates :name, :slug, :author, :lat, :lon, presence: true
+
+  validates :slug, format: { 
+    with: /^[\w-]*$/, 
+    message: "must only include permitted URL safe character types: 
+              alphanumerics, dashes, and underscores. It will be in your map's 
+              URL path (i.e., https://mapknitter.org/maps/your-map-name)." 
+  }, uniqueness: true, on: :create
+
+  validates :location, presence: { 
+    message: ' cannot be found. 
+              Try entering a latitude and longitude if this problem persists.'
+  }
+  #  validates :tile_url, format { with:
+  #   /^(http|https):\/\/[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.
+  #   [a-z]{2,5}(:[0-9]{1,5})?(\/.*)?$/ix
+  # }
+  validates :lat, :lon, NotAtOrigin: true
 
   has_many :exports, :dependent => :destroy
   has_many :tags, :dependent => :destroy
@@ -72,7 +73,8 @@ def anonymous?
   end
 
   def self.bbox(minlat, minlon, maxlat, maxlon)
-    Map.where(['lat > ? AND lat < ? AND lon > ? AND lon < ?', minlat, maxlat, minlon, maxlon])
+    Map.where(['lat > ? AND lat < ? AND lon > ? AND lon < ?', 
+               minlat, maxlat, minlon, maxlon])
   end
 
   def exporting?
@@ -88,21 +90,67 @@ def latest_export
   end
 
   def self.authors(limit = 50)
-    Map.limit(limit)
+    Map.where(archived: false, password: '')
+       .limit(limit)
        .order("maps.id DESC")
-       .where('password = "" AND archived = "false"')
        .collect(&:author)
-#       .group("maps.author")
+  end
+
+  def self.search(q)
+    q = q.squeeze(' ').strip
+    Map.active
+       .where(['author LIKE ? OR name LIKE ? 
+                OR location LIKE ? OR description LIKE ?',
+               "%#{q}%", "%#{q}%", "%#{q}%", "%#{q}%"])
+  end
+
+  def self.featured
+    Map.joins(:warpables)
+       .select('maps.*, count(maps.id) as image_count')
+       .group('warpables.map_id')
+       .order('image_count DESC')       
   end
 
   def self.new_maps
-    self.find(:all, :order => "created_at DESC", :limit => 12, :conditions => ['password = "" AND archived = "false"'])
+    Map.find(
+      :all, 
+      order: 'created_at DESC', 
+      limit: 12, 
+      conditions: ['password = "" AND archived = "false"']
+    )
+  end
+
+  def self.map
+    Map.where(archived: false, password: '')
+       .select('author, maps.name, lat, lon, slug, archived, password,
+               users.login as user_login')
+       .joins(:warpables, :user)
+       .group('maps.id')
+  end
+
+  def self.featured_authors
+    maps = Map.active.has_user
+
+    author_counts = maps.group('author')
+                        .select('user_id, author, count(1) as maps_count')
+                        .order('maps_count DESC')
+
+    author_counts.map do |a|
+      user = User.find(a.user_id)
+      { user: user, count: a.maps_count, location: user.maps.first.location }
+    end
+  end
+
+  def self.maps_nearby(lat:, lon:, dist:)
+    Map.active
+       .where(['lat>? AND lat<? AND lon>? AND lon<?',
+               lat-dist, lat+dist, lon-dist, lon+dist])
   end
 
   def nodes
     nodes = {}
     self.warpables.each do |warpable|
-      if warpable.nodes != ''
+      if warpable.nodes
         w_nodes = []
         warpable.nodes.split(',').each do |node|
           node_obj = Node.find(node)
@@ -117,11 +165,15 @@ def nodes
 
   # find all other maps within <dist> degrees lat or lon
   def nearby_maps(dist)
-    if self.lat.to_f == 0.0 || self.lon.to_f == 0.0
-      return []
-    else
-      return Map.find(:all,:conditions => ['id != ? AND lat > ? AND lat < ? AND lon > ? AND lon < ?',self.id,self.lat-dist,self.lat+dist,self.lon-dist,self.lon+dist], :limit => 10)
-    end
+    return [] if self.lat.to_f == 0.0 || self.lon.to_f == 0.0
+    Map.find(
+      :all, 
+      limit: 10,
+      conditions: [
+        'id != ? AND lat > ? AND lat < ? AND lon > ? AND lon < ?',
+        self.id,self.lat-dist,self.lat+dist,self.lon-dist,self.lon+dist
+      ]
+    )
   end
 
   def average_scale
@@ -149,26 +201,22 @@ def best_cm_per_pixel
       scores[i] += hist[i+3] if i < hist.length - 4
     end
     highest = 0
-    scores.each_with_index do |s,i|
-      highest = i if s > scores[highest]
-    end
+    scores.each_with_index { |s, i| highest = i if s > scores[highest] }
     highest
   end
 
   def average_cm_per_pixel
     if self.warpables.length > 0
       scales = []
       count = 0
-      average = 0
       self.placed_warpables.each do |warpable|
         count += 1
         res = warpable.cm_per_pixel
         res = 1 if res == 0 # let's not ever try to go for infinite resolution
         scales << res unless res == nil
       end
       total_sum = (scales.inject {|sum, n| sum + n }) if scales
-      average = total_sum/count if total_sum
-      average
+      return total_sum/count if total_sum
     else
       0
     end
@@ -208,11 +256,9 @@ def grouped_images_histogram(binsize)
   # we'll eventually replace this with a JavaScript call to initiate an external export process:
   def run_export(user, resolution)
     key = APP_CONFIG ? APP_CONFIG["google_maps_api_key"] : "AIzaSyAOLUQngEmJv0_zcG1xkGq-CXIPpLQY8iQ"
-    unless export
-      new_export = Export.new({
-        :map_id => id
-      })
-    end    
+
+    new_export = Export.new(map_id: id) unless export
+
     Exporter.run_export(user,
       resolution,
       self.export || new_export,
@@ -246,37 +292,7 @@ def has_tag(tagname)
   def add_tag(tagname, user)
     tagname = tagname.downcase
     unless self.has_tag(tagname)
-      self.tags.create({
-        :name => tagname,
-        :user_id => user.id,
-        :map_id => self.id
-      })
+      self.tags.create(name: tagname, user_id: user.id, map_id: id)
     end
   end
-
-  def self.map
-    Map.where(archived: false, password: '')
-       .select('author, maps.name, lat, lon, slug, archived, password,
-               users.login as user_login')
-       .joins(:warpables, :user)
-       .group('maps.id')
-  end
-
-  def self.featured_authors
-    maps = Map.active.has_user
-
-    author_counts = maps.group('author')
-                        .select('user_id, author, count(1) as maps_count')
-                        .order('maps_count DESC')
-
-    author_counts.map do |a|
-      user = User.find(a.user_id)
-      { user: user, count: a.maps_count, location: user.maps.first.location }
-    end
-  end
-
-  def self.maps_nearby(lat:, lon:, dist:)
-    Map.active
-       .where('lat>? AND lat<? AND lon>? AND lon<?',lat-dist, lat+dist, lon-dist, lon+dist)
-  end
 end
diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb
@@ -6,7 +6,7 @@
 
   <div class="collapse navbar-collapse" id="header-navbar-collapse">
     <ul class="navbar-nav">
-      <form action="/search" method='get' class="navbar-form navbar-left">
+      <form action="/search" method='get' class="navbar-form navbar-left" autocomplete="off">
         <div class="input-group" style="min-width: 160px;">
           <input class="form-control bg-light" id="search-input" type="text" name='q' placeholder="Search maps" value='<%= params[:id] if params[:action] == "search" %>'>
           <span class="input-group-append">
@@ -34,3 +34,4 @@
     </ul>
   </div>
 </nav>
+
diff --git a/config/application.rb b/config/application.rb
@@ -17,7 +17,7 @@ class Application < Rails::Application
 
     # Custom directories with classes and modules you want to be autoloadable.
     # config.autoload_paths += %W(#{config.root}/extras)
-    config.autoload_paths += %W(#{config.root}/lib)
+    config.autoload_paths += %W(#{config.root}/lib/)
 
     # Only load the plugins named here, in the order given (default is alphabetical).
     # :all can be used as a placeholder for all plugins not explicitly named.

diff --git a/lib/not_at_origin_validator.rb b/lib/not_at_origin_validator.rb
@@ -0,0 +1,12 @@
+require 'open3'
+
+# custom validator used in map.rb
+class NotAtOriginValidator < ActiveModel::Validator
+  def validate(rec)
+    rec.errors[:base] << '0,0 is an unlikely locale.' if null_island?(rec)
+  end
+
+  def null_island?(rec)
+    rec.lat.zero? || rec.lon.zero?
+  end
+end