Add a reopen API call for notes

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

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"

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

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"

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
 

config/routes.rb

@@ -90,6 +90,7 @@ OpenStreetMap::Application.routes.draw do
       member do
         post 'comment'
         post 'close'
+        post 'reopen'
       end
     end
 

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)

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