Skip to content

Conversation

@andrest50
Copy link
Contributor

@andrest50 andrest50 commented Nov 21, 2025

Summary

  • Add a new NATS event that is published when project settings are updated
  • The event contains both old and new settings states, allowing downstream services (like the meeting service) to react to changes
  • The event is published asynchronously using NATS Publish

Changes

  • Add ProjectSettingsUpdatedSubject constant (lfx.projects-api.project_settings.updated)
  • Add ProjectSettingsUpdatedMessage model with project_uid, old_settings, new_settings fields
  • Add generic SendProjectEventMessage method to MessageBuilder interface for publishing project events
  • Integrate event publishing into UpdateProjectSettings operation
  • Add tests for SendProjectEventMessage
  • Update README with documentation for the new NATS event

Message Format

{
  "project_uid": "string",
  "old_settings": { /* ProjectSettings object */ },
  "new_settings": { /* ProjectSettings object */ }
}

Test plan

  • Unit tests for SendProjectEventMessage pass
  • All existing tests pass
  • Manually verify event is published when updating project settings

Ticket

LFXV2-787

🤖 Generated with Claude Code

Add a new NATS event that is published whenever project settings are updated.
This allows downstream services (like the meeting service) to react to
settings changes by receiving both the old and new settings state.

Changes:
- Add ProjectSettingsUpdatedSubject constant (lfx.projects-api.project_settings.updated)
- Add ProjectSettingsUpdatedMessage model with project_uid, old_settings, new_settings
- Add SendProjectEventMessage method to MessageBuilder interface and implementation
- Integrate event publishing into UpdateProjectSettings operation
- Add tests for SendProjectEventMessage
- Update README with documentation for the new NATS event

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Andres Tobon <[email protected]>
Copilot AI review requested due to automatic review settings November 21, 2025 17:06
@andrest50 andrest50 requested a review from a team as a code owner November 21, 2025 17:06
@coderabbitai
Copy link

coderabbitai bot commented Nov 21, 2025

Walkthrough

The pull request adds a feature to publish project settings update events to NATS messaging. A new SendProjectEventMessage method is introduced to the MessageBuilder interface with implementations in the infrastructure layer. ProjectSettingsUpdatedMessage carries old and new settings data. The service layer integrates event emission during project settings updates.

Changes

Cohort / File(s) Change Summary
Interface Definitions
internal/domain/message.go, internal/domain/mock.go
Added SendProjectEventMessage(ctx context.Context, subject string, message any) error method to MessageBuilder interface and MockMessageBuilder mock implementation to support project event messaging in tests.
Message Models
internal/domain/models/message.go
Added ProjectSettingsUpdatedMessage struct with ProjectUID, OldSettings, and NewSettings fields to represent project settings changes.
NATS Infrastructure
internal/infrastructure/nats/message.go, internal/infrastructure/nats/message_test.go
Implemented SendProjectEventMessage method with JSON serialization and async NATS publish; added corresponding unit tests covering successful publish and error paths.
Service Integration
internal/service/project_operations.go
Integrated SendProjectEventMessage call in UpdateProjectSettings to emit ProjectSettingsUpdatedMessage alongside existing indexer and access messages.
Constants
pkg/constants/nats.go
Added ProjectSettingsUpdatedSubject constant with value "lfx.projects-api.project_settings.updated".
Documentation
README.md
Added NATS events publishing section documenting the project_settings.updated event with payload structure.

Sequence Diagram

