From: Tom Hughes Date: Sat, 1 Nov 2014 11:43:21 +0000 (+0000) Subject: Merge branch 'comments' X-Git-Tag: live~4287 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/2f228437324d80cb8a408d27912cc716a36aa3b0?hp=1d9fe46af1399dd71233505862b384f586260407 Merge branch 'comments' --- diff --git a/.gitignore b/.gitignore index e507f9430..edfd387ab 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,6 @@ tmp *~ doc .vagrant +.ruby-gemset +.ruby-version +.idea \ No newline at end of file diff --git a/app/assets/javascripts/index.js b/app/assets/javascripts/index.js index a259954f2..0cf08f845 100644 --- a/app/assets/javascripts/index.js +++ b/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") { diff --git a/app/assets/javascripts/index/changeset.js b/app/assets/javascripts/index/changeset.js new file mode 100644 index 000000000..57d98dc24 --- /dev/null +++ b/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; +}; \ No newline at end of file diff --git a/app/assets/stylesheets/common.css.scss b/app/assets/stylesheets/common.css.scss index 961923979..37a6aa7a9 100644 --- a/app/assets/stylesheets/common.css.scss +++ b/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; diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb index e16ec2914..f0b92f4b4 100644 --- a/app/controllers/browse_controller.rb +++ b/app/controllers/browse_controller.rb @@ -58,6 +58,11 @@ class BrowseController < ApplicationController def changeset @type = "changeset" @changeset = Changeset.find(params[:id]) + if @user and @user.moderator? + @comments = @changeset.comments.unscope(:where => :visible).includes(:author) + else + @comments = @changeset.comments.includes(:author) + end @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') diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index b236ba6b5..646ba2fa3 100644 --- a/app/controllers/changeset_controller.rb +++ b/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,145 @@ 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 + comment = changeset.comments.create({ + :changeset => changeset, + :body => body, + :author => @user + }) + + # Notify current subscribers of the new comment + changeset.subscribers.each do |user| + if @user != user + Notifier.changeset_comment_notification(comment, user).deliver + end + end + + # Add the commenter to the subscribers if necessary + 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) + + # Add the subscriber + 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) + + # Remove the subscriber + 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) + + # Hide the comment + comment.update(:visible => false) + + # Return a copy of the updated changeset + render :text => comment.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) + + # Unhide the comment + comment.update(:visible => true) + + # Return a copy of the updated changeset + render :text => comment.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) + + # Return comments for this changeset only + @comments = changeset.comments.includes(:author, :changeset).limit(comments_limit) + else + # Return comments + @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 +580,17 @@ 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 diff --git a/app/models/changeset.rb b/app/models/changeset.rb index adb41b204..ab6e05ccf 100644 --- a/app/models/changeset.rb +++ b/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. diff --git a/app/models/changeset_comment.rb b/app/models/changeset_comment.rb new file mode 100644 index 000000000..a010674a3 --- /dev/null +++ b/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 diff --git a/app/models/notifier.rb b/app/models/notifier.rb index be6679c41..b1a94a77d 100644 --- a/app/models/notifier.rb +++ b/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) diff --git a/app/models/user.rb b/app/models/user.rb index 9c1d82ba8..ed0813bee 100644 --- a/app/models/user.rb +++ b/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 diff --git a/app/views/browse/changeset.html.erb b/app/views/browse/changeset.html.erb index 84d85df3c..bbe227921 100644 --- a/app/views/browse/changeset.html.erb +++ b/app/views/browse/changeset.html.erb @@ -11,6 +11,69 @@ <%= render :partial => "tag_details", :object => @changeset.tags.except('comment') %> +

<%= t('browse.changeset.discussion') %>

+ +
+
+ <% if @changeset.subscribers.exists?(@user) %> + + <% else %> + + <% end %> +
+
+ +
+ + <% if @comments.length > 0 %> +
+
+
    + <% @comments.each do |comment| %> + <% if comment.visible %> +
  • + + <%= 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? %> + — <%= t('javascripts.changesets.show.hide_comment') %> + <% end %> + + <%= comment.body.to_html %> +
    • + <% elsif @user and @user.moderator? %> +
  • + + <%= 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 %> + — <%= t('javascripts.changesets.show.unhide_comment') %> + + <%= comment.body.to_html %> +
    • + <% end %> + <% end %> +
+
+
+ <% end %> + +
+ <%= link_to(t("browse.changeset.join_discussion"), :controller => 'user', :action => 'login', :referer => request.fullpath) %> +
+ + <% unless @changeset.is_open? %> +
+ +
+ +
+
+ <% end %> + <% unless @ways.empty? %>

<%= type_and_paginated_count('way', @way_pages) %> diff --git a/app/views/changeset/_comment.html.erb b/app/views/changeset/_comment.html.erb new file mode 100644 index 000000000..dfd91167b --- /dev/null +++ b/app/views/changeset/_comment.html.erb @@ -0,0 +1,6 @@ +

<%= t "changeset.rss.comment", :author => comment.author.display_name, + :changeset_id => comment.changeset.id.to_s %>

+
+
<%= t "changeset.rss.commented_at_by_html", :when => friendly_date(comment.created_at), :user => comment.author.display_name %>
+
<%= comment.body %>
+
diff --git a/app/views/changeset/_comments.rss.builder b/app/views/changeset/_comments.rss.builder new file mode 100644 index 000000000..8ad5cbaa7 --- /dev/null +++ b/app/views/changeset/_comments.rss.builder @@ -0,0 +1,18 @@ +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 diff --git a/app/views/changeset/comments_feed.rss.builder b/app/views/changeset/comments_feed.rss.builder new file mode 100644 index 000000000..60a229a30 --- /dev/null +++ b/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 + diff --git a/app/views/notifier/changeset_comment_notification.html.erb b/app/views/notifier/changeset_comment_notification.html.erb new file mode 100644 index 000000000..b7646a886 --- /dev/null +++ b/app/views/notifier/changeset_comment_notification.html.erb @@ -0,0 +1,20 @@ +

<%= 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_html %> +== + +

<%= raw t 'notifier.changeset_comment_notification.details', :url => link_to(@changeset_url, @changeset_url) %>

