Skip to content

Commit b9c62f2

Browse files
committed
make head repo selection clearer
1 parent d386cc8 commit b9c62f2

File tree

2 files changed

+19
-9
lines changed

2 files changed

+19
-9
lines changed

routers/web/repo/compare.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,13 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo {
249249

250250
// If there is no head repository, it means compare between the same repository.
251251
headInfos := strings.Split(infos[1], ":")
252-
if len(headInfos) == 1 {
252+
if len(headInfos) == 1 { // {:headBranch} case, guaranteed baseRepo is headRepo
253253
isSameRepo = true
254254
ci.HeadUser = ctx.Repo.Owner
255-
ci.HeadBranch, ci.RawDiffType = parseRefForRawDiff(ctx, ctx.Repo.Repository, headInfos[0])
256-
} else if len(headInfos) == 2 {
255+
ci.HeadBranch, ci.RawDiffType = parseRefForRawDiff(ctx, baseRepo, headInfos[0])
256+
} else if len(headInfos) == 2 { // {:headOwner}:{:headBranch} or {:headOwner}/{:headRepoName}:{:headBranch} case
257257
headInfosSplit := strings.Split(headInfos[0], "/")
258-
if len(headInfosSplit) == 1 {
258+
if len(headInfosSplit) == 1 { // {:headOwner}:{:headBranch} case, guaranteed baseRepo.Name is headRepo.Name
259259
ci.HeadUser, err = user_model.GetUserByName(ctx, headInfos[0])
260260
if err != nil {
261261
if user_model.IsErrUserNotExist(err) {
@@ -265,13 +265,23 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo {
265265
}
266266
return nil
267267
}
268-
// FIXME: how to correctly choose the head repository? The logic below (3-8) is quite complex, the real head repo is determined there
269-
ci.HeadBranch, ci.RawDiffType = parseRefForRawDiff(ctx, ..., headInfos[1])
268+
269+
headRepo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ci.HeadUser.Name, baseRepo.Name)
270+
if err != nil {
271+
if repo_model.IsErrRepoNotExist(err) {
272+
ctx.NotFound(nil)
273+
} else {
274+
ctx.ServerError("GetRepositoryByOwnerAndName", err)
275+
}
276+
return nil
277+
}
278+
ci.HeadBranch, ci.RawDiffType = parseRefForRawDiff(ctx, headRepo, headInfos[1])
279+
270280
isSameRepo = ci.HeadUser.ID == ctx.Repo.Owner.ID
271-
if isSameRepo {
281+
if isSameRepo { // not a fork
272282
ci.HeadRepo = baseRepo
273283
}
274-
} else {
284+
} else { // {:headOwner}/{:headRepoName}:{:headBranch} case, across forks
275285
ci.HeadRepo, err = repo_model.GetRepositoryByOwnerAndName(ctx, headInfosSplit[0], headInfosSplit[1])
276286
if err != nil {
277287
if repo_model.IsErrRepoNotExist(err) {

tests/integration/compare_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func TestCompareRawDiffPatch(t *testing.T) {
228228
respTs = respTs.In(time.Local)
229229

230230
// Format the timestamp to match the expected format in the patch
231-
customFormat := "Mon, 02 Jan 2006 15:04:05 -0700"
231+
customFormat := "Mon, 2 Jan 2006 15:04:05 -0700"
232232
respTsStr := respTs.Format(customFormat)
233233

234234
req := NewRequest(t, "GET", fmt.Sprintf("/user1/test_raw_diff/compare/%s...%s.patch", oldRef.ID.String(), newRef.ID.String()))

0 commit comments

Comments
 (0)