Implemented changeset tags updating via the update method.

diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb
index 8c042ef8d463abf11e7e253eff9709e579bd11c6..cd49176e63e105e16819cdc00f392189594e58d9 100644 (file)
--- a/app/controllers/changeset_controller.rb
+++ b/app/controllers/changeset_controller.rb
@@ -257,6 +257,42 @@ class ChangesetController < ApplicationController
   rescue String => s
     render :text => s, :content_type => "text/plain", :status => :bad_request
   end
   rescue String => s
     render :text => s, :content_type => "text/plain", :status => :bad_request
   end

+  
+  ##
+  # updates a changeset's tags. none of the changeset's attributes are
+  # user-modifiable, so they will be ignored.
+  #
+  # changesets are not (yet?) versioned, so we don't have to deal with
+  # history tables here. changesets are locked to a single user, however.
+  #
+  # after succesful update, returns the XML of the changeset.
+  def update
+    # request *must* be a PUT.
+    unless request.put?
+      render :nothing => true, :status => :method_not_allowed
+      return
+    end
+    
+    changeset = Changeset.find(params[:id])
+    new_changeset = Changeset.from_xml(request.raw_post)
+
+    unless new_changeset.nil?
+      changeset.update_from(new_changeset, @user)
+      render :text => changeset.to_xml, :mime_type => "text/xml"
+    else
+      
+      render :nothing => true, :status => :bad_request
+    end
+      
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
+  rescue OSM::APIError => ex
+    render ex.render_opts
+  end
+
+  #------------------------------------------------------------
+  # utility functions below.
+  #------------------------------------------------------------  

 
   ##
   # merge two conditions
 
   ##
   # merge two conditions

diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index 4a4d12124b69b786689d883073cd0a22e3b63b83..047569dca5cf15db682c9c1fbd8fe72a85b3a942 100644 (file)
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -104,13 +104,13 @@ class Changeset < ActiveRecord::Base
   def save_with_tags!
     t = Time.now
 
   def save_with_tags!
     t = Time.now
 

+    # do the changeset update and the changeset tags update in the
+    # same transaction to ensure consistency.

     Changeset.transaction do
       # fixme update modified_at time?
       # FIXME there is no modified_at time, should it be added
       self.save!
     Changeset.transaction do
       # fixme update modified_at time?
       # FIXME there is no modified_at time, should it be added
       self.save!

-    end

 
 

-    ChangesetTag.transaction do

       tags = self.tags
       ChangesetTag.delete_all(['id = ?', self.id])
 
       tags = self.tags
       ChangesetTag.delete_all(['id = ?', self.id])
 


@@ -168,4 +168,25 @@ class Changeset < ActiveRecord::Base
 
     return el1
   end
 
     return el1
   end

+
+  ##
+  # update this instance from another instance given and the user who is
+  # doing the updating. note that this method is not for updating the
+  # bounding box, only the tags of the changeset.
+  def update_from(other, user)
+    # ensure that only the user who opened the changeset may modify it.
+    unless user.id == self.user_id 
+      raise OSM::APIUserChangesetMismatchError 
+    end
+    
+    # can't change a closed changeset
+    unless open
+      raise OSM::APIChangesetAlreadyClosedError
+    end
+
+    # copy the other's tags
+    self.tags = other.tags
+
+    save_with_tags!
+  end

 end
 end

diff --git a/config/routes.rb b/config/routes.rb
index a835dc5523ed0e014a5c3a85cff2f5cfa21e48f5..88fa0a55196e1eb25e3b1447092d2a1a8e74286b 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -7,7 +7,8 @@ ActionController::Routing::Routes.draw do |map|
   map.connect "api/#{API_VERSION}/changeset/:id/upload", :controller => 'changeset', :action => 'upload', :id => /\d+/
   map.connect "api/#{API_VERSION}/changeset/:id/download", :controller => 'changeset', :action => 'download', :id => /\d+/
   map.connect "api/#{API_VERSION}/changeset/:id/include", :controller => 'changeset', :action => 'include', :id => /\d+/
   map.connect "api/#{API_VERSION}/changeset/:id/upload", :controller => 'changeset', :action => 'upload', :id => /\d+/
   map.connect "api/#{API_VERSION}/changeset/:id/download", :controller => 'changeset', :action => 'download', :id => /\d+/
   map.connect "api/#{API_VERSION}/changeset/:id/include", :controller => 'changeset', :action => 'include', :id => /\d+/

-  map.connect "api/#{API_VERSION}/changeset/:id", :controller => 'changeset', :action => 'read', :id => /\d+/
+  map.connect "api/#{API_VERSION}/changeset/:id", :controller => 'changeset', :action => 'read', :id => /\d+/, :conditions => { :method => :get }
+  map.connect "api/#{API_VERSION}/changeset/:id", :controller => 'changeset', :action => 'update', :id => /\d+/, :conditions => { :method => :put }

   map.connect "api/#{API_VERSION}/changeset/:id/close", :controller => 'changeset', :action => 'close', :id =>/\d+/
   map.connect "api/#{API_VERSION}/changesets", :controller => 'changeset', :action => 'query'
   
   map.connect "api/#{API_VERSION}/changeset/:id/close", :controller => 'changeset', :action => 'close', :id =>/\d+/
   map.connect "api/#{API_VERSION}/changesets", :controller => 'changeset', :action => 'query'
   

diff --git a/test/functional/changeset_controller_test.rb b/test/functional/changeset_controller_test.rb
index d81bf46b894adb7cbb90a6b01820a0528cddbf14..1b0c63e2d32d72da15cd564f2dd99f627ab7f929 100644 (file)
--- a/test/functional/changeset_controller_test.rb
+++ b/test/functional/changeset_controller_test.rb
@@ -584,6 +584,45 @@ EOF
     # FIXME: write the actual test bit after fixing the fixtures!
   end
 
     # FIXME: write the actual test bit after fixing the fixtures!
   end
 

+  ##
+  # check updating tags on a changeset
+  def test_changeset_update
+    basic_authorization "test@openstreetmap.org", "test"
+
+    changeset = changesets(:normal_user_first_change)
+    new_changeset = changeset.to_xml
+    new_tag = XML::Node.new "tag"
+    new_tag['k'] = "testing"
+    new_tag['v'] = "testing"
+    new_changeset.find("//osm/changeset").first << new_tag
+
+    content new_changeset
+    put :update, :id => changeset.id
+    assert_response :success
+
+    assert_select "osm>changeset[id=#{changeset.id}]", 1
+    assert_select "osm>changeset>tag", 2
+    assert_select "osm>changeset>tag[k=testing][v=testing]", 1
+  end
+  
+  ##
+  # check that a user different from the one who opened the changeset
+  # can't modify it.
+  def test_changeset_update_invalid
+    basic_authorization "test@example.com", "test"
+
+    changeset = changesets(:normal_user_first_change)
+    new_changeset = changeset.to_xml
+    new_tag = XML::Node.new "tag"
+    new_tag['k'] = "testing"
+    new_tag['v'] = "testing"
+    new_changeset.find("//osm/changeset").first << new_tag
+
+    content new_changeset
+    put :update, :id => changeset.id
+    assert_response :conflict
+  end
+

   #------------------------------------------------------------
   # utility functions
   #------------------------------------------------------------
   #------------------------------------------------------------
   # utility functions
   #------------------------------------------------------------