fix(review): address PR #135 review feedback

Address review comments from copilot-pull-request-reviewer and coderabbitai:

- reindex_invites.go: apply KV backfill to fresh invite from GetInvite
- reindex_invites_test.go: assert access publish and committee cache hits
- total_members_attribute_test.go: fix invitesByCommittee typo, track GetBase calls
- committee_service.go: log GetSettings failures for organization_required
- committee_service_test.go: assert committee_name and organization_required
- gen/http: restore accept-invite optional-body patches reverted by apigen

Resolves 10 review threads.

Signed-off-by: Andres Tobon <andrest2455@gmail.com>

Author: andrest50

cmd/committee-api/service/committee_service.go

@@ -660,7 +660,11 @@ func (s *committeeServicesrvc) CreateInvite(ctx context.Context, p *committeeser
 	}
 
 	// Best-effort: settings drive organization_required; missing settings means false.
-	committeeSettings, _, _ := s.storage.GetSettings(ctx, p.UID)
+	committeeSettings, _, settingsErr := s.storage.GetSettings(ctx, p.UID)
+	if settingsErr != nil {
+		slog.WarnContext(ctx, "CreateInvite: failed to get committee settings for organization_required",
+			"committee_uid", p.UID, "error", settingsErr)
+	}
 	orgRequired := committeeBase.EnableVoting || (committeeSettings != nil && committeeSettings.BusinessEmailRequired)
 
 	var inviteOrgID, inviteOrgName, inviteOrgWebsite *string
@@ -1338,7 +1342,11 @@ func (s *committeeServicesrvc) enrichInviteFromCommittee(ctx context.Context, in
 	if invite.CommitteeName == "" {
 		invite.CommitteeName = cb.Name
 	}
-	settings, _, _ := s.storage.GetSettings(ctx, committeeUID)
+	settings, _, settingsErr := s.storage.GetSettings(ctx, committeeUID)
+	if settingsErr != nil {
+		slog.WarnContext(ctx, "enrichInviteFromCommittee: failed to get committee settings",
+			"committee_uid", committeeUID, "error", settingsErr)
+	}
 	invite.OrganizationRequired = cb.EnableVoting || (settings != nil && settings.BusinessEmailRequired)
 }
 

cmd/committee-api/service/committee_service_test.go

@@ -681,6 +681,8 @@ func TestGetInvite(t *testing.T) {
 				require.NoError(t, err)
 				require.NotNil(t, result)
 				assert.Equal(t, tt.payload.InviteUID, *result.UID)
+				assert.Equal(t, "Technical Advisory Committee", *result.CommitteeName)
+				assert.True(t, *result.OrganizationRequired)
 			}
 		})
 	}
