[PM-33981] feat: Add device management models and API layer#2870
[PM-33981] feat: Add device management models and API layer#2870andrebispo5 wants to merge 5 commits into
Conversation
- Promote DeviceType from Int typealias to a proper enum in BitwardenKit with a safe Decodable fallback to .unknownBrowser for unknown values; update DefaultHeadersRequestHandler and IdentityTokenRequestModel to use .rawValue where an Int is required - Add DeviceTypeCategory and DeviceType display extensions (category, platform, displayName, pendingRequestMatchKey) in BitwardenShared Core - Add DeviceActivityStatus enum mapping server dates to activity buckets using calendar-day comparison - Add DeviceListItem domain model, DeviceResponse/DevicesListResponse API models, CurrentDeviceRequest/DevicesListRequest, and DeviceAPIService with getCurrentDevice(appId:) and getDevices() methods - Add deviceAPIService as a proper init parameter to ServiceContainer (consistent with all other services); update withMocks() default to MockDeviceAPIService() - Add tests for all new types and request/response models; add JSON fixtures CurrentDevice.json and DevicesList.json
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the device management data and API layer: the Code Review DetailsNo blocking findings. The two remaining open threads appear handled: the |
There was a problem hiding this comment.
Pull request overview
Adds the device-management data/API layer (no UI), including new device request/response models, domain mapping helpers, and a promoted DeviceAPIService dependency in ServiceContainer.
Changes:
- Introduces device API models/requests (
DeviceResponse,DevicesListResponse,CurrentDeviceRequest,DevicesListRequest) and extendsDeviceAPIServicewithgetCurrentDevice(appId:)andgetDevices(). - Promotes
DeviceTypeinBitwardenKitfrom anInttypealias to a realenumwith safe decoding fallback; updates dependent request/header building to userawValue. - Adds device display/mapping utilities (
DeviceTypeCategory,DeviceActivityStatus,DeviceListItem) plus fixtures and unit tests.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift | Adds injectable deviceAPIService to mocks container defaults. |
| BitwardenShared/Core/Platform/Services/ServiceContainer.swift | Promotes deviceAPIService to an initializer dependency and stores it instead of computing from apiService. |
| BitwardenShared/Core/Auth/Services/API/Device/Requests/DevicesListRequestTests.swift | Adds Swift Testing coverage for DevicesListRequest defaults. |
| BitwardenShared/Core/Auth/Services/API/Device/Requests/DevicesListRequest.swift | Adds GET /devices request model. |
| BitwardenShared/Core/Auth/Services/API/Device/Requests/CurrentDeviceRequestTests.swift | Adds Swift Testing coverage for CurrentDeviceRequest defaults/path interpolation. |
| BitwardenShared/Core/Auth/Services/API/Device/Requests/CurrentDeviceRequest.swift | Adds GET /devices/identifier/{appId} request model. |
| BitwardenShared/Core/Auth/Services/API/Device/Fixtures/DevicesList.json | Adds JSON fixture for devices list response decoding/tests. |
| BitwardenShared/Core/Auth/Services/API/Device/Fixtures/CurrentDevice.json | Adds JSON fixture for current device response decoding/tests. |
| BitwardenShared/Core/Auth/Services/API/Device/Fixtures/APITestData+Device.swift | Registers device fixtures for API tests. |
| BitwardenShared/Core/Auth/Services/API/Device/DeviceAPIServiceTests.swift | Adds unit tests for new DeviceAPIService endpoints. |
| BitwardenShared/Core/Auth/Services/API/Device/DeviceAPIService.swift | Extends protocol + APIService conformance with getCurrentDevice/getDevices. |
| BitwardenShared/Core/Auth/Models/Response/Fixtures/DeviceResponse+Fixtures.swift | Adds test fixture builder for DeviceResponse. |
| BitwardenShared/Core/Auth/Models/Response/DevicesListResponseTests.swift | Adds decoding test for DevicesListResponse. |
| BitwardenShared/Core/Auth/Models/Response/DevicesListResponse.swift | Adds response wrapper model for list endpoint. |
| BitwardenShared/Core/Auth/Models/Response/DeviceResponseTests.swift | Adds decoding test for DeviceResponse. |
| BitwardenShared/Core/Auth/Models/Response/DeviceResponse.swift | Adds device API response model. |
| BitwardenShared/Core/Auth/Models/Request/IdentityTokenRequestModel.swift | Updates device type form encoding to use DeviceType.rawValue. |
| BitwardenShared/Core/Auth/Models/Enum/DeviceType.swift | Adds device type categorization + display utilities on BitwardenKit.DeviceType. |
| BitwardenShared/Core/Auth/Models/Enum/DeviceActivityStatusTests.swift | Adds tests for activity bucketing and localization strings. |
| BitwardenShared/Core/Auth/Models/Enum/DeviceActivityStatus.swift | Adds calendar-day-based bucketing for “last active” timestamps. |
| BitwardenShared/Core/Auth/Models/Domain/Fixtures/DeviceListItem+Fixtures.swift | Adds test fixture builder for DeviceListItem. |
| BitwardenShared/Core/Auth/Models/Domain/DeviceListItem.swift | Adds UI-friendly domain model + mapping init from DeviceResponse. |
| BitwardenResources/Localizations/en.lproj/Localizable.strings | Adds localization keys for device category/status/display formatting. |
| BitwardenKit/Core/Platform/Utilities/Constants.swift | Removes DeviceType typealias and updates constants to use new enum. |
| BitwardenKit/Core/Platform/Services/API/Handlers/DefaultHeadersRequestHandler.swift | Updates device-type header value to use rawValue. |
| BitwardenKit/Core/Platform/Models/Enum/DeviceType.swift | Introduces DeviceType: Int enum with safe decoding fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2870 +/- ##
==========================================
- Coverage 81.27% 79.01% -2.27%
==========================================
Files 1027 1157 +130
Lines 66109 73642 +7533
==========================================
+ Hits 53733 58189 +4456
- Misses 12376 15453 +3077 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…or backward compat Tests that configure device API responses via MockHTTPClient (e.g. LoginProcessorTests) continue to work unchanged. Callers that need precise mock control pass MockDeviceAPIService explicitly.
| case 15 ... 30: | ||
| self = .thisMonth |
There was a problem hiding this comment.
🤔 Shouldn't "this month" only be the case if the month components are the same? If it's July 15, and the device was last active June 20 (25 days ago), this would return .thisMonth, but that's actually last month.
- Use the DefaultValue property wrapper for DeviceResponse.type instead of a hand-rolled Decodable init, matching the CollectionType/ CipherRepromptType pattern; add Sendable conformance to DefaultValue so it composes with Sendable response models - Fix DeviceActivityStatus to classify "this month" by actual calendar month instead of a fixed 15-30 day range, which mislabeled dates from a previous calendar month (e.g. June 20 relative to a July 15 "now") - Move DeviceListItem's DeviceResponse-based init into an extension so the compiler synthesizes the memberwise init instead of hand-writing it - Rename BitwardenShared DeviceType.swift to DeviceTypeCategory.swift to match the DeviceTypeCategory enum it defines - Add missing coverage: DeviceType/DeviceResponse unknown-value decode fallback, and DeviceTypeCategory/DeviceType category-platform- displayName-pendingRequestMatchKey mappings
| // Beyond the last two weeks, only classify as `.thisMonth` if the date actually | ||
| // falls within the current calendar month; a day count alone can't tell June 20 | ||
| // apart from July 20 relative to a July 15 "now". | ||
| self = calendar.isDate(date, equalTo: now, toGranularity: .month) ? .thisMonth : .overThirtyDaysAgo |
There was a problem hiding this comment.
🤔 I still feel like this isn't quite right. Now if it's July 15, and the last activity was June 20, it would return .overThirtyDaysAgo even though it's less than 30 days. Is it worth adding an "X days ago" label to handle the case where it isn't this month but it also isn't over 30 days ago?
There was a problem hiding this comment.
I just noticed that this changed to match other clients, fixing it now.
Replace the calendar-month check for the .thisMonth bucket with pure day-count thresholds (14-29 days), mirroring the Android app's toLastActivityLabel logic so both platforms show identical "last active" labels for the same date.
…ding Rename ThisWeek/LastWeek/ThisMonth to PastSevenDays/PastFourteenDays/ PastThirtyDays and update their English values to "Past 7/14/30 days", mirroring the Android app's past_seven_days/past_fourteen_days/ past_thirty_days string resources.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-33981
📔 Objective
Adds the complete data and API layer for the device management feature — no UI changes in this PR.
DeviceTypefrom anInttypealias to a properenum DeviceType: Intin BitwardenKit, with a safeDecodablefallback to.unknownBrowserfor unknown server valuesDeviceTypeCategoryandDeviceTypedisplay extensions (category,platform,displayName,pendingRequestMatchKey) in BitwardenShared CoreDeviceActivityStatusenum mapping server timestamps to activity buckets using calendar-day comparisonDeviceListItemdomain model,DeviceResponse/DevicesListResponseAPI models,CurrentDeviceRequest/DevicesListRequest, andDeviceAPIServicewithgetCurrentDevice(appId:)andgetDevices()endpointsdeviceAPIServiceto a properServiceContainerinit parameter (consistent with all other services); updateswithMocks()default toMockDeviceAPIService()📸 Screenshots
No UI changes in this PR.