From 8b89563bf1031089a218e6d05dc61047281b35ee Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Wed, 12 Jul 2023 23:10:35 +0900 Subject: [PATCH] Fix empty project displayed in issue sidebar (#25802) Before: ![image](https://github.com/go-gitea/gitea/assets/18380374/1ab476dc-2f9b-4c85-9e87-105fc73af1ee) After: ![image](https://github.com/go-gitea/gitea/assets/18380374/786f984d-5c27-4eff-b3d9-159f68034ce4) This issue comes from the change in #25468. `LoadProject` will always return at least one record, so we use `ProjectID` to check whether an issue is linked to a project in the old code. As other `issue.LoadXXX` functions, we need to check the return value from `xorm.Session.Get`. In recent unit tests, we only test `issueList.LoadAttributes()` but don't test `issue.LoadAttributes()`. So I added a new test for `issue.LoadAttributes()` in this PR. --------- Co-authored-by: Denys Konovalov --- models/issues/issue_list_test.go | 4 +-- models/issues/issue_project.go | 9 ++++--- models/issues/issue_test.go | 45 ++++++++++++++++++++++++++++++++ routers/web/org/projects.go | 3 +-- routers/web/repo/projects.go | 3 +-- 5 files changed, 53 insertions(+), 11 deletions(-) diff --git a/models/issues/issue_list_test.go b/models/issues/issue_list_test.go index 97ce9e43b..696c3b765 100644 --- a/models/issues/issue_list_test.go +++ b/models/issues/issue_list_test.go @@ -67,9 +67,7 @@ func TestIssueList_LoadAttributes(t *testing.T) { if issue.ID == int64(1) { assert.Equal(t, int64(400), issue.TotalTrackedTime) assert.NotNil(t, issue.Project) - } else if issue.ID == int64(2) { - assert.Equal(t, int64(3682), issue.TotalTrackedTime) - assert.Nil(t, issue.Project) + assert.Equal(t, int64(1), issue.Project.ID) } else { assert.Nil(t, issue.Project) } diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index b163c6835..782638d99 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -16,13 +16,14 @@ import ( func (issue *Issue) LoadProject(ctx context.Context) (err error) { if issue.Project == nil { var p project_model.Project - if _, err = db.GetEngine(ctx).Table("project"). + has, err := db.GetEngine(ctx).Table("project"). Join("INNER", "project_issue", "project.id=project_issue.project_id"). - Where("project_issue.issue_id = ?", issue.ID). - Get(&p); err != nil { + Where("project_issue.issue_id = ?", issue.ID).Get(&p) + if err != nil { return err + } else if has { + issue.Project = &p } - issue.Project = &p } return err } diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index 80699a57b..7f1eab197 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -17,6 +17,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -539,3 +540,47 @@ func TestCountIssues(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, 18, count) } + +func TestIssueLoadAttributes(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + setting.Service.EnableTimetracking = true + + issueList := issues_model.IssueList{ + unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}), + unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 4}), + } + + for _, issue := range issueList { + assert.NoError(t, issue.LoadAttributes(db.DefaultContext)) + assert.EqualValues(t, issue.RepoID, issue.Repo.ID) + for _, label := range issue.Labels { + assert.EqualValues(t, issue.RepoID, label.RepoID) + unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: label.ID}) + } + if issue.PosterID > 0 { + assert.EqualValues(t, issue.PosterID, issue.Poster.ID) + } + if issue.AssigneeID > 0 { + assert.EqualValues(t, issue.AssigneeID, issue.Assignee.ID) + } + if issue.MilestoneID > 0 { + assert.EqualValues(t, issue.MilestoneID, issue.Milestone.ID) + } + if issue.IsPull { + assert.EqualValues(t, issue.ID, issue.PullRequest.IssueID) + } + for _, attachment := range issue.Attachments { + assert.EqualValues(t, issue.ID, attachment.IssueID) + } + for _, comment := range issue.Comments { + assert.EqualValues(t, issue.ID, comment.IssueID) + } + if issue.ID == int64(1) { + assert.Equal(t, int64(400), issue.TotalTrackedTime) + assert.NotNil(t, issue.Project) + assert.Equal(t, int64(1), issue.Project.ID) + } else { + assert.Nil(t, issue.Project) + } + } +} diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 60032c777..50bb5591e 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -438,8 +438,7 @@ func UpdateIssueProject(ctx *context.Context) { projectID := ctx.FormInt64("id") for _, issue := range issues { if issue.Project != nil { - oldProjectID := issue.Project.ID - if oldProjectID == projectID { + if issue.Project.ID == projectID { continue } } diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 6bfb21c13..1574c90c0 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -386,8 +386,7 @@ func UpdateIssueProject(ctx *context.Context) { projectID := ctx.FormInt64("id") for _, issue := range issues { if issue.Project != nil { - oldProjectID := issue.Project.ID - if oldProjectID == projectID { + if issue.Project.ID == projectID { continue } }