Rework project visibility

src/admintools/adminproject.go

@@ -47,8 +47,8 @@ func addCreateProjectCommand(projectCommand *cobra.Command) {
 			defer tx.Rollback(ctx)
 
 			p, err := hmndata.FetchProject(ctx, tx, nil, models.HMNProjectID, hmndata.ProjectsQuery{
-				IncludeHidden: true,
 				Lifecycles:    models.AllProjectLifecycles,
+				IncludeHidden: true,
 			})
 			if err != nil {
 				panic(err)

src/hmndata/project_helper.go

@@ -20,8 +20,9 @@ const (
 )
 
 type ProjectsQuery struct {
-	// Available on all project queries
+	// Available on all project queries. By default, you will get projects that
-	Lifecycles    []models.ProjectLifecycle // if empty, defaults to models.VisibleProjectLifecycles
+	// are generally visible to all users.
+	Lifecycles    []models.ProjectLifecycle // If empty, defaults to visible lifecycles. Do not conflate this with permissions; those are checked separately.
 	Types         ProjectTypeQuery          // bitfield
 	IncludeHidden bool
 
@@ -65,11 +66,6 @@ func FetchProjects(
 	perf.StartBlock("SQL", "Fetch projects")
 	defer perf.EndBlock()
 
-	var currentUserID *int
-	if currentUser != nil {
-		currentUserID = &currentUser.ID
-	}
-
 	tx, err := dbConn.Begin(ctx)
 	if err != nil {
 		return nil, oops.New(err, "failed to start transaction")
@@ -80,11 +76,9 @@ func FetchProjects(
 		Project        models.Project `db:"project"`
 		LogoLightAsset *models.Asset  `db:"logolight_asset"`
 		LogoDarkAsset  *models.Asset  `db:"logodark_asset"`
+		Tag            *models.Tag    `db:"tags"`
 	}
 
-	// If true, join against the project owners table and check visibility permissions
-	checkOwnerVisibility := q.IncludeHidden && currentUser != nil
-
 	// Fetch all valid projects (not yet subject to user permission checks)
 	var qb db.QueryBuilder
 	if len(q.OrderBy) > 0 {
@@ -96,31 +90,31 @@ func FetchProjects(
 			handmade_project AS project
 			LEFT JOIN handmade_asset AS logolight_asset ON logolight_asset.id = project.logolight_asset_id
 			LEFT JOIN handmade_asset AS logodark_asset ON logodark_asset.id = project.logodark_asset_id
+			LEFT JOIN tags ON project.tag = tags.id
 	`)
 	if len(q.OwnerIDs) > 0 {
-		qb.Add(`
+		qb.Add(
-			INNER JOIN handmade_user_projects AS owner_filter ON owner_filter.project_id = project.id
+			`
-		`)
+			JOIN (
-	}
+				SELECT project_id, array_agg(user_id) AS owner_ids
-	if checkOwnerVisibility {
+				FROM handmade_user_projects
-		qb.Add(`
+				WHERE user_id = ANY ($?)
-			LEFT JOIN handmade_user_projects AS owner_visibility ON owner_visibility.project_id = project.id
+				GROUP BY project_id
-		`)
+			) AS owner_filter ON project.id = owner_filter.project_id
+			`,
+			q.OwnerIDs,
+		)
 	}
+
+	// Filters (permissions are checked after the query, in Go)
 	qb.Add(`
 		WHERE
 			TRUE
 	`)
-
+	if len(q.Lifecycles) > 0 {
-	// Filters
+		qb.Add(`AND project.lifecycle = ANY ($?)`, q.Lifecycles)
-	if len(q.ProjectIDs) > 0 {
+	} else {
-		qb.Add(`AND project.id = ANY ($?)`, q.ProjectIDs)
+		qb.Add(`AND project.lifecycle = ANY ($?)`, models.VisibleProjectLifecycles)
-	}
-	if len(q.Slugs) > 0 {
-		qb.Add(`AND (project.slug != '' AND project.slug = ANY ($?))`, q.Slugs)
-	}
-	if len(q.OwnerIDs) > 0 {
-		qb.Add(`AND (owner_filter.user_id = ANY ($?))`, q.OwnerIDs)
 	}
 	if q.Types != 0 {
 		qb.Add(`AND (FALSE`)
@@ -132,21 +126,14 @@ func FetchProjects(
 		}
 		qb.Add(`)`)
 	}
-
-	// Visibility
-	if checkOwnerVisibility {
-		qb.Add(`AND ($? OR owner_visibility.user_id = $? OR (TRUE`, currentUser.IsStaff, currentUser.ID)
-	}
 	if !q.IncludeHidden {
-		qb.Add(`AND NOT hidden`)
+		qb.Add(`AND NOT project.hidden`)
 	}
-	if len(q.Lifecycles) > 0 {
+	if len(q.ProjectIDs) > 0 {
-		qb.Add(`AND project.lifecycle = ANY($?)`, q.Lifecycles)
+		qb.Add(`AND project.id = ANY ($?)`, q.ProjectIDs)
-	} else {
-		qb.Add(`AND project.lifecycle = ANY($?)`, models.VisibleProjectLifecycles)
 	}
-	if checkOwnerVisibility {
+	if len(q.Slugs) > 0 {
-		qb.Add(`))`)
+		qb.Add(`AND (project.slug != '' AND project.slug = ANY ($?))`, q.Slugs)
 	}
 
 	// Output
@@ -164,21 +151,6 @@ func FetchProjects(
 	}
 	iprojects := itProjects.ToSlice()
 
-	// Fetch project tags
-	var tagIDs []int
-	for _, iproject := range iprojects {
-		tagID := iproject.(*projectRow).Project.TagID
-		if tagID != nil {
-			tagIDs = append(tagIDs, *tagID)
-		}
-	}
-	tags, err := FetchTags(ctx, tx, TagQuery{
-		IDs: tagIDs,
-	})
-	if err != nil {
-		return nil, err
-	}
-
 	// Fetch project owners to do permission checks
 	projectIds := make([]int, len(iprojects))
 	for i, iproject := range iprojects {
@@ -195,49 +167,55 @@ func FetchProjects(
 		owners := projectOwners[i].Owners
 
 		/*
-			Per our spec, a user can see a project if:
+			Here's the rundown on project permissions:
+
+			- In general, users can only see projects that are Generally Visible.
+			- As an exception, users can always see projects that they own.
+			- As an exception, staff can always see every project.
+
+			A project is Generally Visible if all the following conditions are true:
+			- The project has a "visible" lifecycle (per models.VisibleProjectLifecycles)
+			- The project is not hidden
+			- One of the following is true:
 				- The project is official
 				- The project is personal and all of the project's owners are approved
-			- The project is personal and the current user is a collaborator (regardless of user status)
+
+			As an exception, the HMN project is always generally visible.
 
 			See https://www.notion.so/handmade-network/Technical-Plan-a11aaa9ea2f14d9a95f7d7780edd789c
 		*/
 
-		var projectVisible bool
+		currentUserIsOwner := false
-		if row.Project.Personal {
 		allOwnersApproved := true
 		for _, owner := range owners {
 			if owner.Status != models.UserStatusApproved {
 				allOwnersApproved = false
 			}
-				if currentUserID != nil && *currentUserID == owner.ID {
+			if currentUser != nil && owner.ID == currentUser.ID {
-					projectVisible = true
+				currentUserIsOwner = true
 			}
 		}
-			if allOwnersApproved {
+
-				projectVisible = true
+		projectGenerallyVisible := true &&
-			}
+			row.Project.Lifecycle.In(models.VisibleProjectLifecycles) &&
-		} else {
+			!row.Project.Hidden &&
-			projectVisible = true
+			(!row.Project.Personal || allOwnersApproved || row.Project.IsHMN())
+		if row.Project.IsHMN() {
+			projectGenerallyVisible = true // hard override
 		}
 
+		projectVisible := false ||
+			projectGenerallyVisible ||
+			currentUserIsOwner ||
+			(currentUser != nil && currentUser.IsStaff)
+
 		if projectVisible {
-			var projectTag *models.Tag
-			if row.Project.TagID != nil {
-				for _, tag := range tags {
-					if tag.ID == *row.Project.TagID {
-						projectTag = tag
-						break
-					}
-				}
-			}
-
 			res = append(res, ProjectAndStuff{
 				Project:        row.Project,
 				LogoLightAsset: row.LogoLightAsset,
 				LogoDarkAsset:  row.LogoDarkAsset,
 				Owners:         owners,
-				Tag:            projectTag,
+				Tag:            row.Tag,
 			})
 		}
 	}
@@ -485,8 +463,8 @@ func SetProjectTag(
 	defer tx.Rollback(ctx)
 
 	p, err := FetchProject(ctx, tx, nil, projectID, ProjectsQuery{
-		IncludeHidden: true,
 		Lifecycles:    models.AllProjectLifecycles,
+		IncludeHidden: true,
 	})
 	if err != nil {
 		return nil, err

src/hmndata/tag_helper.go

@@ -18,7 +18,7 @@ type TagQuery struct {
 
 func FetchTags(ctx context.Context, dbConn db.ConnOrTx, q TagQuery) ([]*models.Tag, error) {
 	perf := perf.ExtractPerf(ctx)
-	perf.StartBlock("SQL", "Fetch snippets")
+	perf.StartBlock("SQL", "Fetch tags")
 	defer perf.EndBlock()
 
 	var qb db.QueryBuilder

src/models/project.go

@@ -36,7 +36,7 @@ var AllProjectLifecycles = []ProjectLifecycle{
 	ProjectLifecycleLTS,
 }
 
-// NOTE(asaf): Just checking the lifecycle is not sufficient. Visible projects also must have flags = 0.
+// NOTE(asaf): Just checking the lifecycle is not sufficient. Visible projects also must not be hidden.
 var VisibleProjectLifecycles = []ProjectLifecycle{
 	ProjectLifecycleActive,
 	ProjectLifecycleHiatus,
@@ -44,6 +44,15 @@ var VisibleProjectLifecycles = []ProjectLifecycle{
 	ProjectLifecycleLTS,
 }
 
+func (lc ProjectLifecycle) In(lcs []ProjectLifecycle) bool {
+	for _, v := range lcs {
+		if lc == v {
+			return true
+		}
+	}
+	return false
+}
+
 const RecentProjectUpdateTimespanSec = 60 * 60 * 24 * 28 // NOTE(asaf): Four weeks
 
 type Project struct {

src/website/feed.go

@@ -157,7 +157,6 @@ func AtomFeed(c *RequestContext) ResponseData {
 				itemsPerFeed = 100000
 			}
 			projectsAndStuff, err := hmndata.FetchProjects(c.Context(), c.Conn, nil, hmndata.ProjectsQuery{
-				Lifecycles: models.VisibleProjectLifecycles,
 				Limit:   itemsPerFeed,
 				Types:   hmndata.OfficialProjects,
 				OrderBy: "date_approved DESC",

src/website/projects.go

@@ -265,7 +265,10 @@ func ProjectHomepage(c *RequestContext) ResponseData {
 		Value:    c.CurrentProject.Blurb,
 	})
 
-	p, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, c.CurrentProject.ID, hmndata.ProjectsQuery{})
+	p, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, c.CurrentProject.ID, hmndata.ProjectsQuery{
+		Lifecycles:    models.AllProjectLifecycles,
+		IncludeHidden: true,
+	})
 	if err != nil {
 		return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "failed to fetch project details"))
 	}
@@ -491,16 +494,16 @@ func ProjectEdit(c *RequestContext) ResponseData {
 		c.Context(), c.Conn,
 		c.CurrentUser, c.CurrentProject.ID,
 		hmndata.ProjectsQuery{
+			Lifecycles:    models.AllProjectLifecycles,
 			IncludeHidden: true,
 		},
 	)
-	owners, err := hmndata.FetchProjectOwners(c.Context(), c.Conn, p.Project.ID)
 	if err != nil {
-		return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "Failed to fetch project owners"))
+		return c.ErrorResponse(http.StatusInternalServerError, err)
 	}
 	projectSettings := templates.ProjectToProjectSettings(
 		&p.Project,
-		owners,
+		p.Owners,
 		p.TagText(),
 		p.LogoURL("light"), p.LogoURL("dark"),
 		c.Theme,

src/website/routes.go

@@ -300,7 +300,10 @@ func NewWebsiteRoutes(longRequestContext context.Context, conn *pgxpool.Pool) ht
 				if err != nil {
 					panic(oops.New(err, "project id was not numeric (bad regex in routing)"))
 				}
-				p, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, id, hmndata.ProjectsQuery{})
+				p, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, id, hmndata.ProjectsQuery{
+					Lifecycles:    models.AllProjectLifecycles,
+					IncludeHidden: true,
+				})
 				if err != nil {
 					if errors.Is(err, db.NotFound) {
 						return FourOhFour(c)
@@ -465,7 +468,10 @@ func LoadCommonWebsiteData(c *RequestContext) (bool, ResponseData) {
 		var owners []*models.User
 
 		if len(slug) > 0 {
-			dbProject, err := hmndata.FetchProjectBySlug(c.Context(), c.Conn, c.CurrentUser, slug, hmndata.ProjectsQuery{})
+			dbProject, err := hmndata.FetchProjectBySlug(c.Context(), c.Conn, c.CurrentUser, slug, hmndata.ProjectsQuery{
+				Lifecycles:    models.AllProjectLifecycles,
+				IncludeHidden: true,
+			})
 			if err == nil {
 				c.CurrentProject = &dbProject.Project
 				owners = dbProject.Owners
@@ -480,6 +486,7 @@ func LoadCommonWebsiteData(c *RequestContext) (bool, ResponseData) {
 
 		if c.CurrentProject == nil {
 			dbProject, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, models.HMNProjectID, hmndata.ProjectsQuery{
+				Lifecycles:    models.AllProjectLifecycles,
 				IncludeHidden: true,
 			})
 			if err != nil {

src/website/user.go

@@ -102,13 +102,12 @@ func UserProfile(c *RequestContext) ResponseData {
 	}
 	c.Perf.EndBlock()
 
-	projectsQuery := hmndata.ProjectsQuery{
+	projectsAndStuff, err := hmndata.FetchProjects(c.Context(), c.Conn, c.CurrentUser, hmndata.ProjectsQuery{
 		OwnerIDs:      []int{profileUser.ID},
-		Lifecycles: models.VisibleProjectLifecycles,
+		Lifecycles:    models.AllProjectLifecycles,
+		IncludeHidden: true,
 		OrderBy:       "all_last_updated DESC",
-	}
+	})
-
-	projectsAndStuff, err := hmndata.FetchProjects(c.Context(), c.Conn, c.CurrentUser, projectsQuery)
 	templateProjects := make([]templates.Project, 0, len(projectsAndStuff))
 	numPersonalProjects := 0
 	for _, p := range projectsAndStuff {