From 893cda904affd70c49e0c60ca43eab994dc5f84d Mon Sep 17 00:00:00 2001 From: Peter Smit Date: Mon, 16 Feb 2015 13:16:24 +0200 Subject: [PATCH 1/2] Fix that owners also see actions on their repositories This is a balance between speed and nice code, where speed has won. To prevent a repository query for each action the ownername is match with the current user. It would be "cleaner" or "better" if we fetch the repository each time. Another option is to add the RepoOwnerID to action --- routers/user/home.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/routers/user/home.go b/routers/user/home.go index 35407534f..574c6387d 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -103,7 +103,12 @@ func Dashboard(ctx *middleware.Context) { feeds := make([]*models.Action, 0, len(actions)) for _, act := range actions { if act.IsPrivate { - if has, _ := models.HasAccess(ctx.User, &models.Repository{Id: act.RepoId, IsPrivate: true}, models.ACCESS_MODE_READ); !has { + repo := &models.Repository{Id: act.RepoId, IsPrivate: true} + // This prevents having to retrieve the repository for each action + if act.RepoUserName == ctx.User.LowerName { + repo.OwnerId = ctx.User.Id + } + if has, _ := models.HasAccess(ctx.User, repo, models.ACCESS_MODE_READ); !has { continue } } @@ -210,11 +215,12 @@ func Profile(ctx *middleware.Context) { if !ctx.IsSigned { continue } - if has, _ := models.HasAccess(ctx.User, - &models.Repository{ - Id: act.RepoId, - IsPrivate: true, - }, models.ACCESS_MODE_READ); !has { + repo := &models.Repository{Id: act.RepoId, IsPrivate: true} + // This prevents having to retrieve the repository for each action + if act.RepoUserName == ctx.User.LowerName { + repo.OwnerId = ctx.User.Id + } + if has, _ := models.HasAccess(ctx.User, repo, models.ACCESS_MODE_READ); !has { continue } } From 4ad9104c52b90184cbd472dc46cde4902bb7b377 Mon Sep 17 00:00:00 2001 From: Peter Smit Date: Wed, 18 Feb 2015 08:59:22 +0200 Subject: [PATCH 2/2] Update/simplify fix that owners also see actions on their repositories --- routers/user/home.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/routers/user/home.go b/routers/user/home.go index 574c6387d..0a1d9dd21 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -103,14 +103,14 @@ func Dashboard(ctx *middleware.Context) { feeds := make([]*models.Action, 0, len(actions)) for _, act := range actions { if act.IsPrivate { - repo := &models.Repository{Id: act.RepoId, IsPrivate: true} // This prevents having to retrieve the repository for each action - if act.RepoUserName == ctx.User.LowerName { - repo.OwnerId = ctx.User.Id - } - if has, _ := models.HasAccess(ctx.User, repo, models.ACCESS_MODE_READ); !has { - continue + repo := &models.Repository{Id: act.RepoId, IsPrivate: true} + if act.RepoUserName != ctx.User.LowerName { + if has, _ := models.HasAccess(ctx.User, repo, models.ACCESS_MODE_READ); !has { + continue + } } + } // FIXME: cache results? u, err := models.GetUserByName(act.ActUserName) @@ -215,14 +215,14 @@ func Profile(ctx *middleware.Context) { if !ctx.IsSigned { continue } - repo := &models.Repository{Id: act.RepoId, IsPrivate: true} // This prevents having to retrieve the repository for each action - if act.RepoUserName == ctx.User.LowerName { - repo.OwnerId = ctx.User.Id - } - if has, _ := models.HasAccess(ctx.User, repo, models.ACCESS_MODE_READ); !has { - continue + repo := &models.Repository{Id: act.RepoId, IsPrivate: true} + if act.RepoUserName != ctx.User.LowerName { + if has, _ := models.HasAccess(ctx.User, repo, models.ACCESS_MODE_READ); !has { + continue + } } + } // FIXME: cache results? u, err := models.GetUserByName(act.ActUserName)