Ensure permission records parsed for groups and users

domain/permission/endpoint.go

@@ -656,11 +656,10 @@ func (h *Handler) SetDocumentPermissions(w http.ResponseWriter, r *http.Request)
 		perm.OrgID = ctx.OrgID
 		perm.DocumentID = id
 
+		// get group records for just this group
 		isGroup := perm.Who == permission.GroupPermission
 		groupRecords := []group.Record{}
-
 		if isGroup {
-			// get group records for just this group
 			groupRecords = group.FilterGroupRecords(groupMembers, perm.WhoID)
 		}
 

domain/permission/permission.go

@@ -15,6 +15,7 @@ import (
 	"database/sql"
 
 	"github.com/documize/community/domain"
+	group "github.com/documize/community/model/group"
 	pm "github.com/documize/community/model/permission"
 	u "github.com/documize/community/model/user"
 )
@@ -193,34 +194,63 @@ func GetDocumentApprovers(ctx domain.RequestContext, s domain.Store, spaceID, do
 	users = []u.User{}
 	prev := make(map[string]bool) // used to ensure we only process user once
 
-	// check space permissions
+	// Permissions can be assigned to both groups and individual users.
-	sp, err := s.Permission.GetSpacePermissions(ctx, spaceID)
+	// Pre-fetch users with group membership to help us work out
-	for _, p := range sp {
+	// if user belongs to a group with permissions.
-		if p.Action == pm.DocumentApprove {
+	groupMembers, err := s.Group.GetMembers(ctx)
-			user, err := s.User.Get(ctx, p.WhoID)
+	if err != nil {
-			if err == nil {
+		return users, err
-				prev[user.RefID] = true
-				users = append(users, user)
-			} else {
-				return users, err
-			}
-		}
 	}
 
-	// check document permissions
+	// space permissions
+	sp, err := s.Permission.GetSpacePermissions(ctx, spaceID)
+	if err != nil {
+		return users, err
+	}
+	// document permissions
 	dp, err := s.Permission.GetDocumentPermissions(ctx, documentID)
-	for _, p := range dp {
+	if err != nil {
-		if p.Action == pm.DocumentApprove {
+		return users, err
-			user, err := s.User.Get(ctx, p.WhoID)
+	}
-			if err == nil {
+
+	// all permissions
+	all := sp
+	all = append(all, dp...)
+
+	for _, p := range all {
+		// only approvers
+		if p.Action != pm.DocumentApprove {
+			continue
+		}
+
+		if p.Who == pm.GroupPermission {
+			// get group records for just this group
+			groupRecords := group.FilterGroupRecords(groupMembers, p.WhoID)
+
+			for i := range groupRecords {
+				user, err := s.User.Get(ctx, groupRecords[i].UserID)
+				if err != nil {
+					return users, err
+				}
 				if _, isExisting := prev[user.RefID]; !isExisting {
 					users = append(users, user)
+					prev[user.RefID] = true
 				}
-			} else {
+			}
+		}
+
+		if p.Who == pm.UserPermission {
+			user, err := s.User.Get(ctx, p.WhoID)
+			if err != nil {
 				return users, err
 			}
+
+			if _, isExisting := prev[user.RefID]; !isExisting {
+				users = append(users, user)
+				prev[user.RefID] = true
+			}
 		}
 	}
 
-	return
+	return users, err
 }