Skip to content

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

Open
marcus8448 wants to merge 3 commits into
otwcode:masterfrom
marcus8448:AO3-4583
Open

AO3-4583 Fix redirect when creating top-level comment on work with multiple pages of comments#5653
marcus8448 wants to merge 3 commits into
otwcode:masterfrom
marcus8448:AO3-4583

Conversation

@marcus8448
Copy link
Copy Markdown
Member

Issue

https://otwarchive.atlassian.net/browse/AO3-4583

Purpose

Fixes the comment creation redirect to point at the page of the comment you just created instead of the page of comments you were on.

Credit

marcus8448 (he/him)

@github-actions github-actions Bot added Scope: Tests Only Only changes automated tests or test configuration Awaiting Review labels Mar 22, 2026
@marcus8448 marcus8448 removed the Scope: Tests Only Only changes automated tests or test configuration label Mar 22, 2026
@brianjaustin brianjaustin self-requested a review April 9, 2026 07:55
Comment on lines +428 to +429
view_full_work = params[:view_full_work] == "true"
parent = view_full_work || current_user&.preference&.view_full_works? ? @comment.ultimate_parent : @comment.parent
Copy link
Copy Markdown
Member

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

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

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?

@Bilka2
Copy link
Copy Markdown
Contributor

Bilka2 commented Apr 9, 2026

This code overlaps with #5524 in a non-desirable way: The user should stay in chapter by chapter if they were in chapter by chapter, regardless of preference. That PR has other issues that I'll try to dig up from my memory to properly review (I gave up because it turned out more complicated than I had patience for at the time) but generally I think these PRs will need to work around each other

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants