cleanup, comments, tests for all user types

Signed-off-by: Sarah Funkhouser <147884153+golanglemonade@users.noreply.github.com>

Author: golanglemonade

.task/checksum/generate-graphql-smart

@@ -1 +1 @@
-1bc21c33a19692f13176d0956c2e1008
+ed032ddbc0789ea1bfa827e3110c275e

internal/graphapi/checksum/.schema_checksum

@@ -1 +1 @@
-0494d41712434de1c809d2a60437092b14dc2b2973c918ffc032e80ba649d279
+e052aa5acf22c7ab4d3565fa783527a773dc66f934f288e368ab513374289019

internal/graphapi/clientschema/checksum/.schema_checksum

@@ -1 +1 @@
-58c690e645495566bbf80f0934eab7e7ac8b9de6bd685130153b6be4be7016e0
+3ac40c271013a9e75eddeee25d102bfb893597abbfa025aff64412f852a3d63d

internal/graphapi/clientschema/schema.graphql

@@ -9139,7 +9139,13 @@ type ControlEvidence {
 	the most severe evidence status among all linked evidence items
 	"""
 	worstStatus: EvidenceEvidenceStatus
+	"""
+	number of evidence items with auditor-approved status
+	"""
 	approvedCount: Int!
+	"""
+	breakdown of evidence item counts by status
+	"""
 	countByStatus: [EvidenceCountByStatus!]
 }
 """
@@ -28364,8 +28370,17 @@ type EvidenceConnection {
 	"""
 	totalCount: Int!
 }
+"""
+EvidenceCountByStatus pairs an evidence status with the number of evidence items in that state
+"""
 type EvidenceCountByStatus {
+	"""
+	the evidence status value
+	"""
 	status: EvidenceEvidenceStatus!
+	"""
+	number of evidence items with this status
+	"""
 	totalCount: Int!
 }
 """

internal/graphapi/controlhelpers.go

@@ -281,6 +281,8 @@ func normalizeFramework(framework *string) string {
 }
 
 // getMappedControlsBySubcontrolID returns mapped control records that include the given subcontrol ID on either side
+// it passes in the owner id so that it can bypass authorization requests since all users in an organization have
+// read access to (sub)controls and mappings
 func getMappedControlsBySubcontrolID(ctx context.Context, subcontrolID string) ([]*generated.MappedControl, error) {
 	orgIDs, err := auth.GetOrganizationIDsFromContext(ctx)
 	if err != nil {
@@ -292,8 +294,8 @@ func getMappedControlsBySubcontrolID(ctx context.Context, subcontrolID string) (
 		subcontrol.OwnerIDIn(orgIDs...),
 	)
 
+	// skip filters, this is already filtered on organization
 	allowCtx := privacy.DecisionContext(ctx, privacy.Allow)
-
 	return withTransactionalMutation(ctx).MappedControl.Query().
 		Where(
 			mappedcontrol.Or(
@@ -306,6 +308,8 @@ func getMappedControlsBySubcontrolID(ctx context.Context, subcontrolID string) (
 }
 
 // getControlMappings returns the controls and subcontrols mapped to a control based on the ref code and framework
+// it passes in the owner id so that it can bypass authorization requests since all users in an organization have
+// read access to (sub)controls and mappings
 func getControlMappings(ctx context.Context, refCode string, framework *string, parentControlID *string) ([]*generated.MappedControl, error) {
 	fullWhere, err := prepMappedControlQuery(ctx, refCode, framework, parentControlID)
 	if err != nil {
@@ -382,6 +386,10 @@ func prepMappedControlQuery(ctx context.Context, refCode string, framework *stri
 	}, nil
 }
 
+// getOrgMappedControlsInfo returns the organization control data for the mapped controls. If the control
+// is the same as the one being looked up, it is skipped. If the mapping came from the organization and is already
+// returning controls in the organization, it is added directly. If the mapping came from the system mappings, it looks
+// up the corresponding organization control if it exists
 func getOrgMappedControlsInfo(ctx context.Context, controls map[string]*model.ControlInfo, refCode string, framework *string) []*model.ControlInfo {
 	if len(controls) == 0 {
 		return nil
@@ -408,7 +416,8 @@ func getOrgMappedControlsInfo(ctx context.Context, controls map[string]*model.Co
 
 }
 
-// findOrganizationControlInfoForMappings
+// findOrganizationControlInfoForMappings returns the organization controls for the system control based on the framework
+// and refCode
 func findOrganizationControlInfoForMappings(ctx context.Context, controls map[string]*model.ControlInfo) ([]*model.ControlInfo, bool) {
 	// get orgs to filter, this will allow us to skip expensive authz checks
 	orgIDs, err := auth.GetOrganizationIDsFromContext(ctx)
@@ -423,8 +432,6 @@ func findOrganizationControlInfoForMappings(ctx context.Context, controls map[st
 
 	for _, c := range controls {
 		if c.ReferenceFramework == nil {
-			logx.FromContext(ctx).Warn().Str("id", c.ID).Str("ref_code", c.RefCode).Msg("found system control without a reference framework")
-
 			continue
 		}
 
@@ -437,6 +444,7 @@ func findOrganizationControlInfoForMappings(ctx context.Context, controls map[st
 
 	}
 
+	// use allowContext because this is filtered on authorized organizations already
 	allowCtx := privacy.DecisionContext(ctx, privacy.Allow)
 	if len(subcontrolRefCodes) > 0 {
 		orClauses := make([]predicate.Subcontrol, 0, len(subcontrolRefCodes))
@@ -496,6 +504,8 @@ func findOrganizationControlInfoForMappings(ctx context.Context, controls map[st
 	return results, len(results) > 0
 }
 
+// isSameControlInfo checks if the refCode and framework combination match, if so they are considered the
+// same control and returns false
 func isSameControlInfo(refCode string, framework *string, mappedControl *model.ControlInfo) bool {
 	if refCode != mappedControl.RefCode {
 		return false
@@ -512,6 +522,7 @@ func isSameControlInfo(refCode string, framework *string, mappedControl *model.C
 	return currentFramework == mappedFramework
 }
 
+// generateMapControlKey creates a key for a map based on the ref code and framework
 func generateMapControlKey(refCode string, framework *string) string {
 	f := normalizeFramework(framework)
 

internal/graphapi/controlhelpers_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/samber/lo"
 	"github.com/theopenlane/core/internal/ent/generated"
 	"github.com/theopenlane/core/internal/ent/generated/predicate"
 	"github.com/theopenlane/core/internal/graphapi/common"
@@ -15,8 +16,6 @@ import (
 	is "gotest.tools/v3/assert/cmp"
 )
 
-func strPtr(s string) *string { return &s }
-
 func TestGetStandardRefCodes(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -111,25 +110,25 @@ func TestNormalizeFramework(t *testing.T) {
 		},
 		{
 			name:      "non-nil framework returns its value",
-			framework: strPtr("ISO27001"),
+			framework: lo.ToPtr("ISO27001"),
 			expected:  "ISO27001",
 		},
 		{
 			name:      "empty string framework returns empty string",
-			framework: strPtr(""),
+			framework: lo.ToPtr(""),
 			expected:  "",
 		},
 		{
 			name:      "SOC2 framework",
-			framework: strPtr("SOC2"),
+			framework: lo.ToPtr("SOC2"),
 			expected:  "SOC2",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			result := normalizeFramework(tt.framework)
-			assert.Equal(t, tt.expected, result)
+			assert.Check(t, is.Equal(tt.expected, result))
 		})
 	}
 }
@@ -147,20 +146,20 @@ func TestGetFrameworkName(t *testing.T) {
 		},
 		{
 			name:     "non-nil reference framework returns its value",
-			control:  &generated.Control{ReferenceFramework: strPtr("NIST800-53")},
+			control:  &generated.Control{ReferenceFramework: lo.ToPtr("NIST800-53")},
 			expected: "NIST800-53",
 		},
 		{
 			name:     "SOC2 reference framework",
-			control:  &generated.Control{ReferenceFramework: strPtr("SOC2")},
+			control:  &generated.Control{ReferenceFramework: lo.ToPtr("SOC2")},
 			expected: "SOC2",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			result := getFrameworkName(tt.control)
-			assert.Equal(t, tt.expected, result)
+			assert.Check(t, is.Equal(tt.expected, result))
 		})
 	}
 }
@@ -181,13 +180,13 @@ func TestGenerateMapControlKey(t *testing.T) {
 		{
 			name:      "non-nil framework uses framework value",
 			refCode:   "CC1.1",
-			framework: strPtr("SOC2"),
+			framework: lo.ToPtr("SOC2"),
 			expected:  "CC1.1::SOC2",
 		},
 		{
 			name:      "empty ref code",
 			refCode:   "",
-			framework: strPtr("ISO27001"),
+			framework: lo.ToPtr("ISO27001"),
 			expected:  "::ISO27001",
 		},
 	}
@@ -221,7 +220,7 @@ func TestPrepMappedControlQuery(t *testing.T) {
 			name:            "no auth context returns error",
 			ctx:             context.Background(),
 			refCode:         "CC1.1",
-			framework:       strPtr("SOC2"),
+			framework:       lo.ToPtr("SOC2"),
 			parentControlID: nil,
 			wantErr:         true,
 		},
@@ -238,7 +237,7 @@ func TestPrepMappedControlQuery(t *testing.T) {
 			name:            "control query with non-nil framework",
 			ctx:             authedCtx,
 			refCode:         "CC1.1",
-			framework:       strPtr("SOC2"),
+			framework:       lo.ToPtr("SOC2"),
 			parentControlID: nil,
 			wantErr:         false,
 			wantPredicates:  1,
@@ -247,7 +246,7 @@ func TestPrepMappedControlQuery(t *testing.T) {
 			name:            "subcontrol query with parent control ID",
 			ctx:             authedCtx,
 			refCode:         "SC-1",
-			framework:       strPtr("NIST800-53"),
+			framework:       lo.ToPtr("NIST800-53"),
 			parentControlID: &parentID,
 			wantErr:         false,
 			wantPredicates:  1,
@@ -264,7 +263,7 @@ func TestPrepMappedControlQuery(t *testing.T) {
 			}
 
 			assert.NilError(t, err)
-			assert.Equal(t, tt.wantPredicates, len(result))
+			assert.Check(t, is.Equal(tt.wantPredicates, len(result)))
 		})
 	}
 }
@@ -283,7 +282,7 @@ func TestIsSameControlInfo(t *testing.T) {
 		{
 			name:          "different ref codes",
 			refCode:       "CC1.1",
-			framework:     strPtr("SOC2"),
+			framework:     lo.ToPtr("SOC2"),
 			mappedControl: &model.ControlInfo{RefCode: "CC2.1", ReferenceFramework: &soc2},
 			expected:      false,
 		},
@@ -344,7 +343,7 @@ func TestGetControlWherePredicate(t *testing.T) {
 		},
 		{
 			name:    "input with filter returns non-nil predicate",
-			where:   &generated.ControlWhereInput{RefCode: strPtr("CC1.1")},
+			where:   &generated.ControlWhereInput{RefCode: lo.ToPtr("CC1.1")},
 			wantNil: false,
 		},
 	}
@@ -399,7 +398,7 @@ func TestConstructWherePredicatesFromStandardRefCodes(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			result := constructWherePredicatesFromStandardRefCodes[predicate.Control](context.Background(), tt.input)
-			assert.Equal(t, tt.wantLen, len(result))
+			assert.Check(t, is.Equal(tt.wantLen, len(result)))
 		})
 	}
 }

internal/graphapi/controlreport.resolvers.go

@@ -151,7 +151,7 @@ func seedControlReportTestData(ctx context.Context, t *testing.T, primaryControl
 func TestQueryControlReports(t *testing.T) {
 	t.Parallel()
 
-	localTestOrg := suite.seedOrgOwner(t)
+	localTestOrg := suite.seedFreshOrgUsers(t)
 	orgUser := suite.seedOrgOwner(t)
 
 	// create 8 filler controls first (oldest) so that the enriched controls and the
@@ -194,21 +194,21 @@ func TestQueryControlReports(t *testing.T) {
 			expectedResults: 5,
 		},
 		{
-			name:            "happy path, with last set",
+			name:            "happy path, with last set by admin",
 			last:            lo.ToPtr(int64(3)),
-			ctx:             localTestOrg.owner.UserCtx,
+			ctx:             localTestOrg.admin.UserCtx,
 			expectedResults: 3,
 		},
 		{
-			name:            "first set over max (10 in test)",
+			name:            "first set over max (10 in test) by member",
 			first:           &orgOwnedCount,
-			ctx:             localTestOrg.owner.UserCtx,
+			ctx:             localTestOrg.member.UserCtx,
 			expectedResults: testutils.MaxResultLimit,
 		},
 		{
-			name:            "last set over max (10 in test)",
+			name:            "last set over max (10 in test) by auditor",
 			last:            &orgOwnedCount,
-			ctx:             localTestOrg.owner.UserCtx,
+			ctx:             localTestOrg.auditor.UserCtx,
 			expectedResults: testutils.MaxResultLimit,
 		},
 		{