From 83821816359187fa2b877a29b7ab7d9f974c17b5 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sun, 22 May 2011 17:02:48 +0100 Subject: [PATCH] Tidy up the note controller Go through most of the note controller, tidying things up, fixing a few bugs, and making sure we have tests for everything. --- app/controllers/note_controller.rb | 292 ++++++++++++++-------- app/models/note.rb | 15 +- test/functional/note_controller_test.rb | 316 +++++++++++++++++------- 3 files changed, 431 insertions(+), 192 deletions(-) diff --git a/app/controllers/note_controller.rb b/app/controllers/note_controller.rb index 486ce38d3..c46145045 100644 --- a/app/controllers/note_controller.rb +++ b/app/controllers/note_controller.rb @@ -5,7 +5,6 @@ class NoteController < ApplicationController before_filter :check_api_readable before_filter :authorize_web, :only => [:create, :close, :update, :delete, :mine] before_filter :check_api_writable, :only => [:create, :close, :update, :delete] - before_filter :require_moderator, :only => [:delete] before_filter :set_locale, :only => [:mine] after_filter :compress_output around_filter :api_call_handle_error, :api_call_timeout @@ -13,57 +12,68 @@ class NoteController < ApplicationController # 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 - bbox = params['bbox'] + # 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 - if bbox and bbox.count(',') == 3 - bbox = bbox.split(',') - @min_lon, @min_lat, @max_lon, @max_lat = sanitise_boundaries(bbox) + bbox = params[:bbox].split(",") else - #Fallback to old style, this is deprecated and should not be used - 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'] - - @min_lon = params['l'].to_f - @max_lon = params['r'].to_f - @min_lat = params['b'].to_f - @max_lat = params['t'].to_f + 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] ] end - limit = getLimit - conditions = closedCondition - + # 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 + + # Check that the boundaries are valid check_boundaries(@min_lon, @min_lat, @max_lon, @max_lat, MAX_NOTE_REQUEST_AREA) - @notes = Note.find_by_area(@min_lat, @min_lon, @max_lat, @max_lon, :include => :comments, :order => "updated_at DESC", :limit => limit, :conditions => conditions) + # 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) + # Render the result respond_to do |format| - format.html {render :template => 'note/list.rjs', :content_type => "text/javascript"} - format.rss {render :template => 'note/list.rss'} + format.html { render :format => :rjs, :content_type => "text/javascript" } + format.rss format.js - format.xml {render :template => 'note/list.xml'} - format.json { render :json => @notes.to_json(:methods => [:lat, :lon], :only => [:id, :status, :created_at], :include => { :comments => { :only => [:author_name, :created_at, :body]}}) } - format.gpx {render :template => 'note/list.gpx'} + format.xml + format.json { render :json => @notes.to_json } + format.gpx end end + ## + # Create a new note def create - raise OSM::APIBadUserInput.new("No lat was given") unless params['lat'] - raise OSM::APIBadUserInput.new("No lon was given") unless params['lon'] - raise OSM::APIBadUserInput.new("No text was given") unless params['text'] - - lon = params['lon'].to_f - lat = params['lat'].to_f - comment = params['text'] - - name = "NoName" - name = params['name'] if params['name'] - - #Include in a transaction to ensure that there is always a note_comment for every note + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No lat was given") unless params[:lat] + raise OSM::APIBadUserInput.new("No lon was given") unless params[:lon] + raise OSM::APIBadUserInput.new("No text was given") unless params[:text] + + # Extract the arguments + lon = params[:lon].to_f + lat = params[:lat].to_f + comment = params[:text] + name = params[:name] + + # Include in a transaction to ensure that there is always a note_comment for every note Note.transaction do + # Create the note @note = Note.create(:lat => lat, :lon => lon) raise OSM::APIBadUserInput.new("The note is outside this world") unless @note.in_world? @@ -81,116 +91,173 @@ class NoteController < ApplicationController @note.nearby_place = "unknown" end + # Save the note @note.save + # Add a comment to the note add_comment(@note, comment, name, "opened") end - + + # Send an OK response render_ok end + ## + # Add a comment to an existing note def update - raise OSM::APIBadUserInput.new("No id was given") unless params['id'] - raise OSM::APIBadUserInput.new("No text was given") unless params['text'] + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + raise OSM::APIBadUserInput.new("No text was given") unless params[:text] - name = "NoName" - name = params['name'] if params['name'] - - id = params['id'].to_i + # Extract the arguments + id = params[:id].to_i + comment = params[:text] + name = params[:name] or "NoName" + # Find the note and check it is valid note = Note.find(id) raise OSM::APINotFoundError unless note raise OSM::APIAlreadyDeletedError unless note.visible? + # Add a comment to the note Note.transaction do - add_comment(note, params['text'], name, "commented") + add_comment(note, comment, name, "commented") end + # Send an OK response render_ok end + ## + # Close a note def close - raise OSM::APIBadUserInput.new("No id was given") unless params['id'] - - id = params['id'].to_i - name = "NoName" - name = params['name'] if params['name'] + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + # Extract the arguments + id = params[:id].to_i + name = params[:name] + + # Find the note and check it is valid note = Note.find_by_id(id) raise OSM::APINotFoundError unless note raise OSM::APIAlreadyDeletedError unless note.visible? + # Close the note and add a comment Note.transaction do note.close - add_comment(note, :nil, name, "closed") + + add_comment(note, nil, name, "closed") end + # Send an OK response render_ok end + ## + # Get a feed of recent notes and comments def rss - limit = getLimit - conditions = closedCondition + # Get any conditions that need to be applied + conditions = closed_condition - # Figure out the bbox - bbox = params['bbox'] + # Process any bbox + if params[:bbox] + raise OSM::APIBadUserInput.new("Invalid bbox") unless params[:bbox].count(",") == 3 - if bbox and bbox.count(',') == 3 - bbox = bbox.split(',') - @min_lon, @min_lat, @max_lon, @max_lat = sanitise_boundaries(bbox) + @min_lon, @min_lat, @max_lon, @max_lat = sanitise_boundaries(params[:bbox].split(',')) 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)] + conditions = cond_merge conditions, [OSM.sql_for_area(@min_lat, @min_lon, @max_lat, @max_lon, "notes.")] end - @comments = NoteComment.find(:all, :limit => limit, :order => "created_at DESC", :joins => :note, :include => :note, :conditions => conditions) - render :template => 'note/rss.rss' + # Find the comments we want to return + @comments = NoteComment.find(:all, + :conditions => conditions, + :order => "created_at DESC", + :limit => result_limit, + :joins => :note, + :include => :note) + + # Render the result + respond_to do |format| + format.rss + end end + ## + # Read a note def read - @note = Note.find(params['id']) + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + + # Find the note and check it is valid + @note = Note.find(params[:id]) raise OSM::APINotFoundError unless @note raise OSM::APIAlreadyDeletedError unless @note.visible? + # Render the result respond_to do |format| - format.rss format.xml - format.json { render :json => @note.to_json(:methods => [:lat, :lon], :only => [:id, :status, :created_at], :include => { :comments => { :only => [:author_name, :created_at, :body]}}) } + format.rss + format.json { render :json => @note.to_json } format.gpx end end + ## + # Delete (hide) a note def delete - note = note.find(params['id']) + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + + # Extract the arguments + id = params[:id].to_i + name = params[:name] + + # Find the note and check it is valid + note = Note.find(id) raise OSM::APINotFoundError unless note raise OSM::APIAlreadyDeletedError unless note.visible? + # Mark the note as hidden Note.transaction do note.status = "hidden" note.save - add_comment(note, :nil, name, "hidden") + + add_comment(note, nil, name, "hidden") end + # Render the result render :text => "ok\n", :content_type => "text/html" end + ## + # Return a list of notes matching a given string def search - raise OSM::APIBadUserInput.new("No query string was given") unless params['q'] - limit = getLimit - conditions = closedCondition - conditions = cond_merge conditions, ['note_comments.body ~ ?', params['q']] + # Check the arguments are sane + 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]] - #TODO: There should be a better way to do this. CloseConditions are ignored at the moment + # 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 = Note.find(:all, :limit => limit, :order => "updated_at DESC", :joins => :comments, :include => :comments, :conditions => conditions).uniq + # Render the result respond_to do |format| - format.html {render :template => 'note/list.rjs', :content_type => "text/javascript"} - format.rss {render :template => 'note/list.rss'} + format.html { render :action => :list, :format => :rjs, :content_type => "text/javascript"} + format.rss { render :action => :list } format.js - format.xml {render :template => 'note/list.xml'} - format.json { render :json => @notes.to_json(:methods => [:lat, :lon], :only => [:id, :status, :created_at], :include => { :comments => { :only => [:author_name, :created_at, :body]}}) } - format.gpx {render :template => 'note/list.gpx'} + format.xml { render :action => :list } + format.json { render :json => @notes.to_json } + format.gpx { render :action => :list } end end @@ -252,32 +319,41 @@ private end end + ## + # Render an OK response def render_ok - output_js = :false - output_js = :true if params['format'] == "js" - - if output_js == :true + if params[:format] == "js" render :text => "osbResponse();", :content_type => "text/javascript" else - render :text => "ok " + @note.id.to_s + "\n", :content_type => "text/html" if @note - render :text => "ok\n", :content_type => "text/html" unless @note + render :text => "ok " + @note.id.to_s + "\n", :content_type => "text/plain" if @note + render :text => "ok\n", :content_type => "text/plain" unless @note end end - def getLimit - limit = 100 - limit = params['limit'] if ((params['limit']) && (params['limit'].to_i < 10000) && (params['limit'].to_i > 0)) - return limit + ## + # Get the maximum number of results to return + def result_limit + if params[:limit] and params[:limit].to_i > 0 and params[:limit].to_i < 10000 + params[:limit].to_i + else + 100 + end end - def closedCondition - closed_since = 7 unless params['closed'] - closed_since = params['closed'].to_i if params['closed'] + ## + # Generate a condition to choose which bugs we want based + # on their status and the user's request parameters + def closed_condition + if params[:closed] + closed_since = params[:closed].to_i + else + closed_since = 7 + end if closed_since < 0 conditions = ["status != 'hidden'"] elsif closed_since > 0 - conditions = ["((status = 'open') OR ((status = 'closed' ) AND (closed_at > '" + (Time.now - closed_since.days).to_s + "')))"] + conditions = ["(status = 'open' OR (status = 'closed' AND closed_at > '#{Time.now - closed_since.days}'))"] else conditions = ["status = 'open'"] end @@ -285,26 +361,26 @@ private return conditions end - def add_comment(note, text, name, event) - comment = note.comments.create(:visible => true, :event => event) - comment.body = text unless text == :nil + ## + # Add a comment to a note + def add_comment(note, text, name, event) + name = "NoName" if name.nil? + + attributes = { :visible => true, :event => event, :body => text } + if @user - comment.author_id = @user.id - comment.author_name = @user.display_name + attributes[:author_id] = @user.id + attributes[:author_name] = @user.display_name else - comment.author_ip = request.remote_ip - comment.author_name = name + " (a)" + attributes[:author_ip] = request.remote_ip + attributes[:author_name] = name + " (a)" end - comment.save - note.save - - sent_to = Set.new - note.comments.each do | cmt | - if cmt.author - unless sent_to.include?(cmt.author) - Notifier.deliver_note_comment_notification(note_comment, cmt.author) unless cmt.author == @user - sent_to.add(cmt.author) - end + + note.comments.create(attributes) + + note.comments.map { |c| c.author }.uniq.each do |user| + if user and user != @user + Notifier.deliver_note_comment_notification(comment, user) end end end diff --git a/app/models/note.rb b/app/models/note.rb index 0512639e1..892ada1aa 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -4,7 +4,7 @@ class Note < ActiveRecord::Base has_many :comments, :class_name => "NoteComment", :foreign_key => :note_id, :order => :created_at, - :conditions => "visible = true AND body IS NOT NULL" + :conditions => { :visible => true } validates_presence_of :id, :on => :update validates_uniqueness_of :id @@ -72,4 +72,17 @@ class Note < ActiveRecord::Base def author_name self.comments.first.author_name end + + # Custom JSON output routine for notes + def to_json(options = {}) + super options.reverse_merge( + :methods => [ :lat, :lon ], + :only => [ :id, :status, :created_at ], + :include => { + :comments => { + :only => [ :event, :author_name, :created_at, :body ] + } + } + ) + end end diff --git a/test/functional/note_controller_test.rb b/test/functional/note_controller_test.rb index 955f1c24c..2e4a01b3e 100644 --- a/test/functional/note_controller_test.rb +++ b/test/functional/note_controller_test.rb @@ -3,7 +3,7 @@ require File.dirname(__FILE__) + '/../test_helper' class NoteControllerTest < ActionController::TestCase fixtures :users, :notes, :note_comments - def test_map_note_create_success + def test_note_create_success assert_difference('Note.count') do assert_difference('NoteComment.count') do post :create, {:lat => -1.0, :lon => -1.0, :name => "new_tester", :text => "This is a comment"} @@ -11,122 +11,304 @@ class NoteControllerTest < ActionController::TestCase end assert_response :success id = @response.body.sub(/ok/,"").to_i + get :read, {:id => id, :format => 'json'} assert_response :success - js = @response.body - assert_match "\"status\":\"open\"", js - assert_match "\"body\":\"This is a comment\"", js - assert_match "\"author_name\":\"new_tester (a)\"", js + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal id, js["note"]["id"] + assert_equal "open", js["note"]["status"] + assert_equal "opened", js["note"]["comments"].last["event"] + assert_equal "This is a comment", js["note"]["comments"].last["body"] + assert_equal "new_tester (a)", js["note"]["comments"].last["author_name"] end - def test_map_note_comment_create_success + def test_note_create_fail + assert_no_difference('Note.count') do + assert_no_difference('NoteComment.count') do + post :create, {:lon => -1.0, :name => "new_tester", :text => "This is a comment"} + end + end + assert_response :bad_request + + assert_no_difference('Note.count') do + assert_no_difference('NoteComment.count') do + post :create, {:lat => -1.0, :name => "new_tester", :text => "This is a comment"} + end + end + assert_response :bad_request + + assert_no_difference('Note.count') do + assert_no_difference('NoteComment.count') do + post :create, {:lat => -1.0, :lon => -1.0, :name => "new_tester"} + end + end + assert_response :bad_request + + assert_no_difference('Note.count') do + assert_no_difference('NoteComment.count') do + post :create, {:lat => -100.0, :lon => -1.0, :name => "new_tester", :text => "This is a comment"} + end + end + assert_response :bad_request + + assert_no_difference('Note.count') do + assert_no_difference('NoteComment.count') do + post :create, {:lat => -1.0, :lon => -200.0, :name => "new_tester", :text => "This is a comment"} + end + end + assert_response :bad_request + end + + def test_note_comment_create_success assert_difference('NoteComment.count') do - post :update, {:id => 2, :name => "new_tester2", :text => "This is an additional comment"} + post :update, {:id => notes(:open_note_with_comment).id, :name => "new_tester2", :text => "This is an additional comment"} end assert_response :success - get :read, {:id => 2, :format => 'json'} + get :read, {:id => notes(:open_note_with_comment).id, :format => 'json'} assert_response :success - js = @response.body - assert_match "\"id\":2", js - assert_match "\"status\":\"open\"", js - assert_match "\"body\":\"This is an additional comment\"", js - assert_match "\"author_name\":\"new_tester2 (a)\"", js + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal notes(:open_note_with_comment).id, js["note"]["id"] + assert_equal "open", js["note"]["status"] + assert_equal "commented", js["note"]["comments"].last["event"] + assert_equal "This is an additional comment", js["note"]["comments"].last["body"] + assert_equal "new_tester2 (a)", js["note"]["comments"].last["author_name"] end - def test_map_note_read_success - get :read, {:id => 1} + def test_note_comment_create_fail + assert_no_difference('NoteComment.count') do + post :update, {:name => "new_tester2", :text => "This is an additional comment"} + end + assert_response :bad_request + + assert_no_difference('NoteComment.count') do + post :update, {:id => notes(:open_note_with_comment).id, :name => "new_tester2"} + end + assert_response :bad_request + + assert_no_difference('NoteComment.count') do + post :update, {:id => 12345, :name => "new_tester2", :text => "This is an additional comment"} + end + assert_response :not_found + + assert_no_difference('NoteComment.count') do + post :update, {:id => notes(:hidden_note_with_comment).id, :name => "new_tester2", :text => "This is an additional comment"} + end + assert_response :gone + end + + def test_note_close_success + post :close, {:id => notes(:open_note_with_comment).id} + assert_response :success + + get :read, {:id => notes(:open_note_with_comment).id, :format => 'json'} + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal notes(:open_note_with_comment).id, js["note"]["id"] + assert_equal "closed", js["note"]["status"] + assert_equal "closed", js["note"]["comments"].last["event"] + assert_equal "NoName (a)", js["note"]["comments"].last["author_name"] + end + + def test_note_close_fail + post :close + assert_response :bad_request + + post :close, {:id => 12345} + assert_response :not_found + + post :close, {:id => notes(:hidden_note_with_comment).id} + assert_response :gone + end + + def test_note_read_success + get :read, {:id => notes(:open_note).id} assert_response :success + assert_equal "application/xml", @response.content_type - get :read, {:id => 1, :format => 'xml'} + get :read, {:id => notes(:open_note).id, :format => "xml"} assert_response :success + assert_equal "application/xml", @response.content_type - get :read, {:id => 1, :format => 'rss'} + get :read, {:id => notes(:open_note).id, :format => "rss"} assert_response :success + assert_equal "application/rss+xml", @response.content_type - get :read, {:id => 1, :format => 'json'} + get :read, {:id => notes(:open_note).id, :format => "json"} assert_response :success + assert_equal "application/json", @response.content_type - get :read, {:id => 1, :format => 'gpx'} + get :read, {:id => notes(:open_note).id, :format => "gpx"} assert_response :success + assert_equal "application/gpx+xml", @response.content_type end - def test_map_note_close_success - post :close, {:id => 2} + def test_note_read_hidden_comment + get :read, {:id => notes(:note_with_hidden_comment).id, :format => 'json'} assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal notes(:note_with_hidden_comment).id, js["note"]["id"] + assert_equal 2, js["note"]["comments"].count + assert_equal "Valid comment for note 5", js["note"]["comments"][0]["body"] + assert_equal "Another valid comment for note 5", js["note"]["comments"][1]["body"] + end - get :read, {:id => 2, :format => 'json'} - js = @response.body - assert_match "\"id\":2", js - assert_match "\"status\":\"closed\"", js + def test_note_read_fail + post :read + assert_response :bad_request + + get :read, {:id => 12345} + assert_response :not_found + + get :read, {:id => notes(:hidden_note_with_comment).id} + assert_response :gone + end + + def test_note_delete_success + delete :delete, {:id => notes(:open_note_with_comment).id} + assert_response :success + + get :read, {:id => notes(:open_note_with_comment).id, :format => 'json'} + assert_response :gone + end + + def test_note_delete_fail + delete :delete + assert_response :bad_request + + delete :delete, {:id => 12345} + assert_response :not_found + + delete :delete, {:id => notes(:hidden_note_with_comment).id} + assert_response :gone end def test_get_notes_success - get :list, {:bbox=>'1,1,1.2,1.2'} + get :list, {:bbox => '1,1,1.2,1.2'} assert_response :success + assert_equal "text/javascript", @response.content_type - get :list, {:bbox=>'1,1,1.2,1.2', :format => 'rss'} + get :list, {:bbox => '1,1,1.2,1.2', :format => 'rss'} assert_response :success + assert_equal "application/rss+xml", @response.content_type - get :list, {:bbox=>'1,1,1.2,1.2', :format => 'json'} + get :list, {:bbox => '1,1,1.2,1.2', :format => 'json'} assert_response :success + assert_equal "application/json", @response.content_type - get :list, {:bbox=>'1,1,1.2,1.2', :format => 'xml'} + get :list, {:bbox => '1,1,1.2,1.2', :format => 'xml'} assert_response :success + assert_equal "application/xml", @response.content_type - get :list, {:bbox=>'1,1,1.2,1.2', :format => 'gpx'} + get :list, {:bbox => '1,1,1.2,1.2', :format => 'gpx'} assert_response :success + assert_equal "application/gpx+xml", @response.content_type end - def test_get_notes_large_area_success - get :list, {:bbox=>'-2.5,-2.5,2.5,2.5'} + def test_get_notes_large_area + get :list, {:bbox => '-2.5,-2.5,2.5,2.5'} assert_response :success - end - def test_get_notes_large_area_bad_request - get :list, {:bbox=>'-10,-10,12,12'} + get :list, {:l => '-2.5', :b => '-2.5', :r => '2.5', :t => '2.5'} + assert_response :success + + get :list, {:bbox => '-10,-10,12,12'} + assert_response :bad_request + + get :list, {:l => '-10', :b => '-10', :r => '12', :t => '12'} assert_response :bad_request end - def test_get_notes_closed_7_success - get :list, {:bbox=>'1,1,1.2,1.2', :closed => '7'} + def test_get_notes_closed + get :list, {:bbox=>'1,1,1.7,1.7', :closed => '7', :format => 'json'} assert_response :success - end + assert_equal "application/json", @response.content_type + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 4, js.count - def test_get_notes_closed_0_success - get :list, {:bbox=>'1,1,1.2,1.2', :closed => '0'} + get :list, {:bbox=>'1,1,1.7,1.7', :closed => '0', :format => 'json'} assert_response :success - end + assert_equal "application/json", @response.content_type + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 4, js.count - def test_get_notes_closed_n1_success - get :list, {:bbox=>'1,1,1.2,1.2', :closed => '-1'} + get :list, {:bbox=>'1,1,1.7,1.7', :closed => '-1', :format => 'json'} assert_response :success + assert_equal "application/json", @response.content_type + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal 6, js.count end + def test_get_notes_bad_params + get :list, {:bbox => '-2.5,-2.5,2.5'} + assert_response :bad_request + + get :list, {:bbox => '-2.5,-2.5,2.5,2.5,2.5'} + assert_response :bad_request + + get :list, {:b => '-2.5', :r => '2.5', :t => '2.5'} + assert_response :bad_request + + get :list, {:l => '-2.5', :r => '2.5', :t => '2.5'} + assert_response :bad_request + + get :list, {:l => '-2.5', :b => '-2.5', :t => '2.5'} + assert_response :bad_request + + get :list, {:l => '-2.5', :b => '-2.5', :r => '2.5'} + assert_response :bad_request + end def test_search_success - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'note 1'} + get :search, {:q => 'note 1'} assert_response :success + assert_equal "text/javascript", @response.content_type - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'note 1', :format => 'xml'} + get :search, {:q => 'note 1', :format => 'xml'} assert_response :success + assert_equal "application/xml", @response.content_type - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'note 1', :format => 'json'} + get :search, {:q => 'note 1', :format => 'json'} assert_response :success + assert_equal "application/json", @response.content_type - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'note 1', :format => 'rss'} + get :search, {:q => 'note 1', :format => 'rss'} assert_response :success + assert_equal "application/rss+xml", @response.content_type - get :search, {:bbox=>'1,1,1.2,1.2', :q => 'note 1', :format => 'gpx'} + get :search, {:q => 'note 1', :format => 'gpx'} assert_response :success + assert_equal "application/gpx+xml", @response.content_type + end + + def test_search_bad_params + get :search + assert_response :bad_request end def test_rss_success - get :rss, {:bbox=>'1,1,1.2,1.2'} - assert_response :success - get :rss assert_response :success + assert_equal "application/rss+xml", @response.content_type + + get :rss, {:bbox=>'1,1,1.2,1.2'} + assert_response :success + assert_equal "application/rss+xml", @response.content_type + end + + def test_rss_fail + get :rss, {:bbox=>'1,1,1.2'} + assert_response :bad_request + + get :rss, {:bbox=>'1,1,1.2,1.2,1.2'} + assert_response :bad_request end def test_user_notes_success @@ -139,36 +321,4 @@ class NoteControllerTest < ActionController::TestCase get :mine, {:display_name=>'non-existent'} assert_response :not_found end - - def test_map_note_comment_create_not_found - assert_no_difference('NoteComment.count') do - post :update, {:id => 12345, :name => "new_tester", :text => "This is an additional comment"} - end - assert_response :not_found - end - - def test_map_note_close_not_found - post :close, {:id => 12345} - assert_response :not_found - end - - def test_map_note_read_not_found - get :read, {:id => 12345} - assert_response :not_found - end - - def test_map_note_read_gone - get :read, {:id => 4} - assert_response :gone - end - - def test_map_note_hidden_comment - get :read, {:id => 5, :format => 'json'} - assert_response :success - js = @response.body - assert_match "\"id\":5", js - assert_match "\"body\":\"Valid comment for note 5\"", js - assert_match "\"body\":\"Another valid comment for note 5\"", js - assert_no_match /\"body\":\"Spam for note 5\"/, js - end end -- 2.39.5