IFC-1633 Add events related to proposed change actions #6875
Conversation
WalkthroughThis change introduces a comprehensive event system for proposed change reviews and merges. It adds new event types, event classes, and GraphQL types, integrates event emission into review and merge workflows, updates documentation, and extends tests to verify event logging. Supporting utilities and filters for event querying are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GraphQL API
participant Event Service
participant Prefect Events
participant DB
User->>GraphQL API: Submit review/merge mutation
GraphQL API->>DB: Update proposed change state/relationships
GraphQL API->>Event Service: Emit ProposedChange*Event (with metadata)
Event Service->>Prefect Events: Log event
GraphQL API-->>User: Return mutation result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (11)
⏰ 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). (9)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
the
group/backend
CodSpeed Performance ReportMerging #6875 will not alter performanceComparing Summary
|
github-actions
bot
added
the
type/documentation
gmazoyer
force-pushed
the
gma-20250722-ifc1633
branch
3 times, most recently
from
9d1a42f to
5d84916
Compare
gmazoyer
force-pushed
the
gma-20250722-ifc1633
branch
2 times, most recently
from
9a6400e to
ef6e955
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/docs/reference/infrahub-events.mdx (1)
339-518: LGTM! Comprehensive documentation for proposed change events.The documentation is thorough and follows the established format with detailed payload descriptions. All eight event types are properly covered with consistent structure and clear descriptions.
Consider organizing the events in alphabetical order within the "Proposed events" section for better readability and consistency with documentation standards.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
backend/infrahub/core/constants/__init__.py(1 hunks)backend/infrahub/events/__init__.py(2 hunks)backend/infrahub/events/proposed_change_action.py(1 hunks)backend/infrahub/graphql/mutations/proposed_change.py(5 hunks)backend/infrahub/graphql/types/event.py(2 hunks)backend/infrahub/proposed_change/tasks.py(3 hunks)backend/infrahub/task_manager/event.py(2 hunks)backend/tests/functional/proposed_change/test_proposed_change_review.py(6 hunks)backend/tests/helpers/events.py(2 hunks)docs/docs/reference/infrahub-events.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
backend/tests/**/*
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Place backend tests in
backend/tests/
Files:
backend/tests/helpers/events.pybackend/tests/functional/proposed_change/test_proposed_change_review.py
backend/**/*
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Run backend tests with
pytestor viainvoketasks
Files:
backend/tests/helpers/events.pybackend/infrahub/events/__init__.pybackend/infrahub/core/constants/__init__.pybackend/infrahub/proposed_change/tasks.pybackend/infrahub/graphql/mutations/proposed_change.pybackend/infrahub/graphql/types/event.pybackend/infrahub/events/proposed_change_action.pybackend/tests/functional/proposed_change/test_proposed_change_review.pybackend/infrahub/task_manager/event.py
docs/docs/reference/**/*.{md,mdx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Write reference documentation in
docs/docs/reference/
Files:
docs/docs/reference/infrahub-events.mdx
docs/**/*.mdx
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Use Docusaurus markdown/MDX features in documentation
Files:
docs/docs/reference/infrahub-events.mdx
docs/**/*
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Develop documentation in
docs/, preview withinvoke docs.build docs.serve, and validate withinvoke docs.validate
Files:
docs/docs/reference/infrahub-events.mdx
🧠 Learnings (1)
docs/docs/reference/infrahub-events.mdx (1)
Learnt from: CR
PR: opsmill/infrahub#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-22T08:13:56.198Z
Learning: Documentation generated for Infrahub must reflect its novel approach, providing clarity around new concepts and demonstrating integration with familiar patterns from existing tools like Git, infrastructure-as-code, and CI/CD pipelines
⏰ 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: Cloudflare Pages
🔇 Additional comments (26)
backend/tests/helpers/events.py (2)
5-6: LGTM: Clean import additionThe import statement correctly adds
EventNameFilteralongside existing event filter imports.
56-62: LGTM: Well-implemented event query helperThe
query_events_by_namefunction follows the established pattern fromquery_eventand provides a clean interface for querying events by name. The implementation correctly:
- Uses
EventNameFilterfor filtering by event name- Handles the POST request to
/events/filterendpoint- Validates the response and returns typed Event objects
- Maintains consistency with existing helper functions
backend/infrahub/core/constants/__init__.py (1)
64-69: LGTM: Well-structured event type constantsThe new proposed change event type constants follow the established naming conventions and patterns:
- Consistent use of
EVENT_NAMESPACEprefix- Clear, descriptive names that align with the proposed change lifecycle
- Proper ordering and grouping with related constants
The constants cover all the key proposed change actions mentioned in the PR objectives.
backend/infrahub/events/__init__.py (2)
6-13: LGTM: Clean module import organizationThe import statement for proposed change action events is well-organized, grouping all related event classes together in a clear, readable format.
32-37: LGTM: Proper public API exposureAll imported proposed change event classes are correctly added to
__all__, ensuring they're properly exposed as part of the module's public API.backend/infrahub/proposed_change/tasks.py (3)
46-46: LGTM: Appropriate import additionThe
NodeManagerimport is needed for retrieving user information for the merge event.
55-55: LGTM: Event-related importsThe imports for
EventMetaandProposedChangeMergedEventare correctly added to support the new event emission functionality.
210-223: LGTM: Well-implemented merge event emissionThe event emission code is properly structured:
- Retrieves the current user using the appropriate NodeManager method
- Creates a comprehensive
ProposedChangeMergedEventwith all relevant fields- Uses
EventMeta.from_context()for proper event metadata- Sends the event through the event service
- Positioned correctly after successful merge state transition
- Will be covered by existing error handling
The implementation follows established patterns and provides good event tracking for proposed change merges.
backend/infrahub/task_manager/event.py (2)
175-185: LGTM: Well-structured reviewer info extraction methodsThe three new private methods follow the established pattern for event data extraction:
- Use descriptive method names and return clear dictionary structures
- Employ safe
self.resource.get()calls to avoid KeyError exceptions- Return consistent data formats that align with GraphQL schema expectations
- Maintain separation of concerns by splitting reviewer info, current decision, and former decision
207-216: LGTM: Appropriate event-specific data handlingThe new match cases properly handle proposed change review events:
- Correctly differentiate between approval/rejection events and revocation events
- Use dictionary unpacking to combine reviewer information with relevant decision data
- Follow the established pattern for other event types in the match statement
- Provide the right data context for each event type (current decision vs former decision)
backend/infrahub/graphql/mutations/proposed_change.py (4)
19-25: LGTM! Event imports are well-structured.The imported event classes follow proper naming conventions and the EventMeta import enables proper event metadata handling.
38-42: LGTM! Type imports properly structured.The type imports are correctly placed within the TYPE_CHECKING block to avoid circular imports while maintaining type safety.
272-286: LGTM! Context parameter addition is appropriate.Adding the
contextparameter enables proper event metadata creation from the GraphQL context, which is essential for the event system integration.
293-371: LGTM! Event creation and dispatching logic is well-implemented.The event creation follows excellent patterns:
- Proper use of EventMeta from context
- Comprehensive event data including all relevant proposed change and reviewer details
- Appropriate event class selection for each decision type
- Correct async event service usage with conditional sending
backend/infrahub/graphql/types/event.py (2)
116-137: LGTM! GraphQL event types are well-structured.The new ObjectTypes properly implement the EventNodeInterface with descriptive fields for reviewer information and decisions. The separation between review and revoked events is logical and clear.
189-192: LGTM! Event type mapping is logical and consistent.The mapping correctly associates approval/rejection events with the review event type, and revocation events with the revoked event type, reflecting their different data structures appropriately.
backend/tests/functional/proposed_change/test_proposed_change_review.py (7)
3-22: LGTM! Test imports are appropriate for event verification.The new imports support the event verification functionality with proper async patterns and type annotations.
48-51: LGTM! Prefect client fixture is well-implemented.The fixture uses proper async patterns with context manager and appropriate scope for efficient test execution.
53-60: LGTM! Event assertion method with reasonable retry logic.The polling approach with 10 retries and 1-second intervals provides good balance between test reliability and execution time. The clear failure message aids debugging.
111-112: LGTM! Event assertion for approval action is properly placed.The event verification occurs after the approval mutation with the correct event name.
133-134: LGTM! Event assertion for rejection action is properly placed.The event verification occurs after the rejection mutation with the correct event name.
195-196: LGTM! Event assertion for approval revocation is properly placed.The event verification occurs after the cancel approve mutation with the correct event name.
257-258: LGTM! Event assertion for rejection revocation is properly placed.The event verification occurs after the cancel reject mutation with the correct event name.
backend/infrahub/events/proposed_change_action.py (3)
9-21: LGTM! Well-designed base event class.The ProposedChangeEvent base class properly extends InfrahubEvent with common fields and standardized resource dictionary format. Field descriptions enhance documentation.
23-51: LGTM! Well-structured specialized event classes.The ProposedChangeReviewEvent and ProposedChangeReviewRevokedEvent classes properly extend the base class with relevant fields and correctly override get_resource to include branch context.
53-113: LGTM! Excellent inheritance hierarchy and event class structure.The concrete event classes demonstrate good design with:
- Proper ClassVar usage for event names with consistent namespace
- Logical inheritance from appropriate parent classes
- Clear separation between review events (with decision) and revoked events (with former_decision)
- Comprehensive field coverage for all event types
gmazoyer
force-pushed
the
gma-20250722-ifc1633
branch
from
d6e0435 to
8be3bb6
Compare
gmazoyer
force-pushed
the
gma-20250722-ifc1633
branch
3 times, most recently
from
7cfedf6 to
aa75a45
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/infrahub/core/constants/__init__.py(1 hunks)backend/infrahub/events/__init__.py(2 hunks)backend/infrahub/events/proposed_change_action.py(1 hunks)backend/infrahub/graphql/mutations/proposed_change.py(5 hunks)backend/infrahub/graphql/types/event.py(2 hunks)backend/infrahub/proposed_change/tasks.py(3 hunks)backend/infrahub/task_manager/event.py(2 hunks)backend/tests/functional/proposed_change/test_proposed_change_review.py(6 hunks)backend/tests/helpers/events.py(2 hunks)changelog/+ifc1633.added.md(1 hunks)docs/docs/reference/infrahub-events.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- changelog/+ifc1633.added.md
- backend/tests/helpers/events.py
- backend/infrahub/events/init.py
- backend/infrahub/core/constants/init.py
- backend/tests/functional/proposed_change/test_proposed_change_review.py
- backend/infrahub/task_manager/event.py
- backend/infrahub/proposed_change/tasks.py
- docs/docs/reference/infrahub-events.mdx
- backend/infrahub/events/proposed_change_action.py
- backend/infrahub/graphql/types/event.py
🧰 Additional context used
📓 Path-based instructions (1)
backend/**/*
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Run backend tests with
pytestor viainvoketasks
Files:
backend/infrahub/graphql/mutations/proposed_change.py
⏰ 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). (6)
- GitHub Check: documentation
- GitHub Check: validate-generated-documentation
- GitHub Check: backend-docker-integration
- GitHub Check: backend-tests-integration
- GitHub Check: backend-tests-unit
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
backend/infrahub/graphql/mutations/proposed_change.py (4)
19-25: LGTM! Well-organized imports for event functionality.The new imports are properly organized and align with the event integration requirements. Event classes are imported from the appropriate module, and type imports are correctly placed within the TYPE_CHECKING block.
Also applies to: 30-30, 38-42
272-272: LGTM! Proper context propagation for event handling.The method signature change correctly propagates the GraphQL context to enable event metadata extraction and dispatching. The parameter naming follows existing conventions.
Also applies to: 285-285
293-294: LGTM! Proper event metadata extraction.The event variable initialization and metadata extraction from the GraphQL context is implemented correctly.
304-312: LGTM! Consistent event creation pattern across all decision types.The event creation logic follows a consistent pattern across all four decision types with appropriate event parameters. The event data includes all necessary information for tracking proposed change review actions.
Also applies to: 321-329, 338-346, 355-363
gmazoyer
force-pushed
the
gma-20250722-ifc1633
branch
from
aa75a45 to
3aab420
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
backend/infrahub/task_manager/event.py (1)
235-246: Implementation covers all required proposed change event types.The implementation correctly handles all the proposed change event types mentioned in the PR objectives:
approvedandrejectedevents include both user info and current decisionapproval_revokedandrejection_revokedevents include both user info and former decisionreview_requestedandmergedevents include only user infoThis addresses the previous review comment about required event types - all necessary event types are now handled appropriately.
🧹 Nitpick comments (1)
backend/infrahub/task_manager/event.py (1)
175-207: Consider handling multiple related resources.The method currently processes only the first matching resource with role "infrahub.related.node" due to the
continuestatement. If there are multiple related nodes, only the first one will be processed and subsequent ones will be ignored.Consider whether this is the intended behavior or if all matching resources should be processed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/infrahub/events/proposed_change_action.py(1 hunks)backend/infrahub/task_manager/event.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/infrahub/events/proposed_change_action.py
🧰 Additional context used
📓 Path-based instructions (1)
backend/**/*
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Run backend tests with
pytestor viainvoketasks
Files:
backend/infrahub/task_manager/event.py
⏰ 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: Cloudflare Pages
🔇 Additional comments (2)
backend/infrahub/task_manager/event.py (2)
209-210: LGTM!Simple and focused method that correctly extracts the reviewer decision from the main resource.
212-213: LGTM!Consistent with the previous method and correctly extracts the former reviewer decision.
gmazoyer
force-pushed
the
gma-20250722-ifc1633
branch
from
d768cce to
388fa92
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/tests/functional/proposed_change/test_review.py (1)
41-48: Consider event count flexibility for robustness.The event assertion logic is well-implemented with proper retry handling. However, the strict check for exactly 1 event (
len(events) == 1) might be fragile if duplicate events are legitimately emitted or if test isolation isn't perfect.Consider whether checking for
len(events) >= 1would be more robust, unless the exact count verification is specifically required for test correctness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/infrahub/core/constants/__init__.py(1 hunks)backend/infrahub/events/__init__.py(2 hunks)backend/infrahub/events/proposed_change_action.py(1 hunks)backend/infrahub/graphql/mutations/proposed_change.py(6 hunks)backend/infrahub/graphql/types/event.py(2 hunks)backend/infrahub/proposed_change/tasks.py(3 hunks)backend/infrahub/task_manager/event.py(2 hunks)backend/tests/functional/proposed_change/test_review.py(6 hunks)backend/tests/helpers/events.py(2 hunks)changelog/+ifc1633.added.md(1 hunks)docs/docs/reference/infrahub-events.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- changelog/+ifc1633.added.md
- backend/infrahub/core/constants/init.py
- backend/infrahub/events/init.py
- backend/infrahub/proposed_change/tasks.py
- backend/tests/helpers/events.py
- backend/infrahub/graphql/mutations/proposed_change.py
- backend/infrahub/task_manager/event.py
- docs/docs/reference/infrahub-events.mdx
- backend/infrahub/events/proposed_change_action.py
- backend/infrahub/graphql/types/event.py
🧰 Additional context used
📓 Path-based instructions (2)
backend/tests/**/*
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Place backend tests in
backend/tests/
Files:
backend/tests/functional/proposed_change/test_review.py
backend/**/*
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Run backend tests with
pytestor viainvoketasks
Files:
backend/tests/functional/proposed_change/test_review.py
⏰ 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). (9)
- GitHub Check: E2E-testing-version-upgrade / From 1.1.0
- GitHub Check: backend-benchmark
- GitHub Check: E2E-testing-playwright
- GitHub Check: backend-docker-integration
- GitHub Check: backend-tests-functional
- GitHub Check: validate-generated-documentation
- GitHub Check: backend-tests-unit
- GitHub Check: documentation
- GitHub Check: backend-tests-integration
🔇 Additional comments (3)
backend/tests/functional/proposed_change/test_review.py (3)
3-3: LGTM! Imports are appropriate for event verification functionality.The new imports properly support the Prefect event verification functionality being added to the tests.
Also applies to: 8-9, 18-18, 21-21
36-39: LGTM! Well-implemented async fixture.The fixture properly uses an async context manager for resource cleanup and has appropriate class-level scoping for reuse across test methods.
56-56: LGTM! Event assertions properly verify the new event system.The event assertions are well-integrated into the existing test flow:
- Correct event names matching the PR objectives
- Proper placement after business logic verification
- Good coverage of all review actions (approve, reject, revoke)
- Non-intrusive integration with existing test structure
Also applies to: 99-100, 122-122, 130-130, 183-184, 192-192, 245-246
gmazoyer
force-pushed
the
gma-20250722-ifc1633
branch
from
388fa92 to
1986c59
Compare
This change introduce events related to actions taken on a proposed change. For now, several new events are part of the PR:
infrahub.proposed_change.mergedinfrahub.proposed_change.approvedinfrahub.proposed_change.rejectedinfrahub.proposed_change.approval_revokedinfrahub.proposed_change.rejection_revokedSummary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores