From 455fad0fbd85137af5d3efec93d2b974e9383b98 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 35407534f9..574c6387dc 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 0fa209d07b6155943bfd235257a4ed6a484a9a85 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 574c6387dc..0a1d9dd217 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)