]> git.openstreetmap.org Git - rails.git/blobdiff - app/controllers/note_controller.rb
Fix some bugs found by the note controller tests
[rails.git] / app / controllers / note_controller.rb
index c461450451375d217e8652f0839e9506e119b39f..32acea89f48175a846a4c903e321750601754021 100644 (file)
@@ -9,50 +9,39 @@ class NoteController < ApplicationController
   after_filter :compress_output
   around_filter :api_call_handle_error, :api_call_timeout
 
-  # Help methods for checking boundary sanity and area size
-  include MapBoundary
-
   ##
   # Return a list of notes in a given area
   def list
     # Figure out the bbox - we prefer a bbox argument but also
     # support the old, deprecated, method with four arguments
     if params[:bbox]
-      raise OSM::APIBadUserInput.new("Invalid bbox") unless params[:bbox].count(",") == 3
-
-      bbox = params[:bbox].split(",")
+      bbox = BoundingBox.from_bbox_params(params)
     else
       raise OSM::APIBadUserInput.new("No l was given") unless params[:l]
       raise OSM::APIBadUserInput.new("No r was given") unless params[:r]
       raise OSM::APIBadUserInput.new("No b was given") unless params[:b]
       raise OSM::APIBadUserInput.new("No t was given") unless params[:t]
 
-      bbox = [ params[:l], params[:b], params[:r], params[:t] ]
+      bbox = BoundingBox.from_lrbt_params(params)
     end
 
-    # Get the sanitised boundaries
-    @min_lon, @min_lat, @max_lon, @max_lat = sanitise_boundaries(bbox)
-
     # Get any conditions that need to be applied
-    conditions = closed_condition
+    notes = closed_condition(Note.scoped)
 
     # Check that the boundaries are valid
-    check_boundaries(@min_lon, @min_lat, @max_lon, @max_lat, MAX_NOTE_REQUEST_AREA)
+    bbox.check_boundaries
+
+    # Check the the bounding box is not too big
+    bbox.check_size(MAX_NOTE_REQUEST_AREA)
 
     # Find the notes we want to return
-    @notes = Note.find_by_area(@min_lat, @min_lon, @max_lat, @max_lon,
-                               :include => :comments, 
-                               :conditions => conditions,
-                               :order => "updated_at DESC", 
-                               :limit => result_limit)
+    @notes = notes.bbox(bbox).order("updated_at DESC").limit(result_limit).preload(:comments)
 
     # Render the result
     respond_to do |format|
-      format.html { render :format => :rjs, :content_type => "text/javascript" }
       format.rss
-      format.js
       format.xml
-      format.json { render :json => @notes.to_json }
+      format.json
       format.gpx
     end
   end
@@ -92,7 +81,7 @@ class NoteController < ApplicationController
       end
 
       # Save the note
-      @note.save
+      @note.save!
 
       # Add a comment to the note
       add_comment(@note, comment, name, "opened")
@@ -158,26 +147,20 @@ class NoteController < ApplicationController
   # Get a feed of recent notes and comments
   def rss
     # Get any conditions that need to be applied
-    conditions = closed_condition
+    notes = closed_condition(Note.scoped)
 
     # Process any bbox
     if params[:bbox]
-      raise OSM::APIBadUserInput.new("Invalid bbox") unless params[:bbox].count(",") == 3
+      bbox = BoundingBox.from_bbox_params(params)
 
-      @min_lon, @min_lat, @max_lon, @max_lat = sanitise_boundaries(params[:bbox].split(','))
+      bbox.check_boundaries
+      bbox.check_size(MAX_NOTE_REQUEST_AREA)
 
-      check_boundaries(@min_lon, @min_lat, @max_lon, @max_lat, MAX_NOTE_REQUEST_AREA)
-
-      conditions = cond_merge conditions, [OSM.sql_for_area(@min_lat, @min_lon, @max_lat, @max_lon, "notes.")]
+      notes = notes.bbox(bbox)
     end
 
     # Find the comments we want to return
-    @comments = NoteComment.find(:all, 
-                                 :conditions => conditions,
-                                 :order => "created_at DESC",
-                                 :limit => result_limit,
-                                 :joins => :note, 
-                                 :include => :note)
+    @comments = NoteComment.where(:note_id => notes).order("created_at DESC").limit(result_limit).preload(:note)
 
     # Render the result
     respond_to do |format|
@@ -200,7 +183,7 @@ class NoteController < ApplicationController
     respond_to do |format|
       format.xml
       format.rss
-      format.json { render :json => @note.to_json }
+      format.json
       format.gpx
     end
   end
@@ -239,24 +222,17 @@ class NoteController < ApplicationController
     raise OSM::APIBadUserInput.new("No query string was given") unless params[:q]
 
     # Get any conditions that need to be applied
-    conditions = closed_condition
-    conditions = cond_merge conditions, ['note_comments.body ~ ?', params[:q]]
-       
+    @notes = closed_condition(Note.scoped)
+    @notes = @notes.joins(:comments).where("note_comments.body ~ ?", params[:q])
+
     # Find the notes we want to return
-    @notes = Note.find(:all, 
-                       :conditions => conditions,
-                       :order => "updated_at DESC",
-                       :limit => result_limit,
-                       :joins => :comments,
-                       :include => :comments)
+    @notes = @notes.order("updated_at DESC").limit(result_limit).preload(:comments)
 
     # Render the result
     respond_to do |format|
-      format.html { render :action => :list, :format => :rjs, :content_type => "text/javascript"}
       format.rss { render :action => :list }
-      format.js
       format.xml { render :action => :list }
-      format.json { render :json => @notes.to_json }
+      format.json { render :action => :list }
       format.gpx { render :action => :list }
     end
   end
@@ -304,21 +280,6 @@ private
   # utility functions below. 
   #------------------------------------------------------------   
  
-  ## 
-  # merge two conditions 
-  # TODO: this is a copy from changeset_controler.rb and should be factored out to share
-  def cond_merge(a, b) 
-    if a and b 
-      a_str = a.shift 
-      b_str = b.shift 
-      return [ a_str + " AND " + b_str ] + a + b 
-    elsif a  
-      return a 
-    else b 
-      return b 
-    end 
-  end 
-
   ##
   # Render an OK response
   def render_ok
@@ -343,7 +304,7 @@ private
   ##
   # Generate a condition to choose which bugs we want based
   # on their status and the user's request parameters
-  def closed_condition
+  def closed_condition(notes)
     if params[:closed]
       closed_since = params[:closed].to_i
     else
@@ -351,14 +312,14 @@ private
     end
        
     if closed_since < 0
-      conditions = ["status != 'hidden'"]
+      notes = notes.where("status != 'hidden'")
     elsif closed_since > 0
-      conditions = ["(status = 'open' OR (status = 'closed' AND closed_at > '#{Time.now - closed_since.days}'))"]
+      notes = notes.where("(status = 'open' OR (status = 'closed' AND closed_at > '#{Time.now - closed_since.days}'))")
     else
-      conditions = ["status = 'open'"]
+      notes = notes.where("status = 'open'")
     end
 
-    return conditions
+    return notes
   end
 
   ##
@@ -376,7 +337,7 @@ private
       attributes[:author_name] = name + " (a)"
     end
 
-    note.comments.create(attributes)
+    note.comments.create(attributes, :without_protection => true)
 
     note.comments.map { |c| c.author }.uniq.each do |user|
       if user and user != @user