tom/forkjo
tom/forkjo
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions 1

Merge pull request '[v9.0/forgejo] fix: issue labels are not set after deleting one label' (#5844) from bp-v9.0/forgejo-db899c1-f06bdb0 into v9.0/forgejo

Browse source 
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5844
Reviewed-by: Otto <otto@codeberg.org>
This commit is contained in:
Otto 2024-11-07 11:22:14 +00:00
commit 66b6917923
2 changed files with 124 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
models/issues/issue_label.go
View file

@ -111,9 +111,7 @@ func NewIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_m
return err
}
issue.isLabelsLoaded = false
issue.Labels = nil
if err = issue.LoadLabels(ctx); err != nil {
if err = issue.ReloadLabels(ctx); err != nil {
return err
}
@ -161,10 +159,7 @@ func NewIssueLabels(ctx context.Context, issue *Issue, labels []*Label, doer *us
return err
}
// reload all labels
issue.isLabelsLoaded = false
issue.Labels = nil
if err = issue.LoadLabels(ctx); err != nil {
if err = issue.ReloadLabels(ctx); err != nil {
return err
}
@ -205,8 +200,7 @@ func DeleteIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *use
return err
}
issue.Labels = nil
return issue.LoadLabels(ctx)
return issue.ReloadLabels(ctx)
}
// DeleteLabelsByRepoID deletes labels of some repository
@ -326,14 +320,23 @@ func FixIssueLabelWithOutsideLabels(ctx context.Context) (int64, error) {
return res.RowsAffected()
}
// LoadLabels loads labels
// LoadLabels only if they are not already set
func (issue *Issue) LoadLabels(ctx context.Context) (err error) {
if !issue.isLabelsLoaded && issue.Labels == nil && issue.ID != 0 {
if !issue.isLabelsLoaded && issue.Labels == nil {
if err := issue.ReloadLabels(ctx); err != nil {
return err
}
issue.isLabelsLoaded = true
}
return nil
}
func (issue *Issue) ReloadLabels(ctx context.Context) (err error) {
if issue.ID != 0 {
issue.Labels, err = GetLabelsByIssueID(ctx, issue.ID)
if err != nil {
return fmt.Errorf("getLabelsByIssueID [%d]: %w", issue.ID, err)
}
issue.isLabelsLoaded = true
}
return nil
}
@ -496,9 +499,7 @@ func ReplaceIssueLabels(ctx context.Context, issue *Issue, labels []*Label, doer
}
}
issue.isLabelsLoaded = false
issue.Labels = nil
if err = issue.LoadLabels(ctx); err != nil {
if err = issue.ReloadLabels(ctx); err != nil {
return err
}

108
models/issues/issue_label_test.go
View file

@ -15,6 +15,114 @@ import (
"github.com/stretchr/testify/require"
)
func TestIssueNewIssueLabels(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
label1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1})
label2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 4})
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
label3 := &issues_model.Label{RepoID: 1, Name: "label3", Color: "#123"}
require.NoError(t, issues_model.NewLabel(db.DefaultContext, label3))
// label1 is already set, do nothing
// label3 is new, add it
require.NoError(t, issues_model.NewIssueLabels(db.DefaultContext, issue, []*issues_model.Label{label1, label3}, doer))
assert.Len(t, issue.Labels, 3)
// check that the pre-existing label1 is still present
assert.Equal(t, label1.ID, issue.Labels[0].ID)
// check that new label3 was added
assert.Equal(t, label3.ID, issue.Labels[1].ID)
// check that pre-existing label2 was not removed
assert.Equal(t, label2.ID, issue.Labels[2].ID)
}
func TestIssueNewIssueLabel(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3})
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
label := &issues_model.Label{RepoID: 1, Name: "label3", Color: "#123"}
require.NoError(t, issues_model.NewLabel(db.DefaultContext, label))
require.NoError(t, issues_model.NewIssueLabel(db.DefaultContext, issue, label, doer))
assert.Len(t, issue.Labels, 1)
assert.Equal(t, label.ID, issue.Labels[0].ID)
}
func TestIssueReplaceIssueLabels(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
label1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1})
label2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 4})
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
label3 := &issues_model.Label{RepoID: 1, Name: "label3", Color: "#123"}
require.NoError(t, issues_model.NewLabel(db.DefaultContext, label3))
issue.LoadLabels(db.DefaultContext)
assert.Len(t, issue.Labels, 2)
assert.Equal(t, label1.ID, issue.Labels[0].ID)
assert.Equal(t, label2.ID, issue.Labels[1].ID)
// label1 is already set, do nothing
// label3 is new, add it
// label2 is not in the list but already set, remove it
require.NoError(t, issues_model.ReplaceIssueLabels(db.DefaultContext, issue, []*issues_model.Label{label1, label3}, doer))
assert.Len(t, issue.Labels, 2)
assert.Equal(t, label1.ID, issue.Labels[0].ID)
assert.Equal(t, label3.ID, issue.Labels[1].ID)
}
func TestIssueDeleteIssueLabel(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
label1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1})
label2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 4})
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
issue.LoadLabels(db.DefaultContext)
assert.Len(t, issue.Labels, 2)
assert.Equal(t, label1.ID, issue.Labels[0].ID)
assert.Equal(t, label2.ID, issue.Labels[1].ID)
require.NoError(t, issues_model.DeleteIssueLabel(db.DefaultContext, issue, label2, doer))
assert.Len(t, issue.Labels, 1)
assert.Equal(t, label1.ID, issue.Labels[0].ID)
}
func TestIssueLoadLabels(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
label1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1})
label2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 4})
assert.Empty(t, issue.Labels)
issue.LoadLabels(db.DefaultContext)
assert.Len(t, issue.Labels, 2)
assert.Equal(t, label1.ID, issue.Labels[0].ID)
assert.Equal(t, label2.ID, issue.Labels[1].ID)
unittest.AssertSuccessfulDelete(t, &issues_model.IssueLabel{IssueID: issue.ID, LabelID: label2.ID})
// the database change is not noticed because the labels are cached
issue.LoadLabels(db.DefaultContext)
assert.Len(t, issue.Labels, 2)
issue.ReloadLabels(db.DefaultContext)
assert.Len(t, issue.Labels, 1)
assert.Equal(t, label1.ID, issue.Labels[0].ID)
}
func TestNewIssueLabelsScope(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())