-
Notifications
You must be signed in to change notification settings - Fork 0
Add Issue Events and Timeline functionality #12
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
Implements comprehensive support for GitHub Issue Events and Timeline tracking:
- Event data object with support for labeled, unlabeled, assigned, unassigned, milestoned, demilestoned, closed, and reopened events
- TimelineEvent data object with extended properties for comments, cross-references, and state changes
- ManagesIssueEvents trait with methods to list issue events, get specific events, and list timeline events
- Full test coverage for both data objects with validation of all event types
- Integration with IssuesService via ManagesIssueEventsInterface
API endpoints covered:
- GET /repos/{owner}/{repo}/issues/{issue_number}/events
- GET /repos/{owner}/{repo}/issues/{issue_number}/timeline
- GET /repos/{owner}/{repo}/issues/events/{event_id}
Closes #10
WalkthroughIntroduces GitHub Issue Events and Timeline support by adding a new event management interface, corresponding data models for Events and TimelineEvents, a trait for API integration, and comprehensive unit tests. Updates the IssuesServiceInterface to extend the new ManagesIssueEventsInterface. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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 (3)
src/Contracts/ManagesIssueEventsInterface.php (1)
7-8: Add missing TimelineEvent import for better IDE support.The interface references
TimelineEventin the PHPDoc at line 20, but the class isn't imported. While this doesn't affect runtime behavior, adding the import improves IDE autocomplete and static analysis.Apply this diff to add the missing import:
use ConduitUI\Issue\Data\Event; +use ConduitUI\Issue\Data\TimelineEvent; use Illuminate\Support\Collection;src/Data/Event.php (1)
56-74: Consider using strict comparison in predicate helpers.The predicate methods correctly identify event types. For consistency with PHP best practices, consider adding the
strict: trueparameter toin_array()calls.Example for isLabelEvent (apply similar pattern to other predicates):
public function isLabelEvent(): bool { - return in_array($this->event, ['labeled', 'unlabeled']); + return in_array($this->event, ['labeled', 'unlabeled'], strict: true); }src/Data/TimelineEvent.php (1)
77-105: Consider using strict comparison in predicate helpers.The predicate methods correctly identify event types, including the timeline-specific
isCrossReferenced()check. For consistency with PHP best practices, consider adding thestrict: trueparameter toin_array()calls and using strict equality (===) for the string comparisons.Example refactor:
public function isComment(): bool { - return $this->event === 'commented'; + return $this->event === 'commented'; // Already strict - good! } public function isLabelEvent(): bool { - return in_array($this->event, ['labeled', 'unlabeled']); + return in_array($this->event, ['labeled', 'unlabeled'], strict: true); } public function isCrossReferenced(): bool { - return $this->event === 'cross-referenced'; + return $this->event === 'cross-referenced'; // Already strict - good! }Note:
isComment()andisCrossReferenced()already use strict equality (===), which is excellent. Only thein_array()calls in the other methods need the strict parameter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Contracts/IssuesServiceInterface.php(1 hunks)src/Contracts/ManagesIssueEventsInterface.php(1 hunks)src/Data/Event.php(1 hunks)src/Data/TimelineEvent.php(1 hunks)src/Services/IssuesService.php(1 hunks)src/Traits/ManagesIssueEvents.php(1 hunks)tests/Unit/Data/EventTest.php(1 hunks)tests/Unit/Data/TimelineEventTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/Contracts/ManagesIssueEventsInterface.php (3)
src/Data/Issue.php (1)
Issue(9-86)src/Data/Event.php (1)
Event(9-75)src/Traits/ManagesIssueEvents.php (3)
listIssueEvents(16-24)getIssueEvent(26-33)listIssueTimeline(38-46)
tests/Unit/Data/EventTest.php (3)
src/Data/Event.php (7)
Event(9-75)fromArray(24-38)toArray(40-54)isLabelEvent(56-59)isAssigneeEvent(61-64)isMilestoneEvent(66-69)isStateEvent(71-74)src/Data/Label.php (1)
Label(7-35)src/Data/User.php (1)
User(7-38)
src/Traits/ManagesIssueEvents.php (4)
src/Data/Issue.php (1)
Issue(9-86)src/Data/Event.php (2)
Event(9-75)fromArray(24-38)src/Data/TimelineEvent.php (2)
TimelineEvent(9-106)fromArray(31-52)src/Contracts/ManagesIssueEventsInterface.php (3)
listIssueEvents(15-15)getIssueEvent(17-17)listIssueTimeline(22-22)
tests/Unit/Data/TimelineEventTest.php (1)
src/Data/TimelineEvent.php (9)
TimelineEvent(9-106)fromArray(31-52)toArray(54-75)isComment(77-80)isLabelEvent(82-85)isAssigneeEvent(87-90)isMilestoneEvent(92-95)isStateEvent(97-100)isCrossReferenced(102-105)
src/Data/Event.php (2)
src/Data/User.php (1)
User(7-38)src/Data/Label.php (1)
Label(7-35)
src/Data/TimelineEvent.php (3)
src/Data/Event.php (3)
__construct(11-22)fromArray(24-38)toArray(40-54)src/Data/User.php (1)
User(7-38)src/Data/Label.php (1)
Label(7-35)
🔇 Additional comments (12)
src/Services/IssuesService.php (1)
10-10: LGTM! Clean trait integration.The ManagesIssueEvents trait is correctly imported and included, following the existing architectural pattern used for other traits in the service.
Also applies to: 17-17
src/Contracts/IssuesServiceInterface.php (1)
7-7: LGTM! Interface extended correctly.The ManagesIssueEventsInterface is properly added to the interface extension list, expanding the service contract to include event management capabilities.
tests/Unit/Data/TimelineEventTest.php (1)
9-196: Excellent test coverage for TimelineEvent.The test suite comprehensively validates:
- Object creation from arrays with nested structures (User, Label)
- Array serialization with proper snake_case conversion
- All predicate helpers (isComment, isLabelEvent, isAssigneeEvent, isMilestoneEvent, isStateEvent, isCrossReferenced)
- Optional fields (body, user, updatedAt, authorAssociation, state, stateReason, source)
This provides strong confidence in the TimelineEvent data model behavior.
tests/Unit/Data/EventTest.php (1)
9-183: Excellent test coverage for Event.The test suite comprehensively validates:
- Object creation from arrays with nested structures (User, Label)
- Array serialization with proper snake_case conversion
- All predicate helpers (isLabelEvent, isAssigneeEvent, isMilestoneEvent, isStateEvent)
- Null handling for optional fields (actor, commitId, commitUrl, label)
- Optional nested structures (assignee, milestone)
This provides strong confidence in the Event data model behavior.
src/Contracts/ManagesIssueEventsInterface.php (1)
15-15: LGTM! Well-defined interface methods.The interface clearly defines the event management contract with proper type hints and PHPDoc annotations for collection types.
Also applies to: 22-22
src/Traits/ManagesIssueEvents.php (3)
16-24: LGTM! Clean implementation of listIssueEvents.The method correctly:
- Constructs the GitHub API endpoint with owner, repo, and issue number
- Passes filters as query parameters
- Maps the JSON response array to Event objects using fromArray
26-33: LGTM! Clean implementation of getIssueEvent.The method correctly fetches a single event by ID and converts it to an Event object.
38-46: LGTM! Clean implementation of listIssueTimeline.The method correctly fetches the timeline and maps each item to a TimelineEvent object using fromArray.
src/Data/Event.php (2)
24-38: LGTM! Well-structured fromArray factory method.The method correctly:
- Handles required fields (id, event, createdAt)
- Uses isset() for conditional nested object parsing (actor, label, assignee)
- Uses null coalescing for optional scalar fields
- Properly delegates to User::fromArray and Label::fromArray
40-54: LGTM! Clean toArray serialization.The method correctly serializes all fields, including nested objects via their toArray() methods, and filters out null values to produce a clean output matching the GitHub API format.
src/Data/TimelineEvent.php (2)
31-52: LGTM! Comprehensive fromArray factory method.The method correctly handles:
- Required fields (id, event, createdAt)
- Optional nested objects (actor, label, assignee, user) with isset() checks
- Optional DateTime parsing for updatedAt
- Multiple optional fields for comments, state changes, and cross-references
This provides excellent flexibility for the diverse timeline event types.
54-75: LGTM! Clean toArray serialization.The method correctly serializes all fields, including multiple optional nested objects and timestamps, filtering out null values to produce clean output.
|
Features already shipped in #14 (bundled with gate workflow) |
Summary
Implements comprehensive support for GitHub Issue Events and Timeline tracking to provide complete visibility into issue history.
Eventdata object supporting all major event types (labeled, unlabeled, assigned, unassigned, milestoned, demilestoned, closed, reopened)TimelineEventdata object with extended properties for comments, cross-references, and state changesManagesIssueEventstrait with three API methods:listIssueEvents()- List all events for an issuegetIssueEvent()- Get specific event details by IDlistIssueTimeline()- Get full timeline with comments and eventsIssuesServiceviaManagesIssueEventsInterfaceAPI Coverage
GET /repos/{owner}/{repo}/issues/{issue_number}/eventsGET /repos/{owner}/{repo}/issues/{issue_number}/timelineGET /repos/{owner}/{repo}/issues/events/{event_id}Test Plan
Closes #10
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.