Skip to content

Show package count in header #26467

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

Closed
wants to merge 6 commits into from
Closed
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
17 changes: 15 additions & 2 deletions models/packages/package_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/modules/util"

"xorm.io/builder"
"xorm.io/xorm"
)

// ErrDuplicatePackageVersion indicates a duplicated package version error
Expand Down Expand Up @@ -298,8 +299,7 @@ func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*Package
return pvs, count, err
}

// SearchLatestVersions gets the latest version of every package matching the search options
func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) {
func buildLatestVersionsSession(ctx context.Context, opts *PackageSearchOptions) *xorm.Session {
cond := opts.ToConds().
And(builder.Expr("pv2.id IS NULL"))

Expand All @@ -314,6 +314,13 @@ func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*P
Join("INNER", "package", "package.id = package_version.package_id").
Where(cond)

return sess
}

// SearchLatestVersions gets the latest version of every package matching the search options
func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) {
sess := buildLatestVersionsSession(ctx, opts)

opts.configureOrderBy(sess)

if opts.Paginator != nil {
Expand All @@ -325,6 +332,12 @@ func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*P
return pvs, count, err
}

// CountLatestVersions counts the latest version of every package matching the search options
func CountLatestVersions(ctx context.Context, opts *PackageSearchOptions) (int64, error) {
sess := buildLatestVersionsSession(ctx, opts)
return sess.Count(new(PackageVersion))
}

// ExistVersion checks if a version matching the search options exist
func ExistVersion(ctx context.Context, opts *PackageSearchOptions) (bool, error) {
return db.GetEngine(ctx).
Expand Down
11 changes: 11 additions & 0 deletions modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
packages_model "code.gitea.io/gitea/models/packages"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
Comment on lines 18 to 22
Copy link
Member

Choose a reason for hiding this comment

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

This file needs a serious refactoring.
A modules/context file, which is one of the most essential files in the hierarchy, should not depend on models and services
But that's not a problem for this PR.

unit_model "code.gitea.io/gitea/models/unit"
Expand Down Expand Up @@ -550,6 +551,16 @@ func RepoAssignment(ctx *Context) context.CancelFunc {
return nil
}

ctx.Data["NumPackages"], err = packages_model.CountLatestVersions(ctx, &packages_model.PackageSearchOptions{
OwnerID: ctx.ContextUser.ID,
RepoID: ctx.Repo.Repository.ID,
IsInternal: util.OptionalBoolFalse,
})
if err != nil {
ctx.ServerError("CountLatestPackagesVersions", err)
return nil
}

ctx.Data["Title"] = owner.Name + "/" + repo.Name
ctx.Data["Repository"] = repo
ctx.Data["Owner"] = ctx.Repo.Repository.Owner
Expand Down
1 change: 1 addition & 0 deletions modules/structs/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type Organization struct {
Location string `json:"location"`
Visibility string `json:"visibility"`
RepoAdminChangeTeamAccess bool `json:"repo_admin_change_team_access"`
Packages int `json:"package_count"`
// deprecated
UserName string `json:"username"`
}
Expand Down
1 change: 1 addition & 0 deletions modules/structs/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type Repository struct {
OpenIssues int `json:"open_issues_count"`
OpenPulls int `json:"open_pr_counter"`
Releases int `json:"release_counter"`
Packages int `json:"package_counter"`
DefaultBranch string `json:"default_branch"`
Archived bool `json:"archived"`
// swagger:strfmt date-time
Expand Down
1 change: 1 addition & 0 deletions modules/structs/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type User struct {
Followers int `json:"followers_count"`
Following int `json:"following_count"`
StarredRepos int `json:"starred_repos_count"`
Packages int `json:"packages_count"`
}

// MarshalJSON implements the json.Marshaler interface for User, adding field(s) for backward compatibility
Expand Down
10 changes: 10 additions & 0 deletions routers/web/shared/user/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package user
import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
packages_model "code.gitea.io/gitea/models/packages"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/context"
Expand Down Expand Up @@ -127,5 +128,14 @@ func LoadHeaderCount(ctx *context.Context) error {
}
ctx.Data["RepoCount"] = repoCount

packageCount, err := packages_model.CountLatestVersions(ctx, &packages_model.PackageSearchOptions{
OwnerID: ctx.ContextUser.ID,
IsInternal: util.OptionalBoolFalse,
})
if err != nil {
return err
}
ctx.Data["PackageCount"] = packageCount

return nil
}
4 changes: 4 additions & 0 deletions services/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
packages_model "code.gitea.io/gitea/models/packages"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
Expand Down Expand Up @@ -283,6 +284,8 @@ func ToDeployKey(apiLink string, key *asymkey_model.DeployKey) *api.DeployKey {

// ToOrganization convert user_model.User to api.Organization
func ToOrganization(ctx context.Context, org *organization.Organization) *api.Organization {
numPackages, _ := packages_model.CountLatestVersions(ctx, &packages_model.PackageSearchOptions{OwnerID: org.ID, IsInternal: util.OptionalBoolFalse})
Comment on lines 286 to +287
Copy link
Member

Choose a reason for hiding this comment

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

This change looks really strange and not at all how the code should be structured.
This method should not make database queries.
It's a much better idea to store the information inside organization.Organization, if at all:
After all, there are already API routes for getting this information (-> X-Total-Count).
Furthermore, if we do that, I can already see the requests to also add it for issues, labels, milestones, PRs, releases, tags, projects, …
So in summary, I'm against modifying the API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To the API response in general: The Number is shown on the Profile, so it should also be returned by the API, so 3rd Party Apps can add this Feature easily. As we don't show Issues or Milestones on the profile, there is no need to return a count.

To the location: There are also other places in the convert API ehere databse querrys are done, but I can move it to the User7org Modell and add a LoadCount function.


return &api.Organization{
ID: org.ID,
AvatarURL: org.AsUser().AvatarLink(ctx),
Expand All @@ -295,6 +298,7 @@ func ToOrganization(ctx context.Context, org *organization.Organization) *api.Or
Location: org.Location,
Visibility: org.Visibility.String(),
RepoAdminChangeTeamAccess: org.RepoAdminChangeTeamAccess,
Packages: int(numPackages),
}
}

Expand Down
9 changes: 9 additions & 0 deletions services/convert/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import (
"time"

"code.gitea.io/gitea/models"
packages_model "code.gitea.io/gitea/models/packages"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
unit_model "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/modules/log"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
)

// ToRepo converts a Repository to api.Repository
Expand Down Expand Up @@ -135,6 +137,12 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR

numReleases, _ := repo_model.GetReleaseCountByRepoID(ctx, repo.ID, repo_model.FindReleasesOptions{IncludeDrafts: false, IncludeTags: false})

numPackages, _ := packages_model.CountLatestVersions(ctx, &packages_model.PackageSearchOptions{
OwnerID: repo.Owner.ID,
RepoID: repo.ID,
IsInternal: util.OptionalBoolFalse,
})

mirrorInterval := ""
var mirrorUpdated time.Time
if repo.IsMirror {
Expand Down Expand Up @@ -193,6 +201,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR
OpenIssues: repo.NumOpenIssues,
OpenPulls: repo.NumOpenPulls,
Releases: int(numReleases),
Packages: int(numPackages),
Copy link
Member

Choose a reason for hiding this comment

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

See comment above.

DefaultBranch: repo.DefaultBranch,
Created: repo.CreatedUnix.AsTime(),
Updated: repo.UpdatedUnix.AsTime(),
Expand Down
5 changes: 5 additions & 0 deletions services/convert/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ package convert
import (
"context"

packages_model "code.gitea.io/gitea/models/packages"
"code.gitea.io/gitea/models/perm"
user_model "code.gitea.io/gitea/models/user"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
)

// ToUser convert user_model.User to api.User
Expand Down Expand Up @@ -47,6 +49,8 @@ func ToUserWithAccessMode(ctx context.Context, user *user_model.User, accessMode
// toUser convert user_model.User to api.User
// signed shall only be set if requester is logged in. authed shall only be set if user is site admin or user himself
func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *api.User {
numPackages, _ := packages_model.CountLatestVersions(ctx, &packages_model.PackageSearchOptions{OwnerID: user.ID, IsInternal: util.OptionalBoolFalse})

result := &api.User{
ID: user.ID,
UserName: user.Name,
Expand All @@ -62,6 +66,7 @@ func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *ap
Followers: user.NumFollowers,
Following: user.NumFollowing,
StarredRepos: user.NumStars,
Packages: int(numPackages),
Copy link
Member

Choose a reason for hiding this comment

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

See comment above.

}

result.Visibility = user.Visibility.String()
Expand Down
3 changes: 3 additions & 0 deletions templates/org/menu.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
{{if and .IsPackageEnabled .CanReadPackages}}
<a class="item" href="{{$.Org.HomeLink}}/-/packages">
{{svg "octicon-package"}} {{.locale.Tr "packages.title"}}
{{if .PackageCount}}
<div class="ui small label">{{.PackageCount}}</div>
{{end}}
</a>
{{end}}
{{if and .IsRepoIndexerEnabled .CanReadCode}}
Expand Down
3 changes: 3 additions & 0 deletions templates/repo/header.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@
{{if .Permission.CanRead $.UnitTypePackages}}
<a href="{{.RepoLink}}/packages" class="{{if .IsPackagesPage}}active {{end}}item">
{{svg "octicon-package"}} {{.locale.Tr "packages.title"}}
{{if .NumPackages}}
<span class="ui small label">{{CountFmt .NumPackages}}</span>
{{end}}
</a>
{{end}}

Expand Down
15 changes: 15 additions & 0 deletions templates/swagger/v1_json.tmpl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions templates/user/overview/header.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
{{if and .IsPackageEnabled (or .ContextUser.IsIndividual (and .ContextUser.IsOrganization .CanReadPackages))}}
<a href="{{.ContextUser.HomeLink}}/-/packages" class="{{if .IsPackagesPage}}active {{end}}item">
{{svg "octicon-package"}} {{.locale.Tr "packages.title"}}
{{if .PackageCount}}
<div class="ui small label">{{.PackageCount}}</div>
{{end}}
</a>
{{end}}
{{if and .IsRepoIndexerEnabled (or .ContextUser.IsIndividual (and .ContextUser.IsOrganization .CanReadCode))}}
Expand Down
39 changes: 39 additions & 0 deletions tests/integration/api_org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package integration

import (
"bytes"
"fmt"
"net/http"
"net/url"
Expand Down Expand Up @@ -218,3 +219,41 @@ func TestAPIOrgSearchEmptyTeam(t *testing.T) {
}
})
}

func TestAPIOrgPackageCount(t *testing.T) {
defer tests.PrepareTestEnv(t)()

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
token := getUserToken(t, user.Name, auth_model.AccessTokenScopeWriteOrganization)
orgName := "PackageOrg"

req := NewRequestWithJSON(t, "POST", "/api/v1/orgs?token="+token, &api.CreateOrgOption{
UserName: orgName,
})
MakeRequest(t, req, http.StatusCreated)

orgURL := "api/v1/orgs/" + orgName
req = NewRequest(t, "GET", orgURL)
resp := MakeRequest(t, req, http.StatusOK)

var orgInfo *api.Organization
DecodeJSON(t, resp, &orgInfo)

assert.Equal(t, 0, orgInfo.Packages)

packageName := "test-package"
packageVersion := "1.0.3"
filename := "file.bin"

url := fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", orgName, packageName, packageVersion, filename)
req = NewRequestWithBody(t, "PUT", url, bytes.NewReader([]byte{}))
AddBasicAuthHeader(req, user.Name)
MakeRequest(t, req, http.StatusCreated)

req = NewRequest(t, "GET", orgURL)
resp = MakeRequest(t, req, http.StatusOK)

DecodeJSON(t, resp, &orgInfo)

assert.Equal(t, 1, orgInfo.Packages)
}
32 changes: 32 additions & 0 deletions tests/integration/api_user_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package integration

import (
"bytes"
"fmt"
"net/http"
"testing"
Expand Down Expand Up @@ -65,3 +66,34 @@ func TestAPIUserInfo(t *testing.T) {
assert.Equal(t, user, u.UserName)
})
}

func TestAPIUserPackageCount(t *testing.T) {
defer tests.PrepareTestEnv(t)()

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})

userURL := "api/v1/users/" + user.Name
req := NewRequest(t, "GET", userURL)
resp := MakeRequest(t, req, http.StatusOK)

var userInfo *api.User
DecodeJSON(t, resp, &userInfo)

assert.Equal(t, 0, userInfo.Packages)

packageName := "test-package"
packageVersion := "1.0.3"
filename := "file.bin"

url := fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", user.Name, packageName, packageVersion, filename)
req = NewRequestWithBody(t, "PUT", url, bytes.NewReader([]byte{}))
AddBasicAuthHeader(req, user.Name)
MakeRequest(t, req, http.StatusCreated)

req = NewRequest(t, "GET", userURL)
resp = MakeRequest(t, req, http.StatusOK)

DecodeJSON(t, resp, &userInfo)

assert.Equal(t, 1, userInfo.Packages)
}