@@ -732,6 +734,8 @@ func TestCreateInvite(t *testing.T) {
 				assert.Equal(t, tt.payload.UID, *result.CommitteeUID)
 				assert.Equal(t, tt.payload.InviteeEmail, *result.InviteeEmail)
 				assert.Equal(t, "pending", result.Status)
+				assert.Equal(t, "Technical Advisory Committee", *result.CommitteeName)
+				assert.True(t, *result.OrganizationRequired)
 
 				require.Len(t, sender.calls, 1)
 				call := sender.calls[0]

cmd/committee-cli/commands/sync/reindex_invites.go

@@ -140,19 +140,27 @@ func (s *reindexInvitesSubcommand) Run(ctx context.Context, rc commands.RunConte
 		failed := false
 
 		if needsKVUpdate {
-			_, rev, getErr := rc.CommitteeReader.GetInvite(ctx, invite.UID)
+			freshInvite, rev, getErr := rc.CommitteeReader.GetInvite(ctx, invite.UID)
 			if getErr != nil {
 				slog.WarnContext(ctx, "failed to fetch invite revision for KV update",
 					"error", getErr,
 					"invite_uid", invite.UID,
 				)
 				failed = true
-			} else if updateErr := rc.CommitteeInviteWriter.UpdateInvite(ctx, invite, rev); updateErr != nil {
-				slog.WarnContext(ctx, "failed to update invite in NATS KV",
-					"error", updateErr,
-					"invite_uid", invite.UID,
-				)
-				failed = true
+			} else {
+				if freshInvite.CommitteeName == "" && snap.name != "" {
+					freshInvite.CommitteeName = snap.name
+				}
+				freshInvite.OrganizationRequired = snap.organizationRequired
+				if updateErr := rc.CommitteeInviteWriter.UpdateInvite(ctx, freshInvite, rev); updateErr != nil {
+					slog.WarnContext(ctx, "failed to update invite in NATS KV",
+						"error", updateErr,
+						"invite_uid", invite.UID,
+					)
+					failed = true
+				} else {
+					invite = freshInvite
+				}
 			}
 		}
 

cmd/committee-cli/commands/sync/reindex_invites_test.go

@@ -152,6 +152,7 @@ func TestReindexInvites_NewInvite_NoKVUpdate(t *testing.T) {
 	require.NoError(t, err)
 	assert.Empty(t, iw.updated, "no KV write expected when fields already match")
 	assert.Equal(t, 1, pub.indexerCalls)
+	assert.Equal(t, 1, pub.accessCalls)
 }
 
 func TestReindexInvites_OrgRequiredMismatch_UpdatesKV(t *testing.T) {
@@ -236,11 +237,6 @@ func TestReindexInvites_FilterByCommitteeUID(t *testing.T) {
 }
 
 func TestReindexInvites_CommitteeCache_FetchedOncePerCommittee(t *testing.T) {
-	fetchCount := 0
-	// Use a custom reader that counts GetBase calls by piggy-backing on baseErr.
-	// We use a regular mockReader but verify via the committee cache: two invites for
-	// the same committee should result in only one publish pair each, and the bases
-	// map having one entry verifies the cache lookup path via the existing mock.
 	r := &mockReader{
 		bases: map[string]*model.CommitteeBase{
 			"c1": {Name: "TSC"},
@@ -250,12 +246,12 @@ func TestReindexInvites_CommitteeCache_FetchedOncePerCommittee(t *testing.T) {
 			{UID: "i2", CommitteeUID: "c1", CommitteeName: "TSC", Status: "pending"},
 		},
 	}
-	_ = fetchCount
 	iw := &mockInviteWriter{}
 	pub := &mockPublisher{}
 
 	err := (&reindexInvitesSubcommand{}).Run(context.Background(), newReindexRC(r, iw, pub))
 	require.NoError(t, err)
+	assert.Equal(t, 1, r.getBaseCalls, "committee base should be fetched once per unique committee")
 	assert.Equal(t, 2, pub.indexerCalls, "both invites published")
 }
 

cmd/committee-cli/commands/sync/total_members_attribute_test.go

@@ -27,17 +27,19 @@ type mockReader struct {
 	settings    map[string]*model.CommitteeSettings
 	settingsErr map[string]error
 
-	invites           []*model.CommitteeInvite
-	invitesByCommitte map[string][]*model.CommitteeInvite
-	inviteRevision    map[string]uint64
-	inviteGetErr      map[string]error
+	invites            []*model.CommitteeInvite
+	invitesByCommittee map[string][]*model.CommitteeInvite
+	inviteRevision     map[string]uint64
+	inviteGetErr       map[string]error
+	getBaseCalls       int
 }
 
 func (r *mockReader) ListAllUIDs(_ context.Context) ([]string, error) {
 	return r.uids, r.listUIDErr
 }
 
 func (r *mockReader) GetBase(_ context.Context, uid string) (*model.CommitteeBase, uint64, error) {
+	r.getBaseCalls++
 	if err, ok := r.baseErr[uid]; ok {
 		return nil, 0, err
 	}
@@ -87,8 +89,8 @@ func (r *mockReader) GetInvite(_ context.Context, uid string) (*model.CommitteeI
 	return nil, rev, nil
 }
 func (r *mockReader) ListInvites(_ context.Context, committeeUID string) ([]*model.CommitteeInvite, error) {
-	if r.invitesByCommitte != nil {
-		return r.invitesByCommitte[committeeUID], nil
+	if r.invitesByCommittee != nil {
+		return r.invitesByCommittee[committeeUID], nil
 	}
 	var out []*model.CommitteeInvite
 	for _, inv := range r.invites {

gen/http/cli/committee/cli.go

@@ -1377,7 +1377,7 @@ paths:
                 - name: Accept-InviteRequestBody
                   in: body
                   description: Optional JSON body
-                  required: true
+                  required: false
                   schema:
                     $ref: '#/definitions/AcceptInviteOptionalBody'
             responses: