fix(pull): handle empty pull request files view to allow reviews#37783
Conversation
b84ddc6 to
774fbd5
Compare
|
Why should we handle empty pull requests? |
|
Thank you for the fix, will do some fine tunes. |
A use-case is to use an empty PR as placeholder to trigger early reviews for a new branch. |
|
Made some changes:
|
|
A PR also becomes empty after the changes get manually merged, outside of the PR. |
I can see 2 different cases:
And IIRC there are also some bugs reports for these edge cases, so maybe it needs more refactoring work in the future. (yes, not in this PR's scope 😄 ) |
There was a problem hiding this comment.
Pull request overview
This PR fixes the “Files Changed” view for empty pull requests (where the compare commit list can be empty because merge-base == head), so the page renders normally instead of erroring out, enabling users to submit PR reviews even when there are no file changes.
Changes:
- Adjusts
viewPullFilescommit resolution to gracefully handle empty PRs by falling back to loadingHeadCommitID/CompareBasedirectly from the git repo when they are not present in the PR commit list. - Replaces the previous “commit not found in PR commits” 400 behavior with a
NotFoundresponse for invalid commit selections. - Adds an integration test asserting that the PR “Files” tab renders and shows the “Diff Content Not Available” placeholder for an empty PR.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
routers/web/repo/pull.go |
Makes the pull files view resilient to empty PR commit lists by fetching missing base/head commits from the repo. |
tests/integration/pull_status_test.go |
Adds coverage to ensure empty PRs can load the /files tab and display the expected placeholder content. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@eliroca does the new change look good to you?
Update: thanks to bircni , backport here: #37785 |
) Backport #37783. Manually adapted for the pre-refactor pull view code (preparePullViewPullInfo / MergeBase).
|
@wxiaoguang: with this change, on repos with unrelated git histories, looking at the PR throws: I specifically tested that, as I've split the changes in this PR from a commit tackling both: empty PRs and unrelated histories diff: eliroca@a12e6e0 Example PR with unrelated histories, no diff is shown: https://gitea.com/eliroca/freetype2/pulls/1 logs here |
IIRC the "unrelated git histories" problems should have been fixed by Fix compare dropdown for branches without common history (#37470) Is there a change in this PR related to "unrelated git histories"? |
And there is no "routers/web/repo/pull.go:595:prepareViewPullInfo() " on main branch |
|
I was testing with 1.26.1, sorry for the noise if this is meanwhile fixed in main. |
|
So merge? |
|
Can you please verify that the changes don't break PRs with unrelated histories as in my test? |
Not quite sure about what you are referring to. The changes made by me:
If you'd like to improve, feel free to make more improvements and/or add more tests. |
|
It is not related to the changes here, my assumption was wrong. Please proceed here, I'll continue with a test for unrelated histories PRs tomorrow, and will follow up with a PR possibly. Sorry again for the noise. |
* origin/main: (104 commits) fix(deps): update module github.com/go-git/go-git/v5 to v5.19.1 [security] (go-gitea#37786) fix(pull): handle empty pull request files view to allow reviews (go-gitea#37783) fix(markup): make RenderString never fail (go-gitea#37779) fix(markup): wrap indented code blocks for the code-copy button (go-gitea#37748) fix(permissions): Fix reading permission (go-gitea#37769) fix: add natural sort to sortTreeViewNodes (go-gitea#37772) fix: package creation unique conflict (go-gitea#37774) fix(deps): update npm dependencies (go-gitea#37768) fix(deps): update module gitlab.com/gitlab-org/api/client-go/v2 to v2.26.0 (go-gitea#37771) ci: split giteabot workflow (go-gitea#37770) [skip ci] Updated translations via Crowdin fix: Unify public-only token filtering in API queries and repo access checks (go-gitea#37118) fix(deps): update module google.golang.org/grpc to v1.81.1 (go-gitea#37762) chore: make DefaultTitleSource default to auto to match GitHub (go-gitea#37767) ci: fix cache-related issues (go-gitea#37761) chore: fix tests (go-gitea#37760) refactor(waitgroup): replace Add/Done goroutines with WaitGroup.Go (go-gitea#37764) fix(deps): update go dependencies (go-gitea#37752) chore(deps): update action dependencies (go-gitea#37751) fix(deps): update module github.com/google/go-github/v85 to v86 (go-gitea#37754) ... # Conflicts: # .github/workflows/pull-db-tests.yml # modules/storage/s3_test.go
This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [code.gitea.io/gitea](https://github.com/go-gitea/gitea) | `v1.26.1` → `v1.26.2` |  |  | --- ### Release Notes <details> <summary>go-gitea/gitea (code.gitea.io/gitea)</summary> ### [`v1.26.2`](https://github.com/go-gitea/gitea/releases/tag/v1.26.2) [Compare Source](go-gitea/gitea@v1.26.1...v1.26.2) - SECURITY - fix(permissions): Fix reading permission ([#​37769](go-gitea/gitea#37769)) - fix(actions): make artifact signature payloads unambiguous ([#​37707](go-gitea/gitea#37707)) - fix: Unify public-only token filtering in API queries and repo access checks ([#​37118](go-gitea/gitea#37118)) - fix: Add missed token scope checking ([#​37735](go-gitea/gitea#37735)) - fix(oauth): bind token exchanges to the original client request ([#​37704](go-gitea/gitea#37704)) - fix(oauth): strengthen PKCE validation and refresh token replay protection ([#​37706](go-gitea/gitea#37706)) - fix(web): enforce token scopes on raw, media, and attachment downloads ([#​37698](go-gitea/gitea#37698)) - fix(security): enforce wiki git writes and LFS token access at request time ([#​37695](go-gitea/gitea#37695)) - feat(api): encrypt AWS creds ([#​37679](go-gitea/gitea#37679)) - fix(deps): update dependency mermaid to v11.15.0 \[security], add e2e test - fix(packages): Add label for private and internal package and fix composor package source permission check ([#​37610](go-gitea/gitea#37610)) - fix(git): Fix smart http request scope bug ([#​37583](go-gitea/gitea#37583)) - Fix basic auth bug ([#​37503](go-gitea/gitea#37503)) - Fix allow maintainer edit permission check ([#​37479](go-gitea/gitea#37479)) ([#​37484](go-gitea/gitea#37484)) - Fix URL sanitization to handle schemeless credentials ([#​37440](go-gitea/gitea#37440)) ([#​37471](go-gitea/gitea#37471)) - Fix attachment Content-Security-Policy ([#​37455](go-gitea/gitea#37455)) ([#​37464](go-gitea/gitea#37464)) - chore(deps): bump go-git/go-git/v5 to 5.19.0 ([#​37608](go-gitea/gitea#37608)) - BUGFIXES - fix(pull): handle empty pull request files view to allow reviews ([#​37783](go-gitea/gitea#37783)) - fix(markup): make RenderString never fail ([#​37779](go-gitea/gitea#37779)) - fix: add natural sort to sortTreeViewNodes ([#​37772](go-gitea/gitea#37772)) - fix: package creation unique conflict ([#​37774](go-gitea/gitea#37774)) - fix!: add DEFAULT\_TITLE\_SOURCE setting for pull request title default behavior ([#​37465](go-gitea/gitea#37465)) - fix: Allow direct commits for unprotected files with push restrictions ([#​37657](go-gitea/gitea#37657)) - fix(actions): wrong assumption that run id always >= job id ([#​37737](go-gitea/gitea#37737)) - fix(auth): set User-Agent on avatar fetch and sync avatar on link-account register ([#​37564](go-gitea/gitea#37564)) ([#​37588](go-gitea/gitea#37588)) - fix(actions): deadlock between PrepareRunAndInsert and UpdateTaskByState ([#​37692](go-gitea/gitea#37692)) - fix(repo): /generate must sync the branch table for the new repo ([#​37693](go-gitea/gitea#37693)) - build: Fix snap build (1.26) - fix(actions): run TransferLogs on UpdateLog{Rows:\[], NoMore:true} ([#​37631](go-gitea/gitea#37631)) - fix show correct mergebase - fix: make clone URL respect public URL detection setting ([#​37615](go-gitea/gitea#37615)) - fix: "run as root" check ([#​37622](go-gitea/gitea#37622)) - chore(deps): update dependency go to v1.26.3 ([#​37601](go-gitea/gitea#37601)) - Compare dropdown fails when selecting branch with no common merge-base ([#​37470](go-gitea/gitea#37470)) - fix: treat email addresses case-insensitively ([#​37600](go-gitea/gitea#37600)) - fix(actions): fix blank lines after ::endgroup:: ([#​37597](go-gitea/gitea#37597)) - fix(actions): report individual step status in workflow job API response ([#​37592](go-gitea/gitea#37592)) - fix: Invalid UTF-8 commit messages in JSON API responses ([#​37542](go-gitea/gitea#37542)) - fix: use consistent GetUser family functions ([#​37553](go-gitea/gitea#37553)) - fix(api): return 409 message instead of empty JSON for wrong commit id ([#​37572](go-gitea/gitea#37572)) - fix(actions): prevent panic when workflow contains null jobs ([#​37570](go-gitea/gitea#37570)) - Make ServeSetHeaders default to download attachment if filename exists ([#​37552](go-gitea/gitea#37552)) ([#​37555](go-gitea/gitea#37555)) - Fix(actions): validate workflow param to prevent 500 error ([#​37546](go-gitea/gitea#37546)) ([#​37554](go-gitea/gitea#37554)) - Don't unblock run-level-concurrency-blocked runs in the resolver ([#​37461](go-gitea/gitea#37461)) ([#​37538](go-gitea/gitea#37538)) - Fix(packages): use file names for generic web downloads ([#​37514](go-gitea/gitea#37514)) ([#​37520](go-gitea/gitea#37520)) - Fix merge autodetect can't close other PRs but only the last one when multiple PRs are pushed at once ([#​37512](go-gitea/gitea#37512)) ([#​37516](go-gitea/gitea#37516)) - Fix update branch protection order ([#​37508](go-gitea/gitea#37508)) ([#​37513](go-gitea/gitea#37513)) - Fix mCaptcha broken after Vite migration ([#​37492](go-gitea/gitea#37492)) ([#​37509](go-gitea/gitea#37509)) - Fix review submission from single-commit PR view ([#​37475](go-gitea/gitea#37475)) ([#​37485](go-gitea/gitea#37485)) - Fix scheduled action panic with null event payload ([#​37459](go-gitea/gitea#37459)) ([#​37466](go-gitea/gitea#37466)) - Make GetPossibleUserByID can handle deleted user ([#​37430](go-gitea/gitea#37430)) ([#​37431](go-gitea/gitea#37431)) - Remove excessive quote from terraform instructions ([#​37424](go-gitea/gitea#37424)) ([#​37426](go-gitea/gitea#37426)) - Fix color regressions, add `priority` color ([#​37417](go-gitea/gitea#37417)) ([#​37421](go-gitea/gitea#37421)) - MISC - Add CurrentURL template variable back ([#​37444](go-gitea/gitea#37444)) ([#​37449](go-gitea/gitea#37449)) Instances on **[Gitea Cloud](https://cloud.gitea.com)** will be automatically upgraded to this version during the specified maintenance window. </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL3BhdGNoIl19--> Reviewed-on: https://git.erwanleboucher.dev/eleboucher/apoci/pulls/47
Example: https://gitea.com/eliroca/empty-pull-request/pulls/1
And the "Files Changed" tab: https://gitea.com/eliroca/empty-pull-request/pulls/1/files
Instead, let's handle empty pull requests and show the usual UI.