From 1257639109e322a18f2b96143ca7c03993e82475 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 26 Dec 2016 22:24:01 -0600 Subject: [PATCH 1/4] Add 'post as moderator' option for comments. * Add 'post as moderator' option to comment form. This creates a so-called sticky comment. * Downvotes have no effect on stickied comments; they're always visible, regardless of comment thresholds. * Only mods may sticky comments. * Mods may sticky comments by other users. --- app/assets/stylesheets/specific/comments.css.scss | 13 +++++++++---- app/controllers/comments_controller.rb | 12 ++++++++++-- app/models/comment.rb | 11 ++++++----- app/views/comments/_form.html.erb | 11 ++++++++++- app/views/comments/edit.html.erb | 8 +------- app/views/comments/partials/index/_list.html.erb | 2 +- app/views/comments/partials/new/_form.html.erb | 10 ---------- app/views/comments/partials/show/_comment.html.erb | 4 ++-- db/migrate/20161227003428_add_sticky_to_comments.rb | 5 +++++ 9 files changed, 44 insertions(+), 32 deletions(-) delete mode 100644 app/views/comments/partials/new/_form.html.erb create mode 100644 db/migrate/20161227003428_add_sticky_to_comments.rb diff --git a/app/assets/stylesheets/specific/comments.css.scss b/app/assets/stylesheets/specific/comments.css.scss index 36a7360a6..6fe78eb3a 100644 --- a/app/assets/stylesheets/specific/comments.css.scss +++ b/app/assets/stylesheets/specific/comments.css.scss @@ -48,10 +48,6 @@ div.comments-for-post { opacity: 1.0; } } - - div.comment-preview { - margin-bottom: 2em; - } } div#c-posts { @@ -146,3 +142,12 @@ div#c-comments { } } } + +form.edit_comment div.input.boolean { + display: inline-block; + + label { + font-weight: normal; + vertical-align: initial; + } +} diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index ff38ab0f6..75a33ec0c 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -23,12 +23,12 @@ class CommentsController < ApplicationController def update @comment = Comment.find(params[:id]) check_privilege(@comment) - @comment.update_attributes(params[:comment].permit(:body)) + @comment.update(update_params, :as => CurrentUser.role) respond_with(@comment, :location => post_path(@comment.post_id)) end def create - @comment = Comment.create(params[:comment]) + @comment = Comment.create(create_params, :as => CurrentUser.role) respond_with(@comment) do |format| format.html do if @comment.errors.any? @@ -110,4 +110,12 @@ private raise User::PrivilegeError end end + + def create_params + params.require(:comment).permit(:post_id, :body, :do_not_bump_post, :is_sticky) + end + + def update_params + params.require(:comment).permit(:body, :is_deleted, :is_sticky) + end end diff --git a/app/models/comment.rb b/app/models/comment.rb index aa1a24f54..ad1d2606d 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -12,7 +12,8 @@ class Comment < ActiveRecord::Base before_validation :initialize_updater after_create :update_last_commented_at_on_create after_destroy :update_last_commented_at_on_destroy - attr_accessible :body, :post_id, :do_not_bump_post, :is_deleted + attr_accessible :body, :post_id, :do_not_bump_post, :is_deleted, :as => [:member, :gold, :platinum, :builder, :janitor, :moderator, :admin] + attr_accessible :is_sticky, :as => [:moderator, :admin] mentionable( :message_field => :body, :user_field => :creator_id, @@ -34,11 +35,11 @@ class Comment < ActiveRecord::Base end def hidden(user) - where("score < ?", user.comment_threshold) + where("score < ? and is_sticky = false", user.comment_threshold) end def visible(user) - where("score >= ?", user.comment_threshold) + where("score >= ? or is_sticky = true", user.comment_threshold) end def deleted @@ -208,11 +209,11 @@ class Comment < ActiveRecord::Base end def delete! - update_attributes(:is_deleted => true) + update({ :is_deleted => true }, :as => CurrentUser.role) end def undelete! - update_attributes(:is_deleted => false) + update({ :is_deleted => false }, :as => CurrentUser.role) end end diff --git a/app/views/comments/_form.html.erb b/app/views/comments/_form.html.erb index 0c5b47949..6117a6051 100644 --- a/app/views/comments/_form.html.erb +++ b/app/views/comments/_form.html.erb @@ -1,5 +1,14 @@ +<%= error_messages_for :comment %> + <%= simple_form_for(comment, :html => {:class => "edit_comment"}) do |f| %> + <%= f.hidden_field :post_id %> <%= dtext_field "comment", "body", :value => comment.body, :input_id => "comment_body_for_#{comment.id}", :preview_id => "dtext-preview-for-#{comment.id}" %> - <%= f.button :submit, "Submit" %> + <%= f.button :submit, "Submit", :data => { :disable_with => "Submitting..." } %> <%= dtext_preview_button "comment", "body", :input_id => "comment_body_for_#{comment.id}", :preview_id => "dtext-preview-for-#{comment.id}" %> + <% if comment.new_record? %> + <%= f.input :do_not_bump_post, :label => "No bump" %> + <% end %> + <% if CurrentUser.is_moderator? %> + <%= f.input :is_sticky, :label => "Post as moderator" %> + <% end %> <% end %> diff --git a/app/views/comments/edit.html.erb b/app/views/comments/edit.html.erb index e68f0087c..26e62c7c5 100644 --- a/app/views/comments/edit.html.erb +++ b/app/views/comments/edit.html.erb @@ -2,13 +2,7 @@

