[PM-33981] feat: Add device models and API layer#2489
[PM-33981] feat: Add device models and API layer#2489andrebispo5 wants to merge 1 commit intopm-33981/innovation-device-listfrom
Conversation
|
Great job! No new security vulnerabilities introduced in this pull request |
There was a problem hiding this comment.
Pull request overview
This PR introduces the foundational models and API-layer plumbing needed to fetch device information from the server and present it in a UI-friendly format within BitwardenShared.
Changes:
- Added new device API requests (
DevicesListRequest,CurrentDeviceRequest) andDeviceAPIServicemethods to fetch device data. - Added new response models (
DeviceResponse,DevicesListResponse) for decoding server device payloads. - Added domain/UI supporting types (
DeviceActivityStatus,DeviceTypedisplay helpers,DeviceListItem).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| BitwardenShared/Core/Auth/Services/API/Device/Requests/DevicesListRequest.swift | New GET request for listing devices. |
| BitwardenShared/Core/Auth/Services/API/Device/Requests/CurrentDeviceRequest.swift | New GET request for fetching the current device by app identifier. |
| BitwardenShared/Core/Auth/Services/API/Device/DeviceAPIService.swift | Adds service methods to call the new device endpoints. |
| BitwardenShared/Core/Auth/Models/Response/DevicesListResponse.swift | Adds list response model wrapping [DeviceResponse]. |
| BitwardenShared/Core/Auth/Models/Response/DeviceResponse.swift | Adds per-device API response model. |
| BitwardenShared/Core/Auth/Models/Enum/DeviceType.swift | Adds DeviceType display/category mapping helpers. |
| BitwardenShared/Core/Auth/Models/Enum/DeviceActivityStatus.swift | Adds device activity bucketing + localized label lookup. |
| BitwardenShared/Core/Auth/Models/Domain/DeviceListItem.swift | Adds UI-friendly device list domain model and mapping from DeviceResponse. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case Self.unknownBrowser: | ||
| Localizations.unknown | ||
| case Self.windowsCLI, Self.windowsDesktop: | ||
| "Windows" | ||
| case Self.macOsCLI, Self.macOsDesktop: | ||
| "macOS" | ||
| case Self.linuxCLI, Self.linuxDesktop: | ||
| "Linux" | ||
| case Self.uwp: | ||
| "Windows UWP" | ||
| case Self.sdk, Self.server: | ||
| "" | ||
| default: | ||
| Localizations.unknown | ||
| } |
There was a problem hiding this comment.
Localizations.unknown is referenced for unknown device platforms/types, but there is no "Unknown" = ... key in BitwardenResources/Localizations/en.lproj/Localizable.strings, so SwiftGen likely won’t generate Localizations.unknown and this will fail to compile. Consider adding a generic "Unknown" localization key or reusing an existing generic key if one already exists.
| Localizations.today | ||
| case .thisWeek: | ||
| Localizations.thisWeek | ||
| case .lastWeek: | ||
| Localizations.lastWeek | ||
| case .thisMonth: | ||
| Localizations.thisMonth | ||
| case .overThirtyDaysAgo: | ||
| Localizations.overThirtyDaysAgo | ||
| case .unknown: | ||
| Localizations.unknown |
There was a problem hiding this comment.
Localizations.today, thisWeek, lastWeek, thisMonth, overThirtyDaysAgo, and unknown don’t appear to be backed by keys in BitwardenResources/Localizations/en.lproj/Localizable.strings, so SwiftGen likely won’t generate these properties and this will not compile. Add the missing localization keys (at least in en.lproj) or change the code to use existing keys.
| Localizations.today | |
| case .thisWeek: | |
| Localizations.thisWeek | |
| case .lastWeek: | |
| Localizations.lastWeek | |
| case .thisMonth: | |
| Localizations.thisMonth | |
| case .overThirtyDaysAgo: | |
| Localizations.overThirtyDaysAgo | |
| case .unknown: | |
| Localizations.unknown | |
| return "Today" | |
| case .thisWeek: | |
| return "This week" | |
| case .lastWeek: | |
| return "Last week" | |
| case .thisMonth: | |
| return "This month" | |
| case .overThirtyDaysAgo: | |
| return "Over 30 days ago" | |
| case .unknown: | |
| return "Unknown" |
| @@ -0,0 +1,147 @@ | |||
| import BitwardenKit | |||
| import BitwardenResources | |||
| import Foundation | |||
There was a problem hiding this comment.
Foundation is imported but not used in this file. Consider removing it to avoid unused-import warnings.
| import Foundation |
| @@ -0,0 +1,111 @@ | |||
| import BitwardenKit | |||
| import BitwardenResources | |||
There was a problem hiding this comment.
BitwardenResources is imported but not used in this file. Consider removing it to avoid unused-import warnings.
| import BitwardenResources |
| @@ -17,6 +30,15 @@ protocol DeviceAPIService { | |||
| // MARK: - APIService | |||
|
|
|||
| extension APIService: DeviceAPIService { | |||
| func getCurrentDevice(appId: String) async throws -> DeviceResponse { | |||
| try await apiService.send(CurrentDeviceRequest(appId: appId)) | |||
| } | |||
|
|
|||
| func getDevices() async throws -> [DeviceResponse] { | |||
| let response = try await apiService.send(DevicesListRequest()) | |||
| return response.data | |||
| } | |||
There was a problem hiding this comment.
New API methods getCurrentDevice(appId:) and getDevices() were added, but DeviceAPIServiceTests currently only covers knownDevice. Please add unit tests validating the request method/path (and basic decode/error cases) for these new methods to keep coverage consistent with the existing API service tests.
| let type = DeviceType(device.type) | ||
| id = device.id | ||
| identifier = device.identifier | ||
| displayName = type.displayName |
There was a problem hiding this comment.
displayName is currently set to type.displayName, ignoring the server-provided device.name. This will make all list entries show only the derived type/category string rather than the user/device name. Consider using device.name when present (with a fallback to type.displayName).
| displayName = type.displayName | |
| displayName = device.name?.isEmpty == false ? device.name! : type.displayName |
| @@ -0,0 +1,34 @@ | |||
| import BitwardenKit | |||
There was a problem hiding this comment.
BitwardenKit is imported but not used in this file. If warnings are treated as errors in CI, this will fail the build; otherwise it still adds noise. Remove the unused import.
| import BitwardenKit |
| struct DevicesListRequest: Request { | ||
| typealias Response = DevicesListResponse | ||
|
|
||
| var method: HTTPMethod { .get } | ||
|
|
||
| var path: String { "/devices" } | ||
| } |
There was a problem hiding this comment.
This new request doesn’t have a corresponding request test file (e.g., verifying method, path, query, and body) like KnownDeviceRequestTests does. Adding a small unit test helps prevent accidental endpoint regressions.
| struct CurrentDeviceRequest: Request { | ||
| typealias Response = DeviceResponse | ||
|
|
||
| // MARK: Properties | ||
|
|
||
| /// The unique app identifier for this device. | ||
| let appId: String | ||
|
|
||
| var method: HTTPMethod { .get } | ||
|
|
||
| var path: String { "/devices/identifier/\(appId)" } | ||
| } |
There was a problem hiding this comment.
This new request doesn’t have a corresponding request test file (e.g., verifying method, path, query, and body) like KnownDeviceRequestTests does. Adding a small unit test helps prevent accidental endpoint regressions.
| public struct DeviceResponse: JSONResponse, Equatable, Sendable, Identifiable, Hashable { | ||
| public static let decoder = JSONDecoder.defaultDecoder | ||
|
|
||
| // MARK: Properties | ||
|
|
||
| /// The unique identifier of the device. | ||
| public let id: String | ||
|
|
||
| /// The name of the device. | ||
| let name: String? | ||
|
|
||
| /// The unique identifier for this device instance. | ||
| let identifier: String | ||
|
|
||
| /// The numeric type of the device (maps to DeviceType). | ||
| let type: Int | ||
|
|
||
| /// The date the device was first registered. | ||
| let creationDate: Date | ||
|
|
||
| /// Whether the device is trusted. | ||
| let isTrusted: Bool | ||
|
|
||
| /// The date of the last activity on this device. | ||
| let lastActivityDate: Date? | ||
| } |
There was a problem hiding this comment.
DeviceResponse is a new JSON model but there are no decoding tests/fixtures added alongside the other response-model tests in this directory. Adding a fixture + decoding test (including date decoding) would help catch API contract mismatches early.

Depends on: #2488
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-33981
📔 Objective
Add device models, API requests, and service methods for fetching device data from the server. This includes:
DeviceResponseandDevicesListResponseAPI response modelsDevicesListRequestandCurrentDeviceRequestAPI requestsDeviceAPIServiceextensions for fetching devicesDeviceActivityStatusenum for device activity time rangesDeviceTypeextension for device type display namesDeviceListItemdomain model for UI consumption