sequenceDiagram
    participant Client as Caller
    participant Service as UpdateProjectSettings
    participant MessageBuilder as NATS MessageBuilder
    participant NATS as NATS Broker
    
    Client->>Service: UpdateProjectSettings(...)
    Service->>Service: Update settings in database
    
    par Concurrent Event Emission
        Service->>MessageBuilder: SendProjectEventMessage(ctx, ProjectSettingsUpdatedSubject, message)
        MessageBuilder->>MessageBuilder: JSON serialize message
        MessageBuilder->>NATS: Publish JSON payload
        NATS-->>MessageBuilder: Ack
        MessageBuilder-->>Service: Return error or nil
    end
    
    Service-->>Client: Return result
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • The changes follow a consistent, repetitive pattern: adding the same method signature across the interface hierarchy (interface → mock → implementation).
  • Logic is straightforward: JSON serialization and NATS publish with basic error handling.
  • Good test coverage with clear expected behaviors.
  • Minor attention areas: verify that the JSON tags in ProjectSettingsUpdatedMessage match the documented payload structure in README.md, and confirm the subject constant value matches the documented event name.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'LFXV2-787 Add NATS event for project settings updates' clearly and concisely summarizes the main change: adding a NATS event for project settings updates.
Description check ✅ Passed The description is well-structured, relating directly to the changeset by detailing the new NATS event implementation, affected files, message format, and testing status.
Linked Issues check ✅ Passed All coding requirements from LFXV2-787 are met: NATS event emission on project settings update is implemented, and the event provides necessary context (project_uid, old_settings, new_settings) for downstream consumers.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue requirement of emitting a NATS event for project setting updates; no out-of-scope modifications detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch andrest50/project-settings-event

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between db85f95 and 54bbb0f.

📒 Files selected for processing (8)
  • README.md (1 hunks)
  • internal/domain/message.go (1 hunks)
  • internal/domain/mock.go (1 hunks)
  • internal/domain/models/message.go (1 hunks)
  • internal/infrastructure/nats/message.go (1 hunks)
  • internal/infrastructure/nats/message_test.go (1 hunks)
  • internal/service/project_operations.go (1 hunks)
  • pkg/constants/nats.go (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Ensure code formatting and linting pass (make fmt, make lint, make check)

Files:

  • internal/domain/message.go
  • internal/domain/mock.go
  • pkg/constants/nats.go
  • internal/service/project_operations.go
  • internal/infrastructure/nats/message.go
  • internal/domain/models/message.go
  • internal/infrastructure/nats/message_test.go
README.md

📄 CodeRabbit inference engine (CLAUDE.md)

Keep README focused on quick start, API endpoints, and deployment setup

Files:

  • README.md
**/*_test.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*_test.go: Place unit tests alongside implementation with the same filename and *_test.go suffix
Each production function must have exactly one corresponding TestFunction test (table with multiple cases allowed)
Use table-driven tests for coverage (subtests over a single TestFunction)
Mock all external dependencies (e.g., repositories, message builders) in unit tests
Focus unit tests on exported package functions

Files:

  • internal/infrastructure/nats/message_test.go
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: linuxfoundation/lfx-v2-project-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-29T19:12:32.725Z
Learning: All data modifications must publish appropriate NATS events (index, access control updates)
📚 Learning: 2025-09-29T19:12:32.725Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-project-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-29T19:12:32.725Z
Learning: Applies to **/*_test.go : Mock all external dependencies (e.g., repositories, message builders) in unit tests

Applied to files:

  • internal/domain/mock.go
  • internal/infrastructure/nats/message_test.go
🧬 Code graph analysis (4)
internal/service/project_operations.go (4)
internal/domain/models/message.go (1)
  • ProjectSettingsUpdatedMessage (59-63)
internal/domain/message.go (1)
  • MessageBuilder (23-27)
internal/infrastructure/nats/message.go (1)
  • MessageBuilder (22-24)
pkg/constants/nats.go (1)
  • ProjectSettingsUpdatedSubject (44-44)
internal/infrastructure/nats/message.go (2)
internal/domain/message.go (1)
  • MessageBuilder (23-27)
pkg/constants/app.go (1)
  • ErrKey (9-9)
internal/domain/models/message.go (1)
internal/domain/models/project.go (1)
  • ProjectSettings (47-56)
internal/infrastructure/nats/message_test.go (6)
internal/infrastructure/nats/mock.go (1)
  • MockNATSConn (25-27)
pkg/constants/nats.go (1)
  • ProjectSettingsUpdatedSubject (44-44)
internal/domain/models/message.go (1)
  • ProjectSettingsUpdatedMessage (59-63)
internal/domain/models/project.go (1)
  • ProjectSettings (47-56)
internal/domain/message.go (1)
  • MessageBuilder (23-27)
internal/infrastructure/nats/message.go (1)
  • MessageBuilder (22-24)
⏰ 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). (4)
  • GitHub Check: CodeQL analysis (go)
  • GitHub Check: Agent
  • GitHub Check: MegaLinter
  • GitHub Check: Build and Test
🔇 Additional comments (8)
README.md (1)

33-45: LGTM! Clear documentation for the new NATS event.

The documentation clearly describes the event subject, purpose, and payload structure. The JSON example aligns with the ProjectSettingsUpdatedMessage model.

pkg/constants/nats.go (1)

40-44: LGTM! Constant follows naming conventions.

The ProjectSettingsUpdatedSubject constant is well-documented and follows the established naming pattern for NATS subjects in this codebase.

internal/domain/models/message.go (1)

57-63: LGTM! Message structure is well-defined.

The ProjectSettingsUpdatedMessage struct appropriately captures both old and new settings states, enabling downstream services to react to changes. The JSON tags align with the documented payload format.

internal/service/project_operations.go (1)

596-603: LGTM! Event emission follows established patterns.

The implementation correctly:

  • Captures old settings before the update (line 516)
  • Emits the event after the successful update
  • Uses the errgroup pattern consistent with indexer and access messages
  • Properly dereferences the settings structs for the message payload

Based on learnings

internal/infrastructure/nats/message.go (1)

175-192: LGTM! Clean implementation with appropriate async behavior.

The method correctly:

  • Marshals the message to JSON
  • Publishes asynchronously (appropriate for event notifications)
  • Includes comprehensive error handling and logging
  • Follows the established pattern in this file

Note that unlike SendIndexerMessage and SendAccessMessage, this method publishes asynchronously without a sync option, which is appropriate for event notifications.

internal/domain/message.go (1)

26-26: LGTM! Interface method is well-defined.

The new SendProjectEventMessage method signature is appropriate for publishing project event notifications without a sync parameter, which aligns with the async-only implementation.

internal/domain/mock.go (1)

125-128: LGTM! Mock implementation follows established patterns.

The mock method correctly implements the SendProjectEventMessage interface using testify's standard mocking pattern.

internal/infrastructure/nats/message_test.go (1)

389-461: LGTM! Comprehensive test coverage.

The test suite properly covers:

  • Successful publishing with JSON payload validation
  • Error handling when NATS publish fails
  • Correct subject usage
  • Proper message structure with old and new settings

The tests follow the established table-driven pattern and validate the critical aspects of the new functionality.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a 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 a new NATS event that is published when project settings are updated, allowing downstream services to react to changes by receiving both the old and new settings states. The implementation follows existing patterns for NATS message publishing using asynchronous publish.

Key Changes:

  • Added ProjectSettingsUpdatedSubject constant and ProjectSettingsUpdatedMessage model for the new event
  • Implemented SendProjectEventMessage method in MessageBuilder for publishing project events asynchronously
  • Integrated event publishing into the UpdateProjectSettings operation using errgroup for concurrent message delivery

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/constants/nats.go Added new NATS subject constant for project settings update events
internal/domain/models/message.go Added ProjectSettingsUpdatedMessage model containing project UID and both old/new settings
internal/domain/message.go Extended MessageBuilder interface with SendProjectEventMessage method
internal/domain/mock.go Added mock implementation for SendProjectEventMessage
internal/infrastructure/nats/message.go Implemented SendProjectEventMessage to marshal and publish events asynchronously
internal/infrastructure/nats/message_test.go Added comprehensive unit tests for the new message sending method
internal/service/project_operations.go Integrated event publishing into UpdateProjectSettings using errgroup
README.md Documented the new NATS event format and purpose

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@andrest50 andrest50 merged commit 3c912d7 into main Nov 21, 2025
12 checks passed
@andrest50 andrest50 deleted the andrest50/project-settings-event branch November 21, 2025 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants