Skip to content

Support Forums: Return a 404 for topic pages past the last page of replies - #1207

Closed
dhrupo wants to merge 1 commit into
WordPress:trunkfrom
dhrupo:fix/4412-topic-page-404
Closed

dhrupo wants to merge 1 commit into
WordPress:trunkfrom
dhrupo:fix/4412-topic-page-404

Conversation

@dhrupo

@dhrupo dhrupo commented Nov 8, 2026 •

Copy link
Copy Markdown

Fixes https://meta.trac.wordpress.org/ticket/4412

A request for a topic page past its last page of replies, like /support/topic/<slug>/page/45/ on a topic with one page of replies, returns a 200 with only the lead post. It should be a 404.

Nothing checks the page number against the topic. bbp_has_replies() runs the reply query for any page and simply finds no replies, and WPORG_Page_Limiter only stops pages above 49 for logged-out users.

This adds a check on bbp_template_redirect, next to redirect_page_zero_one(). On a single topic with paged above 1, it counts the pages the same way bbp_get_topic_pagination() does (public reply count, plus the topic when it is part of the reply loop) and adds the hidden reply count when the user is viewing all replies (bbp_get_view_all( 'edit_others_replies' )), which is when bbp_has_replies() includes them. A page past that count gets bbp_set_404(). Threaded topics are skipped, as bbp_get_topic_pagination() skips them. Users with the moderate capability are skipped too: Report_Topic::maybe_include_reports() adds reported_topics posts to their reply loop, and no stored count includes those, so a report could push a real page past the count.

  • Page 9 of a topic with 2 pages of replies: 404, with the theme's 404 template
  • Page 2 of the same topic: 200 with replies 16 to 20, unchanged
  • Page 1: unchanged
  • Topic with 14 public and 2 pending replies: page 2 is a 404 for a visitor; a moderator with ?view=all gets page 2 with the pending replies
  • Topic with 15 public replies and 1 report: page 2 is a 404 for a visitor; a moderator gets page 2 with the report

Testing

  1. npm run support:env start in environments/. (On Docker Desktop this currently fails on the wporg-ratings.php mount; Environments: Fix jobs, WordPress.tv and Support Forums envs failing to start #1205 fixes that.)
  2. Create a topic with 20 replies. With 15 replies per page, it has 2 pages.
  3. Open /topic/<slug>/page/9/. It returns a 404 and shows the 404 page.
  4. Open /topic/<slug>/page/2/. It returns a 200 with the last 5 replies.

A Playwright run against the local environment failed on trunk with Expected: 404 Received: 200 (3 failed, 4 passed) and passes with this change (7 passed).

AI assistance: Yes. Tool: Claude Code. Used for: finding the cause and drafting the change; I reviewed and tested it myself.

Summary by CodeRabbit

  • Bug Fixes
    • Paginated, non-threaded topic pages beyond the available replies now return a not-found response instead of loading an invalid page. Page limits reflect replies visible to the visitor and whether the lead topic appears. Moderator access and other topic pages remain unchanged.

…plies

Requests like /topic/<slug>/page/45/ on a topic with one page of replies
returned a 200 with only the lead post. Count the pages the same way
bbp_get_topic_pagination() does, adding hidden replies when viewing all
replies, and set a 404 when the requested page is beyond them.

Moderators are skipped: their reply loop also includes topic reports,
which no stored count tracks.

See https://meta.trac.wordpress.org/ticket/4412
@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props dhrupo.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

@coderabbitai

coderabbitai Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The performance optimizations class now checks paginated, non-threaded topic pages during bbPress template redirects. It calculates the last page using visible replies and applicable hidden replies and lead-topic display settings. It sets a 404 for requests beyond that page, except for moderators.

Changes: Topic Pagination

Layer / File(s) Summary
Register and apply topic page limits
wordpress.org/public_html/wp-content/plugins/support-forums/inc/class-performance-optimizations.php
The constructor registers topic_page_out_of_range_404() on bbp_template_redirect. The method checks eligible topic pages, calculates the last page, skips moderators, and sets a 404 when the requested page exceeds the limit.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: jjj, obenland

Merge Risk: Minimal · up to ae25b

The pagination change appears mergeable after normal checks; no concrete issue requiring a pre-merge fix was identified.

Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title states the main change: topic pages requested past the last page of replies now return a 404. The raw summary confirms a new topic_page_out_of_range_404() method that sets a 404 when the req…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title states the main change: topic pages requested past the last page of replies now return a 404. The raw summary confirms a new topic_page_out_of_range_404() method that sets a 404 when the requested page exceeds the calculated last page. The objectives describe the same goal, so the title matches the changeset.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b2394185-5399-4cfa-b69f-f0de7d9382bf
Commits

Reviewing files that changed from the base of the PR and between 82f0414 and ae25bf4.

Files selected for processing: 1

  • wordpress.org/public_html/wp-content/plugins/support-forums/inc/class-performance-optimizations.php

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant