Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AO3-5860 Prevent leaving comments or kudos when logged in as admin #4378

Merged
merged 47 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
cc10ee2
Ask admins to log out before commenting
ceithir Nov 9, 2022
1fba796
Ask admins to log out before leaving kudos
ceithir Nov 9, 2022
1ee02e0
Clean up now irrelevant code
ceithir Nov 9, 2022
9142a82
Update reminder text
ceithir Nov 9, 2022
12c31ba
Controller tests instead of integration tests
ceithir Nov 9, 2022
7aa02d3
Hide kudos/comments button as admin
ceithir Nov 9, 2022
4ed8ade
Restrict commenting on works only?
ceithir Nov 9, 2022
33b89e0
Attempt at disabling all admin comments
ceithir Nov 9, 2022
0228eed
Revert "Attempt at disabling all admin comments"
ceithir Nov 9, 2022
731229a
Fix test to not restrict admin comments on tags
ceithir Nov 9, 2022
d43483b
? spacing
ceithir Nov 9, 2022
60da2d0
Hound
ceithir Nov 9, 2022
f33f7d3
Merge remote-tracking branch 'upstream/master' into AO3-5860
ceithir Nov 17, 2022
f50f30b
Hide whole form, not solely button
ceithir Nov 17, 2022
f876634
Prevent admin from commenting on anything
ceithir Nov 17, 2022
1416d90
Update old test for new behavior
ceithir Nov 17, 2022
2bbe0f8
Merge remote-tracking branch 'upstream/master' into AO3-5860
ceithir Dec 28, 2022
05d8843
Check message and redirection at once
ceithir Dec 28, 2022
485ff75
Add test for no reply comment
ceithir Dec 28, 2022
dde9452
Remove empty if
ceithir Dec 28, 2022
bac38ce
Now useless admin only form
ceithir Dec 28, 2022
45d007d
Lost EOF newline
ceithir Dec 28, 2022
75ba8e7
Actually hide Reply link
ceithir Dec 29, 2022
5d79d12
Check not admin on all "add comment" routes
ceithir Jan 2, 2023
987c85e
Consistent location for check
ceithir Jan 2, 2023
21a2048
Updated test for new behavior
ceithir Jan 2, 2023
68c056b
Unneeded change
ceithir Jan 3, 2023
a1c39fa
Merge remote-tracking branch 'upstream/master' into AO3-5860
ceithir Jun 1, 2023
face994
More precise message
ceithir Jun 1, 2023
6a402bd
Better test description
ceithir Jun 1, 2023
24c925b
Missing test
ceithir Jun 1, 2023
ba071ea
Normalize i18n files
ceithir Jun 1, 2023
079c090
Merge remote-tracking branch 'upstream/master' into AO3-5860
ceithir Jun 16, 2023
bdb5b41
Forgot to update similar tests indeed
ceithir Jun 16, 2023
5ddfb2e
Handle :js format
ceithir Jun 16, 2023
70f4e2a
Revert "Handle :js format"
ceithir Jun 16, 2023
8923da3
Make sure admin cannot create kudos through AJAX
ceithir Jun 17, 2023
8ab8775
Merge remote-tracking branch 'upstream/master' into AO3-5860
ceithir Jun 30, 2023
493d6fa
Oops
ceithir Jun 30, 2023
5e8e94f
Doing two things as once...
ceithir Jun 30, 2023
0ec17bf
Actual non silly merge conflict
ceithir Jun 30, 2023
9266392
Intended behavior?
ceithir Jun 30, 2023
b329103
Merge remote-tracking branch 'upstream/master' into AO3-5860
ceithir Apr 11, 2024
7155047
Update ts( to t(
ceithir Apr 11, 2024
9c7a080
If you start doing something...
ceithir Apr 11, 2024
55b0885
Test update
ceithir Apr 11, 2024
ce366d6
Acceptable HTML
ceithir Apr 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/controllers/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class CommentsController < ApplicationController
before_action :check_permission_to_moderate, only: [:approve, :reject]
before_action :check_permission_to_modify_frozen_status, only: [:freeze, :unfreeze]
before_action :check_permission_to_modify_hidden_status, only: [:hide, :unhide]
before_action :admin_logout_required, only: [:create]
ceithir marked this conversation as resolved.
Show resolved Hide resolved

include BlockHelper

Expand Down
1 change: 1 addition & 0 deletions app/controllers/kudos_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class KudosController < ApplicationController
skip_before_action :store_location
before_action :admin_logout_required, only: [:create]

def index
@work = Work.find(params[:work_id])
Expand Down
197 changes: 101 additions & 96 deletions app/views/comments/_comment_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,115 +2,120 @@
<% if !commentable && @commentable %>
<% commentable = @commentable %>
<% end %>
<div class="post comment" id="comment_form_for_<%= commentable.id %>">
<%= form_for value_for_comment_form(commentable, comment), :remote => !comment.new_record?, :html => {:id => "comment_for_#{commentable.id}"} do |f| %>
<fieldset>
<legend><%= ts("Post Comment") %></legend>

<% # here come the hacks (hidden fields to transmit various info to the create action) %>
<% if commentable.is_a?(Tag) %>
<%= hidden_field_tag :tag_id, commentable.name %>
<% end %>
<% if logged_in_as_admin? %>
<% # Hide form for admins %>
<% else %>
ceithir marked this conversation as resolved.
Show resolved Hide resolved
<div class="post comment" id="comment_form_for_<%= commentable.id %>">
<%= form_for value_for_comment_form(commentable, comment), :remote => !comment.new_record?, :html => {:id => "comment_for_#{commentable.id}"} do |f| %>
<fieldset>
<legend><%= ts("Post Comment") %></legend>

<% if params[:view_full_work] %>
<%= hidden_field_tag :view_full_work, params[:view_full_work] %>
<% end %>
<% # here come the hacks (hidden fields to transmit various info to the create action) %>
<% if commentable.is_a?(Tag) %>
<%= hidden_field_tag :tag_id, commentable.name %>
<% end %>

<% if controller.controller_name == "inbox" && params[:filters] %>
<%= hidden_field_tag "filters[read]", params[:filters][:read] %>
<%= hidden_field_tag "filters[replied_to]", params[:filters][:replied_to] %>
<%= hidden_field_tag "filters[date]", params[:filters][:date] %>
<% end %>
<% if params[:view_full_work] %>
<%= hidden_field_tag :view_full_work, params[:view_full_work] %>
<% end %>

<% if params[:page] %>
<%= hidden_field_tag :page, params[:page] %>
<% end %>
<% if controller.controller_name == "inbox" && params[:filters] %>
<%= hidden_field_tag "filters[read]", params[:filters][:read] %>
<%= hidden_field_tag "filters[replied_to]", params[:filters][:replied_to] %>
<%= hidden_field_tag "filters[date]", params[:filters][:date] %>
<% end %>

<% if comments_are_moderated(commentable) && !current_user_is_work_creator(commentable) %>
<p class="notice">
<%= ts("This work's creator has chosen to moderate comments on the work. Your comment will not appear until it has been approved by the creator.") %>
</p>
<% end %>
<% if params[:page] %>
<%= hidden_field_tag :page, params[:page] %>
<% end %>

<% if logged_in? %>
<% if current_user_is_anonymous_creator(commentable) %>
<% if comments_are_moderated(commentable) && !current_user_is_work_creator(commentable) %>
<p class="notice">
<%= ts("While this work is anonymous, comments you post will also be listed anonymously.") %>
<%= ts("This work's creator has chosen to moderate comments on the work. Your comment will not appear until it has been approved by the creator.") %>
</p>
<% end %>

<% if current_user.pseuds.count > 1 %>
<h4 class="heading"><%= ts("Comment as") %> <%= f.collection_select :pseud_id, current_user.pseuds, :id, :name, {:selected => (comment.pseud ? comment.pseud.id.to_s : current_user.default_pseud.id.to_s)}, :id => "comment_pseud_id_for_#{commentable.id}", :title => ts("Choose Name") %>
<% if controller.controller_name == "inbox" %>
<% if commentable.by_anonymous_creator? %>
<span><%= ts("to") %> <%= "Anonymous Creator" %> <%= ts("on") %> <%= commentable_description_link(commentable) %></span>
<% else %>
<span><%= ts("to") %> <%= get_commenter_pseud_or_name(commentable) %> <%= ts("on") %> <%= commentable_description_link(commentable) %></span>
<% if logged_in? %>
<% if current_user_is_anonymous_creator(commentable) %>
<p class="notice">
<%= ts("While this work is anonymous, comments you post will also be listed anonymously.") %>
</p>
<% end %>

<% if current_user.pseuds.count > 1 %>
<h4 class="heading"><%= ts("Comment as") %> <%= f.collection_select :pseud_id, current_user.pseuds, :id, :name, {:selected => (comment.pseud ? comment.pseud.id.to_s : current_user.default_pseud.id.to_s)}, :id => "comment_pseud_id_for_#{commentable.id}", :title => ts("Choose Name") %>
<% if controller.controller_name == "inbox" %>
<% if commentable.by_anonymous_creator? %>
<span><%= ts("to") %> <%= "Anonymous Creator" %> <%= ts("on") %> <%= commentable_description_link(commentable) %></span>
<% else %>
<span><%= ts("to") %> <%= get_commenter_pseud_or_name(commentable) %> <%= ts("on") %> <%= commentable_description_link(commentable) %></span>
<% end %>
<% end %>
<% end %>
(<%= allowed_html_instructions(show_list=false) %>)
</h4>
<% else %>
<h4 class="heading"><%= ts("Comment as") %> <span class="byline"><%= current_user.default_pseud.name %></span>
<%= f.hidden_field :pseud_id, :value => "#{current_user.default_pseud.id}", :id => "comment_pseud_id_for_#{commentable.id}" %>
<% if controller.controller_name == "inbox" %>
<% if commentable.by_anonymous_creator? %>
<span><%= ts("to") %> <%= "Anonymous Creator" %> <%= ts("on") %> <%= commentable_description_link(commentable) %></span>
<% else %>
<span><%= ts("to") %> <%= get_commenter_pseud_or_name(commentable) %> <%= ts("on") %> <%= commentable_description_link(commentable) %></span>
(<%= allowed_html_instructions(show_list=false) %>)
</h4>
<% else %>
<h4 class="heading"><%= ts("Comment as") %> <span class="byline"><%= current_user.default_pseud.name %></span>
<%= f.hidden_field :pseud_id, :value => "#{current_user.default_pseud.id}", :id => "comment_pseud_id_for_#{commentable.id}" %>
<% if controller.controller_name == "inbox" %>
<% if commentable.by_anonymous_creator? %>
<span><%= ts("to") %> <%= "Anonymous Creator" %> <%= ts("on") %> <%= commentable_description_link(commentable) %></span>
<% else %>
<span><%= ts("to") %> <%= get_commenter_pseud_or_name(commentable) %> <%= ts("on") %> <%= commentable_description_link(commentable) %></span>
<% end %>
<% end %>
<% end %>
</h4>
<p class="footnote">(<%= allowed_html_instructions %>)</p>
<% end %>

<% elsif logged_in_as_admin? %>
ceithir marked this conversation as resolved.
Show resolved Hide resolved
<h4 class="heading"><%= ts("Comment as") %> <span class="byline"><%= current_admin.login %></span>
<%= f.hidden_field :name, :value => "#{current_admin.login}", :id => "comment_name_for_#{commentable.id}" %>
<%= f.hidden_field :email, :value => "#{current_admin.email}", :id => "comment_email_for_#{commentable.id}" %>
</h4>
<p class="footnote">(<%= allowed_html_instructions %>)</p>
<% end %>

<% elsif logged_in_as_admin? %>
<h4 class="heading"><%= ts("Comment as") %> <span class="byline"><%= current_admin.login %></span>
<%= f.hidden_field :name, :value => "#{current_admin.login}", :id => "comment_name_for_#{commentable.id}" %>
<%= f.hidden_field :email, :value => "#{current_admin.email}", :id => "comment_email_for_#{commentable.id}" %>
</h4>
<p class="footnote">(<%= allowed_html_instructions %>)</p>

<% else %>
<dl>
<dt class="landmark"><%= ts("Note") %>:</dt>
<dd class="instructions comment_form"><%=ts("All fields are required. Your email address will not be published.") %></dd>
<dt><%= f.label "name_for_#{commentable.id}", ts("Guest name: ") %></dt>
<dd>
<%= f.text_field :name, :id => "comment_name_for_#{commentable.id}" %>
<%= live_validation_for_field("comment_name_for_#{commentable.id}", :failureMessage => ts('Please enter your name.')) %>
</dd>
<dt><%= f.label "email_for_#{commentable.id}", ts("Guest email: ") %></dt>
<dd>
<%= f.text_field :email, :id => "comment_email_for_#{commentable.id}" %>
<%= live_validation_for_field("comment_email_for_#{commentable.id}", :failureMessage => ts('Please enter your email address.')) %>
</dd>
</dl>
<p class="footnote">(<%= allowed_html_instructions %>)</p>
<% end %>

<p>
<% content_id = "comment_content_for_#{commentable.id}" %>
<label for="<%= content_id %>" class="landmark"><%= ts("Comment") %></label>
<%= f.text_area :comment_content, :id => content_id, :class => "comment_form observe_textlength", :title => ts("Enter Comment") %>
<input type="hidden" id="controller_name_for_<%= commentable.id %>" name="controller_name" value="<%= @controller_name ||= controller.controller_name %>" />
</p>
<%= generate_countdown_html("comment_content_for_#{commentable.id}", ArchiveConfig.COMMENT_MAX) %>
<%= live_validation_for_field("comment_content_for_#{commentable.id}",
:failureMessage => ts('Brevity is the soul of wit, but we need your comment to have text in it.'),
:maximum_length => ArchiveConfig.COMMENT_MAX,
:tooLongMessage => ts("must be less than %{count} characters long.", :count => ArchiveConfig.COMMENT_MAX)) %>
<p class="submit actions">
<%= f.submit button_name, :id => "comment_submit_for_#{commentable.id}", data: {disable_with: ts("Please wait...")} %>
<% if controller.controller_name == 'inbox' %>
<a name="comment_cancel" id="comment_cancel"><%= ts("Cancel") %></a>
<% elsif comment.persisted? %>
<%= cancel_edit_comment_link(comment) %>
<% elsif commentable.is_a?(Comment) || commentable.is_a?(CommentDecorator) %>
<%= cancel_comment_reply_link(commentable) %>
<% else %>
<dl>
<dt class="landmark"><%= ts("Note") %>:</dt>
<dd class="instructions comment_form"><%=ts("All fields are required. Your email address will not be published.") %></dd>
<dt><%= f.label "name_for_#{commentable.id}", ts("Guest name: ") %></dt>
<dd>
<%= f.text_field :name, :id => "comment_name_for_#{commentable.id}" %>
<%= live_validation_for_field("comment_name_for_#{commentable.id}", :failureMessage => ts('Please enter your name.')) %>
</dd>
<dt><%= f.label "email_for_#{commentable.id}", ts("Guest email: ") %></dt>
<dd>
<%= f.text_field :email, :id => "comment_email_for_#{commentable.id}" %>
<%= live_validation_for_field("comment_email_for_#{commentable.id}", :failureMessage => ts('Please enter your email address.')) %>
</dd>
</dl>
<p class="footnote">(<%= allowed_html_instructions %>)</p>
<% end %>
</p>
</fieldset>
<% end %>
</div>
<div class="clear"></div>

<p>
<% content_id = "comment_content_for_#{commentable.id}" %>
<label for="<%= content_id %>" class="landmark"><%= ts("Comment") %></label>
<%= f.text_area :comment_content, :id => content_id, :class => "comment_form observe_textlength", :title => ts("Enter Comment") %>
<input type="hidden" id="controller_name_for_<%= commentable.id %>" name="controller_name" value="<%= @controller_name ||= controller.controller_name %>" />
</p>
<%= generate_countdown_html("comment_content_for_#{commentable.id}", ArchiveConfig.COMMENT_MAX) %>
<%= live_validation_for_field("comment_content_for_#{commentable.id}",
:failureMessage => ts('Brevity is the soul of wit, but we need your comment to have text in it.'),
:maximum_length => ArchiveConfig.COMMENT_MAX,
:tooLongMessage => ts("must be less than %{count} characters long.", :count => ArchiveConfig.COMMENT_MAX)) %>
<p class="submit actions">
<%= f.submit button_name, :id => "comment_submit_for_#{commentable.id}", data: {disable_with: ts("Please wait...")} %>
<% if controller.controller_name == 'inbox' %>
<a name="comment_cancel" id="comment_cancel"><%= ts("Cancel") %></a>
<% elsif comment.persisted? %>
<%= cancel_edit_comment_link(comment) %>
<% elsif commentable.is_a?(Comment) || commentable.is_a?(CommentDecorator) %>
<%= cancel_comment_reply_link(commentable) %>
<% end %>
</p>
</fieldset>
<% end %>
</div>
<div class="clear"></div>
<% end %>
2 changes: 1 addition & 1 deletion app/views/comments/_commentable.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<li><%= link_to ts('Back to AO3 News Index'), admin_posts_path %></li>
<% end %>

<% if @work && !is_author_of?(@work) %>
<% if @work && !is_author_of?(@work) && !logged_in_as_admin? %>
<li>
<%= form_for(:kudo, url: kudos_path, html: { method: 'post', id: 'new_kudo' }) do |kudo_form| %>
<%= kudo_form.hidden_field :commentable_id, :value => @work.id %>
Expand Down
2 changes: 1 addition & 1 deletion config/locales/views/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ en:
page_title: "Hi, %{login}!"
confidentiality_reminder: "You are now logged in as an admin. That means you will probably encounter information that is personal or confidential (e.g. usernames, email and IP addresses, creator names on anonymous works, etc). Please do not use this information in ways unrelated to your OTW role. If you have questions about what you can or cannot do with information you see here, contact your committee chair(s)."
responsibility: "With great power comes great responsibility."
log_out_reminder: "Please remember to log out before resuming your normal site activity and leaving comments and kudos!"
log_out_reminder: "Please remember to log out before resuming your normal site activity!"
roles:
heading: "Your admin roles:"
none: "You currently have no admin roles assigned to you."
Expand Down
9 changes: 9 additions & 0 deletions features/comments_and_kudos/add_comment.feature
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,12 @@ Scenario: Try to post a comment with a < angle bracket before a linebreak, with
"""
And I press "Comment"
Then I should see "Comment created!"

Scenario: Cannot comment (no form) while logged as admin

Given the work "Generic Work"
And I am logged in as an admin
And I view the work "Generic Work"
Then I should see "Generic Work"
And I should not see "Post Comment"
And I should not see a "Comment" button
7 changes: 7 additions & 0 deletions features/comments_and_kudos/kudos.feature
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,10 @@ Feature: Kudos
And the email should contain "Meh Story"
And the email should not contain "0 guests"
And the email should not contain "translation missing"

Scenario: Cannot leave kudos (no button) while logged as admin

Given I am logged in as an admin
And I view the work "Awesome Story"
Then I should see "Awesome Story"
And I should not see a "Kudos ♥" button
15 changes: 4 additions & 11 deletions features/tags_and_wrangling/tag_comment.feature
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,17 @@ I'd like to comment on a tag'
Scenario: Multiple comments on a tag increment correctly

Given the following activated tag wranglers exist
| login |
| dizmo |
| login |
| dizmo |
| someone_else |
And a fandom exists with name: "Stargate Atlantis", canonical: true
When I am logged in as "dizmo"
When I post the comment "Yep, we should have a Stargate franchise metatag." on the tag "Stargate Atlantis"
When I am logged in as an admin
When I am logged in as "someone_else"
When I post the comment "Important policy decision" on the tag "Stargate Atlantis"
When I view the tag "Stargate Atlantis"
Then I should see "2 comments"

Scenario: admin can also comment on tags, issue 1428

Given a fandom exists with name: "Stargate Atlantis", canonical: true
When I am logged in as an admin
When I post the comment "Important policy decision" on the tag "Stargate Atlantis" via web
When I view the tag "Stargate Atlantis"
Then I should see "1 comment"

Scenario: Issue 2185: email notifications for tag commenting; TO DO: replies to comments

Given the following activated tag wranglers exist
Expand Down
20 changes: 13 additions & 7 deletions spec/controllers/comments_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -249,14 +249,9 @@

it "posts the comment and shows it in context" do
post :create, params: { tag_id: fandom.name, comment: anon_comment_attributes }
expect(flash[:notice]).to eq("Please log out of your admin account first!")
comment = Comment.last
expect(comment.commentable).to eq fandom
expect(comment.name).to eq anon_comment_attributes[:name]
expect(comment.email).to eq anon_comment_attributes[:email]
expect(comment.comment_content).to include anon_comment_attributes[:comment_content]
path = comments_path(tag_id: fandom.to_param,
anchor: "comment_#{comment.id}")
expect(response).to redirect_to path
expect(comment).to eq nil
end
end

Expand Down Expand Up @@ -340,6 +335,17 @@
expect(cookies[:flash_is_set]).to eq(1)
end
end

context "when logged in as an admin" do
let(:work) { create(:work) }

before { fake_login_admin(create(:admin)) }

it "asks to log out first" do
ceithir marked this conversation as resolved.
Show resolved Hide resolved
post :create, params: { work_id: work.id, comment: anon_comment_attributes }
expect(flash[:notice]).to eq("Please log out of your admin account first!")
end
end
end

context "when the commentable is an admin post" do
Expand Down
12 changes: 12 additions & 0 deletions spec/controllers/kudos_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -182,5 +182,17 @@
end
end
end

context "when kudos giver is admin" do
let(:work) { create(:work) }
let(:admin) { create(:admin) }

before { fake_login_admin(admin) }

it "asks to log out first" do
post :create, params: { kudo: { commentable_id: work.id, commentable_type: "Work" } }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the behavior like with format: :js?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely not great, as <html><body>You are being <a href="http://www.example.com/">redirected</a>.</body></html> is not valid JSON.

Could be handled more nicely like 5ddfb2e... But seems a bit overkill for what I understand is an edge case (maybe I'm missing something?), where the user clicks in the Kudos button in one tab after having logged as admin in another.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that does seem like overkill for the situation. Maybe just add a test showing no kudos is created and we'll leave it at that?

expect(flash[:notice]).to eq("Please log out of your admin account first!")
end
end
end
end