Skip to content

AO3-4583 Fix redirect when creating top-level comment on work with multiple pages of comments #5653

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.

Sign up for GitHub

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

Open
marcus8448 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

AO3-4583 Fix redirect when creating top-level comment on work with multiple pages of comments #5653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
29be43d
AO3-4583 Failing tests first
marcus8448 Mar 22, 2026
41c8f36
AO3-4583 Redirect to last page of comments when creating new top-leve…
marcus8448 Mar 22, 2026
faa32f9
Add comment to explain page selection reasoning
marcus8448 Apr 2, 2026
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
8 changes: 7 additions & 1 deletion app/controllers/comments_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,13 @@ def create
elsif @comment.unreviewed?
redirect_to_all_comments(@commentable)
else
redirect_to_comment(@comment, { view_full_work: (params[:view_full_work] == "true"), page: params[:page] })
# rubocop:disable Metrics/BlockNesting
view_full_work = params[:view_full_work] == "true"
parent = view_full_work || current_user&.preference&.view_full_works? ? @comment.ultimate_parent : @comment.parent
Comment on lines +428 to +429
Copy link
Copy Markdown
Member

@brianjaustin brianjaustin Apr 9, 2026

Choose a reason for hiding this comment

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

Nitpick: since the second condition is also a case of view_full_work, I think it would make more sense to lift the current user check up into that variable.

Suggested change
view_full_work = params[:view_full_work] == "true"
parent = view_full_work || current_user&.preference&.view_full_works? ? @comment.ultimate_parent : @comment.parent
view_full_work = params[:view_full_work] == "true" || current_user&.preference&.view_full_works?
parent = view_full_work ? @comment.ultimate_parent : @comment.parent

# if this comment starts a new top-level thread, it will always be on the last page of comments
page = parent.comments.reviewed.count.ceildiv(Comment.per_page) if @comment.thread == @comment.id
redirect_to_comment(@comment, { view_full_work: view_full_work, page: page || params[:page] })
# rubocop:enable Metrics/BlockNesting
end
end
end
Expand Down
52 changes: 52 additions & 0 deletions features/comments_and_kudos/comments_redirect.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,55 @@ Scenario: Posting top level comment on a middle chapter, while in temporary view
And I should see "Chapter 2" within "div#chapters"
# Once you've commented, it defaults back to your preference
And I should see "Chapter 1" within "div#chapters"

Scenario: Posting a top level comment on a one-chapter work, when there is more than one page of comments
Given the work "The Collection" with 2 comments setup
And 2 comments displayed per page
And I am logged in as a random user
When I view the work "The Collection"
And I post a comment "wow, that's a lot of comments"
Then I should see "wow, that's a lot of comments"
When I view the work "The Collection"
And I post a comment "very over 1000 amazing"
Then I should see "very over 1000 amazing"
When I post a comment "have another comment"
Then I should see "have another comment"
And I should not see "wow, that's a lot of comments"

Scenario: Posting a top level comment on a multi-chapter work, when there is more than one page of comments,
with view full work in the preferences
Given the work "Welcome"
And 2 comments displayed per page
And a comment "hello." by "somebody" on the work "Welcome"
And a chapter is added to "Welcome"
And a comment "hi!" by "somebody" on the work "Welcome"
And a chapter is added to "Welcome"
And a comment "heyo" by "somebody" on the work "Welcome"
And I am logged in as a random user
And I set my preferences to View Full Work mode by default
When I view the work "Welcome"
And I post a comment "sup?"
Then I should see "sup?"
And I should see "heyo"
And I should not see "hi!"

Scenario: Posting a top level comment on the middle chapter of a work, when there is more than one page of comments,
Copy link
Copy Markdown
Member

@brianjaustin brianjaustin Apr 9, 2026

Choose a reason for hiding this comment

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

There are 3 cases for top level comments. Shouldn't we also have at least a few cases for non-top-level ones too?

