Add a reopen API call for notes

diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index e037a48c9c596d7f8808c0db6c078c46798481b7..3923cb2d04ac44c4b5a4ab1797f7d3dc24242586 100644 (file)
--- 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 490ff43cb8299beba1259b405bc03070d6d06818..10b74d8a669a46f431a03b476f94494c38064476 100644 (file)
--- 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 22d9283ef7a8aff3a8b8c0af4df3225864ae5db3..b6d0855368cd1f8113f18ce8c48fc6d3e3ce93f8 100644 (file)
--- 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 985bcf79c6f8f7126ddecfbf466e165ab6ae0f91..f8872c717c184c9da9aab84e79150dd1075d242f 100644 (file)
--- 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 1128f35ebf4477c4ce640da2804ae56e623c1aa6..42e69fec2a7def80f70aa3a47d98b274a0d33ee1 100644 (file)
--- 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 f53f208617d9ab8cb7f12e8e89b1260687369809..6896fcc01f7bc39d3cf51094d61d77f1b2124d5c 100644 (file)
--- 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 ba28378f398c52610140cc6d04c6c2f3a0b1c492..d4c13ad7feab520f97d1f207b46ac3e36ef3528a 100644 (file)
--- 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 816ba4f6eada13bf0b59adc7ec1fa38bc3c43066..0b52d035e682aeb7980a19222b0b82d93ca0976c 100644 (file)
--- 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