Changeset discussions

Add support for commenting on changesets with RSS feeds and email
notification of comments to other commenters and people that have
chosen to subscribe to a changeset.

.gitignore

@@ -8,3 +8,6 @@ tmp
 *~
 doc
 .vagrant
+.ruby-gemset
+.ruby-version
+.idea

app/assets/javascripts/index.js

@@ -12,6 +12,7 @@
 //= require index/history
 //= require index/note
 //= require index/new_note
+//= require index/changeset
 //= require router
 
 (function() {
@@ -304,7 +305,7 @@ $(document).ready(function () {
     "/node/:id(/history)":         OSM.Browse(map, 'node'),
     "/way/:id(/history)":          OSM.Browse(map, 'way'),
     "/relation/:id(/history)":     OSM.Browse(map, 'relation'),
-    "/changeset/:id":              OSM.Browse(map, 'changeset')
+    "/changeset/:id":              OSM.Changeset(map)
   });
 
   if (OSM.preferred_editor == "remote" && document.location.pathname == "/edit") {

app/assets/javascripts/index/changeset.js

@@ -0,0 +1,80 @@
+OSM.Changeset = function (map) {
+  var page = {},
+    content = $('#sidebar_content'),
+    currentChangesetId;
+
+  page.pushstate = page.popstate = function(path, id) {
+    OSM.loadSidebarContent(path, function() {
+      page.load(path, id);
+    });
+  };
+
+  page.load = function(path, id) {
+    if(id)
+      currentChangesetId = id;
+    initialize();
+    addChangeset(currentChangesetId, true);
+  };
+
+  function addChangeset(id, center) {
+    var bounds = map.addObject({type: 'changeset', id: parseInt(id)}, function(bounds) {
+      if (!window.location.hash && bounds.isValid() &&
+          (center || !map.getBounds().contains(bounds))) {
+        OSM.router.withoutMoveListener(function () {
+          map.fitBounds(bounds);
+        });
+      }
+    });
+  }
+
+  function updateChangeset(form, method, url, include_data) {
+    $(form).find("input[type=submit]").prop("disabled", true);
+    if(include_data) {
+      data = {text: $(form.text).val()};
+    } else {
+      data = {};
+    }
+
+    $.ajax({
+      url: url,
+      type: method,
+      oauth: true,
+      data: data,
+      success: function () {
+        OSM.loadSidebarContent(window.location.pathname, page.load);
+      }
+    });
+  }
+
+  function initialize() {
+    content.find("input[name=comment]").on("click", function (e) {
+      e.preventDefault();
+      var data = $(e.target).data();
+      updateChangeset(e.target.form, data.method, data.url, true);
+    });
+
+    content.find(".action-button").on("click", function (e) {
+      e.preventDefault();
+      var data = $(e.target).data();
+      updateChangeset(e.target.form, data.method, data.url);
+    });
+
+    content.find("textarea").on("input", function (e) {
+      var form = e.target.form;
+
+      if ($(e.target).val() == "") {
+        $(form.comment).prop("disabled", true);
+      } else {
+        $(form.comment).prop("disabled", false);
+      }
+    });
+
+    content.find("textarea").val('').trigger("input");
+  };
+
+  page.unload = function() {
+    map.removeObject();
+  };
+
+  return page;
+};

app/assets/stylesheets/common.css.scss

@@ -1103,7 +1103,7 @@ header .search_form {
     font-size: 90%;
   }
 
-  .note-comments li {
+  .note-comments li, .changeset-comments li {
     margin: $lineheight/2 0;
 
     p {
@@ -1111,6 +1111,27 @@ header .search_form {
     }
   }
 
+  .comments-header {
+    float:left;
+  }
+
+  .subscribe-buttons {
+    float:left;
+    margin: 18px 10px 10px;
+    min-width: 80px;
+  }
+
+  .subscribe-buttons input {
+    font-size: 90%;
+    line-height: 15px;
+    min-height: 20px;
+  }
+
+  span.action-button:hover {
+    cursor:pointer;
+    text-decoration:underline;
+  }
+
   .note-description {
     overflow: hidden;
     margin: 0 0 10px 10px;

app/controllers/browse_controller.rb

@@ -58,6 +58,8 @@ class BrowseController < ApplicationController
   def changeset
     @type = "changeset"
     @changeset = Changeset.find(params[:id])
+    @comments = @changeset.comments.unscope(:where => :visible).includes(:author) if @user and @user.moderator?
+    @comments ||= @changeset.comments.includes(:author)
     @node_pages, @nodes = paginate(:old_nodes, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'node_page')
     @way_pages, @ways = paginate(:old_ways, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'way_page')
     @relation_pages, @relations = paginate(:old_relations, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'relation_page')

app/controllers/changeset_controller.rb

@@ -7,11 +7,12 @@ class ChangesetController < ApplicationController
   skip_before_filter :verify_authenticity_token, :except => [:list]
   before_filter :authorize_web, :only => [:list, :feed]
   before_filter :set_locale, :only => [:list, :feed]
-  before_filter :authorize, :only => [:create, :update, :delete, :upload, :include, :close]
-  before_filter :require_allow_write_api, :only => [:create, :update, :delete, :upload, :include, :close]
-  before_filter :require_public_data, :only => [:create, :update, :delete, :upload, :include, :close]
-  before_filter :check_api_writable, :only => [:create, :update, :delete, :upload, :include]
-  before_filter :check_api_readable, :except => [:create, :update, :delete, :upload, :download, :query, :list, :feed]
+  before_filter :authorize, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
+  before_filter :require_moderator, :only => [:hide_comment, :unhide_comment]
+  before_filter :require_allow_write_api, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
+  before_filter :require_public_data, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe]
+  before_filter :check_api_writable, :only => [:create, :update, :delete, :upload, :include, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
+  before_filter :check_api_readable, :except => [:create, :update, :delete, :upload, :download, :query, :list, :feed, :comment, :subscribe, :unsubscribe, :comments_feed]
   before_filter(:only => [:list, :feed]) { |c| c.check_database_readable(true) }
   after_filter :compress_output
   around_filter :api_call_handle_error, :except => [:list, :feed]
@@ -29,6 +30,10 @@ class ChangesetController < ApplicationController
     # Assume that Changeset.from_xml has thrown an exception if there is an error parsing the xml
     cs.user_id = @user.id
     cs.save_with_tags!
+
+    # Subscribe user to changeset comments
+    cs.subscribers << @user
+
     render :text => cs.id.to_s, :content_type => "text/plain"
   end
 
@@ -37,7 +42,8 @@ class ChangesetController < ApplicationController
   # return anything about the nodes, ways and relations in the changeset.
   def read
     changeset = Changeset.find(params[:id])
-    render :text => changeset.to_xml.to_s, :content_type => "text/xml"
+
+    render :text => changeset.to_xml(params[:include_discussion].presence).to_s, :content_type => "text/xml"
   end
 
   ##
@@ -305,6 +311,141 @@ class ChangesetController < ApplicationController
     list
   end
 
+  ##
+  # Add a comment to a changeset
+  def comment
+    # Check the arguments are sane
+    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+    raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
+
+    # Extract the arguments
+    id = params[:id].to_i
+    body = params[:text]
+
+    # Find the changeset and check it is valid
+    changeset = Changeset.find(id)
+    raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
+
+    # Add a comment to the changeset
+    attributes = {
+      :changeset => changeset,
+      :body => body,
+      :author => @user
+    }
+
+    comment = changeset.comments.create(attributes)
+
+    changeset.subscribers.each do |user|
+      if @user != user
+        Notifier.changeset_comment_notification(comment, user).deliver
+      end
+    end
+
+    changeset.subscribers << @user unless changeset.subscribers.exists?(@user)
+
+    # Return a copy of the updated changeset
+    render :text => changeset.to_xml.to_s, :content_type => "text/xml"
+  end
+
+  ## 
+  # Adds a subscriber to the changeset
+  def subscribe
+    # Check the arguments are sane
+    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+
+    # Extract the arguments
+    id = params[:id].to_i
+
+    # Find the changeset and check it is valid
+    changeset = Changeset.find(id)
+    raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
+    raise OSM::APIChangesetAlreadySubscribedError.new(changeset) if changeset.subscribers.exists?(@user)
+
+    changeset.subscribers << @user
+    # Return a copy of the updated changeset
+    render :text => changeset.to_xml.to_s, :content_type => "text/xml"
+  end
+
+  ## 
+  # Removes a subscriber from the changeset
+  def unsubscribe 
+    # Check the arguments are sane
+    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+
+    # Extract the arguments
+    id = params[:id].to_i
+
+    # Find the changeset and check it is valid
+    changeset = Changeset.find(id)
+    raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
+    raise OSM::APIChangesetNotSubscribedError.new(changeset) unless changeset.subscribers.exists?(@user)
+
+    changeset.subscribers.delete(@user)
+
+    # Return a copy of the updated changeset
+    render :text => changeset.to_xml.to_s, :content_type => "text/xml"
+  end
+
+  ## 
+  # Sets visible flag on comment to false
+  def hide_comment
+    # Check the arguments are sane
+    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+
+    # Extract the arguments
+    id = params[:id].to_i
+
+    # Find the changeset
+    @comment = ChangesetComment.find(id)
+    changeset = @comment.changeset
+
+    @comment.update(:visible => false)
+
+    # Return a copy of the updated changeset
+    render :text => changeset.to_xml.to_s, :content_type => "text/xml"
+  end
+
+  ## 
+  # Sets visible flag on comment to true
+  def unhide_comment
+    # Check the arguments are sane
+    raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
+
+    # Extract the arguments
+    id = params[:id].to_i
+
+    # Find the changeset
+    @comment = ChangesetComment.find(id)
+    changeset = @comment.changeset
+
+    @comment.update :visible => true
+
+    # Return a copy of the updated changeset
+    render :text => changeset.to_xml.to_s, :content_type => "text/xml"
+  end
+
+  ##
+  # Get a feed of recent changeset comments
+  def comments_feed
+    if params[:id]
+      # Extract the arguments
+      id = params[:id].to_i
+
+      # Find the changeset
+      changeset = Changeset.find(id)
+
+      # Find the comments we want to return
+      @comments = changeset.comments.includes(:author, :changeset).limit(comments_limit)
+    else
+      @comments = ChangesetComment.includes(:author, :changeset).where(:visible => :true).order("created_at DESC").limit(comments_limit).preload(:changeset)
+    end
+
+    # Render the result
+    respond_to do |format|
+      format.rss
+    end
+  end
+
 private
   #------------------------------------------------------------
   # utility functions below.
@@ -435,4 +576,18 @@ private
     return changesets.where("num_changes > 0")
   end
 
+  ##
+  # Get the maximum number of comments to return
+  def comments_limit
+    if params[:limit]
+      if params[:limit].to_i > 0 and params[:limit].to_i <= 10000
+        params[:limit].to_i
+      else
+        raise OSM::APIBadUserInput.new("Comments limit must be between 1 and 10000")
+      end
+    else
+      100
+    end
+  end
+
 end

app/models/changeset.rb

@@ -11,6 +11,9 @@ class Changeset < ActiveRecord::Base
   has_many :old_nodes
   has_many :old_ways
   has_many :old_relations
+  
+  has_many :comments, -> { where(:visible => true).order(:created_at) }, :class_name => "ChangesetComment"
+  has_and_belongs_to_many :subscribers, :class_name => 'User', :join_table => 'changesets_subscribers', :association_foreign_key => 'subscriber_id'
 
   validates_presence_of :id, :on => :update
   validates_presence_of :user_id, :created_at, :closed_at, :num_changes
@@ -179,13 +182,13 @@ class Changeset < ActiveRecord::Base
     end
   end
 
-  def to_xml
+  def to_xml(include_discussion = false)
     doc = OSM::API.new.get_xml_doc
-    doc.root << to_xml_node()
+    doc.root << to_xml_node(nil, include_discussion)
     return doc
   end
 
-  def to_xml_node(user_display_name_cache = nil)
+  def to_xml_node(user_display_name_cache = nil, include_discussion = false)
     el1 = XML::Node.new 'changeset'
     el1['id'] = self.id.to_s
 
@@ -217,6 +220,23 @@ class Changeset < ActiveRecord::Base
       bbox.to_unscaled.add_bounds_to(el1, '_')
     end
 
+    el1['comments_count'] = self.comments.count.to_s
+
+    if include_discussion
+      el2 = XML::Node.new('discussion')
+      self.comments.includes(:author).each do |comment|
+        el3 = XML::Node.new('comment')
+        el3['date'] = comment.created_at.xmlschema
+        el3['uid'] = comment.author.id.to_s if comment.author.data_public?
+        el3['user'] = comment.author.display_name.to_s if comment.author.data_public?
+        el4 = XML::Node.new('text')
+        el4.content = comment.body.to_s
+        el3 << el4
+        el2 << el3
+      end
+      el1 << el2
+    end
+
     # NOTE: changesets don't include the XML of the changes within them,
     # they are just structures for tagging. to get the osmChange of a
     # changeset, see the download method of the controller.

app/models/changeset_comment.rb

@@ -0,0 +1,17 @@
+class ChangesetComment < ActiveRecord::Base
+  belongs_to :changeset
+  belongs_to :author, :class_name => "User"
+
+  validates_presence_of :id, :on => :update # is it necessary?
+  validates_uniqueness_of :id
+  validates_presence_of :changeset
+  validates_associated :changeset
+  validates_presence_of :author
+  validates_associated :author
+  validates :visible, :inclusion => { :in => [true,false] }
+  
+  # Return the comment text
+  def body
+    RichText.new("text", read_attribute(:body))
+  end
+end

app/models/notifier.rb

@@ -146,6 +146,26 @@ class Notifier < ActionMailer::Base
     end
   end
 
+  def changeset_comment_notification(comment, recipient)
+    with_recipient_locale recipient do
+      @changeset_url = changeset_url(comment.changeset, :host => SERVER_URL)
+      @comment = comment.body
+      @owner = recipient == comment.changeset.user
+      @commenter = comment.author.display_name
+      @changeset_comment = comment.changeset.tags['comment'].presence
+      @time = comment.created_at
+      @changeset_author = comment.changeset.user.display_name
+
+      if @owner
+        subject = I18n.t("notifier.changeset_comment_notification.commented.subject_own", :commenter => @commenter)
+      else
+        subject = I18n.t("notifier.changeset_comment_notification.commented.subject_other", :commenter => @commenter)
+      end
+
+      mail :to => recipient.email, :subject => subject
+    end
+  end
+
 private
 
   def with_recipient_locale(recipient)

app/models/user.rb

@@ -12,6 +12,8 @@ class User < ActiveRecord::Base
   has_many :tokens, :class_name => "UserToken"
   has_many :preferences, :class_name => "UserPreference"
   has_many :changesets, -> { order(:created_at => :desc) }
+  has_many :changeset_comments, :foreign_key =>  'author_id'
+  has_and_belongs_to_many :changeset_subscriptions, :class_name => 'Changeset', :join_table => 'changesets_subscribers', :foreign_key => 'subscriber_id'
   has_many :note_comments, :foreign_key => :author_id
   has_many :notes, :through => :note_comments
 

app/views/browse/changeset.html.erb

@@ -11,6 +11,69 @@
 
   <%= render :partial => "tag_details", :object => @changeset.tags.except('comment') %>
 
+  <h4 class="comments-header"><%= t('browse.changeset.discussion') %></h4>
+  
+  <div class="buttons clearfix subscribe-buttons">
+    <form action="#" class="hide_unless_logged_in">
+      <% if @changeset.subscribers.exists?(@user) %>
+        <input class="action-button" type="submit" name="unsubscribe" value="<%= t('javascripts.changesets.show.unsubscribe') %>" data-method="POST" data-url="<%= changeset_unsubscribe_url(@changeset) %>" />
+      <% else %>
+        <input class="action-button" type="submit" name="subscribe" value="<%= t('javascripts.changesets.show.subscribe') %>" data-method="POST" data-url="<%= changeset_subscribe_url(@changeset) %>" />
+      <% end %>
+    </form>
+  </div>
+
+  <div class="clearfix"></div>
+
+  <% if @comments.length > 0 %>
+    <div class='changeset-comments'>
+      <form action="#">
+        <ul>
+          <% @comments.each do |comment| %>
+            <% if comment.visible %>
+              <li id="c<%= comment.id %>">
+                <small class='deemphasize'>
+                  <%= t("browse.changeset.commented_by",
+                    :when => friendly_date(comment.created_at), :exact_time => l(comment.created_at),
+                    :user => link_to(h(comment.author.display_name), {:controller => "user", :action => "view",
+                    :display_name => comment.author.display_name})).html_safe %>
+                  <% if @user and @user.moderator? %>
+                    — <span class="action-button deemphasize" data-comment-id="<%= comment.id %>" data-method="POST" data-url="<%= changeset_comment_hide_url(comment.id) %>"><%= t('javascripts.changesets.show.hide_comment') %></span>
+                  <% end %>
+                </small>
+                <%= comment.body.to_html %>
+              </li>
+            <% elsif @user and @user.moderator? %>
+              <li id="c<%= comment.id %>">
+                <small class='deemphasize'>
+                  <%= t("browse.changeset.hidden_commented_by",
+                    :when => friendly_date(comment.created_at), :exact_time => l(comment.created_at),
+                    :user => link_to(h(comment.author.display_name), {:controller => "user", :action => "view",
+                    :display_name => comment.author.display_name})).html_safe %>
+                  — <span class="action-button deemphasize" data-comment-id="<%= comment.id %>" data-method="POST" data-url="<%= changeset_comment_unhide_url(comment.id) %>"><%= t('javascripts.changesets.show.unhide_comment') %></span>
+                 </small>
+                <%= comment.body.to_html %>
+              </li>
+            <% end %>
+          <% end %>
+        </ul>
+      </form>
+    </div>
+  <% end %>
+
+  <div class="notice hide_if_logged_in">
+    <%= link_to(t("browse.changeset.join_discussion"), :controller => 'user', :action => 'login', :referer => request.fullpath) %>
+  </div>
+
+  <% unless @changeset.is_open? %>
+    <form action="#" class="hide_unless_logged_in">
+      <textarea class="comment" name="text" cols="40" rows="5"></textarea>
+      <div class="buttons clearfix">
+        <input type="submit" name="comment" value="<%= t('javascripts.changesets.show.comment') %>" data-changeset-id="<%= @changeset.id %>" data-method="POST" data-url="<%= changeset_comment_url(@changeset) %>" disabled="1"/>
+      </div>
+    </form>
+  <% end %>
+
   <% unless @ways.empty? %>
     <h4>
       <%= type_and_paginated_count('way', @way_pages) %>

app/views/changeset/_comment.html.erb

@@ -0,0 +1,6 @@
+<h2><%= t "changeset.rss.comment", :author => comment.author.display_name, 
+  :changeset_id => comment.changeset.id.to_s %></h2>
+<div class="changeset-comment" style="margin-top: 5px">
+  <div class="changeset-comment-description" style="font-size: smaller; color: #999999"><%= t "changeset.rss.commented_at_by_html", :when => friendly_date(comment.created_at), :user => comment.author.display_name %></div>
+  <div class="changeset-comment-text"><%= comment.body %></div>
+</div>

app/views/changeset/_comments.rss.builder

@@ -0,0 +1,19 @@
+comments.each do |comment|
+
+  xml.item do
+    xml.title t("changeset.rss.comment", :author => comment.author.display_name, :changeset_id => comment.changeset.id.to_s)
+    
+    xml.link url_for(:controller => "browse", :action => "changeset", :id => comment.changeset.id, :anchor => "c#{comment.id}", :only_path => false)
+    xml.guid url_for(:controller => "browse", :action => "changeset", :id => comment.changeset.id, :anchor => "c#{comment.id}", :only_path => false)
+
+    xml.description do
+      xml.cdata! render(:partial => "comment", :object => comment, :formats => [ :html ])
+    end
+
+    if comment.author
+      xml.dc :creator, comment.author.display_name
+    end
+
+    xml.pubDate comment.created_at.to_s(:rfc822)
+  end
+end

app/views/changeset/comments_feed.rss.builder

@@ -0,0 +1,14 @@
+xml.rss("version" => "2.0", 
+        "xmlns:dc" => "http://purl.org/dc/elements/1.1/") do
+  xml.channel do
+    if @changeset
+      xml.title t('changeset.rss.title_particular', :changeset_id => @changeset.id)
+    else
+      xml.title t('changeset.rss.title_all')
+    end
+    xml.link url_for(:controller => "site", :action => "index", :only_path => false)
+
+    xml << render(:partial => "comments", :object => @comments)
+
+  end
+end

app/views/notifier/changeset_comment_notification.html.erb

@@ -0,0 +1,22 @@
+<p><%= t 'notifier.changeset_comment_notification.greeting' %></p>
+
+
+<p>
+  <% if @owner %>
+    <%= t "notifier.changeset_comment_notification.commented.your_changeset", :commenter => @commenter, :time => @time %>
+  <% else %>
+    <%= t "notifier.changeset_comment_notification.commented.commented_changeset", :commenter => @commenter, :time => @time, :changeset_author => @changeset_author %>
+  <% end %>
+  <% if @changeset_comment %>
+    <%= t "notifier.changeset_comment_notification.commented.partial_changeset_with_comment", :changeset_comment => @changeset_comment %>
+  <% else %>
+    <%= t "notifier.changeset_comment_notification.commented.partial_changeset_without_comment" %>
+  <% end %>
+</p>
+
+
+==
+<%= @comment.to_html %>
+==
+
+<p><%= raw t 'notifier.changeset_comment_notification.details', :url => link_to(@changeset_url, @changeset_url) %></p>

app/views/notifier/changeset_comment_notification.text.erb

@@ -0,0 +1,18 @@
+<%= t 'notifier.changeset_comment_notification.greeting' %>
+
+<% if @owner %>
+  <%= t "notifier.changeset_comment_notification.commented.your_changeset", :commenter => @commenter, :time => @time %>
+<% else %>
+  <%= t "notifier.changeset_comment_notification.commented.commented_changeset", :commenter => @commenter, :time => @time, :changeset_author => @changeset_author %>
+<% end %>
+<% if @changeset_comment %>
+  <%= t "notifier.changeset_comment_notification.commented.partial_changeset_with_comment", :changeset_comment => @changeset_comment %>
+<% else %>
+  <%= t "notifier.changeset_comment_notification.commented.partial_changeset_without_comment" %>
+<% end %>
+
+==
+<%= @comment.to_text %>
+==
+
+<%= t 'notifier.changeset_comment_notification.details', :url => @changeset_url %>

config/locales/en.yml

@@ -121,11 +121,16 @@ en:
       way_paginated: "Ways (%{x}-%{y} of %{count})"
       relation: "Relations (%{count})"
       relation_paginated: "Relations (%{x}-%{y} of %{count})"
+      comment: "Comments (%{count})"
+      hidden_commented_by: "Hidden comment from %{user} <abbr title='%{exact_time}'>%{when} ago</abbr>"
+      commented_by: "Comment from %{user} <abbr title='%{exact_time}'>%{when} ago</abbr>"
       changesetxml: "Changeset XML"
       osmchangexml: "osmChange XML"
       feed:
         title: "Changeset %{id}"
         title_comment: "Changeset %{id} - %{comment}"
+      join_discussion: "Log in to join the discussion"
+      discussion: Discussion
     node:
       title: "Node: %{name}"
       history_title: "Node History: %{name}"
@@ -228,6 +233,13 @@ en:
       load_more: "Load more"
     timeout:
       sorry: "Sorry, the list of changesets you requested took too long to retrieve."
+    rss:
+      title_all: OpenStreetMap changeset discussion
+      title_particular: "OpenStreetMap changeset #%{changeset_id} discussion"
+      comment: "New comment on changeset #%{changeset_id} by %{author}"
+      commented_at_html: "Updated %{when} ago"
+      commented_at_by_html: "Updated %{when} ago by %{user}"
+      full: Full discussion
   diary_entry:
     new:
       title: New Diary Entry
@@ -1234,6 +1246,16 @@ en:
         your_note: "%{commenter} has reactivated one of your map notes near %{place}."
         commented_note: "%{commenter} has reactivated a map note you have commented on. The note is near %{place}."
       details: "More details about the note can be found at %{url}."
+    changeset_comment_notification:
+      greeting: "Hi,"
+      commented:
+        subject_own: "[OpenStreetMap] %{commenter} has commented on one of your changesets"
+        subject_other: "[OpenStreetMap] %{commenter} has commented on a changeset you are interested in"
+        your_changeset: "%{commenter} has left a comment on one of your changesets created at %{time}"
+        commented_changeset: "%{commenter} has left a comment on a map changeset you are watching created by %{changeset_author} at %{time}"
+        partial_changeset_with_comment: "with comment '%{changeset_comment}'"
+        partial_changeset_without_comment: "without comment"
+      details: "More details about the changeset can be found at %{url}."
   message:
     inbox:
       title: "Inbox"
@@ -2104,6 +2126,13 @@ en:
       createnote_disabled_tooltip: Zoom in to add a note to the map
       map_notes_zoom_in_tooltip: Zoom in to see map notes
       map_data_zoom_in_tooltip: Zoom in to see map data
+    changesets:
+      show:
+        comment: "Comment"
+        subscribe: "Subscribe"
+        unsubscribe: "Unsubscribe"
+        hide_comment: "hide"
+        unhide_comment: "unhide"
     notes:
       new:
         intro: "Spotted a mistake or something missing? Let other mappers know so we can fix it. Move the marker to the correct position and type a note to explain the problem. (Please don't enter personal information or information from copyrighted maps or directory listings.)"

config/routes.rb

@@ -8,10 +8,16 @@ OpenStreetMap::Application.routes.draw do
   match 'api/0.6/changeset/:id/upload' => 'changeset#upload', :via => :post, :id => /\d+/
   match 'api/0.6/changeset/:id/download' => 'changeset#download', :via => :get, :as => :changeset_download, :id => /\d+/
   match 'api/0.6/changeset/:id/expand_bbox' => 'changeset#expand_bbox', :via => :post, :id => /\d+/
-  match 'api/0.6/changeset/:id' => 'changeset#read', :via => :get, :as => :changeset_read, :via => :get, :id => /\d+/
+  match 'api/0.6/changeset/:id' => 'changeset#read', :via => :get, :as => :changeset_read, :id => /\d+/
+  match 'api/0.6/changeset/:id/subscribe' => 'changeset#subscribe', :via => :post, :as => :changeset_subscribe, :id => /\d+/
+  match 'api/0.6/changeset/:id/unsubscribe' => 'changeset#unsubscribe', :via => :post, :as => :changeset_unsubscribe, :id => /\d+/
   match 'api/0.6/changeset/:id' => 'changeset#update', :via => :put, :id => /\d+/
   match 'api/0.6/changeset/:id/close' => 'changeset#close', :via => :put, :id => /\d+/
   match 'api/0.6/changesets' => 'changeset#query', :via => :get
+  post 'api/0.6/changeset/:id/comment' => 'changeset#comment', :as => :changeset_comment, :id => /\d+/
+  get 'api/0.6/changeset(/:id)/comments/feed' => 'changeset#comments_feed', :as => :changeset_comments_feed, :id => /\d*/, :defaults => { :format => 'rss' }
+  post 'api/0.6/changeset/comment/:id/hide' => 'changeset#hide_comment', :as => :changeset_comment_hide, :id => /\d+/
+  post 'api/0.6/changeset/comment/:id/unhide' => 'changeset#unhide_comment', :as => :changeset_comment_unhide, :id => /\d+/
 
   match 'api/0.6/node/create' => 'node#create', :via => :put
   match 'api/0.6/node/:id/ways' => 'way#ways_for_node', :via => :get, :id => /\d+/

db/migrate/20140507110937_create_changeset_comments.rb

@@ -0,0 +1,16 @@
+require 'migrate'
+
+class CreateChangesetComments < ActiveRecord::Migration
+  def change
+    create_table :changeset_comments do |t|
+      t.column :changeset_id, :bigint, :null => false
+      t.column :author_id, :bigint, :null =>  false
+      t.text :body, :null => false
+      t.timestamp :created_at, :null => false
+      t.boolean :visible, :null => false
+    end
+    add_foreign_key :changeset_comments, [:changeset_id], :changesets, [:id]
+    add_foreign_key :changeset_comments, [:author_id], :users, [:id]
+    add_index :changeset_comments, :created_at
+  end
+end

db/migrate/20140519141742_add_join_table_between_users_and_changesets.rb

@@ -0,0 +1,13 @@
+require 'migrate'
+class AddJoinTableBetweenUsersAndChangesets < ActiveRecord::Migration
+  def change
+    create_table :changesets_subscribers, id: false do |t|
+      t.column :subscriber_id, :bigint, null: false
+      t.column :changeset_id, :bigint, null: false
+    end
+    add_foreign_key :changesets_subscribers, [:subscriber_id], :users, [:id]
+    add_foreign_key :changesets_subscribers, [:changeset_id], :changesets, [:id]
+    add_index :changesets_subscribers, [:subscriber_id, :changeset_id], { :unique => true }
+    add_index :changesets_subscribers, [:changeset_id]
+  end
+end

db/structure.sql

@@ -126,7 +126,7 @@ CREATE TYPE user_status_enum AS ENUM (
 
 CREATE FUNCTION maptile_for_point(bigint, bigint, integer) RETURNS integer
     LANGUAGE c STRICT
-    AS '/srv/www/master.osm.compton.nu/db/functions/libpgosm.so', 'maptile_for_point';
+    AS '/home/ukasiu/repos/openstreetmap-website/db/functions/libpgosm', 'maptile_for_point';
 
 
 --
@@ -135,7 +135,7 @@ CREATE FUNCTION maptile_for_point(bigint, bigint, integer) RETURNS integer
 
 CREATE FUNCTION tile_for_point(integer, integer) RETURNS bigint
     LANGUAGE c STRICT
-    AS '/srv/www/master.osm.compton.nu/db/functions/libpgosm.so', 'tile_for_point';
+    AS '/home/ukasiu/repos/openstreetmap-website/db/functions/libpgosm', 'tile_for_point';
 
 
 --
@@ -143,8 +143,8 @@ CREATE FUNCTION tile_for_point(integer, integer) RETURNS bigint
 --
 
 CREATE FUNCTION xid_to_int4(xid) RETURNS integer
-    LANGUAGE c IMMUTABLE STRICT
-    AS '/srv/www/master.osm.compton.nu/db/functions/libpgosm.so', 'xid_to_int4';
+    LANGUAGE c STRICT
+    AS '/home/ukasiu/repos/openstreetmap-website/db/functions/libpgosm', 'xid_to_int4';
 
 
 SET default_tablespace = '';
@@ -183,6 +183,39 @@ CREATE SEQUENCE acls_id_seq
 ALTER SEQUENCE acls_id_seq OWNED BY acls.id;
 
 
+--
+-- Name: changeset_comments; Type: TABLE; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE TABLE changeset_comments (
+    id integer NOT NULL,
+    changeset_id bigint NOT NULL,
+    author_id bigint NOT NULL,
+    body text NOT NULL,
+    created_at timestamp without time zone NOT NULL,
+    visible boolean NOT NULL
+);
+
+
+--
+-- Name: changeset_comments_id_seq; Type: SEQUENCE; Schema: public; Owner: -
+--
+
+CREATE SEQUENCE changeset_comments_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+
+--
+-- Name: changeset_comments_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
+--
+
+ALTER SEQUENCE changeset_comments_id_seq OWNED BY changeset_comments.id;
+
+
 --
 -- Name: changeset_tags; Type: TABLE; Schema: public; Owner: -; Tablespace: 
 --
@@ -230,6 +263,16 @@ CREATE SEQUENCE changesets_id_seq
 ALTER SEQUENCE changesets_id_seq OWNED BY changesets.id;
 
 
+--
+-- Name: changesets_subscribers; Type: TABLE; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE TABLE changesets_subscribers (
+    subscriber_id bigint NOT NULL,
+    changeset_id bigint NOT NULL
+);
+
+
 --
 -- Name: client_applications; Type: TABLE; Schema: public; Owner: -; Tablespace: 
 --
@@ -698,7 +741,7 @@ CREATE TABLE nodes (
 --
 
 CREATE TABLE note_comments (
-    id integer NOT NULL,
+    id bigint NOT NULL,
     note_id bigint NOT NULL,
     visible boolean NOT NULL,
     created_at timestamp without time zone NOT NULL,
@@ -733,7 +776,7 @@ ALTER SEQUENCE note_comments_id_seq OWNED BY note_comments.id;
 --
 
 CREATE TABLE notes (
-    id integer NOT NULL,
+    id bigint NOT NULL,
     latitude integer NOT NULL,
     longitude integer NOT NULL,
     tile bigint NOT NULL,
@@ -851,8 +894,8 @@ CREATE TABLE redactions (
     id integer NOT NULL,
     title character varying(255),
     description text,
-    created_at timestamp without time zone NOT NULL,
-    updated_at timestamp without time zone NOT NULL,
+    created_at timestamp without time zone,
+    updated_at timestamp without time zone,
     user_id bigint NOT NULL,
     description_format format_enum DEFAULT 'markdown'::format_enum NOT NULL
 );
@@ -1064,9 +1107,9 @@ CREATE TABLE users (
     status user_status_enum DEFAULT 'pending'::user_status_enum NOT NULL,
     terms_agreed timestamp without time zone,
     consider_pd boolean DEFAULT false NOT NULL,
+    openid_url character varying(255),
     preferred_editor character varying(255),
     terms_seen boolean DEFAULT false NOT NULL,
-    openid_url character varying(255),
     description_format format_enum DEFAULT 'html'::format_enum NOT NULL,
     image_fingerprint character varying(255),
     changesets_count integer DEFAULT 0 NOT NULL,
@@ -1141,6 +1184,13 @@ CREATE TABLE ways (
 ALTER TABLE ONLY acls ALTER COLUMN id SET DEFAULT nextval('acls_id_seq'::regclass);
 
 
+--
+-- Name: id; Type: DEFAULT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY changeset_comments ALTER COLUMN id SET DEFAULT nextval('changeset_comments_id_seq'::regclass);
+
+
 --
 -- Name: id; Type: DEFAULT; Schema: public; Owner: -
 --
@@ -1289,6 +1339,14 @@ ALTER TABLE ONLY acls
     ADD CONSTRAINT acls_pkey PRIMARY KEY (id);
 
 
+--
+-- Name: changeset_comments_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: 
+--
+
+ALTER TABLE ONLY changeset_comments
+    ADD CONSTRAINT changeset_comments_pkey PRIMARY KEY (id);
+
+
 --
 -- Name: changesets_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: 
 --
@@ -1737,6 +1795,34 @@ CREATE INDEX gpx_files_user_id_idx ON gpx_files USING btree (user_id);
 CREATE INDEX gpx_files_visible_visibility_idx ON gpx_files USING btree (visible, visibility);
 
 
+--
+-- Name: index_changeset_comments_on_body; Type: INDEX; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE INDEX index_changeset_comments_on_body ON changeset_comments USING btree (body);
+
+
+--
+-- Name: index_changeset_comments_on_created_at; Type: INDEX; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE INDEX index_changeset_comments_on_created_at ON changeset_comments USING btree (created_at);
+
+
+--
+-- Name: index_changesets_subscribers_on_changeset_id; Type: INDEX; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE INDEX index_changesets_subscribers_on_changeset_id ON changesets_subscribers USING btree (changeset_id);
+
+
+--
+-- Name: index_changesets_subscribers_on_subscriber_id_and_changeset_id; Type: INDEX; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE UNIQUE INDEX index_changesets_subscribers_on_subscriber_id_and_changeset_id ON changesets_subscribers USING btree (subscriber_id, changeset_id);
+
+
 --
 -- Name: index_client_applications_on_key; Type: INDEX; Schema: public; Owner: -; Tablespace: 
 --
@@ -1968,6 +2054,22 @@ CREATE INDEX ways_changeset_id_idx ON ways USING btree (changeset_id);
 CREATE INDEX ways_timestamp_idx ON ways USING btree ("timestamp");
 
 
+--
+-- Name: changeset_comments_author_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY changeset_comments
+    ADD CONSTRAINT changeset_comments_author_id_fkey FOREIGN KEY (author_id) REFERENCES users(id);
+
+
+--
+-- Name: changeset_comments_changeset_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY changeset_comments
+    ADD CONSTRAINT changeset_comments_changeset_id_fkey FOREIGN KEY (changeset_id) REFERENCES changesets(id);
+
+
 --
 -- Name: changeset_tags_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
@@ -1976,6 +2078,22 @@ ALTER TABLE ONLY changeset_tags
     ADD CONSTRAINT changeset_tags_id_fkey FOREIGN KEY (changeset_id) REFERENCES changesets(id);
 
 
+--
+-- Name: changesets_subscribers_changeset_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY changesets_subscribers
+    ADD CONSTRAINT changesets_subscribers_changeset_id_fkey FOREIGN KEY (changeset_id) REFERENCES changesets(id);
+
+
+--
+-- Name: changesets_subscribers_subscriber_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY changesets_subscribers
+    ADD CONSTRAINT changesets_subscribers_subscriber_id_fkey FOREIGN KEY (subscriber_id) REFERENCES users(id);
+
+
 --
 -- Name: changesets_user_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
@@ -2426,6 +2544,10 @@ INSERT INTO schema_migrations (version) VALUES ('20140117185510');
 
 INSERT INTO schema_migrations (version) VALUES ('20140210003018');
 
+INSERT INTO schema_migrations (version) VALUES ('20140507110937');
+
+INSERT INTO schema_migrations (version) VALUES ('20140519141742');
+
 INSERT INTO schema_migrations (version) VALUES ('21');
 
 INSERT INTO schema_migrations (version) VALUES ('22');

lib/osm.rb

@@ -106,6 +106,57 @@ module OSM
     end
   end
 
+  # Raised when the changeset provided is not yet closed
+  class APIChangesetNotYetClosedError < APIError
+    def initialize(changeset)
+      @changeset = changeset
+    end
+
+    attr_reader :changeset
+
+    def status
+      :conflict
+    end
+
+    def to_s
+      "The changeset #{@changeset.id} is not yet closed."
+    end
+  end
+
+  # Raised when a user is already subscribed to the changeset
+  class APIChangesetAlreadySubscribedError < APIError
+    def initialize(changeset)
+      @changeset = changeset
+    end
+
+    attr_reader :changeset
+
+    def status
+      :conflict
+    end
+
+    def to_s
+      "You are already subscribed to changeset #{@changeset.id}."
+    end
+  end
+
+  # Raised when a user is not subscribed to the changeset
+  class APIChangesetNotSubscribedError < APIError
+    def initialize(changeset)
+      @changeset = changeset
+    end
+
+    attr_reader :changeset
+
+    def status
+      :not_found
+    end
+
+    def to_s
+      "You are not subscribed to changeset #{@changeset.id}."
+    end
+  end
+
   # Raised when a change is expecting a changeset, but the changeset doesn't exist
   class APIChangesetMissingError < APIError
     def status

test/controllers/changeset_controller_test.rb

@@ -3,6 +3,7 @@ require 'changeset_controller'
 
 class ChangesetControllerTest < ActionController::TestCase
   api_fixtures
+  fixtures :changeset_comments
 
   ##
   # test all routes which lead to this controller
@@ -35,6 +36,30 @@ class ChangesetControllerTest < ActionController::TestCase
       { :path => "/api/0.6/changeset/1/close", :method => :put },
       { :controller => "changeset", :action => "close", :id => "1" }
     )
+    assert_routing(
+        { :path => "/api/0.6/changeset/1/subscribe", :method => :post },
+        { :controller => "changeset", :action => "subscribe", :id => "1" }
+    )
+    assert_routing(
+        { :path => "/api/0.6/changeset/1/unsubscribe", :method => :post },
+        { :controller => "changeset", :action => "unsubscribe", :id => "1" }
+    )
+    assert_routing(
+        { :path => "/api/0.6/changeset/comment/1/hide", :method => :post },
+        { :controller => "changeset", :action => "hide_comment", :id => "1" }
+    )
+    assert_routing(
+        { :path => "/api/0.6/changeset/comment/1/unhide", :method => :post },
+        { :controller => "changeset", :action => "unhide_comment", :id => "1" }
+    )
+    assert_routing(
+        { :path => "/api/0.6/changeset/comments/feed", :method => :get },
+        { :controller => "changeset", :action => "comments_feed", :format =>"rss" }
+    )
+    assert_routing(
+        { :path => "/api/0.6/changeset/1/comments/feed", :method => :get },
+        { :controller => "changeset", :action => "comments_feed", :id => "1", :format =>"rss" }
+    )
     assert_routing(
       { :path => "/api/0.6/changesets", :method => :get },
       { :controller => "changeset", :action => "query" }
@@ -100,6 +125,9 @@ class ChangesetControllerTest < ActionController::TestCase
       # must be number of seconds...
       assert_equal 3600, duration.round, "initial idle timeout should be an hour (#{cs.created_at} -> #{cs.closed_at})"
     end
+
+    # checks if uploader was subscribed
+    assert_equal 1, cs.subscribers.length
   end
 
   def test_create_invalid
@@ -149,11 +177,15 @@ class ChangesetControllerTest < ActionController::TestCase
   # document structure.
   def test_read
     changeset_id = changesets(:normal_user_first_change).id
-    get :read, :id => changeset_id
+    get :read, :id => changeset_id, :format => :xml
     assert_response :success, "cannot get first changeset"
 
     assert_select "osm[version=#{API_VERSION}][generator=\"OpenStreetMap server\"]", 1
     assert_select "osm>changeset[id=#{changeset_id}]", 1
+    assert_select "osm>changeset>discussion", 0
+
+    get :read, :id => changeset_id, :format => :xml, :include_discussion => true
+    assert_select "osm>changeset>discussion", 1
   end
 
   ##
@@ -1382,7 +1414,8 @@ EOF
     end
 
     # get the bounding box back from the changeset
-    get :read, :id => changeset_id
+    get :read, :id => changeset_id, :format => :xml
+
     assert_response :success, "Couldn't read back changeset."
     assert_select "osm>changeset[min_lon=1.0]", 1
     assert_select "osm>changeset[max_lon=1.0]", 1
@@ -1397,7 +1430,7 @@ EOF
     end
 
     # get the bounding box back from the changeset
-    get :read, :id => changeset_id
+    get :read, :id => changeset_id, :format => :xml
     assert_response :success, "Couldn't read back changeset for the second time."
     assert_select "osm>changeset[min_lon=1.0]", 1
     assert_select "osm>changeset[max_lon=2.0]", 1
@@ -1413,7 +1446,7 @@ EOF
     end
 
     # get the bounding box back from the changeset
-    get :read, :id => changeset_id
+    get :read, :id => changeset_id, :format => 'xml'
     assert_response :success, "Couldn't read back changeset for the third time."
     # note that the 3.1 here is because of the bbox overexpansion
     assert_select "osm>changeset[min_lon=1.0]", 1
@@ -1840,6 +1873,234 @@ EOF
     assert_select "osmChange node[id=17][version=1]", 0
   end
 
+  ##
+  # create comment success
+  def test_create_comment_success
+    basic_authorization(users(:public_user).email, 'test')
+
+    assert_difference('ChangesetComment.count') do
+      post :comment, { :id => changesets(:normal_user_closed_change).id, :text => 'This is a comment', :format => :xml }
+    end
+    assert_response :success
+  end
+
+  ##
+  # create comment fail
+  def test_create_comment_fail
+    # unauthorized
+    post :comment, { :id => changesets(:normal_user_closed_change).id, :text => 'This is a comment' }
+    assert_response :unauthorized
+
+    basic_authorization(users(:public_user).email, 'test')
+
+    # bad changeset id
+    assert_no_difference('ChangesetComment.count') do
+      post :comment, { :id => 999111, :text => 'This is a comment' }
+    end
+    assert_response :not_found
+
+    # not closed changeset
+    assert_no_difference('ChangesetComment.count') do
+      post :comment, { :id => changesets(:normal_user_first_change).id, :text => 'This is a comment' }
+    end
+    assert_response :conflict
+
+    # no text
+    assert_no_difference('ChangesetComment.count') do
+      post :comment, { :id => changesets(:normal_user_closed_change).id }
+    end
+    assert_response :bad_request
+
+    # empty text
+    assert_no_difference('ChangesetComment.count') do
+      post :comment, { :id => changesets(:normal_user_closed_change).id, :text => '' }
+    end
+    assert_response :bad_request    
+  end
+
+  ##
+  # test subscribe success
+  def test_subscribe_success
+    basic_authorization(users(:public_user).email, 'test')
+    changeset = changesets(:normal_user_closed_change)
+
+    assert_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => changeset.id, :format => :xml }
+    end
+    assert_response :success
+  end
+
+  ##
+  # test subscribe fail
+  def test_subscribe_fail
+    changeset = changesets(:normal_user_closed_change)
+    assert_no_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => changeset.id, :format => :xml }
+    end
+    assert_response :unauthorized
+
+    basic_authorization(users(:public_user).email, 'test')
+
+    assert_no_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => 999111 }
+    end
+    assert_response :not_found
+
+    changeset = changesets(:normal_user_first_change)
+    assert_no_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => changeset.id }
+    end
+    assert_response :conflict
+
+    # subscribing
+    changeset = changesets(:normal_user_closed_change)
+    post :subscribe, { :id => changeset.id, :format => :xml }
+    assert_response :success
+
+    # trying to subsrcirbe one more time
+    assert_no_difference('changeset.subscribers.count') do
+      post :subscribe, { :id => changeset.id, :format => :xml }
+    end
+    assert_response :conflict
+  end
+
+  ##
+  # test unsubscribe success
+  def test_unsubscribe_success
+    basic_authorization(users(:public_user).email, 'test')
+    changeset = changesets(:normal_user_closed_change)
+    post :subscribe, { :id => changeset.id, :format => :xml }
+    # unsubscribe
+    assert_difference('changeset.subscribers.count', -1) do
+      post :unsubscribe, { :id => changeset.id, :format => :xml }
+    end
+    assert_response :success
+  end
+
+  ##
+  # test unsubscribe fail
+  def test_unsubscribe_fail
+    changeset = changesets(:normal_user_closed_change)
+    assert_no_difference('changeset.subscribers.count') do
+      post :unsubscribe, { :id => changeset.id }
+    end
+    assert_response :unauthorized
+
+    basic_authorization(users(:public_user).email, 'test')
+
+    assert_no_difference('changeset.subscribers.count', -1) do
+      post :unsubscribe, { :id => 999111 }
+    end
+    assert_response :not_found
+
+    changeset = changesets(:normal_user_first_change)
+    assert_no_difference('changeset.subscribers.count', -1) do
+      post :unsubscribe, { :id => changeset.id }
+    end
+    assert_response :conflict
+
+    # trying to unsubscribe when not subscribed
+    changeset = changesets(:normal_user_closed_change)
+    assert_no_difference('changeset.subscribers.count') do
+      post :unsubscribe, { :id => changeset.id }
+    end
+    assert_response :not_found
+  end
+
+  ##
+  # test hide comment fail
+  def test_hide_comment_fail
+    comment = changeset_comments(:normal_comment_1)
+    assert('comment.visible') do
+      post :hide_comment, { :id => comment.id, :format => "xml" }
+      assert_response :unauthorized
+    end
+    
+
+    basic_authorization(users(:public_user).email, 'test')
+
+    assert('comment.visible') do
+      post :hide_comment, { :id => comment.id, :format => "xml" }
+      assert_response :forbidden
+    end
+
+    basic_authorization(users(:moderator_user).email, 'test')
+
+    post :hide_comment, { :id => 999111, :format => "xml" }
+    assert_response :not_found
+  end
+
+  ##
+  # test hide comment succes
+  def test_hide_comment_success
+    comment = changeset_comments(:normal_comment_1)
+
+    basic_authorization(users(:moderator_user).email, 'test')
+
+    assert('!comment.visible') do
+      post :hide_comment, { :id => comment.id, :format => "xml" }
+    end
+    assert_response :success
+  end
+
+  ##
+  # test unhide comment fail
+  def test_unhide_comment_fail
+    comment = changeset_comments(:normal_comment_1)
+    assert('comment.visible') do
+      post :unhide_comment, { :id => comment.id, :format => "xml" }
+      assert_response :unauthorized
+    end
+    
+
+    basic_authorization(users(:public_user).email, 'test')
+
+    assert('comment.visible') do
+      post :unhide_comment, { :id => comment.id, :format => "xml" }
+      assert_response :forbidden
+    end
+
+    basic_authorization(users(:moderator_user).email, 'test')
+
+    post :unhide_comment, { :id => 999111, :format => "xml" }
+    assert_response :not_found
+  end
+
+  ##
+  # test unhide comment succes
+  def test_unhide_comment_success
+    comment = changeset_comments(:normal_comment_1)
+
+    basic_authorization(users(:moderator_user).email, 'test')
+
+    assert('!comment.visible') do
+      post :unhide_comment, { :id => comment.id, :format => "xml" }
+    end
+    assert_response :success
+  end
+
+  ##
+  # test comments feed
+  def test_comments_feed
+    get :comments_feed, {:format => "rss"}
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
+    assert_select "rss", :count => 1 do
+      assert_select "channel", :count => 1 do
+        assert_select "item", :count => 3
+      end
+    end
+
+    get :comments_feed, { :id => changesets(:normal_user_closed_change), :format => "rss"}
+    assert_response :success
+    assert_equal "application/rss+xml", @response.content_type
+    assert_select "rss", :count => 1 do
+      assert_select "channel", :count => 1 do
+        assert_select "item", :count => 3
+      end
+    end
+  end
+
   #------------------------------------------------------------
   # utility functions
   #------------------------------------------------------------
@@ -1886,5 +2147,4 @@ EOF
     xml.find("//osm/way").first[name] = value.to_s
     return xml
   end
-
 end

test/controllers/relation_controller_test.rb

@@ -859,7 +859,7 @@ OSM
 
     # now download the changeset to check its bounding box
     with_controller(ChangesetController.new) do
-      get :read, :id => changeset_id
+      get :read, :id => changeset_id, :format => :xml # TODO why it doesn't use defaults?
       assert_response :success, "can't re-read changeset for modify test"
       assert_select "osm>changeset", 1, "Changeset element doesn't exist in #{@response.body}"
       assert_select "osm>changeset[id=#{changeset_id}]", 1, "Changeset id=#{changeset_id} doesn't exist in #{@response.body}"

test/fixtures/changeset_comments.yml

@@ -0,0 +1,31 @@
+normal_comment_1:
+  id: 1
+  changeset_id: 3
+  created_at: 2007-01-01 00:00:00
+  author_id: 1
+  body: 'A comment from a logged-in user'
+  visible: true
+
+normal_comment_2:
+  id: 2
+  changeset_id: 3
+  created_at: 2007-02-01 00:00:00
+  author_id: 4
+  body: 'A comment from another logged-in user'
+  visible: true
+
+normal_comment_3:
+  id: 4
+  changeset_id: 3
+  created_at: 2007-02-01 00:00:00
+  author_id: 4
+  body: 'A comment from another logged-in user'
+  visible: true
+
+hidden_comment:
+  id: 3
+  changeset_id: 3
+  created_at: 2007-02-01 00:00:00
+  author_id: 4
+  body: 'A non-visible comment'
+  visible: false

test/integration/user_changeset_comments_test.rb

@@ -0,0 +1,49 @@
+require 'test_helper'
+
+class UserChangesetCommentsTest < ActionDispatch::IntegrationTest
+  fixtures :users, :changesets, :changeset_comments
+
+  # Test 'log in to comment' message for nonlogged in user
+  def test_log_in_message
+    get "/changeset/#{changesets(:normal_user_closed_change).id}"
+    assert_response :success
+    
+    assert_select "div#content" do
+      assert_select "div#sidebar" do
+        assert_select "div#sidebar_content" do
+          assert_select "div.browse-section" do
+            assert_select "div.notice.hide_if_logged_in"
+          end
+        end
+      end
+    end
+  end
+
+  # Test if the form is shown
+  def test_displaying_form
+    get_via_redirect '/login'
+    # We should now be at the login page
+    assert_response :success
+    assert_template 'user/login'
+    # We can now login
+    post  '/login', {'username' => "test@openstreetmap.org", 'password' => "test"}
+    assert_response :redirect
+
+    get "/changeset/#{changesets(:normal_user_closed_change).id}"
+    
+    assert_response :success
+    assert_template 'browse/changeset'
+
+    assert_select "div#content" do
+      assert_select "div#sidebar" do
+        assert_select "div#sidebar_content" do
+          assert_select "div.browse-section" do
+            assert_select "form[action='#']" do
+              assert_select "textarea[name=text]"
+            end
+          end
+        end
+      end
+    end
+  end
+end

test/models/changeset_comment_test.rb

@@ -0,0 +1,41 @@
+require 'test_helper'
+
+class ChangesetCommentTest < ActiveSupport::TestCase
+  fixtures :changesets, :changeset_comments
+
+  def test_changeset_comment_count
+    assert_equal 4, ChangesetComment.count
+  end
+
+  # validations
+  def test_does_not_accept_invalid_author
+    comment = changeset_comments(:normal_comment_1)
+    
+    comment.author = nil
+    assert !comment.valid?
+
+    comment.author_id = 999111
+    assert !comment.valid?
+  end
+
+  def test_does_not_accept_invalid_changeset
+    comment = changeset_comments(:normal_comment_1)
+
+    comment.changeset = nil
+    assert !comment.valid?
+
+    comment.changeset_id = 999111
+    assert !comment.valid?
+  end
+
+  def test_does_not_accept_empty_visible
+    comment = changeset_comments(:normal_comment_1)
+
+    comment.visible = nil
+    assert !comment.valid?
+  end
+
+  def test_comments_of_changeset_count
+    assert_equal 3, Changeset.find(changesets(:normal_user_closed_change)).comments.count
+  end
+end