with view full work in the preferences, and while in temporary view by chapter mode
Given the work "Welcome"
And 2 comments displayed per page
And a comment "hello." by "somebody" on the work "Welcome"
And a chapter is added to "Welcome"
And a comment "hi!" by "somebody" on the work "Welcome"
And a comment "hiya" by "somebody" on the work "Welcome"
And a chapter is added to "Welcome"
And a comment "heyo" by "somebody" on the work "Welcome"
And a comment "hey!" by "somebody" on the work "Welcome"
And I am logged in as a random user
And I set my preferences to View Full Work mode by default
When I view the work "Welcome" in chapter-by-chapter mode
And I view the 2nd chapter
And I post a comment "sup?"
# Once you've commented, it defaults back to your preference (full work)
Then I should see "sup?"
And I should see "hey!"
And I should not see "hiya"
4 changes: 4 additions & 0 deletions features/step_definitions/comment_steps.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@
step %{I am logged in as "commentrecip"}
end

Given "{int} comment(s) displayed per page" do |per_page|
allow(Comment).to receive(:per_page).and_return(per_page)
end

# THEN

Then /^the comment's posted date should be nowish$/ do
Expand Down
15 changes: 9 additions & 6 deletions features/tags_and_wrangling/tag_comment.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,29 +168,32 @@ I'd like to comment on a tag'
And I am logged in as a tag wrangler
When I post the comment "And now things should not break!" on the tag "hack/sign"
Then I should see "Comment created"
And I should see "And now things should not break!"
# all it checks is that the pagination links aren't broken
When I follow "Next" within ".pagination"
Then I should see "And now things should not break!"
When I follow "Previous" within ".pagination"
Then I should not see "And now things should not break!"

Scenario: Comments pagination for a tag with periods in the name

Given a period-containing tag "sign.me" with 34 comments
And I am logged in as a tag wrangler
When I post the comment "And now things should not break!" on the tag "sign.me"
Then I should see "Comment created"
And I should see "And now things should not break!"
# all it checks is that the pagination links aren't broken
When I follow "Next" within ".pagination"
Then I should see "And now things should not break!"
When I follow "Previous" within ".pagination"
Then I should not see "And now things should not break!"

Scenario: Comments pagination for a tag with slashes and periods in the name

Given a tag "hack/sign.me" with 34 comments
And I am logged in as a tag wrangler
When I post the comment "And now things should not break!" on the tag "hack/sign.me"
Then I should see "Comment created"
And I should see "And now things should not break!"
# all it checks is that the pagination links aren't broken
When I follow "Next" within ".pagination"
Then I should see "And now things should not break!"
When I follow "Previous" within ".pagination"
Then I should not see "And now things should not break!"

Scenario: Comments on a tag should not be visible to non-wranglers.

Expand Down
7 changes: 4 additions & 3 deletions spec/controllers/comments/default_rails_actions_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@
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,
page: 1,
anchor: "comment_#{comment.id}")
expect(response).to redirect_to path
end
Expand Down Expand Up @@ -520,7 +521,7 @@
post :create, params: { work_id: work.id, comment: comment_attributes }
comment = Comment.last
expect(flash[:error]).to be_nil
expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, view_full_work: false, anchor: "comment_#{comment.id}"))
expect(response).to redirect_to(chapter_path(comment.commentable, show_comments: true, view_full_work: false, page: 1, anchor: "comment_#{comment.id}"))
end
end
end
Expand Down Expand Up @@ -638,7 +639,7 @@
fake_login_known_user(user)
post :create, params: { work_id: work.id, comment: comment_attributes }
comment = assigns[:comment]
it_redirects_to_with_comment_notice(chapter_path(comment.commentable, show_comments: true, view_full_work: false, anchor: "comment_#{comment.id}"), "Comment created!")
it_redirects_to_with_comment_notice(chapter_path(comment.commentable, show_comments: true, view_full_work: false, page: 1, anchor: "comment_#{comment.id}"), "Comment created!")
expect(comment.user_agent.length).to eq(500)
end
end
Expand All @@ -656,7 +657,7 @@
fake_login_known_user(user)
post :create, params: { work_id: work.id, comment: comment_attributes }
comment = assigns[:comment]
it_redirects_to_with_comment_notice(chapter_path(comment.commentable, show_comments: true, view_full_work: false, anchor: "comment_#{comment.id}"), "Comment created!")
it_redirects_to_with_comment_notice(chapter_path(comment.commentable, show_comments: true, view_full_work: false, page: 1, anchor: "comment_#{comment.id}"), "Comment created!")
expect(comment.user_agent).to be_nil
end
end
Expand Down
Loading