Skip to content

Drop redundant columns from issue_user table #638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 15 additions & 88 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,18 @@ func (issue *Issue) LoadAttributes() error {
return issue.loadAttributes(x)
}

// GetIsRead load the `IsRead` field of the issue
func (issue *Issue) GetIsRead(userID int64) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have notification system, it seems is_read is not needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IsRead field is still used (in /templates/repo/issue/list.tmpl).

While it might make sense to eventually remove the is_read column, it seems outside the scope of this PR.

issueUser := &IssueUser{IssueID: issue.ID, UID: userID}
if has, err := x.Get(issueUser); err != nil {
return err
} else if !has {
return ErrUserNotExist{UID: userID}
}
issue.IsRead = issueUser.IsRead
return nil
}

// HTMLURL returns the absolute URL to this issue.
func (issue *Issue) HTMLURL() string {
var path string
Expand Down Expand Up @@ -473,8 +485,6 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, repo *Repository,

if err = updateIssueCols(e, issue, "is_closed"); err != nil {
return err
} else if err = updateIssueUsersByStatus(e, issue.ID, isClosed); err != nil {
return err
}

// Update issue count of labels
Expand Down Expand Up @@ -999,13 +1009,9 @@ type IssueUser struct {
ID int64 `xorm:"pk autoincr"`
UID int64 `xorm:"INDEX"` // User ID.
IssueID int64
RepoID int64 `xorm:"INDEX"`
MilestoneID int64
IsRead bool
IsAssigned bool
IsMentioned bool
IsPoster bool
IsClosed bool
}

func newIssueUsers(e *xorm.Session, repo *Repository, issue *Issue) error {
Expand All @@ -1021,24 +1027,17 @@ func newIssueUsers(e *xorm.Session, repo *Repository, issue *Issue) error {
// and just waste 1 unit is cheaper than re-allocate memory once.
issueUsers := make([]*IssueUser, 0, len(assignees)+1)
for _, assignee := range assignees {
isPoster := assignee.ID == issue.PosterID
issueUsers = append(issueUsers, &IssueUser{
IssueID: issue.ID,
RepoID: repo.ID,
UID: assignee.ID,
IsPoster: isPoster,
IsAssigned: assignee.ID == issue.AssigneeID,
})
if !isPosterAssignee && isPoster {
isPosterAssignee = true
}
isPosterAssignee = isPosterAssignee || assignee.ID == issue.PosterID
}
if !isPosterAssignee {
issueUsers = append(issueUsers, &IssueUser{
IssueID: issue.ID,
RepoID: repo.ID,
UID: issue.PosterID,
IsPoster: true,
IssueID: issue.ID,
UID: issue.PosterID,
})
}

Expand All @@ -1063,62 +1062,6 @@ func NewIssueUsers(repo *Repository, issue *Issue) (err error) {
return sess.Commit()
}

// PairsContains returns true when pairs list contains given issue.
func PairsContains(ius []*IssueUser, issueID, uid int64) int {
for i := range ius {
if ius[i].IssueID == issueID &&
ius[i].UID == uid {
return i
}
}
return -1
}

// GetIssueUsers returns issue-user pairs by given repository and user.
func GetIssueUsers(rid, uid int64, isClosed bool) ([]*IssueUser, error) {
ius := make([]*IssueUser, 0, 10)
err := x.Where("is_closed=?", isClosed).Find(&ius, &IssueUser{RepoID: rid, UID: uid})
return ius, err
}

// GetIssueUserPairsByRepoIds returns issue-user pairs by given repository IDs.
func GetIssueUserPairsByRepoIds(rids []int64, isClosed bool, page int) ([]*IssueUser, error) {
if len(rids) == 0 {
return []*IssueUser{}, nil
}

ius := make([]*IssueUser, 0, 10)
sess := x.
Limit(20, (page-1)*20).
Where("is_closed=?", isClosed).
In("repo_id", rids)
err := sess.Find(&ius)
return ius, err
}

// GetIssueUserPairsByMode returns issue-user pairs by given repository and user.
func GetIssueUserPairsByMode(uid, rid int64, isClosed bool, page, filterMode int) ([]*IssueUser, error) {
ius := make([]*IssueUser, 0, 10)
sess := x.
Limit(20, (page-1)*20).
Where("uid=?", uid).
And("is_closed=?", isClosed)
if rid > 0 {
sess.And("repo_id=?", rid)
}

switch filterMode {
case FilterModeAssign:
sess.And("is_assigned=?", true)
case FilterModeCreate:
sess.And("is_poster=?", true)
default:
return ius, nil
}
err := sess.Find(&ius)
return ius, err
}

// UpdateIssueMentions extracts mentioned people from content and
// updates issue-user relations for them.
func UpdateIssueMentions(e Engine, issueID int64, mentions []string) error {
Expand Down Expand Up @@ -1348,16 +1291,6 @@ func UpdateIssue(issue *Issue) error {
return updateIssue(x, issue)
}

func updateIssueUsersByStatus(e Engine, issueID int64, isClosed bool) error {
_, err := e.Exec("UPDATE `issue_user` SET is_closed=? WHERE issue_id=?", isClosed, issueID)
return err
}

// UpdateIssueUsersByStatus updates issue-user relations by issue status.
func UpdateIssueUsersByStatus(issueID int64, isClosed bool) error {
return updateIssueUsersByStatus(x, issueID, isClosed)
}

func updateIssueUserByAssignee(e *xorm.Session, issue *Issue) (err error) {
if _, err = e.Exec("UPDATE `issue_user` SET is_assigned = ? WHERE issue_id = ?", false, issue.ID); err != nil {
return err
Expand Down Expand Up @@ -1702,8 +1635,6 @@ func changeMilestoneAssign(e *xorm.Session, issue *Issue, oldMilestoneID int64)

if err = updateMilestone(e, m); err != nil {
return err
} else if _, err = e.Exec("UPDATE `issue_user` SET milestone_id = 0 WHERE issue_id = ?", issue.ID); err != nil {
return err
}
}

Expand All @@ -1720,8 +1651,6 @@ func changeMilestoneAssign(e *xorm.Session, issue *Issue, oldMilestoneID int64)

if err = updateMilestone(e, m); err != nil {
return err
} else if _, err = e.Exec("UPDATE `issue_user` SET milestone_id = ? WHERE issue_id = ?", m.ID, issue.ID); err != nil {
return err
}
}

Expand Down Expand Up @@ -1775,8 +1704,6 @@ func DeleteMilestoneByRepoID(repoID, id int64) error {

if _, err = sess.Exec("UPDATE `issue` SET milestone_id = 0 WHERE milestone_id = ?", m.ID); err != nil {
return err
} else if _, err = sess.Exec("UPDATE `issue_user` SET milestone_id = 0 WHERE milestone_id = ?", m.ID); err != nil {
return err
}
return sess.Commit()
}
Expand Down
2 changes: 1 addition & 1 deletion models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (
"time"

"code.gitea.io/git"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/process"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/sync"
api "code.gitea.io/sdk/gitea"
"github.com/Unknwon/com"
"github.com/go-xorm/xorm"
"code.gitea.io/gitea/modules/base"
)

var pullRequestQueue = sync.NewUniqueQueue(setting.Repository.PullRequestQueueLength)
Expand Down
4 changes: 3 additions & 1 deletion models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -1424,7 +1424,6 @@ func DeleteRepository(uid, repoID int64) error {
&Watch{RepoID: repoID},
&Star{RepoID: repoID},
&Mirror{RepoID: repoID},
&IssueUser{RepoID: repoID},
&Milestone{RepoID: repoID},
&Release{RepoID: repoID},
&Collaboration{RepoID: repoID},
Expand All @@ -1445,6 +1444,9 @@ func DeleteRepository(uid, repoID int64) error {
if _, err = sess.Delete(&Comment{IssueID: issues[i].ID}); err != nil {
return err
}
if _, err = sess.Delete(&IssueUser{IssueID: issues[i].ID}); err != nil {
return err
}

attachments := make([]*Attachment, 0, 5)
if err = sess.
Expand Down
20 changes: 4 additions & 16 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,26 +230,14 @@ func Issues(ctx *context.Context) {
}
}

// Get issue-user relations.
pairs, err := models.GetIssueUsers(repo.ID, posterID, isShowClosed)
if err != nil {
ctx.Handle(500, "GetIssueUsers", err)
return
}

// Get posters.
for i := range issues {
// Check read status
if !ctx.IsSigned {
issues[i].IsRead = true
continue
}

// Check read status.
idx := models.PairsContains(pairs, issues[i].ID, ctx.User.ID)
if idx > -1 {
issues[i].IsRead = pairs[idx].IsRead
} else {
issues[i].IsRead = true
} else if err = issues[i].GetIsRead(ctx.User.ID); err != nil {
ctx.Handle(500, "GetIsRead", err)
return
}
}
ctx.Data["Issues"] = issues
Expand Down