From a8ef5d5125955b6c76ef8d30d5be6221e8e0a477 Mon Sep 17 00:00:00 2001 From: badhezi Date: Mon, 2 Jun 2025 11:53:01 +0300 Subject: [PATCH] make head repo selection clearer --- routers/web/repo/compare.go | 26 ++++++++++++++++++-------- tests/integration/compare_test.go | 2 +- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 846064b4166..7fb99925b9d 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -247,13 +247,13 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { // If there is no head repository, it means compare between the same repository. headInfos := strings.Split(infos[1], ":") - if len(headInfos) == 1 { + if len(headInfos) == 1 { // {:headBranch} case, guaranteed baseRepo is headRepo isSameRepo = true ci.HeadUser = ctx.Repo.Owner - ci.HeadBranch, ci.RawDiffType = parseRefForRawDiff(ctx, ctx.Repo.Repository, headInfos[0]) - } else if len(headInfos) == 2 { + ci.HeadBranch, ci.RawDiffType = parseRefForRawDiff(ctx, baseRepo, headInfos[0]) + } else if len(headInfos) == 2 { // {:headOwner}:{:headBranch} or {:headOwner}/{:headRepoName}:{:headBranch} case headInfosSplit := strings.Split(headInfos[0], "/") - if len(headInfosSplit) == 1 { + if len(headInfosSplit) == 1 { // {:headOwner}:{:headBranch} case, guaranteed baseRepo.Name is headRepo.Name ci.HeadUser, err = user_model.GetUserByName(ctx, headInfos[0]) if err != nil { if user_model.IsErrUserNotExist(err) { @@ -263,13 +263,23 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { } return nil } - // FIXME: how to correctly choose the head repository? The logic below (3-8) is quite complex, the real head repo is determined there - ci.HeadBranch, ci.RawDiffType = parseRefForRawDiff(ctx, ..., headInfos[1]) + + headRepo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ci.HeadUser.Name, baseRepo.Name) + if err != nil { + if repo_model.IsErrRepoNotExist(err) { + ctx.NotFound(nil) + } else { + ctx.ServerError("GetRepositoryByOwnerAndName", err) + } + return nil + } + ci.HeadBranch, ci.RawDiffType = parseRefForRawDiff(ctx, headRepo, headInfos[1]) + isSameRepo = ci.HeadUser.ID == ctx.Repo.Owner.ID - if isSameRepo { + if isSameRepo { // not a fork ci.HeadRepo = baseRepo } - } else { + } else { // {:headOwner}/{:headRepoName}:{:headBranch} case, across forks ci.HeadRepo, err = repo_model.GetRepositoryByOwnerAndName(ctx, headInfosSplit[0], headInfosSplit[1]) if err != nil { if repo_model.IsErrRepoNotExist(err) { diff --git a/tests/integration/compare_test.go b/tests/integration/compare_test.go index f0304de9b75..08e0f285563 100644 --- a/tests/integration/compare_test.go +++ b/tests/integration/compare_test.go @@ -229,7 +229,7 @@ func TestCompareRawDiffPatch(t *testing.T) { respTs = respTs.In(time.Local) // Format the timestamp to match the expected format in the patch - customFormat := "Mon, 02 Jan 2006 15:04:05 -0700" + customFormat := "Mon, 2 Jan 2006 15:04:05 -0700" respTsStr := respTs.Format(customFormat) req := NewRequest(t, "GET", fmt.Sprintf("/user1/test_raw_diff/compare/%s...%s.patch", oldRef.ID.String(), newRef.ID.String()))