Fix stange behavior of DownloadPullDiffOrPatch in incorect index

[pricly-yellow]

Close #17105

[codecov-commenter]

Codecov Report

Merging #17223 (4c65a51) into main (f2e7d54) will decrease coverage by 0.06%.
The diff coverage is 49.61%.

@@            Coverage Diff             @@
##             main   #17223      +/-   ##
==========================================
- Coverage   45.31%   45.25%   -0.07%     
==========================================
  Files         773      779       +6     
  Lines       86873    87287     +414     
==========================================
+ Hits        39366    39501     +135     
- Misses      41151    41369     +218     
- Partials     6356     6417      +61     

Impacted Files	Coverage Δ
cmd/admin.go	0.00% <0.00%> (ø)
models/action.go	48.86% <0.00%> (ø)
models/avatar.go	35.38% <0.00%> (ø)
models/error.go	39.83% <ø> (+0.29%)	⬆️
models/fixture_generation.go	70.00% <0.00%> (ø)
models/gpg_key_verify.go	13.46% <0.00%> (ø)
models/helper.go	52.94% <0.00%> (ø)
models/issue_lock.go	0.00% <0.00%> (ø)
models/migrate.go	0.00% <0.00%> (ø)
models/pull_sign.go	37.64% <0.00%> (ø)
... and 216 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4ea6cc...4c65a51. Read the comment docs.

[a1012112796]

suggest update test code:

diff --git a/models/pull_test.go b/models/pull_test.go
index 2b7ef2f66..173977aaf 100644
--- a/models/pull_test.go
+++ b/models/pull_test.go
@@ -134,6 +134,10 @@ func TestGetPullRequestByIndex(t *testing.T) {
 	_, err = GetPullRequestByIndex(9223372036854775807, 9223372036854775807)
 	assert.Error(t, err)
 	assert.True(t, IsErrPullRequestNotExist(err))
+
+	_, err = GetPullRequestByIndex(1, 0)
+	assert.Error(t, err)
+	assert.True(t, IsErrPullRequestNotExist(err))
 }
 
 func TestGetPullRequestByID(t *testing.T) {

[pricly-yellow]

Good idea, push it now

[delvh]

Apart from this question: LGTM.

[6543]

the (xorm) Get will ignore fields with "default init values" and match with set one

var value int64
fmt.Println(value)

That's why Get is mostly used with the ".ID()" function witch we can not use since index is a normal field for xorm (we do use it ID'like for each repo)

So either check with if if it is 0 and return 404 or add a explisit Where("index=?", pr.index). In this case an if is more elegant since 0 is not an valide index

@delvh (:point_up:)

[6543]

@pricly-yellow thanks for the pull, can you cherry-pick squash-commit to release branch 1.15 and make a pull?

if you need help just tell

[6543]

-> #17227