Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/indexer-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ These fields are indexed and queryable via `filters` or `cel_filter` in the quer
| `sso_group_name` | string (optional) | SSO group name |
| `requires_review` | bool | Whether membership requires review |
| `public` | bool | Whether the committee is publicly visible |
| `public_name` | string (optional) | Human-readable slug for public URLs |
Comment thread
MRashad26 marked this conversation as resolved.
Outdated
| `join_mode` | string (optional) | How members can join |
| `calendar.public` | bool (optional) | Whether the committee calendar is public; omitted when the `calendar` object is empty |
| `parent_uid` | string (optional) | UID of the parent committee (if nested) |
Expand All @@ -75,6 +76,7 @@ These fields are indexed and queryable via `filters` or `cel_filter` in the quer
| `project_slug:{value}` | `project_slug:test-project-slug-1` | Find committees by project slug |
| `parent_uid:{value}` | `parent_uid:9493eae5-cd73-4c4a-b28f-3b8ec5280f6c` | Find child committees of a parent |
| `category:{value}` | `category:Board` | Find committees by category |
| `public_name:{value}` | `public_name:technical-steering-committee` | Find committee by public slug |

### Access Control (IndexingConfig)

Expand All @@ -89,8 +91,8 @@ These fields are indexed and queryable via `filters` or `cel_filter` in the quer

| Field | Value |
|---|---|
| `fulltext` | `name`, `display_name`, `description` |
| `name_and_aliases` | `name`, `display_name` (deduplicated) |
| `fulltext` | `name`, `display_name`, `public_name`, `description` (deduplicated) |
| `name_and_aliases` | `name`, `display_name`, `public_name` (deduplicated) |
Comment thread
MRashad26 marked this conversation as resolved.
Outdated
| `sort_name` | `name` |
| `public` | set from `committee.public` |

Expand Down
10 changes: 10 additions & 0 deletions internal/domain/model/committee_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,16 @@ func (c *Committee) Tags() []string {
tags = append(tags, tag)
}

if c.DisplayName != "" {
tag := fmt.Sprintf("display_name:%s", c.DisplayName)
tags = append(tags, tag)
}

if c.SSOGroupName != "" {
tag := fmt.Sprintf("sso_group_name:%s", c.SSOGroupName)
tags = append(tags, tag)
}

