Exclude COMMITTED events from adapted timeline#679
Conversation
WalkthroughThe update modifies the logic in the GitHub timeline event adaptation method by introducing a new filter: events of type Changes
Sequence Diagram(s)sequenceDiagram
participant GitHubAdapter as GitHubAdapter
participant Event as TimelineEvent
participant AdaptedList as AdaptedEvents
loop For each event in timeline
GitHubAdapter->>Event: Check required fields (timestamp, type, id, user)
alt Event.type != COMMITTED
GitHubAdapter->>AdaptedList: Append event
else Event.type == COMMITTED
Note right of GitHubAdapter: Skip event (no warning logged)
end
end
Poem
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
backend/analytics_server/mhq/exapi/github.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/analytics_server/mhq/exapi/github.py (2)
backend/analytics_server/mhq/store/models/code/enums.py (1)
PullRequestEventType(30-51)backend/analytics_server/mhq/exapi/models/github_timeline.py (3)
timestamp(113-121)id(128-132)user(89-110)
🔇 Additional comments (2)
backend/analytics_server/mhq/exapi/github.py (2)
21-21: Import addition looks correct.The import for
PullRequestEventTypeis properly added to support the enum comparison in the filtering logic.
377-380: Confirmed use ofevent.typeis correct.The
GithubPullRequestTimelineEventsmodel defines a@property def type(self) -> Optional[PullRequestEventType](inbackend/analytics_server/mhq/exapi/models/github_timeline.py), which internally maps the rawevent_typefield to the proper enum. There is no standaloneevent_typeproperty on the instance, so accessingevent.typehere correctly returns thePullRequestEventType. No changes required.
| if ( | ||
| all([event.timestamp, event.type, event.id, event.user]) | ||
| and event.type != PullRequestEventType.COMMITTED | ||
| ): |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding test coverage for the COMMITTED event exclusion.
The filtering logic successfully excludes COMMITTED events from the adapted timeline, which aligns with the PR objectives. However, consider adding unit tests to verify this behavior works correctly and prevents potential regressions.
Would you like me to generate unit tests for this filtering logic to ensure COMMITTED events are properly excluded?
🤖 Prompt for AI Agents
In backend/analytics_server/mhq/exapi/github.py around lines 377 to 380, the
code filters out events of type COMMITTED from the adapted timeline. To ensure
this exclusion works correctly and to prevent regressions, add unit tests that
specifically create events with type COMMITTED and verify they are excluded by
the filtering logic. Also, include tests for other event types to confirm they
are included as expected.
2 participants
Linked Issue(s)
Closes #678
Further comments
Hash can be same for commits in different PRs.
Later we would also need to handle this case for
PullRequestCommittable, as we use currently use hash as Primary Key.Summary by CodeRabbit