Skip to content
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

Fix search by issue type #1914

Merged
merged 2 commits into from
Jun 15, 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
50 changes: 50 additions & 0 deletions integrations/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,30 @@ package integrations

import (
"net/http"
"strconv"
"strings"
"testing"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/setting"

"github.com/PuerkitoBio/goquery"
"github.com/stretchr/testify/assert"
)

func getIssuesSelection(htmlDoc *HtmlDoc) *goquery.Selection {
return htmlDoc.doc.Find(".issue.list").Find("li").Find(".title")
}

func getIssue(t *testing.T, repoID int64, issueSelection *goquery.Selection) *models.Issue {
href, exists := issueSelection.Attr("href")
assert.True(t, exists)
indexStr := href[strings.LastIndexByte(href, '/')+1:]
index, err := strconv.Atoi(indexStr)
assert.NoError(t, err, "Invalid issue href: %s", href)
return models.AssertExistsAndLoadBean(t, &models.Issue{RepoID: repoID, Index: int64(index)}).(*models.Issue)
}

func TestNoLoginViewIssues(t *testing.T) {
prepareTestEnv(t)

Expand All @@ -19,6 +38,37 @@ func TestNoLoginViewIssues(t *testing.T) {
assert.EqualValues(t, http.StatusOK, resp.HeaderCode)
}

func TestNoLoginViewIssuesSortByType(t *testing.T) {
prepareTestEnv(t)

user := models.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User)
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
repo.Owner = models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User)

session := loginUser(t, user.Name, "password")
req := NewRequest(t, "GET", repo.RelLink()+"/issues?type=created_by")
resp := session.MakeRequest(t, req)
assert.EqualValues(t, http.StatusOK, resp.HeaderCode)

htmlDoc, err := NewHtmlParser(resp.Body)
assert.NoError(t, err)
issuesSelection := getIssuesSelection(htmlDoc)
expectedNumIssues := models.GetCount(t,
&models.Issue{RepoID: repo.ID, PosterID: user.ID},
models.Cond("is_closed=?", false),
models.Cond("is_pull=?", false),
)
if expectedNumIssues > setting.UI.IssuePagingNum {
expectedNumIssues = setting.UI.IssuePagingNum
}
assert.EqualValues(t, expectedNumIssues, issuesSelection.Length())

issuesSelection.Each(func(_ int, selection *goquery.Selection) {
issue := getIssue(t, repo.ID, selection)
assert.EqualValues(t, user.ID, issue.PosterID)
})
}

func TestNoLoginViewIssue(t *testing.T) {
prepareTestEnv(t)

Expand Down
52 changes: 13 additions & 39 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,6 @@ func parseCountResult(results []map[string][]byte) int64 {

// IssueStatsOptions contains parameters accepted by GetIssueStats.
type IssueStatsOptions struct {
FilterMode int
RepoID int64
Labels string
MilestoneID int64
Expand All @@ -1241,7 +1240,7 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
countSession := func(opts *IssueStatsOptions) *xorm.Session {
sess := x.
Where("issue.repo_id = ?", opts.RepoID).
And("is_pull = ?", opts.IsPull)
And("issue.is_pull = ?", opts.IsPull)

if len(opts.IssueIDs) > 0 {
sess.In("issue.id", opts.IssueIDs)
Expand All @@ -1252,8 +1251,8 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
if err != nil {
log.Warn("Malformed Labels argument: %s", opts.Labels)
} else if len(labelIDs) > 0 {
sess.Join("INNER", "issue_label", "issue.id = issue_id").
In("label_id", labelIDs)
sess.Join("INNER", "issue_label", "issue.id = issue_label.issue_id").
In("issue_label.label_id", labelIDs)
}
}

Expand All @@ -1262,11 +1261,11 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
}

if opts.AssigneeID > 0 {
sess.And("assignee_id = ?", opts.AssigneeID)
sess.And("issue.assignee_id = ?", opts.AssigneeID)
}

if opts.PosterID > 0 {
sess.And("poster_id = ?", opts.PosterID)
sess.And("issue.poster_id = ?", opts.PosterID)
}

if opts.MentionedID > 0 {
Expand All @@ -1279,40 +1278,15 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
}

var err error
switch opts.FilterMode {
case FilterModeAll, FilterModeAssign:
stats.OpenCount, err = countSession(opts).
And("is_closed = ?", false).
Count(new(Issue))

stats.ClosedCount, err = countSession(opts).
And("is_closed = ?", true).
Count(new(Issue))
case FilterModeCreate:
stats.OpenCount, err = countSession(opts).
And("poster_id = ?", opts.PosterID).
Copy link
Member Author

Choose a reason for hiding this comment

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

These conditions are already handled in countSession(..), so we don't need to repeat them again here.

And("is_closed = ?", false).
Count(new(Issue))

stats.ClosedCount, err = countSession(opts).
And("poster_id = ?", opts.PosterID).
And("is_closed = ?", true).
Count(new(Issue))
case FilterModeMention:
stats.OpenCount, err = countSession(opts).
Join("INNER", "issue_user", "issue.id = issue_user.issue_id").
And("issue_user.uid = ?", opts.PosterID).
And("issue_user.is_mentioned = ?", true).
And("issue.is_closed = ?", false).
Count(new(Issue))

stats.ClosedCount, err = countSession(opts).
Join("INNER", "issue_user", "issue.id = issue_user.issue_id").
And("issue_user.uid = ?", opts.PosterID).
And("issue_user.is_mentioned = ?", true).
And("issue.is_closed = ?", true).
Count(new(Issue))
stats.OpenCount, err = countSession(opts).
And("issue.is_closed = ?", false).
Count(new(Issue))
if err != nil {
return stats, err
}
stats.ClosedCount, err = countSession(opts).
And("issue.is_closed = ?", true).
Count(new(Issue))
return stats, err
}

Expand Down
40 changes: 33 additions & 7 deletions models/unit_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,31 @@ func PrepareTestDatabase() error {
return LoadFixtures()
}

type testCond struct {
query interface{}
args []interface{}
}

// Cond create a condition with arguments for a test
func Cond(query interface{}, args ...interface{}) interface{} {
return &testCond{query: query, args: args}
}

func whereConditions(sess *xorm.Session, conditions []interface{}) {
for _, condition := range conditions {
switch cond := condition.(type) {
case *testCond:
sess.Where(cond.query, cond.args...)
default:
sess.Where(cond)
}
}
}

func loadBeanIfExists(bean interface{}, conditions ...interface{}) (bool, error) {
sess := x.NewSession()
defer sess.Close()

for _, cond := range conditions {
sess = sess.Where(cond)
}
whereConditions(sess, conditions)
return sess.Get(bean)
}

Expand All @@ -65,6 +83,16 @@ func AssertExistsAndLoadBean(t *testing.T, bean interface{}, conditions ...inter
return bean
}

// GetCount get the count of a bean
func GetCount(t *testing.T, bean interface{}, conditions ...interface{}) int {
sess := x.NewSession()
defer sess.Close()
whereConditions(sess, conditions)
count, err := sess.Count(bean)
assert.NoError(t, err)
return int(count)
}

// AssertNotExistsBean assert that a bean does not exist in the test database
func AssertNotExistsBean(t *testing.T, bean interface{}, conditions ...interface{}) {
exists, err := loadBeanIfExists(bean, conditions...)
Expand All @@ -80,7 +108,5 @@ func AssertSuccessfulInsert(t *testing.T, beans ...interface{}) {

// AssertCount assert the count of a bean
func AssertCount(t *testing.T, bean interface{}, expected interface{}) {
actual, err := x.Count(bean)
assert.NoError(t, err)
assert.EqualValues(t, expected, actual)
assert.EqualValues(t, expected, GetCount(t, bean))
}
12 changes: 11 additions & 1 deletion routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,15 @@ func Issues(ctx *context.Context) {
forceEmpty bool
)

if ctx.IsSigned {
switch viewType {
case "created_by":
posterID = ctx.User.ID
case "mentioned":
mentionedID = ctx.User.ID
}
}

repo := ctx.Repo.Repository
selectLabels := ctx.Query("labels")
milestoneID := ctx.QueryInt64("milestone")
Expand Down Expand Up @@ -150,11 +159,12 @@ func Issues(ctx *context.Context) {
MilestoneID: milestoneID,
AssigneeID: assigneeID,
MentionedID: mentionedID,
PosterID: posterID,
IsPull: isPullList,
IssueIDs: issueIDs,
})
if err != nil {
ctx.Error(500, "GetSearchIssueStats")
ctx.Handle(500, "GetIssueStats", err)
return
}
}
Expand Down
26 changes: 14 additions & 12 deletions templates/repo/issue/list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,21 @@
</div>
</div>

<!-- Type -->
<div class="ui dropdown type jump item">
<span class="text">
{{.i18n.Tr "repo.issues.filter_type"}}
<i class="dropdown icon"></i>
</span>
<div class="menu">
<a class="{{if eq .ViewType "all"}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type=all&sort={{$.SortType}}&state={{$.State}}&labels={{.SelectLabels}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}">{{.i18n.Tr "repo.issues.filter_type.all_issues"}}</a>
<a class="{{if eq .ViewType "assigned"}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type=assigned&sort={{$.SortType}}&state={{$.State}}&labels={{.SelectLabels}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}">{{.i18n.Tr "repo.issues.filter_type.assigned_to_you"}}</a>
<a class="{{if eq .ViewType "created_by"}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type=created_by&sort={{$.SortType}}&state={{$.State}}&labels={{.SelectLabels}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}">{{.i18n.Tr "repo.issues.filter_type.created_by_you"}}</a>
<a class="{{if eq .ViewType "mentioned"}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type=mentioned&sort={{$.SortType}}&state={{$.State}}&labels={{.SelectLabels}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}">{{.i18n.Tr "repo.issues.filter_type.mentioning_you"}}</a>
{{if .IsSigned}}
<!-- Type -->
<div class="ui dropdown type jump item">
<span class="text">
{{.i18n.Tr "repo.issues.filter_type"}}
<i class="dropdown icon"></i>
</span>
<div class="menu">
<a class="{{if eq .ViewType "all"}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type=all&sort={{$.SortType}}&state={{$.State}}&labels={{.SelectLabels}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}">{{.i18n.Tr "repo.issues.filter_type.all_issues"}}</a>
<a class="{{if eq .ViewType "assigned"}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type=assigned&sort={{$.SortType}}&state={{$.State}}&labels={{.SelectLabels}}&milestone={{$.MilestoneID}}&assignee={{.SignedUser.ID}}">{{.i18n.Tr "repo.issues.filter_type.assigned_to_you"}}</a>
Copy link
Member Author

Choose a reason for hiding this comment

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

assignee param updated to be .Signed.User.ID (might be hard to tell what the non-whitespace changes are)

<a class="{{if eq .ViewType "created_by"}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type=created_by&sort={{$.SortType}}&state={{$.State}}&labels={{.SelectLabels}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}">{{.i18n.Tr "repo.issues.filter_type.created_by_you"}}</a>
<a class="{{if eq .ViewType "mentioned"}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type=mentioned&sort={{$.SortType}}&state={{$.State}}&labels={{.SelectLabels}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}">{{.i18n.Tr "repo.issues.filter_type.mentioning_you"}}</a>
</div>
</div>
</div>
{{end}}

<!-- Sort -->
<div class="ui dropdown type jump item">
Expand Down