[LFXV2-2453] feat: add committee_name and organization_required to committee invite#135
Merged
Merged
Conversation
Snapshot the committee name onto the invite at creation time so that invitees can see which committee they were invited to without needing read access to the committee itself (relevant for private committees). - Add CommitteeName field to CommitteeInvite domain model - Set CommitteeName from committeeBase.Name on create and reinstate paths - Expose committee_name in the Goa API type and regenerate gen/ - Wire through the response converter - Index the field via the existing data payload (flat_object in OS) - Update docs/indexer-contract.md with the new field Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
…tions Pre-existing invites stored in NATS before committee_name was added will have an empty field. Populate it on-the-fly from the committee record whenever an invite without committee_name is read (GetInvite) or mutated (Accept, Decline, Revoke). Mutations persist the backfilled value and re-publish to OpenSearch so the index gradually self-heals without a one-time migration job. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
Computes organization_required from the committee's current settings at invite creation time (voting enabled || business_email_required), matching the same rule used by validateOrganizationFields when creating a member. The UI uses this field to decide whether to prompt the invitee for an organization when accepting the invite. - Add OrganizationRequired bool to CommitteeInvite domain model - Fetch committee settings alongside base in CreateInvite to compute it - Expose organization_required in the Goa API type and regenerate gen/ - Wire through the response converter - Refactor four per-endpoint lazy-backfill blocks into a single enrichInviteFromCommittee helper that also refreshes OrganizationRequired (GetInvite, AcceptInvite, DeclineInvite, RevokeInvite each call GetBase and GetSettings only once via the helper; CreateInvite reuses its already- fetched values and does not call the helper) - Update docs/indexer-contract.md with the new field Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughTwo new fields, ChangesCommittee Invite Field Enrichment and Reindexing
Sequence DiagramsequenceDiagram
participant Client
participant API as Committee API
participant Enricher as enrichInviteFromCommittee
participant Reader as CommitteeReader
participant KV as NATS KV
participant CLI as reindex-invites
Note over API: Invite lifecycle operation
Client->>API: GetInvite / CreateInvite / RevokeInvite
API->>Enricher: enrich from committee
Enricher->>Reader: GetBase + GetSettings
Reader-->>Enricher: committee data
Enricher-->>API: CommitteeName + OrganizationRequired set
API-->>Client: updated invite response
Note over CLI: CLI reindex run
CLI->>KV: ListInvites
KV-->>CLI: invite list
loop per invite
alt committee cache miss
CLI->>Reader: GetBase + GetSettings
Reader-->>CLI: cached snapshot
end
alt needsKVUpdate
CLI->>KV: GetInvite (revision)
KV-->>CLI: current invite + revision
CLI->>KV: UpdateInvite
KV-->>CLI: success/failure
end
alt KV update succeeded
CLI->>CLI: publishIndexerMessage
CLI->>CLI: publishAccessControlMessage
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the committee-invite domain/API shape to include invite-visible committee metadata (committee_name) and a derived flag (organization_required) so clients (UI + indexer) can render/decide organization prompting without requiring committee read access.
Changes:
- Added
committee_name+organization_requiredto the invite domain model and exposed them via Goa-generated API types and response conversion. - Populated these fields on invite creation and “best-effort” enriched them on invite lifecycle operations (get/accept/decline/revoke).
- Updated indexer contract docs and regenerated OpenAPI + HTTP client/server code.
Reviewed changes
Copilot reviewed 7 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/domain/model/committee_invite.go | Adds CommitteeName + OrganizationRequired fields to the invite domain model. |
| cmd/committee-api/design/type.go | Adds the two attributes to the Goa invite type definition. |
| cmd/committee-api/service/committee_service.go | Computes/sets the new fields on create and enriches invites from committee base/settings in multiple endpoints. |
| cmd/committee-api/service/committee_service_response.go | Wires the new fields into the API response converter. |
| docs/indexer-contract.md | Documents committee_name and organization_required in the invite indexer payload schema. |
| gen/committee_service/service.go | Regenerates Goa service types to include the new invite attributes. |
| gen/http/openapi3.yaml | Regenerates OpenAPI v3 schema/examples (also changes accept-invite requestBody requiredness). |
| gen/http/openapi.yaml | Regenerates Swagger/OpenAPI v2 schema/examples (also changes accept-invite body requiredness). |
| gen/http/openapi.json | Regenerates JSON OpenAPI artifact reflecting the schema and body-required changes. |
| gen/http/committee_service/server/types.go | Regenerates server-side HTTP response mapping to include new invite fields. |
| gen/http/committee_service/client/types.go | Regenerates client-side mappings (includes a new nil-deref risk in accept-invite request builder). |
| gen/http/committee_service/client/cli.go | Regenerates CLI payload builder behavior for accept-invite body parsing. |
| gen/http/cli/committee/cli.go | Regenerates CLI flag defaults (accept-invite body default changed). |
Files not reviewed (5)
- gen/committee_service/service.go: Generated file
- gen/http/cli/committee/cli.go: Generated file
- gen/http/committee_service/client/cli.go: Generated file
- gen/http/committee_service/client/types.go: Generated file
- gen/http/committee_service/server/types.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/committee-api/service/committee_service.go (1)
662-664: ⚡ Quick winConsider extracting the OrganizationRequired computation to a helper.
The formula
committeeBase.EnableVoting || (committeeSettings != nil && committeeSettings.BusinessEmailRequired)is duplicated inenrichInviteFromCommittee(line 1342). Extracting this business logic to a small helper function would prevent divergence if the computation rule changes.♻️ Refactor to eliminate duplication
// computeOrganizationRequired determines whether an organization is required // for committee membership based on voting and email settings. func computeOrganizationRequired(base *model.CommitteeBase, settings *model.CommitteeSettings) bool { return base.EnableVoting || (settings != nil && settings.BusinessEmailRequired) }Then use it in both CreateInvite and enrichInviteFromCommittee:
- orgRequired := committeeBase.EnableVoting || (committeeSettings != nil && committeeSettings.BusinessEmailRequired) + orgRequired := computeOrganizationRequired(committeeBase, committeeSettings)And in enrichInviteFromCommittee (line 1342):
- invite.OrganizationRequired = cb.EnableVoting || (settings != nil && settings.BusinessEmailRequired) + invite.OrganizationRequired = computeOrganizationRequired(cb, settings)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/committee-api/service/committee_service.go` around lines 662 - 664, The organization required computation formula `committeeBase.EnableVoting || (committeeSettings != nil && committeeSettings.BusinessEmailRequired)` is duplicated in two locations: in the current code block (around line 662-664) and in the enrichInviteFromCommittee method (around line 1342). Create a new helper function named computeOrganizationRequired that accepts the CommitteeBase and CommitteeSettings parameters and returns a boolean representing whether an organization is required, then replace both instances of the duplicated formula with calls to this helper function to maintain consistency and prevent divergence if the business logic changes in the future.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/committee-api/service/committee_service.go`:
- Around line 662-664: The organization required computation formula
`committeeBase.EnableVoting || (committeeSettings != nil &&
committeeSettings.BusinessEmailRequired)` is duplicated in two locations: in the
current code block (around line 662-664) and in the enrichInviteFromCommittee
method (around line 1342). Create a new helper function named
computeOrganizationRequired that accepts the CommitteeBase and CommitteeSettings
parameters and returns a boolean representing whether an organization is
required, then replace both instances of the duplicated formula with calls to
this helper function to maintain consistency and prevent divergence if the
business logic changes in the future.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7b08a3e-6ddd-467b-9884-e9fac9417a7d
⛔ Files ignored due to path filters (9)
gen/committee_service/service.gois excluded by!**/gen/**gen/http/cli/committee/cli.gois excluded by!**/gen/**gen/http/committee_service/client/cli.gois excluded by!**/gen/**gen/http/committee_service/client/types.gois excluded by!**/gen/**gen/http/committee_service/server/types.gois excluded by!**/gen/**gen/http/openapi.jsonis excluded by!**/gen/**gen/http/openapi.yamlis excluded by!**/gen/**gen/http/openapi3.jsonis excluded by!**/gen/**gen/http/openapi3.yamlis excluded by!**/gen/**
📒 Files selected for processing (5)
cmd/committee-api/design/type.gocmd/committee-api/service/committee_service.gocmd/committee-api/service/committee_service_response.godocs/indexer-contract.mdinternal/domain/model/committee_invite.go
… reindex-invites The reindex-invites CLI command now populates committee_name and organization_required on each invite before publishing to OpenSearch, and persists the updated values back to NATS KV when they differ from what is stored (i.e. for pre-existing invites that predate these fields). Committee base and settings are fetched once per unique committee UID and cached for the duration of the run, avoiding redundant NATS KV reads across invites that share a committee. The NATS KV update uses GetInvite to obtain the current revision and is only performed when a change is detected; indexer and FGA messages are skipped for that invite if the KV write fails. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
- Add reindex_invites_test.go with 10 table-driven scenarios covering dry-run, old-invite backfill, new-invite no-op, org_required mismatch, committee fetch failure (preserves existing fields), KV update failure (skips publish), committee-uid filter, committee cache, and publish errors - Extend mockReader in total_members_attribute_test.go with invite, settings, and per-invite revision/error fields so all sync tests share one fake - Add mockInviteWriter and mockPublisher to the new test file - Fix a correctness bug in reindex_invites.go: guard all invite field mutations behind snap.fetched so a transient committee lookup failure cannot corrupt correctly-set fields on new invites - Document sync reindex-invites in cmd/committee-cli/README.md Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/committee-cli/commands/sync/reindex_invites.go`:
- Around line 132-147: The code fetches a fresh invite and revision via
GetInvite but discards the returned invite (using underscore), then writes the
stale invite from ListInvites with the fresh revision. Instead, capture the
fresh invite returned from GetInvite and use it in the UpdateInvite call to
prevent overwriting concurrent modifications. Change the underscore placeholder
in the GetInvite return assignment to a variable name, store the fresh invite
data, and pass that fresh invite to the UpdateInvite method instead of the stale
invite variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01fc7021-30da-4721-bacb-57306f387a28
📒 Files selected for processing (3)
cmd/committee-cli/commands/command.gocmd/committee-cli/commands/sync/reindex_invites.gocmd/committee-cli/main.go
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/committee-cli/commands/sync/total_members_attribute_test.go (1)
16-34: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftMove expanded test doubles to the shared mock infrastructure.
This file keeps extending an in-file
mockReaderinstead of usinginternal/infrastructure/mockfakes, which is now increasing maintenance drift across CLI tests.As per coding guidelines, "
**/*_test.go: Use table-driven tests withinternal/infrastructure/mockfakes for test structure".Also applies to: 77-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/committee-cli/commands/sync/total_members_attribute_test.go` around lines 16 - 34, The test file is extending the in-file mockReader struct instead of leveraging shared mock infrastructure from internal/infrastructure/mock, creating maintenance drift. Move the mockReader struct definition and all its associated mock method implementations (covering bases, revision, members, settings, invites and related error handling as indicated by lines 77-118) to use the shared fakes from internal/infrastructure/mock instead. Replace the local mockReader with the appropriate shared mock implementation or create a wrapper that delegates to the shared mock infrastructure to maintain consistency across CLI tests.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/committee-cli/commands/sync/reindex_invites_test.go`:
- Around line 238-260: The test
TestReindexInvites_CommitteeCache_FetchedOncePerCommittee declares fetchCount on
line 239 but never connects it to track actual GetBase calls in the mockReader
and never asserts its final value. Wire fetchCount to increment in the
mockReader's GetBase method, remove the unused variable assignment (the _ =
fetchCount line), and add an assertion at the end of the test to verify that
fetchCount equals 1, proving that the committee cache resulted in only one fetch
for the two invites targeting the same committee.
- Around line 138-155: The test function TestReindexInvites_NewInvite_NoKVUpdate
only asserts on pub.indexerCalls but is missing verification of the access
publish path. Add an additional assertion to check that pub.accessCalls equals 1
(or the expected value) immediately after the existing indexerCalls assertion to
ensure both publish paths are exercised and to prevent regressions in the access
publish branch.
---
Outside diff comments:
In `@cmd/committee-cli/commands/sync/total_members_attribute_test.go`:
- Around line 16-34: The test file is extending the in-file mockReader struct
instead of leveraging shared mock infrastructure from
internal/infrastructure/mock, creating maintenance drift. Move the mockReader
struct definition and all its associated mock method implementations (covering
bases, revision, members, settings, invites and related error handling as
indicated by lines 77-118) to use the shared fakes from
internal/infrastructure/mock instead. Replace the local mockReader with the
appropriate shared mock implementation or create a wrapper that delegates to the
shared mock infrastructure to maintain consistency across CLI tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcc8c962-c286-4a0f-ac3e-d8d98e6b7f96
📒 Files selected for processing (4)
cmd/committee-cli/README.mdcmd/committee-cli/commands/sync/reindex_invites.gocmd/committee-cli/commands/sync/reindex_invites_test.gocmd/committee-cli/commands/sync/total_members_attribute_test.go
✅ Files skipped from review due to trivial changes (1)
- cmd/committee-cli/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/committee-cli/commands/sync/reindex_invites.go
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 20 changed files in this pull request and generated 7 comments.
Files not reviewed (5)
- gen/committee_service/service.go: Generated file
- gen/http/cli/committee/cli.go: Generated file
- gen/http/committee_service/client/cli.go: Generated file
- gen/http/committee_service/client/types.go: Generated file
- gen/http/committee_service/server/types.go: Generated file
Comments suppressed due to low confidence (1)
gen/http/committee_service/client/cli.go:1452
BuildAcceptInvitePayloadnow JSON-unmarshals the raw--bodyflag value. If callers omit the flag (or tooling passes a sentinel like "REQUIRED"), this fails JSON decoding even though the accept-invite body is intended to be optional (an empty object should be accepted).
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>
Contributor
Author
Review Feedback AddressedCommit: 6e1d6ce Changes Made
No Change Needed
Threads Resolved16 review threads addressed in this iteration. |
audigregorie
left a comment
audigregorie
left a comment
There was a problem hiding this comment.
Code Review Summary
Solid, well-tested change. One Major correctness concern on the settings-failure path in enrichInviteFromCommittee; the rest are informational. Build is clean and the sync tests pass locally.
Informational — outside the diff
- Lazy backfill on
GetInviteenriches the response only — it does not persist or re-index. A pre-existing invite that is only ever viewed (never accepted/declined/revoked) won't getcommittee_nameinto OpenSearch until thereindex-invitesCLI runs. Reasonable tradeoff (that's what the reindex tool is for), but the PR description reads as ifGetInvitebackfills the stored record — worth a wording tweak. stats.Skippedis never incremented inreindex_invites.go, yet the README documents askippedfield in the completion log line — it will always be0.
What's done well
- The reindex
fetchedguard avoids corrupting stored fields when a committee is temporarily unreachable; the re-fetch-for-revision + gate-publishes-behind-!failedflow is careful about partial writes. - Strong, focused CLI test coverage (backfill, no-op, mismatch, fetch-failure, KV-write-failure, cache-once-per-committee).
- Coherent snapshot-vs-live semantics (
committee_nameset-when-missing,organization_requiredrefreshed), with the indexer contract doc updated in the same PR.
…enrichInviteFromCommittee When GetSettings fails, the previous code evaluated: invite.OrganizationRequired = cb.EnableVoting || (settings != nil && settings.BusinessEmailRequired) With settings == nil this collapses to cb.EnableVoting only, silently clobbering a correctly-stored OrganizationRequired=true for a committee where BusinessEmailRequired is true but EnableVoting is false. On the Accept/Decline/Revoke paths the wrong value would then be persisted to KV and republished to the index; on GetInvite it would feed the wrong value to the accept-form org prompt. Fix: return early on settings error so the existing invite value is preserved — consistent with the GetBase early-return above it and with the reindex tool's deliberate "don't corrupt correctly-set values" guard. Also updated the doc comment to accurately reflect both early-return paths. Adds TestGetInvite_SettingsFailurePreservesOrganizationRequired to cover this exact regression path. Resolves 1 review thread from @audigregorie. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
Contributor
Author
Review Feedback AddressedCommit: 1540169 Changes Made
Threads Resolved1 of 1 unresolved thread addressed in this iteration. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/committee-api/service/committee_service_test.go (1)
691-726: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd assertion for CommitteeName enrichment.
The test verifies that
OrganizationRequiredis preserved whenGetSettingsfails, but doesn't verify thatCommitteeNameenrichment (which happens before the settings failure) still works correctly. According toenrichInviteFromCommittee,GetBasesucceeds and setsCommitteeNamebefore the early return on settings error.Add an assertion to verify
CommitteeNameis populated even when settings lookup fails:require.NotNil(t, result.OrganizationRequired) assert.True(t, *result.OrganizationRequired, "settings failure must not clobber a correctly-stored OrganizationRequired=true") // Verify partial enrichment: CommitteeName should still be set despite settings failure require.NotNil(t, result.CommitteeName) assert.Equal(t, "No Settings Committee", *result.CommitteeName, "CommitteeName enrichment must succeed even when settings fail")This ensures the test covers the complete behavior: successful
GetBaseenrichment + gracefulGetSettingsfailure handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/committee-api/service/committee_service_test.go` around lines 691 - 726, In the TestGetInvite_SettingsFailurePreservesOrganizationRequired test function, add assertions after the existing OrganizationRequired checks to verify that CommitteeName is properly enriched even when GetSettings fails. Add a require.NotNil check for result.CommitteeName and an assert.Equal check to confirm it equals "No Settings Committee", which verifies that the successful GetBase enrichment happens before the early return on settings error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/committee-api/service/committee_service_test.go`:
- Around line 691-726: In the
TestGetInvite_SettingsFailurePreservesOrganizationRequired test function, add
assertions after the existing OrganizationRequired checks to verify that
CommitteeName is properly enriched even when GetSettings fails. Add a
require.NotNil check for result.CommitteeName and an assert.Equal check to
confirm it equals "No Settings Committee", which verifies that the successful
GetBase enrichment happens before the early return on settings error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 502ee94c-02cf-404e-8015-5c5486c4d003
📒 Files selected for processing (2)
cmd/committee-api/service/committee_service.gocmd/committee-api/service/committee_service_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/committee-api/service/committee_service.go
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 19 changed files in this pull request and generated 4 comments.
Files not reviewed (3)
- gen/committee_service/service.go: Generated file
- gen/http/committee_service/client/types.go: Generated file
- gen/http/committee_service/server/types.go: Generated file
…cess in reindex-invites
Three related fixes in cmd/committee-cli/commands/sync/reindex_invites.go,
plus a doc comment correction in cmd/committee-api/service/committee_service.go.
reindex_invites.go:
- Add settingsFetched bool to committeeSnapshot. lookupCommittee now only
sets settingsFetched when GetSettings returns without an error, so a
transient settings failure cannot cause OrganizationRequired to be
clobbered with a value derived from nil/zero settings.
- Gate the invite-loop OrganizationRequired update on snap.settingsFetched
so the existing value is preserved when settings are unavailable.
- Gate the KV backfill freshInvite.OrganizationRequired assignment on
snap.settingsFetched for the same reason — mirrors the guard the API-side
enrichInviteFromCommittee already applies.
- When GetSettings returns (nil, 0, nil) (no settings configured, no
error), treat as a successful fetch: compute organizationRequired as
base.EnableVoting only (no BusinessEmailRequired contribution).
committee_service.go:
- Update enrichInviteFromCommittee doc comment to accurately describe the
partial-mutation behaviour: a GetBase failure leaves the invite fully
unchanged; a GetSettings failure leaves OrganizationRequired unchanged
but CommitteeName may already have been backfilled. The previous comment
("on any error the invite is left unchanged") was inaccurate.
Resolves 4 review threads from copilot-pull-request-reviewer.
Generated with [Claude Code](https://claude.ai/code)
Signed-off-by: Andres Tobon <andrest2455@gmail.com>
Contributor
Author
Review Feedback AddressedCommit: f271a30 Changes Made
Threads Resolved4 of 4 unresolved threads addressed in this iteration. |
audigregorie
approved these changes
Jun 22, 2026
Merged
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
committee_nametoCommitteeInvite— snapshotted from the committee at creation time so invitees can see which committee invited them without needing read access to the committee itself (relevant for private committees)organization_requiredboolean — computed fromenable_voting || business_email_required, the same rule used byvalidateOrganizationFieldswhen creating a member; the UI uses this to decide whether to prompt for an organization on acceptdatais aflat_object, no schema migration needed)enrichInviteFromCommitteehelper onGetInvite,AcceptInvite,DeclineInvite, andRevokeInvite— pre-existing invites get both fields populated as they are interacted with, without a one-time migration jobTicket
LFXV2-2453
Changes
Goa/API:
cmd/committee-api/design/type.go— addedcommittee_nameandorganization_requiredattributes toCommitteeInviteWithReadonlyAttributes;gen/regenerated viamake apigenDomain/service:
internal/domain/model/committee_invite.go— addedCommitteeNameandOrganizationRequiredfieldscmd/committee-api/service/committee_service.go— set both fields inCreateInvite(using already-fetched base + one newGetSettingscall); addedenrichInviteFromCommitteehelper used byGetInvite,AcceptInvite,DeclineInvite,RevokeInvite(oneGetBase+ oneGetSettingsper call, no duplication)cmd/committee-api/service/committee_service_response.go— wire both fields through the response converterContract docs:
docs/indexer-contract.md— documentedcommittee_nameandorganization_requiredin the Committee Invite data schema🤖 Generated with Claude Code