Edit Comment

- <%= error_messages_for "comment" %> - - <%= simple_form_for(@comment) do |f| %> - <%= dtext_field "comment", "body" %> - <%= f.button :submit, "Submit" %> - <%= dtext_preview_button "comment", "body" %> - <% end %> + <%= render "comments/form", :post => @comment.post, :comment => @comment %>
diff --git a/app/views/comments/partials/index/_list.html.erb b/app/views/comments/partials/index/_list.html.erb index dbe729038..6cf957800 100644 --- a/app/views/comments/partials/index/_list.html.erb +++ b/app/views/comments/partials/index/_list.html.erb @@ -28,7 +28,7 @@ <% if CurrentUser.is_member? %>

<%= link_to "Post comment", new_comment_path, :class => "expand-comment-response" %>

- <%= render "comments/partials/new/form", :post => post %> + <%= render "comments/form", :post => post, :comment => post.comments.new %>
<% end %> diff --git a/app/views/comments/partials/new/_form.html.erb b/app/views/comments/partials/new/_form.html.erb deleted file mode 100644 index 59978a6cd..000000000 --- a/app/views/comments/partials/new/_form.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -
-
- -<%= form_tag(comments_path, :class => "simple_form comment-form") do %> - <%= hidden_field "comment", "post_id", :value => post.id %> - <%= dtext_field "comment", "body", :input_id => "comment_response_for_#{post.id}", :preview_id => "dtext-preview-for-#{post.id}" %> - <%= submit_tag "Post", :data => { :disable_with => "Submitting..." } %> - <%= dtext_preview_button "comment", "body", :input_id => "comment_response_for_#{post.id}", :preview_id => "dtext-preview-for-#{post.id}" %> - <%= check_box "comment", "do_not_bump_post", :id => "comment_do_not_bump_post_#{post.id}" %> -<% end %> diff --git a/app/views/comments/partials/show/_comment.html.erb b/app/views/comments/partials/show/_comment.html.erb index 395804bdd..cbfe8323a 100644 --- a/app/views/comments/partials/show/_comment.html.erb +++ b/app/views/comments/partials/show/_comment.html.erb @@ -1,6 +1,6 @@ <% if CurrentUser.is_moderator? || !comment.is_deleted? %> -
+

<%= link_to_user comment.creator %> @@ -39,7 +39,7 @@ <% end %> <% if comment.editable_by?(CurrentUser.user) %> - <%= render "comments/form", :comment => comment %> + <%= render "comments/form", :post => comment.post, :comment => comment %> <% end %> <% end %>

diff --git a/db/migrate/20161227003428_add_sticky_to_comments.rb b/db/migrate/20161227003428_add_sticky_to_comments.rb new file mode 100644 index 000000000..124da10af --- /dev/null +++ b/db/migrate/20161227003428_add_sticky_to_comments.rb @@ -0,0 +1,5 @@ +class AddStickyToComments < ActiveRecord::Migration + def change + add_column :comments, :is_sticky, :boolean, null: false, default: false + end +end From 91e368f08ae33fb977bcff59942d84e648884d4e Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 26 Dec 2016 22:31:08 -0600 Subject: [PATCH 2/4] Highlight stickied comments; don't dim downvoted stickies. The highlight color uses the same color as the subnav bar background color. Make it a tad brighter so it's more visible. --- app/assets/stylesheets/common/000_vars.css.scss | 2 +- app/assets/stylesheets/specific/comments.css.scss | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/common/000_vars.css.scss b/app/assets/stylesheets/common/000_vars.css.scss index fb8bcf1ed..6279c968f 100644 --- a/app/assets/stylesheets/common/000_vars.css.scss +++ b/app/assets/stylesheets/common/000_vars.css.scss @@ -1,4 +1,4 @@ -$menu_color: #F7F7FF; +$menu_color: #F5F5FF; $link_color: hsl(213, 100%, 50%); $link_hover_color: lighten($link_color, 25%); $link_dark_color: darken($link_color, 25%); diff --git a/app/assets/stylesheets/specific/comments.css.scss b/app/assets/stylesheets/specific/comments.css.scss index 6fe78eb3a..7eaa842bb 100644 --- a/app/assets/stylesheets/specific/comments.css.scss +++ b/app/assets/stylesheets/specific/comments.css.scss @@ -15,6 +15,11 @@ div.comments-for-post { article.comment { margin-bottom: 2em; word-wrap: break-word; + padding: 5px; + + &[data-is-sticky="true"] { + background: $menu_color; + } div.author { width: 12em; @@ -40,7 +45,7 @@ div.comments-for-post { } } - article.comment.below-threshold { + article.comment.below-threshold:not([data-is-sticky="true"]) { opacity: 0.3; } From 82e93b3c1235a8e6f132b1c98d10d552c8c08788 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 26 Dec 2016 22:48:13 -0600 Subject: [PATCH 3/4] Allow downvoting admin comments. --- app/models/comment_vote.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/models/comment_vote.rb b/app/models/comment_vote.rb index 88ec2d084..accf9bc92 100644 --- a/app/models/comment_vote.rb +++ b/app/models/comment_vote.rb @@ -39,9 +39,6 @@ class CommentVote < ActiveRecord::Base if is_positive? && comment.creator == CurrentUser.user errors.add :base, "You cannot upvote your own comments" false - elsif is_negative? && comment.creator.is_admin? - errors.add :base, "You cannot downvote an admin comment" - false else true end From 0e73f3c8a9e01a1f9009ed44186ef28bb3230776 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 26 Dec 2016 23:37:16 -0600 Subject: [PATCH 4/4] Add tests for stickying comments. --- test/functional/comments_controller_test.rb | 81 +++++++++++++++++++-- test/unit/comment_test.rb | 20 +++++ 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/test/functional/comments_controller_test.rb b/test/functional/comments_controller_test.rb index a98ef8d85..eb54597a6 100644 --- a/test/functional/comments_controller_test.rb +++ b/test/functional/comments_controller_test.rb @@ -3,12 +3,17 @@ require 'test_helper' class CommentsControllerTest < ActionController::TestCase context "A comments controller" do setup do - CurrentUser.user = FactoryGirl.create(:user) + @mod = FactoryGirl.create(:moderator_user) + @user = FactoryGirl.create(:member_user) + CurrentUser.user = @user CurrentUser.ip_addr = "127.0.0.1" Danbooru.config.stubs(:member_comment_time_threshold).returns(1.week.from_now) + @post = FactoryGirl.create(:post) @comment = FactoryGirl.create(:comment, :post => @post) - @user = FactoryGirl.create(:moderator_user) + CurrentUser.scoped(@mod) do + @mod_comment = FactoryGirl.create(:comment, :post => @post) + end end teardown do @@ -28,13 +33,64 @@ class CommentsControllerTest < ActionController::TestCase end end + context "search action" do + should "render" do + get :search + assert_response :success + end + end + + context "show action" do + should "render" do + get :show, {:id => @comment.id} + assert_response :success + end + end + + context "edit action" do + should "render" do + get :edit, {:id => @comment.id}, {:user_id => @user.id} + assert_response :success + end + end + context "update action" do - should "update the comment" do + context "when updating another user's comment" do + should "succeed if updater is a moderator" do + post :update, {:id => @comment.id, :comment => {:body => "abc"}}, {:user_id => @mod.id} + assert_equal("abc", @comment.reload.body) + assert_redirected_to post_path(@comment.post) + end + + should "fail if updater is not a moderator" do + post :update, {:id => @mod_comment.id, :comment => {:body => "abc"}}, {:user_id => @user.id} + assert_not_equal("abc", @mod_comment.reload.body) + assert_response 403 + end + end + + context "when stickying a comment" do + should "succeed if updater is a moderator" do + CurrentUser.user = @mod + post :update, {:id => @comment.id, :comment => {:is_sticky => true}}, {:user_id => @mod.id} + assert_equal(true, @comment.reload.is_sticky) + assert_redirected_to @comment.post + end + + should "fail if updater is not a moderator" do + post :update, {:id => @comment.id, :comment => {:is_sticky => true}}, {:user_id => @user.id} + assert_equal(false, @comment.reload.is_sticky) + assert_redirected_to @comment.post + end + end + + should "update the body" do post :update, {:id => @comment.id, :comment => {:body => "abc"}}, {:user_id => @comment.creator_id} + assert_equal("abc", @comment.reload.body) assert_redirected_to post_path(@comment.post) end - should "only allow changing the body" do + should "allow changing the body and is_deleted" do params = { id: @comment.id, comment: { @@ -50,13 +106,20 @@ class CommentsControllerTest < ActionController::TestCase assert_equal("herp derp", @comment.body) assert_equal(false, @comment.do_not_bump_post) - assert_equal(false, @comment.is_deleted) + assert_equal(true, @comment.is_deleted) assert_equal(@post.id, @comment.post_id) assert_redirected_to post_path(@post) end end + context "new action" do + should "redirect" do + get :new, {}, {:user_id => @user.id} + assert_redirected_to comments_path + end + end + context "create action"do should "create a comment" do assert_difference("Comment.count", 1) do @@ -71,5 +134,13 @@ class CommentsControllerTest < ActionController::TestCase assert_response :error end end + + context "destroy action" do + should "mark comment as deleted" do + delete :destroy, {:id => @comment.id}, {:user_id => @user.id} + assert_equal(true, @comment.reload.is_deleted) + assert_redirected_to @comment + end + end end end diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb index 6c7a4d481..862b127ce 100644 --- a/test/unit/comment_test.rb +++ b/test/unit/comment_test.rb @@ -205,6 +205,26 @@ class CommentTest < ActiveSupport::TestCase assert_equal(c2.id, matches.all[0].id) assert_equal(c1.id, matches.all[1].id) end + + context "that is below the score threshold" do + should "be hidden if not stickied" do + user = FactoryGirl.create(:user, :comment_threshold => 0) + post = FactoryGirl.create(:post) + comment = FactoryGirl.create(:comment, :post => post, :score => -5) + + assert_equal([comment], post.comments.hidden(user)) + assert_equal([], post.comments.visible(user)) + end + + should "be visible if stickied" do + user = FactoryGirl.create(:user, :comment_threshold => 0) + post = FactoryGirl.create(:post) + comment = FactoryGirl.create(:comment, :post => post, :score => -5, :is_sticky => true) + + assert_equal([], post.comments.hidden(user)) + assert_equal([comment], post.comments.visible(user)) + end + end end end end