if c.CommitteeBase.UID != "" {
// without prefix
tags = append(tags, c.CommitteeBase.UID)
Expand Down
33 changes: 23 additions & 10 deletions internal/service/committee_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,13 +386,16 @@ func (uc *committeeWriterOrchestrator) mergeCommitteeData(ctx context.Context, e
// Update timestamp
updated.CommitteeBase.UpdatedAt = time.Now()

// Log SSO group name update if applicable
if existing.Name != updated.Name && updated.SSOGroupEnabled {
// Accept new SSO group name when name changed and SSO/public active,
// or when SSO/public just enabled on an existing committee without an SSO name.
if existing.Name != updated.Name && (updated.SSOGroupEnabled || updated.Public) {
slog.DebugContext(ctx, "SSO group name updated",
"old_sso_name", existing.SSOGroupName,
"new_sso_name", updated.SSOGroupName,
)
ssoGroupName = updated.SSOGroupName
} else if (updated.SSOGroupEnabled || updated.Public) && existing.SSOGroupName == "" {
ssoGroupName = updated.SSOGroupName
}
updated.SSOGroupName = ssoGroupName
}
Expand Down Expand Up @@ -480,10 +483,11 @@ func (uc *committeeWriterOrchestrator) Create(ctx context.Context, committee *mo
}
keys = append(keys, uniqueNameProjectKey)

// Check SSO group exists (if specified)
if committee.SSOGroupEnabled {
// Reserve SSO group name when SSO is enabled or committee is public (used as URL slug)
if committee.SSOGroupEnabled || committee.Public {
Comment thread
MRashad26 marked this conversation as resolved.
Comment thread
MRashad26 marked this conversation as resolved.
uniqueSSOName, errCheckReserveSSOName := uc.checkReserveSSOName(ctx, committee, slug)
if errCheckReserveSSOName != nil {
rollbackRequired = true
return nil, errCheckReserveSSOName
}
keys = append(keys, uniqueSSOName)
Expand Down Expand Up @@ -647,8 +651,8 @@ func (uc *committeeWriterOrchestrator) Update(ctx context.Context, committee *mo
oldNameKey := uc.rebuildCommitteeNameIndex(ctx, newNameKey, existing)
staleKeys = append(staleKeys, oldNameKey)
}
// Step 3.1: Handle SSO Group Name changes (if name changed)
if committee.SSOGroupEnabled {
// Step 3.1: Regenerate SSO group name when name changed and SSO/public active
if committee.SSOGroupEnabled || committee.Public {
newSSOKey, errSSOChange := uc.checkReserveSSOName(ctx, committee, slug)
if errSSOChange != nil {
rollbackRequired = true
Expand All @@ -665,7 +669,16 @@ func (uc *committeeWriterOrchestrator) Update(ctx context.Context, committee *mo
}
}
}

} else if (committee.SSOGroupEnabled || committee.Public) && existing.SSOGroupName == "" {
// Step 3.2: Generate SSO name for the first time (public/SSO just enabled, no name change)
newSSOKey, errSSOChange := uc.checkReserveSSOName(ctx, committee, slug)
if errSSOChange != nil {
rollbackRequired = true
return nil, errSSOChange
}
if newSSOKey != "" {
newKeys = append(newKeys, newSSOKey)
}
}

// Step 4: Validate parent change
Expand Down Expand Up @@ -711,6 +724,8 @@ func (uc *committeeWriterOrchestrator) Update(ctx context.Context, committee *mo
"name", committee.Name,
)

updateSucceeded = true

// ******************************************************
// Step 7: Publish messages

Comment thread
MRashad26 marked this conversation as resolved.
Expand Down Expand Up @@ -786,8 +801,6 @@ func (uc *committeeWriterOrchestrator) Update(ctx context.Context, committee *mo
}
// ******************************************************

// Mark update as successful for defer cleanup
updateSucceeded = true
return committee, nil
}

Expand Down Expand Up @@ -964,7 +977,7 @@ func (uc *committeeWriterOrchestrator) Delete(ctx context.Context, uid string, r
indicesToDelete = append(indicesToDelete, nameIndexKey)

// Build SSO group name index key if it exists
if existing.SSOGroupEnabled && existing.SSOGroupName != "" {
if existing.SSOGroupName != "" {
ssoIndexKey := fmt.Sprintf(constants.KVLookupSSOGroupNamePrefix, existing.SSOGroupName)
indicesToDelete = append(indicesToDelete, ssoIndexKey)
}
Expand Down
70 changes: 70 additions & 0 deletions internal/service/committee_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"errors"
"fmt"
"log/slog"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -2515,3 +2516,72 @@ func TestCommitteeWriterOrchestrator_buildMemberAccessControlMessage(t *testing.
})
}
}

func TestBuildCommitteeIndexingConfig_DisplayNameDedup(t *testing.T) {
tests := []struct {
name string
committee *model.Committee
wantAliases []string
wantDisplayNameTag string
wantFulltext string
}{
{
name: "distinct display_name included in aliases and tags",
committee: &model.Committee{
CommitteeBase: model.CommitteeBase{
UID: "uid-1",
ProjectUID: "proj-1",
Name: "TSC",
DisplayName: "Technical Steering Committee",
},
},
wantAliases: []string{"TSC", "Technical Steering Committee"},
wantDisplayNameTag: "display_name:Technical Steering Committee",
wantFulltext: "TSC Technical Steering Committee",
},
{
name: "display_name matching name is deduplicated",
committee: &model.Committee{
CommitteeBase: model.CommitteeBase{
UID: "uid-2",
ProjectUID: "proj-2",
Name: "TSC",
DisplayName: "TSC",
},
},
wantAliases: []string{"TSC"},
wantDisplayNameTag: "display_name:TSC",
wantFulltext: "TSC",
},
{
name: "empty display_name omitted from aliases and tags",
committee: &model.Committee{
CommitteeBase: model.CommitteeBase{
UID: "uid-3",
ProjectUID: "proj-3",
Name: "TSC",
},
},
wantAliases: []string{"TSC"},
wantDisplayNameTag: "",
wantFulltext: "TSC",
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
cfg := buildCommitteeIndexingConfig(tc.committee)
assert.Equal(t, tc.wantAliases, cfg.NameAndAliases)
Comment thread
MRashad26 marked this conversation as resolved.

var foundTag string
for _, tag := range cfg.Tags {
if strings.HasPrefix(tag, "display_name:") {
foundTag = tag
break
}
}
assert.Equal(t, tc.wantDisplayNameTag, foundTag, "display_name tag")
assert.Equal(t, tc.wantFulltext, cfg.Fulltext, "fulltext")
})
}
}
Loading