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 43 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 @@ -30,6 +30,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: [:new, :create, :add_comment_reply]

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
1 change: 1 addition & 0 deletions app/helpers/comments_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ def can_reply_to_comment?(comment)
return false if comment_parent_hidden?(comment)
return false if blocked_by_comment?(comment)
return false if blocked_by?(comment.ultimate_parent)
return false if logged_in_as_admin?

return true unless guest?

Expand Down
9 changes: 1 addition & 8 deletions app/views/comments/_comment_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<% 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.") %>
<%= ts("While this work is anonymous, comments you post will also be listed anonymously.") %>

Check failure on line 38 in app/views/comments/_comment_form.html.erb

View workflow job for this annotation

GitHub Actions / ERB Lint runner

[] reported by reviewdog 🐶 I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards Raw Output: app/views/comments/_comment_form.html.erb:38:14: I18n/DeprecatedHelper: Prefer Rails built-in `t` helper over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards
</p>
<% end %>

Expand Down Expand Up @@ -64,13 +64,6 @@
<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>
Expand Down
6 changes: 5 additions & 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) && !blocked_by?(@work) %>
<% if @work && !is_author_of?(@work) && !blocked_by?(@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 Expand Up @@ -113,6 +113,10 @@
<p class="notice">
<%= t(".blocked") %>
</p>
<% elsif logged_in_as_admin? %>
<p class="notice">
<%= t(".logged_as_admin") %>
</p>
<% else %>
<div id="add_comment_placeholder" title="top level comment">
<div id="add_comment">
Expand Down
3 changes: 2 additions & 1 deletion config/locales/views/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ en:
admins:
index:
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).
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!
page_title: Hi, %{login}!
responsibility: With great power comes great responsibility.
roles:
Expand Down Expand Up @@ -421,6 +421,7 @@ en:
blocked: Sorry, you have been blocked by one or more of this work's creators.
guest_comments_disabled: Sorry, the Archive doesn't allow guests to comment right now.
invite_to_collections_link: Invite To Collections
logged_as_admin: Please log out of your admin account to comment.
permissions:
admin_post:
alt_action: You can however %{support_link} with any feedback or questions.
Expand Down
5 changes: 0 additions & 5 deletions features/admins/admin_settings.feature
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,6 @@ Feature: Admin Settings Page
Scenario: Tag comments are not affected when guest comments are turned off
Given guest comments are off
And a fandom exists with name: "Stargate SG-1", canonical: true
When I am logged in as a super admin
And I view the tag "Stargate SG-1" with comments
Then I should not see "Sorry, the Archive doesn't allow guests to comment right now."
When I post the comment "Important policy decision" on the tag "Stargate SG-1"
Then I should see "Comment created!"
When I am logged in as a tag wrangler
And I view the tag "Stargate SG-1" with comments
Then I should not see "Sorry, the Archive doesn't allow guests to comment right now."
Expand Down
27 changes: 27 additions & 0 deletions features/comments_and_kudos/add_comment.feature
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,30 @@ Scenario: Users with different time zone preferences should see the time in thei
And I view the work "Generic Work" with comments
Then I should see "AEST" within ".posted.datetime"
And I should see "AEST" within ".edited.datetime"

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
And I should see "Please log out of your admin account to comment."

Scenario: Cannot reply to comments (no button) while logged as admin

Given the work "Generic Work"
When I am logged in as "commenter"
And I view the work "Generic Work"
And I post a comment "Woohoo"
When I am logged in as an admin
And I view the work "Generic Work"
And I follow "Comments (1)"
Then I should see "Woohoo"
And I should not see "Reply"
When I am logged out
And I view the work "Generic Work"
And I follow "Comments (1)"
Then I should see "Woohoo"
And I should see "Reply"
6 changes: 6 additions & 0 deletions features/comments_and_kudos/kudos.feature
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,9 @@ Feature: Kudos
And the kudos cache has expired
And I view the work "Interesting beans"
Then I should not see "newusername1 left kudos on this work!"

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
36 changes: 25 additions & 11 deletions spec/controllers/comments_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@
it_redirects_to_with_error(work_path(work), "Sorry, this work doesn't allow comments.")
end
end

context "when logged in as an admin" do
before { fake_login_admin(create(:admin)) }

it "redirects to root with notice prompting log out" do
get :add_comment_reply, params: { comment_id: comment.id }
it_redirects_to_with_notice(root_path, "Please log out of your admin account first!")
end
end
end

shared_examples "guest cannot reply to a user with guest replies disabled" do
Expand Down Expand Up @@ -300,10 +309,9 @@
context "when logged in as an admin" do
before { fake_login_admin(create(:admin)) }

it "renders the :new template" do
it "redirects to root with notice prompting log out" do
get :new, params: { tag_id: fandom.name }
expect(response).to render_template("new")
expect(assigns(:name)).to eq("Fandom")
it_redirects_to_with_notice(root_path, "Please log out of your admin account first!")
end
end

Expand Down Expand Up @@ -531,16 +539,11 @@
context "when logged in as an admin" do
before { fake_login_admin(create(:admin)) }

it "posts the comment and shows it in context" do
it "redirects to root with notice prompting log out" do
post :create, params: { tag_id: fandom.name, comment: anon_comment_attributes }
it_redirects_to_with_notice(root_path, "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 @@ -624,6 +627,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 "redirects to root with notice prompting log out" do
post :create, params: { work_id: work.id, comment: anon_comment_attributes }
it_redirects_to_with_notice(root_path, "Please log out of your admin account first!")
end
end
end

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

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

before { fake_login_admin(admin) }

it "redirects to root with notice prompting log out" 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?

it_redirects_to_with_notice(root_path, "Please log out of your admin account first!")
expect(assigns(:kudo)).to be_nil
end

context "with format: :js" do
it "does not create any kudo" do
post :create, params: { kudo: { commentable_id: work.id, commentable_type: "Work" }, format: :js }
expect(assigns(:kudo)).to be_nil
end
end
end
end
end
Loading