Redirect commit pages when URL branch doesn't contain the commit#93
Merged
Conversation
CommitsController#show used to redirect only when the :branch URL segment didn't match any Branch record. A URL like /main/commits/de16d72 where de16d72 lives only on a feature branch would silently render the commit "in main's context" — same matrix data, but with main's subway map, main's last-clean-commit, and a breadcrumb that lied about where the SHA actually lived. A user who hit one of those links via a stale share couldn't tell what they were looking at. Promote the existing redirect to also catch the "branch exists but doesn't contain this commit" case. Both branches of the fallback share one helper that picks a target branch — prefer main, then the branch with the most recent head, with alphabetical as a tiebreaker — and one helper that builds the flash. The redirect target lives on Commit#preferred_branch so the same precedence is available outside the controller. TestCaseCommitsController#show grew the same fallback, since its URLs have the same branch segment and the same exposure. The flash banner reports both the requested branch and the alternatives the commit is on, so a user landing via a stale link can pick a different containing branch if they want.
wmwolf
added a commit
that referenced
this pull request
May 26, 2026
- Spec count 263 → 336. - New reality-check: singleton submissions with `compiled=nil` are treated as built when test_instances exist (PR #92). - New reality-check: commit-page URLs whose branch doesn't contain the SHA redirect to a containing branch via `Commit#preferred_branch` + the `BranchMismatchRedirect` concern, with the redirect target documented as main → most-recent-head → alphabetical (PR #93). Search results use the same precedence via `TestInstancesHelper#best_branch_name_for`. - New reality-check: only `runtime_minutes` is populated by the ingest factory; `runtime_seconds`, `re_time`, and `total_runtime_seconds` are dead schema columns and any read or filter that hits them silently returns nothing. Documents what the search-runtime fix (PR #94) is restoring.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CommitsController#showandTestCaseCommitsController#shownow redirect to a containing branch whenever the URL's:branchsegment doesn't include this commit — not just when the branch doesn't exist as a record. A flash banner explains the mismatch and lists the branches the commit is actually on.Commit#preferred_branch: prefersmain, then the branch with the most-recent head, alphabetical tiebreaker.BranchMismatchRedirectconcern keeps the predicate + flash builder in one place.Background
A user reported that
/main/commits/de16d72rendered de16d72 "on main" even though it lives only onfeature/superad-turnover-time-limiter. The existing fallback at commits_controller.rb:18 only fired when the URL branch wasn't a valid Branch record. If the user typed (or was sent) a URL wheremainexisted but didn't contain the SHA, no redirect happened — and the page silently presented the commit under the wrong branch's subway map, neighbors, and last-clean-commit.Now any URL whose branch segment doesn't contain the commit redirects to one that does, with a flash like:
When the URL branch doesn't exist (the pre-existing case), the flash flips to "Branch 'X' doesn't exist." and the redirect target follows the same precedence. Multi-branch commits get an "Also on:" suffix so the user can pick another branch.
Changes
app/models/commit.rb—#preferred_branchreturning a Branch (or nil for unbranched commits).app/controllers/concerns/branch_mismatch_redirect.rb— sharedbranch_mismatch?+branch_mismatch_message.app/controllers/commits_controller.rb— folded the old "no such branch" redirect into the same flow; usespreferred_branch. A commit on zero branches now 404s instead of crashing onnil.name.app/controllers/test_case_commits_controller.rb— same fallback, carrying the test_case + module through the redirect.spec/models/commit_spec.rbcovers the fourpreferred_branchcases (no branches / main present / most-recent-head wins / alphabetical tiebreaker);spec/requests/page_renders_spec.rbcovers the four request cases (branch doesn't contain / branch doesn't exist / multi-branch "Also on" / most-recent-head over alphabetical) plus the TCC variant.Test plan
bundle exec rspec— 313 examples, 0 failures/main/commits/de16d72redirects to/feature%2Fsuperad-turnover-time-limiter/commits/de16d72with the expected warning flash/totally-fake-branch/commits/aa27a08still redirects to/main/commits/aa27a08(pre-existing case) with updated phrasing/main/commits/aa27a08(valid URL) renders with no flash/main/commits/de16d72/test_cases/star/1.4M_ms_op_monoredirects to the same path under the containing branch