diff --git a/app/views/notifier/changeset_comment_notification.text.erb b/app/views/notifier/changeset_comment_notification.text.erb new file mode 100644 index 000000000..44a3c1216 --- /dev/null +++ b/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 %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 1e8cf1938..021151052 100644 --- a/config/locales/en.yml +++ b/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} %{when} ago" + commented_by: "Comment from %{user} %{when} ago" 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.)" diff --git a/config/routes.rb b/config/routes.rb index 3357f911d..8cbf8f900 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,10 +8,15 @@ 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+/ + 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+/ @@ -109,6 +114,7 @@ OpenStreetMap::Application.routes.draw do match '/relation/:id' => 'browse#relation', :via => :get, :id => /\d+/, :as => :relation match '/relation/:id/history' => 'browse#relation_history', :via => :get, :id => /\d+/ match '/changeset/:id' => 'browse#changeset', :via => :get, :as => :changeset, :id => /\d+/ + match '/changeset/:id/comments/feed' => 'changeset#comments_feed', :via => :get, :as => :changeset_comments_feed, :id => /\d*/, :defaults => { :format => 'rss' } match '/note/:id' => 'browse#note', :via => :get, :id => /\d+/, :as => "browse_note" match '/note/new' => 'browse#new_note', :via => :get match '/user/:display_name/history' => 'changeset#list', :via => :get @@ -144,6 +150,7 @@ OpenStreetMap::Application.routes.draw do match '/about' => 'site#about', :via => :get, :as => :about match '/history' => 'changeset#list', :via => :get match '/history/feed' => 'changeset#feed', :via => :get, :defaults => { :format => :atom } + match '/history/comments/feed' => 'changeset#comments_feed', :via => :get, :as => :changesets_comments_feed, :defaults => { :format => 'rss' } match '/export' => 'site#export', :via => :get match '/login' => 'user#login', :via => [:get, :post] match '/logout' => 'user#logout', :via => [:get, :post] diff --git a/db/migrate/20140507110937_create_changeset_comments.rb b/db/migrate/20140507110937_create_changeset_comments.rb new file mode 100644 index 000000000..8d2a4599d --- /dev/null +++ b/db/migrate/20140507110937_create_changeset_comments.rb @@ -0,0 +1,18 @@ +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 diff --git a/db/migrate/20140519141742_add_join_table_between_users_and_changesets.rb b/db/migrate/20140519141742_add_join_table_between_users_and_changesets.rb new file mode 100644 index 000000000..d07c6aae9 --- /dev/null +++ b/db/migrate/20140519141742_add_join_table_between_users_and_changesets.rb @@ -0,0 +1,16 @@ +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 diff --git a/db/structure.sql b/db/structure.sql index 1d6170bc4..c287baea3 100644 --- a/db/structure.sql +++ b/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'); diff --git a/lib/osm.rb b/lib/osm.rb index daef8d3f0..c59ab7e26 100644 --- a/lib/osm.rb +++ b/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 diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index fc4f4b681..461738330 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -3,6 +3,7 @@ require 'changeset_controller' class ChangesetControllerTest < ActionController::TestCase api_fixtures + fixtures :changeset_comments, :changesets_subscribers ## # test all routes which lead to this controller @@ -27,6 +28,14 @@ class ChangesetControllerTest < ActionController::TestCase { :path => "/api/0.6/changeset/1", :method => :get }, { :controller => "changeset", :action => "read", :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/1", :method => :put }, { :controller => "changeset", :action => "update", :id => "1" } @@ -35,10 +44,26 @@ 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/comment", :method => :post }, + { :controller => "changeset", :action => "comment", :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/changesets", :method => :get }, { :controller => "changeset", :action => "query" } ) + assert_routing( + { :path => "/changeset/1/comments/feed", :method => :get }, + { :controller => "changeset", :action => "comments_feed", :id => "1", :format =>"rss" } + ) assert_routing( { :path => "/user/name/history", :method => :get }, { :controller => "changeset", :action => "list", :display_name => "name" } @@ -63,6 +88,10 @@ class ChangesetControllerTest < ActionController::TestCase { :path => "/history/feed", :method => :get }, { :controller => "changeset", :action => "feed", :format => :atom } ) + assert_routing( + { :path => "/history/comments/feed", :method => :get }, + { :controller => "changeset", :action => "comments_feed", :format =>"rss" } + ) end # ----------------------- @@ -100,6 +129,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 @@ -154,6 +186,14 @@ class ChangesetControllerTest < ActionController::TestCase 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, :include_discussion => true + assert_response :success, "cannot get first changeset with comments" + + assert_select "osm[version=#{API_VERSION}][generator=\"OpenStreetMap server\"]", 1 + assert_select "osm>changeset[id=#{changeset_id}]", 1 + assert_select "osm>changeset>discussion", 1 end ## @@ -1502,11 +1542,11 @@ EOF basic_authorization "test@openstreetmap.org", "test" get :query, :user => users(:normal_user).id assert_response :success, "can't get changesets by user ID" - assert_changesets [1,3,6] + assert_changesets [1,3,6,8] get :query, :display_name => users(:normal_user).display_name assert_response :success, "can't get changesets by user name" - assert_changesets [1,3,6] + assert_changesets [1,3,6,8] # check that the correct error is given when we provide both UID and name get :query, :user => users(:normal_user).id, :display_name => users(:normal_user).display_name @@ -1534,11 +1574,11 @@ EOF get :query, :closed => 'true' assert_response :success, "can't get changesets by closed-ness" - assert_changesets [3,5,6,7] + assert_changesets [3,5,6,7,8] get :query, :closed => 'true', :user => users(:normal_user).id assert_response :success, "can't get changesets by closed-ness and user" - assert_changesets [3,6] + assert_changesets [3,6,8] get :query, :closed => 'true', :user => users(:public_user).id assert_response :success, "can't get changesets by closed-ness and user" @@ -1840,6 +1880,239 @@ 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' } + 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 } + end + assert_response :success + end + + ## + # test subscribe fail + def test_subscribe_fail + # unauthorized + changeset = changesets(:normal_user_closed_change) + assert_no_difference('changeset.subscribers.count') do + post :subscribe, { :id => changeset.id } + end + assert_response :unauthorized + + basic_authorization(users(:public_user).email, 'test') + + # bad changeset id + assert_no_difference('changeset.subscribers.count') do + post :subscribe, { :id => 999111 } + end + assert_response :not_found + + # not closed changeset + changeset = changesets(:normal_user_first_change) + assert_no_difference('changeset.subscribers.count') do + post :subscribe, { :id => changeset.id } + end + assert_response :conflict + + # trying to subscribe when already subscribed + changeset = changesets(:normal_user_subscribed_change) + assert_no_difference('changeset.subscribers.count') do + post :subscribe, { :id => changeset.id } + end + assert_response :conflict + end + + ## + # test unsubscribe success + def test_unsubscribe_success + basic_authorization(users(:public_user).email, 'test') + changeset = changesets(:normal_user_subscribed_change) + + assert_difference('changeset.subscribers.count', -1) do + post :unsubscribe, { :id => changeset.id } + end + assert_response :success + end + + ## + # test unsubscribe fail + def test_unsubscribe_fail + # unauthorized + 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') + + # bad changeset id + assert_no_difference('changeset.subscribers.count', -1) do + post :unsubscribe, { :id => 999111 } + end + assert_response :not_found + + # not closed changeset + 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 + # unauthorized + comment = changeset_comments(:normal_comment_1) + assert('comment.visible') do + post :hide_comment, { :id => comment.id } + assert_response :unauthorized + end + + basic_authorization(users(:public_user).email, 'test') + + # not a moderator + assert('comment.visible') do + post :hide_comment, { :id => comment.id } + assert_response :forbidden + end + + basic_authorization(users(:moderator_user).email, 'test') + + # bad comment id + post :hide_comment, { :id => 999111 } + 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 } + end + assert_response :success + end + + ## + # test unhide comment fail + def test_unhide_comment_fail + # unauthorized + comment = changeset_comments(:normal_comment_1) + assert('comment.visible') do + post :unhide_comment, { :id => comment.id } + assert_response :unauthorized + end + + basic_authorization(users(:public_user).email, 'test') + + # not a moderator + assert('comment.visible') do + post :unhide_comment, { :id => comment.id } + assert_response :forbidden + end + + basic_authorization(users(:moderator_user).email, 'test') + + # bad comment id + post :unhide_comment, { :id => 999111 } + 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 } + 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 +2159,4 @@ EOF xml.find("//osm/way").first[name] = value.to_s return xml end - end diff --git a/test/fixtures/changeset_comments.yml b/test/fixtures/changeset_comments.yml new file mode 100644 index 000000000..cd7076fdc --- /dev/null +++ b/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 \ No newline at end of file diff --git a/test/fixtures/changesets.yml b/test/fixtures/changesets.yml index d80b517fe..3cfec5bbb 100644 --- a/test/fixtures/changesets.yml +++ b/test/fixtures/changesets.yml @@ -69,3 +69,9 @@ too_many_elements_changeset: max_lat: <%= 4 * SCALE %> num_changes: <%= Changeset::MAX_ELEMENTS + 1 %> +normal_user_subscribed_change: + id: 8 + user_id: 1 + created_at: "2007-01-01 00:00:00" + closed_at: "2007-01-02 00:00:00" + num_changes: 0 diff --git a/test/fixtures/changesets_subscribers.yml b/test/fixtures/changesets_subscribers.yml new file mode 100644 index 000000000..f46c688a2 --- /dev/null +++ b/test/fixtures/changesets_subscribers.yml @@ -0,0 +1,3 @@ +t1: + changeset_id: 8 + subscriber_id: 2 diff --git a/test/integration/user_changeset_comments_test.rb b/test/integration/user_changeset_comments_test.rb new file mode 100644 index 000000000..c984149db --- /dev/null +++ b/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 diff --git a/test/models/changeset_comment_test.rb b/test/models/changeset_comment_test.rb new file mode 100644 index 000000000..5f8efdbbd --- /dev/null +++ b/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 diff --git a/test/models/changeset_test.rb b/test/models/changeset_test.rb index b6e5ba46a..715a90b92 100644 --- a/test/models/changeset_test.rb +++ b/test/models/changeset_test.rb @@ -4,7 +4,7 @@ class ChangesetTest < ActiveSupport::TestCase api_fixtures def test_changeset_count - assert_equal 7, Changeset.count + assert_equal 8, Changeset.count end def test_from_xml_no_text