From d74d4f8d195673250c3f2e841d28f185d74d8849 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Mon, 10 Jun 2013 18:39:52 +0100 Subject: [PATCH] Add a reopen API call for notes --- app/controllers/notes_controller.rb | 36 +++++++++++++++-- app/models/note.rb | 7 ++++ app/views/notes/_note.gpx.builder | 9 ++++- app/views/notes/_note.json.jsonify | 10 ++++- app/views/notes/_note.xml.builder | 10 ++++- config/routes.rb | 1 + lib/osm.rb | 17 ++++++++ test/functional/notes_controller_test.rb | 51 ++++++++++++++++++++++++ 8 files changed, 132 insertions(+), 9 deletions(-) diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index e037a48c9..3923cb2d0 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -5,10 +5,10 @@ class NotesController < ApplicationController before_filter :check_api_readable before_filter :authorize_web, :only => [:mine] before_filter :setup_user_auth, :only => [:create, :comment] - before_filter :authorize, :only => [:close, :destroy] + before_filter :authorize, :only => [:close, :reopen, :destroy] before_filter :require_moderator, :only => [:destroy] - before_filter :check_api_writable, :only => [:create, :comment, :close, :destroy] - before_filter :require_allow_write_notes, :only => [:create, :comment, :close, :destroy] + before_filter :check_api_writable, :only => [:create, :comment, :close, :reopen, :destroy] + before_filter :require_allow_write_notes, :only => [:create, :comment, :close, :reopen, :destroy] before_filter :set_locale after_filter :compress_output around_filter :api_call_handle_error, :api_call_timeout @@ -142,6 +142,36 @@ class NotesController < ApplicationController end end + ## + # Reopen a note + def reopen + # Check the arguments are sane + raise OSM::APIBadUserInput.new("No id was given") unless params[:id] + + # Extract the arguments + id = params[:id].to_i + comment = params[:text] + + # Find the note and check it is valid + @note = Note.find_by_id(id) + raise OSM::APINotFoundError unless @note + raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? + raise OSM::APINoteAlreadyOpenError.new(@note) unless @note.closed? + + # Reopen the note and add a comment + Note.transaction do + @note.reopen + + add_comment(@note, comment, "reopened") + end + + # Return a copy of the updated note + respond_to do |format| + format.xml { render :action => :show } + format.json { render :action => :show } + end + end + ## # Get a feed of recent notes and comments def feed diff --git a/app/models/note.rb b/app/models/note.rb index 490ff43cb..10b74d8a6 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -30,6 +30,13 @@ class Note < ActiveRecord::Base self.save end + # Reopen a note + def reopen + self.status = "open" + self.closed_at = nil + self.save + end + # Check if a note is visible def visible? status != "hidden" diff --git a/app/views/notes/_note.gpx.builder b/app/views/notes/_note.gpx.builder index 22d9283ef..b6d085536 100644 --- a/app/views/notes/_note.gpx.builder +++ b/app/views/notes/_note.gpx.builder @@ -12,7 +12,12 @@ xml.wpt("lon" => note.lon, "lat" => note.lat) do xml.id note.id xml.url note_url(note, :format => params[:format]) - xml.comment_url comment_note_url(note, :format => params[:format]) - xml.close_url close_note_url(note, :format => params[:format]) + + if note.closed? + xml.reopen_url reopen_note_url(note, :format => params[:format]) + else + xml.comment_url comment_note_url(note, :format => params[:format]) + xml.close_url close_note_url(note, :format => params[:format]) + end end end diff --git a/app/views/notes/_note.json.jsonify b/app/views/notes/_note.json.jsonify index 985bcf79c..f8872c717 100644 --- a/app/views/notes/_note.json.jsonify +++ b/app/views/notes/_note.json.jsonify @@ -8,8 +8,14 @@ end json.properties do json.id note.id json.url note_url(note, :format => params[:format]) - json.comment_url comment_note_url(note, :format => params[:format]) - json.close_url close_note_url(note, :format => params[:format]) + + if note.closed? + json.reopen_url reopen_note_url(note, :format => params[:format]) + else + json.comment_url comment_note_url(note, :format => params[:format]) + json.close_url close_note_url(note, :format => params[:format]) + end + json.date_created note.created_at json.status note.status json.closed_at note.closed_at if note.status == "closed" diff --git a/app/views/notes/_note.xml.builder b/app/views/notes/_note.xml.builder index 1128f35eb..42e69fec2 100644 --- a/app/views/notes/_note.xml.builder +++ b/app/views/notes/_note.xml.builder @@ -1,8 +1,14 @@ xml.note("lon" => note.lon, "lat" => note.lat) do xml.id note.id xml.url note_url(note, :format => params[:format]) - xml.comment_url comment_note_url(note, :format => params[:format]) - xml.close_url close_note_url(note, :format => params[:format]) + + if note.closed? + xml.reopen_url reopen_note_url(note, :format => params[:format]) + else + xml.comment_url comment_note_url(note, :format => params[:format]) + xml.close_url close_note_url(note, :format => params[:format]) + end + xml.date_created note.created_at xml.status note.status diff --git a/config/routes.rb b/config/routes.rb index f53f20861..6896fcc01 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -90,6 +90,7 @@ OpenStreetMap::Application.routes.draw do member do post 'comment' post 'close' + post 'reopen' end end diff --git a/lib/osm.rb b/lib/osm.rb index ba28378f3..d4c13ad7f 100644 --- a/lib/osm.rb +++ b/lib/osm.rb @@ -298,6 +298,23 @@ module OSM end end + # Raised when the note provided is already open + class APINoteAlreadyOpenError < APIError + def initialize(note) + @note = note + end + + attr_reader :note + + def status + :conflict + end + + def to_s + "The note #{@note.id} is already open" + end + end + # raised when a two preferences have a duplicate key string. class APIDuplicatePreferenceError < APIError def initialize(key) diff --git a/test/functional/notes_controller_test.rb b/test/functional/notes_controller_test.rb index 816ba4f6e..0b52d035e 100644 --- a/test/functional/notes_controller_test.rb +++ b/test/functional/notes_controller_test.rb @@ -38,6 +38,10 @@ class NotesControllerTest < ActionController::TestCase { :path => "/api/0.6/notes/1/close", :method => :post }, { :controller => "notes", :action => "close", :id => "1", :format => "xml" } ) + assert_routing( + { :path => "/api/0.6/notes/1/reopen", :method => :post }, + { :controller => "notes", :action => "reopen", :id => "1", :format => "xml" } + ) assert_routing( { :path => "/api/0.6/notes/1", :method => :delete }, { :controller => "notes", :action => "destroy", :id => "1", :format => "xml" } @@ -305,6 +309,53 @@ class NotesControllerTest < ActionController::TestCase assert_response :conflict end + def test_reopen_success + post :reopen, {:id => notes(:closed_note_with_comment).id, :text => "This is a reopen comment", :format => "json"} + assert_response :unauthorized + + basic_authorization(users(:public_user).email, "test") + + post :reopen, {:id => notes(:closed_note_with_comment).id, :text => "This is a reopen comment", :format => "json"} + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal "Feature", js["type"] + assert_equal notes(:closed_note_with_comment).id, js["properties"]["id"] + assert_equal "open", js["properties"]["status"] + assert_equal 2, js["properties"]["comments"].count + assert_equal "reopened", js["properties"]["comments"].last["action"] + assert_equal "This is a reopen comment", js["properties"]["comments"].last["text"] + assert_equal "test2", js["properties"]["comments"].last["user"] + + get :show, {:id => notes(:closed_note_with_comment).id, :format => "json"} + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal "Feature", js["type"] + assert_equal notes(:closed_note_with_comment).id, js["properties"]["id"] + assert_equal "open", js["properties"]["status"] + assert_equal 2, js["properties"]["comments"].count + assert_equal "reopened", js["properties"]["comments"].last["action"] + assert_equal "This is a reopen comment", js["properties"]["comments"].last["text"] + assert_equal "test2", js["properties"]["comments"].last["user"] + end + + def test_reopen_fail + post :reopen, {:id => notes(:hidden_note_with_comment).id} + assert_response :unauthorized + + basic_authorization(users(:public_user).email, "test") + + post :reopen, {:id => 12345} + assert_response :not_found + + post :reopen, {:id => notes(:hidden_note_with_comment).id} + assert_response :gone + + post :reopen, {:id => notes(:open_note_with_comment).id} + assert_response :conflict + end + def test_show_success get :show, {:id => notes(:open_note).id, :format => "xml"} assert_response :success -- 2.43.2