Make the API reject changes to closed notes

diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index a7fa03ac7e94e076eaf133099581c807018c522b..0544f8705a527af9379c2368cfdb6cf2455eacc0 100644 (file)
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -94,6 +94,7 @@ class NotesController < ApplicationController
     @note = Note.find(id)
     raise OSM::APINotFoundError unless @note
     raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
+    raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
 
     # Add a comment to the note
     Note.transaction do
@@ -121,6 +122,7 @@ class NotesController < ApplicationController
     @note = Note.find_by_id(id)
     raise OSM::APINotFoundError unless @note
     raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
+    raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
 
     # Close the note and add a comment
     Note.transaction do

diff --git a/app/models/note.rb b/app/models/note.rb
index 801e1f3cbee16a2a9c74014321cdd6490ae41ac7..bb56c5ce0d6e73612b3358b1bdf9cc84226cc0b3 100644 (file)
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -52,6 +52,11 @@ class Note < ActiveRecord::Base
     status != "hidden"
   end
 
+  # Check if a note is closed
+  def closed?
+    not closed_at.nil?
+  end
+
   # Return the author object, derived from the first comment
   def author
     self.comments.first.author

diff --git a/lib/osm.rb b/lib/osm.rb
index 3da9671c13840ccca0e61de2df5ca00626d14d36..393011dac563a4e205b7511c6f4a62e7f2c2f769 100644 (file)
--- a/lib/osm.rb
+++ b/lib/osm.rb
@@ -281,6 +281,23 @@ module OSM
     end
   end
 
+  # Raised when the note provided is already closed
+  class APINoteAlreadyClosedError < APIError
+    def initialize(note)
+      @note = note
+    end
+
+    attr_reader :note
+
+    def status
+      :conflict
+    end
+
+    def to_s
+      "The note #{@note.id} was closed at #{@note.closed_at}"
+    end
+  end
+
   # Helper methods for going to/from mercator and lat/lng.
   class Mercator
     include Math

diff --git a/test/functional/notes_controller_test.rb b/test/functional/notes_controller_test.rb
index 1c0ded3558f567a8417a53bbe20fbe8f85a0a6a0..6e6a3c17c22f436d03a14371e39166b3035d7873 100644 (file)
--- a/test/functional/notes_controller_test.rb
+++ b/test/functional/notes_controller_test.rb
@@ -236,6 +236,11 @@ class NotesControllerTest < ActionController::TestCase
       post :comment, {:id => notes(:hidden_note_with_comment).id, :text => "This is an additional comment"}
     end
     assert_response :gone
+
+    assert_no_difference('NoteComment.count') do
+      post :comment, {:id => notes(:closed_note_with_comment).id, :text => "This is an additional comment"}
+    end
+    assert_response :conflict
   end
 
   def test_note_close_success
@@ -273,6 +278,9 @@ class NotesControllerTest < ActionController::TestCase
 
     post :close, {:id => notes(:hidden_note_with_comment).id}
     assert_response :gone
+
+    post :close, {:id => notes(:closed_note_with_comment).id}
+    assert_response :conflict
   end
 
   def test_note_read_success