From 8598db3233baefa9e455da331ca2579a8df17709 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sun, 19 Oct 2014 21:31:23 +0100 Subject: [PATCH] Tidy up changeset comment code --- app/assets/stylesheets/common.css.scss | 8 ++-- app/controllers/browse_controller.rb | 7 +++- app/controllers/changeset_controller.rb | 41 ++++++++++--------- app/models/user.rb | 2 +- app/views/changeset/_comments.rss.builder | 3 +- app/views/changeset/comments_feed.rss.builder | 4 +- .../changeset_comment_notification.html.erb | 2 - ...0140507110937_create_changeset_comments.rb | 2 + ...join_table_between_users_and_changesets.rb | 5 ++- 9 files changed, 41 insertions(+), 33 deletions(-) diff --git a/app/assets/stylesheets/common.css.scss b/app/assets/stylesheets/common.css.scss index 0276b3e55..37a6aa7a9 100644 --- a/app/assets/stylesheets/common.css.scss +++ b/app/assets/stylesheets/common.css.scss @@ -1112,11 +1112,11 @@ header .search_form { } .comments-header { - float:left; + float: left; } .subscribe-buttons { - float:left; + float: left; margin: 18px 10px 10px; min-width: 80px; } @@ -1128,8 +1128,8 @@ header .search_form { } span.action-button:hover { - cursor:pointer; - text-decoration:underline; + cursor: pointer; + text-decoration: underline; } .note-description { diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb index 7056539cb..f0b92f4b4 100644 --- a/app/controllers/browse_controller.rb +++ b/app/controllers/browse_controller.rb @@ -58,8 +58,11 @@ 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) + 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 ba7cb0cbf..646ba2fa3 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -327,27 +327,27 @@ class ChangesetController < ApplicationController raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open? # Add a comment to the changeset - attributes = { + comment = changeset.comments.create({ :changeset => changeset, :body => body, :author => @user - } - - comment = changeset.comments.create(attributes) + }) + # 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 @@ -361,14 +361,16 @@ class ChangesetController < ApplicationController 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 + def unsubscribe # Check the arguments are sane raise OSM::APIBadUserInput.new("No id was given") unless params[:id] @@ -380,13 +382,14 @@ class ChangesetController < ApplicationController 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 @@ -396,16 +399,16 @@ class ChangesetController < ApplicationController id = params[:id].to_i # Find the changeset - @comment = ChangesetComment.find(id) - changeset = @comment.changeset + comment = ChangesetComment.find(id) - @comment.update(:visible => false) + # Hide the comment + comment.update(:visible => false) # Return a copy of the updated changeset - render :text => changeset.to_xml.to_s, :content_type => "text/xml" + 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 @@ -415,13 +418,13 @@ class ChangesetController < ApplicationController id = params[:id].to_i # Find the changeset - @comment = ChangesetComment.find(id) - changeset = @comment.changeset + comment = ChangesetComment.find(id) - @comment.update :visible => true + # Unhide the comment + comment.update(:visible => true) # Return a copy of the updated changeset - render :text => changeset.to_xml.to_s, :content_type => "text/xml" + render :text => comment.changeset.to_xml.to_s, :content_type => "text/xml" end ## @@ -434,9 +437,10 @@ class ChangesetController < ApplicationController # Find the changeset changeset = Changeset.find(id) - # Find the comments we want to return + # 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 @@ -589,5 +593,4 @@ private 100 end end - end diff --git a/app/models/user.rb b/app/models/user.rb index e5ff1917d..ed0813bee 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,7 +12,7 @@ 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_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/changeset/_comments.rss.builder b/app/views/changeset/_comments.rss.builder index 5effbc788..8ad5cbaa7 100644 --- a/app/views/changeset/_comments.rss.builder +++ b/app/views/changeset/_comments.rss.builder @@ -1,5 +1,4 @@ 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) @@ -16,4 +15,4 @@ comments.each do |comment| xml.pubDate comment.created_at.to_s(:rfc822) end -end \ No newline at end of file +end diff --git a/app/views/changeset/comments_feed.rss.builder b/app/views/changeset/comments_feed.rss.builder index 5224d9f93..60a229a30 100644 --- a/app/views/changeset/comments_feed.rss.builder +++ b/app/views/changeset/comments_feed.rss.builder @@ -9,6 +9,6 @@ xml.rss("version" => "2.0", xml.link url_for(:controller => "site", :action => "index", :only_path => false) xml << render(:partial => "comments", :object => @comments) - end -end \ No newline at end of file +end + diff --git a/app/views/notifier/changeset_comment_notification.html.erb b/app/views/notifier/changeset_comment_notification.html.erb index 2a07032c3..b7646a886 100644 --- a/app/views/notifier/changeset_comment_notification.html.erb +++ b/app/views/notifier/changeset_comment_notification.html.erb @@ -1,6 +1,5 @@

<%= t 'notifier.changeset_comment_notification.greeting' %>

-

<% if @owner %> <%= t "notifier.changeset_comment_notification.commented.your_changeset", :commenter => @commenter, :time => @time %> @@ -14,7 +13,6 @@ <% end %>

- == <%= @comment.to_html %> == diff --git a/db/migrate/20140507110937_create_changeset_comments.rb b/db/migrate/20140507110937_create_changeset_comments.rb index a44fd61ce..8d2a4599d 100644 --- a/db/migrate/20140507110937_create_changeset_comments.rb +++ b/db/migrate/20140507110937_create_changeset_comments.rb @@ -9,8 +9,10 @@ class CreateChangesetComments < ActiveRecord::Migration 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 index 37ec1a220..d07c6aae9 100644 --- a/db/migrate/20140519141742_add_join_table_between_users_and_changesets.rb +++ b/db/migrate/20140519141742_add_join_table_between_users_and_changesets.rb @@ -1,13 +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, [:subscriber_id, :changeset_id], :unique => true add_index :changesets_subscribers, [:changeset_id] end end -- 2.39.5