-
Notifications
You must be signed in to change notification settings - Fork 2
[LFXV2-887] Add V1 meeting RSVP enricher for indexing RSVP data #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Implement V1MeetingRSVPEnricher to handle V1 meeting RSVP messages: - Uses meeting_id for access control object (meeting-level permissions) - Sets access check relation to "auditor" for read-only RSVP access - Sets history check relation to "writer" for modification tracking - Extracts email field as sort name and alias for searchability - Registers new enricher in IndexerService Add ObjectTypeV1MeetingRSVP constant to messaging constants. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds V1 Meeting RSVP enrichment: new enricher implementation and constant, registers it in the indexer service, and increments the Helm chart version. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Andres Tobon <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for indexing V1 meeting RSVP data by implementing a dedicated enricher that handles access control, searchability, and data extraction for RSVP messages from V1 systems. The implementation follows existing patterns used for similar V1 meeting-related enrichers (registrants, participants) and enables meeting-level permission inheritance.
Key Changes:
- Implements V1MeetingRSVPEnricher with meeting_id-based access control inheritance
- Extracts email field for sort name and searchability
- Registers enricher in the IndexerService for message processing
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/constants/messaging.go | Adds ObjectTypeV1MeetingRSVP constant following existing V1 object type naming convention |
| internal/enrichers/v1_meeting_rsvp_enricher.go | Implements new enricher with access control using meeting_id, email-based sorting, and name extraction logic |
| internal/domain/services/indexer_service.go | Registers V1MeetingRSVPEnricher in the enricher list for message routing |
| charts/lfx-v2-indexer-service/Chart.yaml | Bumps chart version to 0.4.11 for the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/enrichers/v1_meeting_rsvp_enricher.go (1)
90-122: Email-based sort name and aliases are straightforward; regex use is slightly overkill.
extractSortNameandextractNameAndAliasesreliably derive a trimmed email as sort key and alias, matching the PR description.- For
extractNameAndAliases, compilingregexp.MustCompile("(?i)^(email)$")on every call is a minor inefficiency, and the pattern is equivalent to a simple case-insensitive key check.You could simplify and micro-optimize by replacing the regex with
strings.EqualFold(key, "email"), or by hoisting the compiled regex to a package-levelvarif you prefer the regex form.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
charts/lfx-v2-indexer-service/Chart.yaml(1 hunks)internal/domain/services/indexer_service.go(1 hunks)internal/enrichers/v1_meeting_rsvp_enricher.go(1 hunks)pkg/constants/messaging.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Run golangci-lint before committing code
Run fmt, vet, lint, and test together before committing
Files:
pkg/constants/messaging.gointernal/enrichers/v1_meeting_rsvp_enricher.gointernal/domain/services/indexer_service.go
pkg/constants/messaging.go
📄 CodeRabbit inference engine (CLAUDE.md)
Add new object type constants to pkg/constants/messaging.go
Files:
pkg/constants/messaging.go
internal/enrichers/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/enrichers/**/*.go: Create new enrichers in internal/enrichers/ implementing the Enricher interface
Enricher System: Extensible data processing based on object type with registry pattern
Files:
internal/enrichers/v1_meeting_rsvp_enricher.go
internal/domain/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/domain/**/*.go: Domain layer cannot import from infrastructure or presentation layers - enforce Clean Architecture dependency rule
All external dependencies must use repository interfaces
Files:
internal/domain/services/indexer_service.go
internal/domain/services/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Business logic stays in domain services
Files:
internal/domain/services/indexer_service.go
internal/domain/services/indexer_service.go
📄 CodeRabbit inference engine (CLAUDE.md)
Register new enrichers in IndexerService.NewIndexerService()
Files:
internal/domain/services/indexer_service.go
🧬 Code graph analysis (1)
internal/domain/services/indexer_service.go (1)
internal/enrichers/v1_meeting_rsvp_enricher.go (1)
NewV1MeetingRSVPEnricher(125-134)
🔇 Additional comments (6)
charts/lfx-v2-indexer-service/Chart.yaml (1)
9-9: Chart version bump is appropriate for the new enricher feature.Version update to
0.4.11cleanly reflects the added functionality without changingappVersion.pkg/constants/messaging.go (1)
65-73: New V1 RSVP object type constant is consistent with existing naming.
ObjectTypeV1MeetingRSVP = "v1_meeting_rsvp"fits the established V1 meeting object-type pattern and enables clean registration in the enricher registry.internal/domain/services/indexer_service.go (1)
97-106: V1MeetingRSVPEnricher correctly registered in the enricher registry.Adding
NewV1MeetingRSVPEnricher()alongside the other V1 meeting enrichers ensures RSVP messages are handled without impacting existing types.internal/enrichers/v1_meeting_rsvp_enricher.go (3)
16-29: Enricher type and delegation pattern look sound.
V1MeetingRSVPEnricherdelegatesObjectTypeandEnrichDatato a configureddefaultEnricher, matching the existing registry/option-based pattern and keeping RSVP-specific logic isolated in callbacks.
31-88: ACL defaults correctly implement meeting-level, auditor/writer semantics (verify upstream types).
- When
accessCheck*/historyCheck*fields are absent, ACL objects default tov1_meeting:<meeting_id>(orobjectType:objectIDfallback) and relations default to"auditor"and"writer", which matches the stated requirements.- Explicit values in
datacleanly override the defaults, including the ability to blank them via empty strings.One behavioral assumption to double-check: all of
meeting_id,accessCheckObject,accessCheckRelation,historyCheckObject, andhistoryCheckRelationare expected to be strings in incoming V1 RSVP messages; non-string values are effectively treated as “no override” and result in empty ACL fields (no query) rather than a validation error.Please confirm that the upstream V1 RSVP message schema guarantees these fields are strings so this logic behaves as intended. If not, you may want to add a defensive conversion/logging step for non-string values.
124-134: Constructor wiring aligns with the default enricher option model.
NewV1MeetingRSVPEnrichersets the object type toObjectTypeV1MeetingRSVPand wiresWithAccessControl,WithNameAndAliases, andWithSortNameto the RSVP-specific callbacks, which integrates cleanly with the existing default enricher framework.
Signed-off-by: Andres Tobon <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/enrichers/v1_meeting_rsvp_enricher.go (2)
31-88: Confirm semantics for empty/malformed access control fieldsThe “only apply defaults when fields are completely missing” rule is clear, but it has a subtle consequence: if
accessCheckObject/accessCheckRelation/historyCheckObject/historyCheckRelationare present but either empty strings or non‑string types, no defaults are applied and the resulting object/relation remain empty, so the corresponding*_Queryis never built.If these fields can be populated from upstream JSON, an empty string or wrong type could unintentionally disable access/history checks instead of falling back to safe defaults (
auditor/writerwith meeting‑level object). If the intent is that “explicit empty” should disable checks, this is fine; if not, consider treating empty strings or non‑string values as “missing” and applying the defaults instead.
100-122: Alias extraction is solid; consider precompiling regex as a minor optimizationThe alias collection logic (case‑insensitive match on
username|email, trimming, and dedup viaseen) is robust and should give good searchability.As a small optional improvement, you could move
aliasRegex := regexp.MustCompile(...)to a package‑levelvarso it isn’t recompiled on every call toextractNameAndAliases. This only matters if this enricher runs very frequently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/enrichers/v1_meeting_rsvp_enricher.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Run golangci-lint before committing code
Run fmt, vet, lint, and test together before committing
Files:
internal/enrichers/v1_meeting_rsvp_enricher.go
internal/enrichers/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
internal/enrichers/**/*.go: Create new enrichers in internal/enrichers/ implementing the Enricher interface
Enricher System: Extensible data processing based on object type with registry pattern
Files:
internal/enrichers/v1_meeting_rsvp_enricher.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: MegaLinter
🔇 Additional comments (3)
internal/enrichers/v1_meeting_rsvp_enricher.go (3)
16-29: Composition arounddefaultEnricherlooks goodDelegating
ObjectTypeandEnrichDatato the embeddeddefaultEnricherkeeps this enricher focused on V1 RSVP‑specific hooks without duplicating common logic. This matches the existing enricher pattern cleanly.
90-98: Sort name extraction aligns with specUsing the trimmed
124-134: Constructor wiring matches enricher framework expectations
NewV1MeetingRSVPEnrichercorrectly wires the V1 RSVP object type and hooks (WithAccessControl,WithNameAndAliases,WithSortName) intonewDefaultEnricher, while returning theEnricherinterface. This is consistent with the described registry pattern and should integrate cleanly with the existing indexer wiring.
Ticket
LFXV2-887
Summary
Changes
v1_meeting_rsvp_enricher.gowith custom access control logicObjectTypeV1MeetingRSVPconstant to messaging constants🤖 Generated with Claude Code