From 83b6d032318c7ba082531a9d0060b7109094b828 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Wed, 22 Feb 2017 21:15:14 +0800 Subject: [PATCH] fix: Wrong repo list on Explore page if user already loggin. (#1009) * fix: Wrong repo list on Explore page if user already loggin. * fix: code readable. * fix: declare variable --- models/fixtures/repository.yml | 36 +++++++++++++++++++++++ models/fixtures/user.yml | 15 ++++++++++ models/repo.go | 37 ++++++++++++++++-------- models/repo_test.go | 52 ++++++++++++++++++++++++++++++++++ 4 files changed, 128 insertions(+), 12 deletions(-) diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 0e2aa5fd0d..7c3426b3fc 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -132,3 +132,39 @@ num_pulls: 0 num_closed_pulls: 0 is_mirror: false + +- + id: 12 + owner_id: 14 + lower_name: test_repo_12 + name: test_repo_12 + is_private: false + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + +- + id: 13 + owner_id: 14 + lower_name: test_repo_13 + name: test_repo_13 + is_private: true + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + +- + id: 14 + owner_id: 14 + lower_name: test_repo_14 + name: test_repo_14 + is_private: false + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 8874498c91..5a2dd2f080 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -194,3 +194,18 @@ avatar_email: user13@example.com num_repos: 1 is_active: true + +- + id: 14 + lower_name: user14 + name: user14 + full_name: User 14 + email: user14@example.com + passwd: password + type: 0 # individual + salt: salt + is_admin: false + avatar: avatar14 + avatar_email: user13@example.com + num_repos: 3 + is_active: true diff --git a/models/repo.go b/models/repo.go index 1d4e3dc311..218f15afe5 100644 --- a/models/repo.go +++ b/models/repo.go @@ -30,6 +30,7 @@ import ( "github.com/Unknwon/cae/zip" "github.com/Unknwon/com" + "github.com/go-xorm/builder" "github.com/go-xorm/xorm" version "github.com/mcuadros/go-version" ini "gopkg.in/ini.v1" @@ -1797,7 +1798,11 @@ type SearchRepoOptions struct { // SearchRepositoryByName takes keyword and part of repository name to search, // it returns results in given range and number of total results. func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ int64, _ error) { - var sess *xorm.Session + var ( + sess *xorm.Session + cond builder.Cond = builder.NewCond() + ) + if len(opts.Keyword) == 0 { return repos, 0, nil } @@ -1810,26 +1815,24 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in repos = make([]*Repository, 0, opts.PageSize) if opts.Starred && opts.OwnerID > 0 { - sess = x. - Join("INNER", "star", "star.repo_id = repository.id"). - Where("star.uid = ?", opts.OwnerID). - And("lower_name LIKE ?", "%"+opts.Keyword+"%") - } else { - sess = x.Where("lower_name LIKE ?", "%"+opts.Keyword+"%") + cond = builder.Eq{ + "star.uid": opts.OwnerID, + } } + cond = cond.And(builder.Like{"lower_name", opts.Keyword}) // Append conditions if !opts.Starred && opts.OwnerID > 0 { - sess.And("owner_id = ?", opts.OwnerID) + cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) } if !opts.Private { - sess.And("is_private=?", false) + cond = cond.And(builder.Eq{"is_private": false}) } if opts.Searcher != nil { + var ownerIds []int64 - sess.Or("owner_id = ?", opts.Searcher.ID) - + ownerIds = append(ownerIds, opts.Searcher.ID) err := opts.Searcher.GetOrganizations(true) if err != nil { @@ -1837,14 +1840,24 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in } for _, org := range opts.Searcher.Orgs { - sess.Or("owner_id = ?", org.ID) + ownerIds = append(ownerIds, org.ID) } + + cond = cond.Or(builder.And(builder.Like{"lower_name", opts.Keyword}, builder.In("owner_id", ownerIds))) } if len(opts.OrderBy) == 0 { opts.OrderBy = "name ASC" } + if opts.Starred && opts.OwnerID > 0 { + sess = x. + Join("INNER", "star", "star.repo_id = repository.id"). + Where(cond) + } else { + sess = x.Where(cond) + } + var countSess xorm.Session countSess = *sess count, err := countSess.Count(new(Repository)) diff --git a/models/repo_test.go b/models/repo_test.go index 7c0e94a5ae..ca1259a7ea 100644 --- a/models/repo_test.go +++ b/models/repo_test.go @@ -125,3 +125,55 @@ func TestForkRepository(t *testing.T) { assert.Error(t, err) assert.True(t, IsErrRepoAlreadyExist(err)) } + +func TestSearchRepositoryByName(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + // test search public repository on explore page + repos, count, err := SearchRepositoryByName(&SearchRepoOptions{ + Keyword: "repo_12", + Page: 1, + PageSize: 10, + Searcher: nil, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, "test_repo_12", repos[0].Name) + assert.Equal(t, int64(1), count) + + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Keyword: "test_repo", + Page: 1, + PageSize: 10, + Searcher: nil, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(2), count) + + // test search private repository on explore page + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Keyword: "repo_13", + Page: 1, + PageSize: 10, + Searcher: &User{ID: 14}, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, "test_repo_13", repos[0].Name) + assert.Equal(t, int64(1), count) + + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Keyword: "test_repo", + Page: 1, + PageSize: 10, + Searcher: &User{ID: 14}, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(3), count) +}