Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds interval endpoint integration to support the Leaderboard feature. It follows the existing architecture pattern by introducing domain and model layers with appropriate mapping functionality.
- Introduces new
IntervalModelandIntervalDomaintypes to represent interval data for drivers - Adds
getAllIntervalsmethod toRequestsClientfor fetching interval updates by session - Implements the necessary endpoint, mapper, and data structures following existing codebase patterns
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| RequestsClient.swift | Adds getAllIntervals method to fetch interval data for a session with optional chunk filtering |
| IntervalMapper.swift | Provides mapping logic from IntervalDomain to IntervalModel |
| IntervalModel.swift | Defines the public model struct representing interval data with properties for date, driver, interval, and leader |
| IntervalDomain.swift | Defines the internal domain struct for decoding interval data from the API |
| Endpoints+Interval.swift | Adds the Interval endpoint enum with getAll case for fetching all intervals by session key |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public func getAllIntervals( | ||
| sessionKey: Int, | ||
| chunk: Chunk? = nil | ||
| ) async throws -> [IntervalModel] { |
There was a problem hiding this comment.
The getAllIntervals method is missing documentation. Other similar methods in the RequestsClient (such as getAllWeatherUpdates) include comprehensive documentation describing the parameters, return value, and what exceptions may be thrown. Add a documentation comment following the same pattern used for other methods in this class.
| public func getAllIntervals( | ||
| sessionKey: Int, | ||
| chunk: Chunk? = nil | ||
| ) async throws -> [IntervalModel] { | ||
| let domain = try await request( | ||
| endpoint: Endpoints.Interval.getAll(sessionKey: sessionKey), | ||
| type: [IntervalDomain].self, | ||
| chunk: chunk | ||
| ) | ||
|
|
||
| return domain.map { IntervalMapper.map(from: $0) } | ||
| } |
There was a problem hiding this comment.
Test coverage is missing for the new getAllIntervals functionality. Other endpoint methods in the codebase have corresponding test files (e.g., RequestsClientWeatherTests.swift, RequestsClientDriversTests.swift). Consider adding a test file like RequestsClientIntervalTests.swift to verify the interval endpoint behavior, following the same patterns used in existing test files.
| @@ -0,0 +1,26 @@ | |||
| // | |||
| // Endpoints+Intervale.swift | |||
There was a problem hiding this comment.
The filename in the header comment contains a typo. It should be "Endpoints+Interval.swift" instead of "Endpoints+Intervale.swift".
| // Endpoints+Intervale.swift | |
| // Endpoints+Interval.swift |
| let date: Date | ||
| let driver: Int | ||
| let interval: Float | ||
| let leader: Float |
There was a problem hiding this comment.
The properties of IntervalModel should be public to be accessible from outside the module. Other public model structs in the codebase (like WeatherModel and CarModel) have public properties. Update all properties to use the public access modifier to maintain consistency and ensure the model is fully usable by consumers of this module.
| let date: Date | |
| let driver: Int | |
| let interval: Float | |
| let leader: Float | |
| public let date: Date | |
| public let driver: Int | |
| public let interval: Float | |
| public let leader: Float |
Integration of the intervals for Leaderboard