chore: remove InventoryID from AdvisoryUpdateEvent#2210
Conversation
This field is no longer required as part of the AdvisoryUpdateEvent structure. Updated tests accordingly. Added a TODO for future system context changes in notifications.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRemoves the now-unnecessary InventoryID field from AdvisoryUpdateEvent and its tests, while documenting a future plan to remove system-specific notification Context once aggregation is handled elsewhere. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="base/mqueue/advisory_update_event_test.go" line_range="23" />
<code_context>
event := AdvisoryUpdateEvent{
RhAccountID: 1,
WorkspaceID: testWorkspaceID,
- InventoryID: uuid.New(),
AdvisoryIDs: []int64{101, 202, 303},
ProducedAt: testNow,
</code_context>
<issue_to_address>
**issue (testing):** Add a test that explicitly verifies `inventory_id` is no longer present in the marshaled JSON
The current tests only stop asserting on `InventoryID`; they don’t confirm the field is removed from the payload. Please add or extend a test (e.g., `TestAdvisoryUpdateEventMarshal`) that marshals the event, unmarshals into a `map[string]any` (or `map[string]json.RawMessage`), and asserts that the `"inventory_id"` key is absent to clearly validate this behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| event := AdvisoryUpdateEvent{ | ||
| RhAccountID: 1, | ||
| WorkspaceID: testWorkspaceID, | ||
| InventoryID: uuid.New(), |
There was a problem hiding this comment.
issue (testing): Add a test that explicitly verifies inventory_id is no longer present in the marshaled JSON
The current tests only stop asserting on InventoryID; they don’t confirm the field is removed from the payload. Please add or extend a test (e.g., TestAdvisoryUpdateEventMarshal) that marshals the event, unmarshals into a map[string]any (or map[string]json.RawMessage), and asserts that the "inventory_id" key is absent to clearly validate this behavior.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2210 +/- ##
=======================================
Coverage 59.23% 59.23%
=======================================
Files 137 137
Lines 8795 8795
=======================================
Hits 5210 5210
Misses 3037 3037
Partials 548 548
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
AdvisoryUpdateEventstructureSecure Coding Practices Checklist GitHub Link
Secure Coding Checklist
Summary by Sourcery
Remove inventory identifier from advisory update events and update related tests and notification context documentation.
Enhancements:
Documentation: