Helium migration #218
Helium migration #218
Conversation
WalkthroughThis pull request introduces updates to the device frequency management system across multiple components of the application. Key changes include the addition of a factory method for creating Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 8
🧹 Nitpick comments (3)
wxm-ios/DataLayer/DataLayer/Networking/ApiRequestBuilders/MeApiRequestBuilder.swift (2)
147-149: Consider using the serial number in the path instead of as a parameter.Following RESTful conventions, consider using the serial number in the path (e.g.,
me/devices/{serialNumber}/frequency) instead of as a query parameter. This would be more consistent with resource-oriented URL design.-case setDeviceFrequency(serialNumber: String, frequency: String) +case setDeviceFrequency(frequency: String) private var path: String { switch self { - case .setDeviceFrequency: - return "me/devices/frequency" + case let .setDeviceFrequency(serialNumber, _): + return "me/devices/\(serialNumber)/frequency" private var parameters: Parameters? { switch self { - case let .setDeviceFrequency(serialNumber, frequency): - return [ParameterConstants.Me.serialNumber: serialNumber, - ParameterConstants.Me.freq: frequency] + case let .setDeviceFrequency(frequency): + return [ParameterConstants.Me.freq: frequency]
147-149: Improve code formatting for consistency.The parameter dictionary formatting is inconsistent with other similar blocks. Consider aligning the parameters for better readability.
- return [ParameterConstants.Me.serialNumber: serialNumber, - ParameterConstants.Me.freq: frequency] + return [ + ParameterConstants.Me.serialNumber: serialNumber, + ParameterConstants.Me.freq: frequency + ]PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift (1)
29-35: Fix typo in parameter name.The default value for
frequencyparameter has a typo.- frequency: Frequency? = Frequency.allCases.first) { + frequency: Frequency? = Frequency.allCases.first) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyView.swift(4 hunks)PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift(2 hunks)PresentationLayer/UIComponents/Screens/ClaimDevice/ClaimDeviceContainer/ClaimHeliumContainerViewModel.swift(4 hunks)PresentationLayer/UIComponents/Screens/DeviceInfo/DeviceInfoViewModel.swift(1 hunks)PresentationLayer/UIComponents/ViewModelsFactory.swift(1 hunks)wxm-ios/DataLayer/DataLayer/Networking/ApiRequestBuilders/MeApiRequestBuilder.swift(4 hunks)wxm-ios/DataLayer/DataLayer/Networking/Constants/ParameterConstants.swift(1 hunks)wxm-ios/DataLayer/DataLayer/RepositoryImplementations/DeviceInfoRepositoryImpl.swift(2 hunks)wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift(1 hunks)wxm-ios/DomainLayer/DomainLayer/DomainRepositoryInterfaces/MeRepository.swift(1 hunks)wxm-ios/DomainLayer/DomainLayer/Entities/DomainModels/Frequency.swift(1 hunks)wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift(1 hunks)
👮 Files not reviewed due to content moderation or server errors (4)
- wxm-ios/DataLayer/DataLayer/Networking/ApiRequestBuilders/MeApiRequestBuilder.swift
- PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift
- wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift
- PresentationLayer/UIComponents/ViewModelsFactory.swift
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WeatherXM | Unit tests | Test - iOS
🔇 Additional comments (30)
PresentationLayer/UIComponents/Screens/DeviceInfo/DeviceInfoViewModel.swift (1)
92-92: LGTM! Good use of factory pattern.The change to use
ViewModelsFactoryfor instantiatingChangeFrequencyViewModelimproves dependency management and maintains consistency with other view model instantiations.Let's verify the factory implementation:
✅ Verification successful
Factory pattern implementation verified and consistent
The change is correctly implemented:
- All ChangeFrequencyViewModel instantiations use the factory method consistently
- The factory method signature matches the ViewModel's requirements
- No direct instantiations found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the ViewModelsFactory implementation and its usage # Test 1: Check if ViewModelsFactory exists and has the required method rg -p "class ViewModelsFactory" -A 10 # Test 2: Check for other usages of getChangeFrequencyViewModel to ensure consistent usage rg "getChangeFrequencyViewModel" # Test 3: Check if there are any remaining direct instantiations of ChangeFrequencyViewModel that should be using the factory rg "ChangeFrequencyViewModel\(.*device.*\)"Length of output: 2110
Script:
#!/bin/bash # Get the complete implementation of ViewModelsFactory and ChangeFrequencyViewModel ast-grep --pattern 'class ViewModelsFactory { $$$ }' ast-grep --pattern 'class ChangeFrequencyViewModel { $$$ }'Length of output: 120
Script:
#!/bin/bash # Get the complete implementation with context rg -p "class ViewModelsFactory" -B 2 -A 15 "PresentationLayer/UIComponents/ViewModelsFactory.swift" # Get the ChangeFrequencyViewModel implementation rg -p "class ChangeFrequencyViewModel" -B 2 -A 15Length of output: 997
PresentationLayer/UIComponents/Screens/ClaimDevice/ClaimDeviceContainer/ClaimHeliumContainerViewModel.swift (4)
84-98: Validate non-nildevEUIandclaimingKeybefore proceeding.After retrieving
devEUIandclaimingKey(line 90–91), consider validating that these values are notnil. If they arenil, subsequent flow might fail silently or lead to an unexpected state.
Line range hint
99-109: Good job on delegating frequency setting to a dedicated function.The error handling flow here is straightforward: if there's an error from
setHeliumFrequency(), you immediately handle it and return. This improves clarity and maintainability.
110-117: API frequency handling appears consistent with the Bluetooth flow.You reuse a similar error-handling pattern to synchronize the device frequency with the backend. This consistent approach helps keep the logic predictable and maintainable.
147-147: Performing claim unconditionally after setting frequency.The direct call to
performClaim(retries: 0)ensures the next step proceeds immediately. Double-check whether you need additional retry logic if device info or frequency is not yet fully confirmed.wxm-ios/DomainLayer/DomainLayer/Entities/DomainModels/Frequency.swift (1)
8-8: Good addition of Sendable conformance for concurrency.Marking
FrequencyasSendableis beneficial for concurrent code. Confirm that there are no underlying reference types or non-thread-safe logic that might violateSendablesemantics.wxm-ios/DataLayer/DataLayer/Networking/Constants/ParameterConstants.swift (1)
25-25: Verify alignment with backend parameter naming.Check that the backend expects
"freq"as the frequency parameter key. Using mismatched keys can lead to silent failures or inconsistent data updates.wxm-ios/DomainLayer/DomainLayer/DomainRepositoryInterfaces/MeRepository.swift (1)
47-47: LGTM! Clean protocol extension.The new
setFrequencymethod follows the established pattern and includes appropriate error handling.wxm-ios/DataLayer/DataLayer/RepositoryImplementations/DeviceInfoRepositoryImpl.swift (2)
9-9: LGTM! Simplified import statement.Removing @preconcurrency is appropriate as the module is now fully concurrency-safe.
65-74: Good concurrency improvements!The changes improve thread safety and memory management:
- @mainactor ensures UI updates occur on the main thread
- Weak self prevents potential retain cycles
- Proper error handling with early returns
PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyView.swift (1)
76-76: Good use of factory pattern!Consistent use of ViewModelsFactory across all preview providers improves dependency management and testability.
Also applies to: 85-85, 97-97, 109-109
wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift (1)
58-61: Good async/await implementation.The implementation correctly:
- Uses async/await for better concurrency
- Properly handles errors
- Returns only the error response when needed
wxm-ios/DataLayer/DataLayer/Networking/ApiRequestBuilders/MeApiRequestBuilder.swift (10)
44-44: LGTM!The new case is well-defined with appropriate parameters.
70-70: LGTM!The
setDeviceFrequencycase is correctly added to the POST methods.
103-104: LGTM!The endpoint path follows RESTful conventions and maintains consistency with other device-related endpoints.
44-44: LGTM!The new case is well-defined with appropriate parameters.
70-70: LGTM!Correctly added setDeviceFrequency to POST methods.
103-104: LGTM!The endpoint path follows the API convention.
147-149: Verify parameter names match API contract.The parameter names look correct, but let's verify they match the API contract.
Run the following script to check the API documentation or related tests:
✅ Verification successful
Parameter names match API contract ✅
The implementation uses
serialNumberandfreqas parameter names, which are properly defined in the constants and consistently used throughout the codebase. The naming follows standard REST API conventions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for API contract tests or documentation for the setDeviceFrequency endpoint # Search for API tests rg -A 5 'setDeviceFrequency|set_device_frequency' --type swift # Search for parameter constants rg -A 2 'ParameterConstants\.Me\.(serialNumber|freq)' --type swiftLength of output: 6732
Script:
#!/bin/bash # Search for ParameterConstants.Me definition and its properties rg -A 10 'struct Me|enum Me' --type swiftLength of output: 8849
44-44: LGTM!The new case follows the existing naming convention and has appropriate parameters.
70-70: LGTM!Using POST method is appropriate for setting the device frequency.
103-104: LGTM!The endpoint path follows RESTful conventions and is consistent with other device-related endpoints.
PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift (5)
25-25: LGTM!The property is correctly defined with appropriate access level and type.
29-36: LGTM!The initializer is properly updated to include the new dependency.
25-25: LGTM!The meUseCase property is correctly marked as private.
29-35: LGTM!The initializer is properly updated to include the new dependency.
25-25: LGTM!The property declaration follows the class's conventions.
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift (3)
66-71: LGTM!The method is well-implemented following the repository pattern and maintains consistency with other repository methods.
66-71: LGTM!The implementation follows the repository pattern and correctly uses the ApiClient for the request.
66-71: LGTM!The implementation follows the repository pattern and is consistent with other API methods in the class.
| func setApiHeliumFrequency() async -> NetworkErrorResponse? { | ||
| guard let heliumFrequency, let serialNumber = serialNumber?.replacingOccurrences(of: ":", with: "") else { | ||
| return nil | ||
| } | ||
|
|
||
| let error = try? await useCase.setFrequncy(serialNumber, frequency: heliumFrequency) | ||
| return error | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return an explicit error when serialNumber or heliumFrequency is missing.
Currently, this function returns nil if serialNumber or heliumFrequency is absent (line 162). Consider returning a specialized error to reflect a missing parameter scenario. This practice safeguards against silent failures and improves troubleshooting.
func setApiHeliumFrequency() async -> NetworkErrorResponse? {
guard let heliumFrequency,
let serialNumber = serialNumber?.replacingOccurrences(of: ":", with: "")
else {
- return nil
+ return NetworkErrorResponse.customError("Serial number or helium frequency is missing")
}
...
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func setApiHeliumFrequency() async -> NetworkErrorResponse? { | |
| guard let heliumFrequency, let serialNumber = serialNumber?.replacingOccurrences(of: ":", with: "") else { | |
| return nil | |
| } | |
| let error = try? await useCase.setFrequncy(serialNumber, frequency: heliumFrequency) | |
| return error | |
| } | |
| func setApiHeliumFrequency() async -> NetworkErrorResponse? { | |
| guard let heliumFrequency, let serialNumber = serialNumber?.replacingOccurrences(of: ":", with: "") else { | |
| return NetworkErrorResponse.customError("Serial number or helium frequency is missing") | |
| } | |
| let error = try? await useCase.setFrequncy(serialNumber, frequency: heliumFrequency) | |
| return error | |
| } |
| public func setFrequncy(_ serialNumber: String, frequency: Frequency) async throws -> NetworkErrorResponse? { | ||
| let response = try await meRepository.setFrequency(serialNumber: serialNumber, frequency: frequency.rawValue).toAsync() | ||
| return response.error | ||
| } |
There was a problem hiding this comment.
Fix typo in method name.
The method name contains a typo: setFrequncy should be setFrequency.
Apply this diff to fix the typo:
-public func setFrequncy(_ serialNumber: String, frequency: Frequency) async throws -> NetworkErrorResponse? {
+public func setFrequency(_ serialNumber: String, frequency: Frequency) async throws -> NetworkErrorResponse? {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public func setFrequncy(_ serialNumber: String, frequency: Frequency) async throws -> NetworkErrorResponse? { | |
| let response = try await meRepository.setFrequency(serialNumber: serialNumber, frequency: frequency.rawValue).toAsync() | |
| return response.error | |
| } | |
| public func setFrequency(_ serialNumber: String, frequency: Frequency) async throws -> NetworkErrorResponse? { | |
| let response = try await meRepository.setFrequency(serialNumber: serialNumber, frequency: frequency.rawValue).toAsync() | |
| return response.error | |
| } |
| func upateApiFrequency(frequency: Frequency) { | ||
| guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else { | ||
|
|
||
| return | ||
| } | ||
|
|
||
| Task { @MainActor [weak self] in | ||
| do { | ||
| if let apiError = try await self?.meUseCase?.setFrequncy(serialNumber, frequency: frequency) { | ||
| let uiInfo = apiError.uiInfo | ||
| let obj = uiInfo.defaultFailObject(type: .changeFrequency, | ||
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | ||
| self?.state = .failed(obj) | ||
| return | ||
| } | ||
| } catch { | ||
| return | ||
| } | ||
|
|
||
| let subtitle = LocalizableString.DeviceInfo.stationFrequencyChangedDescription(frequency.rawValue).localized | ||
| let obj = FailSuccessStateObject(type: .changeFrequency, | ||
| title: LocalizableString.DeviceInfo.stationFrequencyChanged.localized, | ||
| subtitle: subtitle.attributedMarkdown, | ||
| cancelTitle: nil, | ||
| retryTitle: LocalizableString.DeviceInfo.stationBackToSettings.localized, | ||
| contactSupportAction: nil, | ||
| cancelAction: nil, | ||
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | ||
| self?.state = .success(obj) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix typo and enhance error handling.
- The method name has a typo: "upateApiFrequency" should be "updateApiFrequency".
- The error handling could be improved:
- The empty
catchblock silently ignores errors - The early return on
serialNumberdoesn't provide error feedback
- The empty
-func upateApiFrequency(frequency: Frequency) {
+func updateApiFrequency(frequency: Frequency) {
guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else {
+ let obj = FailSuccessStateObject(
+ type: .changeFrequency,
+ title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized,
+ subtitle: LocalizableString.DeviceInfo.invalidSerialNumber.localized.attributedMarkdown,
+ cancelTitle: LocalizableString.cancel.localized,
+ retryTitle: nil,
+ contactSupportAction: nil,
+ cancelAction: { [weak self] in self?.dismissToggle.toggle() },
+ retryAction: nil
+ )
+ self.state = .failed(obj)
return
}
Task { @MainActor [weak self] in
do {
if let apiError = try await self?.meUseCase?.setFrequncy(serialNumber, frequency: frequency) {
let uiInfo = apiError.uiInfo
let obj = uiInfo.defaultFailObject(type: .changeFrequency,
retryAction: { [weak self] in self?.dismissToggle.toggle() })
self?.state = .failed(obj)
return
}
- } catch {
+ } catch let error {
+ let obj = FailSuccessStateObject(
+ type: .changeFrequency,
+ title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized,
+ subtitle: error.localizedDescription.attributedMarkdown,
+ cancelTitle: LocalizableString.cancel.localized,
+ retryTitle: LocalizableString.retry.localized,
+ contactSupportAction: nil,
+ cancelAction: { [weak self] in self?.dismissToggle.toggle() },
+ retryAction: { [weak self] in self?.setFrequency() }
+ )
+ self?.state = .failed(obj)
return
}Committable suggestion skipped: line range outside the PR's diff.
🛠️ Refactor suggestion
Fix typo and improve error handling.
The method has a typo in its name and could benefit from improved error handling.
Apply this diff to fix the issues:
-func upateApiFrequency(frequency: Frequency) {
+func updateApiFrequency(frequency: Frequency) {
guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else {
+ let obj = FailSuccessStateObject(type: .changeFrequency,
+ title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized,
+ subtitle: "Missing device serial number".attributedMarkdown,
+ cancelTitle: LocalizableString.cancel.localized,
+ retryTitle: nil,
+ contactSupportAction: nil,
+ cancelAction: { [weak self] in self?.dismissToggle.toggle() },
+ retryAction: nil)
+ self.state = .failed(obj)
return
}
Task { @MainActor [weak self] in
do {
if let apiError = try await self?.meUseCase?.setFrequncy(serialNumber, frequency: frequency) {
let uiInfo = apiError.uiInfo
let obj = uiInfo.defaultFailObject(type: .changeFrequency,
retryAction: { [weak self] in self?.dismissToggle.toggle() })
self?.state = .failed(obj)
return
}
} catch {
+ let obj = FailSuccessStateObject(type: .changeFrequency,
+ title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized,
+ subtitle: error.localizedDescription.attributedMarkdown,
+ cancelTitle: LocalizableString.cancel.localized,
+ retryTitle: LocalizableString.retry.localized,
+ contactSupportAction: nil,
+ cancelAction: { [weak self] in self?.dismissToggle.toggle() },
+ retryAction: { [weak self] in self?.setFrequency() })
+ self?.state = .failed(obj)
return
}
let subtitle = LocalizableString.DeviceInfo.stationFrequencyChangedDescription(frequency.rawValue).localized
let obj = FailSuccessStateObject(type: .changeFrequency,
title: LocalizableString.DeviceInfo.stationFrequencyChanged.localized,
subtitle: subtitle.attributedMarkdown,
cancelTitle: nil,
retryTitle: LocalizableString.DeviceInfo.stationBackToSettings.localized,
contactSupportAction: nil,
cancelAction: nil,
retryAction: { [weak self] in self?.dismissToggle.toggle() })
self?.state = .success(obj)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func upateApiFrequency(frequency: Frequency) { | |
| guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else { | |
| return | |
| } | |
| Task { @MainActor [weak self] in | |
| do { | |
| if let apiError = try await self?.meUseCase?.setFrequncy(serialNumber, frequency: frequency) { | |
| let uiInfo = apiError.uiInfo | |
| let obj = uiInfo.defaultFailObject(type: .changeFrequency, | |
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | |
| self?.state = .failed(obj) | |
| return | |
| } | |
| } catch { | |
| return | |
| } | |
| let subtitle = LocalizableString.DeviceInfo.stationFrequencyChangedDescription(frequency.rawValue).localized | |
| let obj = FailSuccessStateObject(type: .changeFrequency, | |
| title: LocalizableString.DeviceInfo.stationFrequencyChanged.localized, | |
| subtitle: subtitle.attributedMarkdown, | |
| cancelTitle: nil, | |
| retryTitle: LocalizableString.DeviceInfo.stationBackToSettings.localized, | |
| contactSupportAction: nil, | |
| cancelAction: nil, | |
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | |
| self?.state = .success(obj) | |
| } | |
| } | |
| func updateApiFrequency(frequency: Frequency) { | |
| guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else { | |
| let obj = FailSuccessStateObject(type: .changeFrequency, | |
| title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized, | |
| subtitle: "Missing device serial number".attributedMarkdown, | |
| cancelTitle: LocalizableString.cancel.localized, | |
| retryTitle: nil, | |
| contactSupportAction: nil, | |
| cancelAction: { [weak self] in self?.dismissToggle.toggle() }, | |
| retryAction: nil) | |
| self.state = .failed(obj) | |
| return | |
| } | |
| Task { @MainActor [weak self] in | |
| do { | |
| if let apiError = try await self?.meUseCase?.setFrequncy(serialNumber, frequency: frequency) { | |
| let uiInfo = apiError.uiInfo | |
| let obj = uiInfo.defaultFailObject(type: .changeFrequency, | |
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | |
| self?.state = .failed(obj) | |
| return | |
| } | |
| } catch { | |
| let obj = FailSuccessStateObject(type: .changeFrequency, | |
| title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized, | |
| subtitle: error.localizedDescription.attributedMarkdown, | |
| cancelTitle: LocalizableString.cancel.localized, | |
| retryTitle: LocalizableString.retry.localized, | |
| contactSupportAction: nil, | |
| cancelAction: { [weak self] in self?.dismissToggle.toggle() }, | |
| retryAction: { [weak self] in self?.setFrequency() }) | |
| self?.state = .failed(obj) | |
| return | |
| } | |
| let subtitle = LocalizableString.DeviceInfo.stationFrequencyChangedDescription(frequency.rawValue).localized | |
| let obj = FailSuccessStateObject(type: .changeFrequency, | |
| title: LocalizableString.DeviceInfo.stationFrequencyChanged.localized, | |
| subtitle: subtitle.attributedMarkdown, | |
| cancelTitle: nil, | |
| retryTitle: LocalizableString.DeviceInfo.stationBackToSettings.localized, | |
| contactSupportAction: nil, | |
| cancelAction: nil, | |
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | |
| self?.state = .success(obj) | |
| } | |
| } |
🛠️ Refactor suggestion
Fix typos and improve error handling.
The method has several issues that need to be addressed:
- The method name has a typo.
- Error handling needs improvement.
- Code style can be enhanced.
- func upateApiFrequency(frequency: Frequency) {
- guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else {
-
- return
- }
-
+ /// Updates the device frequency on the backend
+ func updateApiFrequency(frequency: Frequency) {
+ guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else {
+ let obj = FailSuccessStateObject(
+ type: .changeFrequency,
+ title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized,
+ subtitle: "Invalid device serial number".attributedMarkdown,
+ retryAction: { [weak self] in self?.setFrequency() }
+ )
+ state = .failed(obj)
+ return
+ }
Task { @MainActor [weak self] in
do {
if let apiError = try await self?.meUseCase?.setFrequncy(serialNumber, frequency: frequency) {
- let uiInfo = apiError.uiInfo
- let obj = uiInfo.defaultFailObject(type: .changeFrequency,
- retryAction: { [weak self] in self?.dismissToggle.toggle() })
+ let obj = apiError.uiInfo.defaultFailObject(
+ type: .changeFrequency,
+ retryAction: { [weak self] in self?.dismissToggle.toggle() }
+ )
self?.state = .failed(obj)
return
}
} catch {
+ let obj = FailSuccessStateObject(
+ type: .changeFrequency,
+ title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized,
+ subtitle: error.localizedDescription.attributedMarkdown,
+ retryAction: { [weak self] in self?.setFrequency() }
+ )
+ self?.state = .failed(obj)
return
}Committable suggestion skipped: line range outside the PR's diff.
| static func getChangeFrequencyViewModel(device: DeviceDetails, frequncy: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel { | ||
| let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self)! | ||
| let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self)! | ||
| return ChangeFrequencyViewModel(device: device, | ||
| useCase: deviceInfoUseCase, | ||
| meUseCase: meUseCase, | ||
| frequency: frequncy) | ||
| } |
There was a problem hiding this comment.
Fix typo in parameter name.
The parameter name has a typo: "frequncy" should be "frequency".
-static func getChangeFrequencyViewModel(device: DeviceDetails, frequncy: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel {
+static func getChangeFrequencyViewModel(device: DeviceDetails, frequency: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel {
let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self)!
let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self)!
return ChangeFrequencyViewModel(device: device,
useCase: deviceInfoUseCase,
meUseCase: meUseCase,
- frequency: frequncy)
+ frequency: frequency)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static func getChangeFrequencyViewModel(device: DeviceDetails, frequncy: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel { | |
| let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self)! | |
| let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self)! | |
| return ChangeFrequencyViewModel(device: device, | |
| useCase: deviceInfoUseCase, | |
| meUseCase: meUseCase, | |
| frequency: frequncy) | |
| } | |
| static func getChangeFrequencyViewModel(device: DeviceDetails, frequency: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel { | |
| let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self)! | |
| let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self)! | |
| return ChangeFrequencyViewModel(device: device, | |
| useCase: deviceInfoUseCase, | |
| meUseCase: meUseCase, | |
| frequency: frequency) | |
| } |
🛠️ Refactor suggestion
Fix typo and add null checks.
The implementation has a typo and should include null checks for resolved dependencies.
Apply this diff to fix the issues:
-static func getChangeFrequencyViewModel(device: DeviceDetails, frequncy: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel {
- let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self)!
- let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self)!
+static func getChangeFrequencyViewModel(device: DeviceDetails, frequency: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel {
+ guard let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self),
+ let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self) else {
+ fatalError("Failed to resolve required dependencies")
+ }
return ChangeFrequencyViewModel(device: device,
useCase: deviceInfoUseCase,
meUseCase: meUseCase,
- frequency: frequncy)
+ frequency: frequency)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static func getChangeFrequencyViewModel(device: DeviceDetails, frequncy: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel { | |
| let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self)! | |
| let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self)! | |
| return ChangeFrequencyViewModel(device: device, | |
| useCase: deviceInfoUseCase, | |
| meUseCase: meUseCase, | |
| frequency: frequncy) | |
| } | |
| static func getChangeFrequencyViewModel(device: DeviceDetails, frequency: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel { | |
| guard let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self), | |
| let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self) else { | |
| fatalError("Failed to resolve required dependencies") | |
| } | |
| return ChangeFrequencyViewModel(device: device, | |
| useCase: deviceInfoUseCase, | |
| meUseCase: meUseCase, | |
| frequency: frequency) | |
| } |
🛠️ Refactor suggestion
Fix typo and consider safer dependency handling.
The method has several issues that need to be addressed:
- Parameter name has a typo.
- Force unwrapping dependencies could be unsafe.
- Indentation is inconsistent.
- static func getChangeFrequencyViewModel(device: DeviceDetails, frequncy: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel {
- let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self)!
- let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self)!
- return ChangeFrequencyViewModel(device: device,
- useCase: deviceInfoUseCase,
- meUseCase: meUseCase,
- frequency: frequncy)
- }
+ static func getChangeFrequencyViewModel(device: DeviceDetails, frequency: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel {
+ guard let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self),
+ let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self) else {
+ fatalError("Required dependencies not registered")
+ }
+ return ChangeFrequencyViewModel(
+ device: device,
+ useCase: deviceInfoUseCase,
+ meUseCase: meUseCase,
+ frequency: frequency
+ )
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static func getChangeFrequencyViewModel(device: DeviceDetails, frequncy: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel { | |
| let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self)! | |
| let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self)! | |
| return ChangeFrequencyViewModel(device: device, | |
| useCase: deviceInfoUseCase, | |
| meUseCase: meUseCase, | |
| frequency: frequncy) | |
| } | |
| static func getChangeFrequencyViewModel(device: DeviceDetails, frequency: Frequency? = Frequency.allCases.first) -> ChangeFrequencyViewModel { | |
| guard let deviceInfoUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(DeviceInfoUseCase.self), | |
| let meUseCase = SwinjectHelper.shared.getContainerForSwinject().resolve(MeUseCase.self) else { | |
| fatalError("Required dependencies not registered") | |
| } | |
| return ChangeFrequencyViewModel( | |
| device: device, | |
| useCase: deviceInfoUseCase, | |
| meUseCase: meUseCase, | |
| frequency: frequency | |
| ) | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
PresentationLayer/UIComponents/Screens/ClaimDevice/ClaimDeviceContainer/ClaimHeliumContainerViewModel.swift (1)
171-178:⚠️ Potential issueImprove error handling in setApiHeliumFrequency.
The current implementation has several issues:
- Using generic NSError with empty domain
- Silently ignoring potential errors with try?
- Returning nil for missing parameters
Consider this improved implementation:
-func setApiHeliumFrequency() async throws -> NetworkErrorResponse? { - guard let heliumFrequency, let serialNumber = serialNumber?.replacingOccurrences(of: ":", with: "") else { - throw NSError(domain: "", code: -1) - } - - let error = try? await useCase.setFrequency(serialNumber, frequency: heliumFrequency) - return error +enum HeliumFrequencyError: Error { + case missingParameters +} + +func setApiHeliumFrequency() async throws -> NetworkErrorResponse? { + guard let heliumFrequency, let serialNumber = serialNumber?.replacingOccurrences(of: ":", with: "") else { + throw HeliumFrequencyError.missingParameters + } + + return try await useCase.setFrequency(serialNumber, frequency: heliumFrequency) }PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift (1)
143-179:⚠️ Potential issueFix error handling and code style issues.
The implementation needs several improvements:
- The empty guard clause needs proper error handling
- Generic error messages should be more specific
- Indentation needs to be consistent
Apply this diff to fix the issues:
func updateApiFrequency(frequency: Frequency) { guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else { - + let obj = FailSuccessStateObject( + type: .changeFrequency, + title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized, + subtitle: LocalizableString.DeviceInfo.invalidSerialNumber.localized.attributedMarkdown, + cancelTitle: LocalizableString.cancel.localized, + retryTitle: nil, + contactSupportAction: nil, + cancelAction: { [weak self] in self?.dismissToggle.toggle() }, + retryAction: nil + ) + self.state = .failed(obj) return } Task { @MainActor [weak self] in do { if let apiError = try await self?.meUseCase?.setFrequency(serialNumber, frequency: frequency) { let uiInfo = apiError.uiInfo let obj = uiInfo.defaultFailObject(type: .changeFrequency, - retryAction: { [weak self] in self?.dismissToggle.toggle() }) + retryAction: { [weak self] in self?.dismissToggle.toggle() }) self?.state = .failed(obj) return } } catch { - let uiInfo = NetworkErrorResponse.UIInfo(title: LocalizableString.Error.genericMessage.localized, - description: nil) - let obj = uiInfo.defaultFailObject(type: .changeFrequency, - retryAction: { [weak self] in self?.dismissToggle.toggle() }) + let obj = FailSuccessStateObject( + type: .changeFrequency, + title: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized, + subtitle: error.localizedDescription.attributedMarkdown, + cancelTitle: LocalizableString.cancel.localized, + retryTitle: LocalizableString.retry.localized, + contactSupportAction: nil, + cancelAction: { [weak self] in self?.dismissToggle.toggle() }, + retryAction: { [weak self] in self?.setFrequency() } + ) self?.state = .failed(obj) - return }
🧹 Nitpick comments (2)
PresentationLayer/UIComponents/Screens/ClaimDevice/ClaimDeviceContainer/ClaimHeliumContainerViewModel.swift (2)
110-127: Refactor duplicated error handling logic.The error handling code is duplicated across different failure scenarios. Consider extracting the common error handling pattern into a reusable method.
Here's a suggested refactor:
+private func handleError(_ error: Error, title: String? = nil) { + let uiInfo = (error as? NetworkErrorResponse)?.uiInfo ?? + NetworkErrorResponse.UIInfo( + title: title ?? LocalizableString.Error.genericMessage.localized, + description: nil + ) + let failObj = uiInfo.defaultFailObject(type: .claimDeviceFlow, failMode: .default) { [weak self] in + self?.showLoading = false + } + loadingState = .fail(failObj) +} do { if let apiFrequencyError = try await self.setApiHeliumFrequency() { - let uiInfo = apiFrequencyError.uiInfo - let failObj = uiInfo.defaultFailObject(type: .claimDeviceFlow, failMode: .default) { [weak self] in - self?.showLoading = false - } - loadingState = .fail(failObj) + handleError(apiFrequencyError) return } } catch { - let uiInfo = NetworkErrorResponse.UIInfo(title: LocalizableString.Error.genericMessage.localized, - description: nil) - let failObj = uiInfo.defaultFailObject(type: .claimDeviceFlow, failMode: .default) { [weak self] in - self?.showLoading = false - } - loadingState = .fail(failObj) + handleError(error) return }
98-98: Fix return arrow spacing.Add a space between the return arrow and return type.
- return + return🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 98-98: Return arrow and return type should be separated by a single space or on a separate line
(return_arrow_whitespace)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
PresentationLayer/Extensions/DomainExtensions/Common/NetworkErrorResponse+.swift(1 hunks)PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift(2 hunks)PresentationLayer/UIComponents/Screens/ClaimDevice/ClaimDeviceContainer/ClaimHeliumContainerViewModel.swift(4 hunks)wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- PresentationLayer/Extensions/DomainExtensions/Common/NetworkErrorResponse+.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift
🧰 Additional context used
🪛 SwiftLint (0.57.0)
PresentationLayer/UIComponents/Screens/ClaimDevice/ClaimDeviceContainer/ClaimHeliumContainerViewModel.swift
[Warning] 98-98: Return arrow and return type should be separated by a single space or on a separate line
(return_arrow_whitespace)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WeatherXM | Unit tests | Test - iOS
🔇 Additional comments (3)
PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift (3)
25-35: LGTM! Clean implementation of the new use case.The addition of
meUseCaseand its initializer parameter follows Swift best practices for dependency injection.
136-136: LGTM! Good integration with the backend update.The placement of
updateApiFrequencycall in the.finishedcase ensures the frequency is only posted to the backend after successful Bluetooth update.
151-151: Verify the API endpoint integration.Let's confirm that the
setFrequencyendpoint is properly integrated with the backend.✅ Verification successful
API endpoint integration verified successfully
The
setFrequencyendpoint is properly integrated through all layers with appropriate error handling and authorization. The implementation follows the established patterns in the codebase for API communication.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the API endpoint definition and usage echo "Searching for setFrequency endpoint definition..." rg -p "setFrequency.*frequency.*Frequency" --type swift echo "Searching for related API tests..." rg -p "test.*setFrequency" --type swiftLength of output: 1232
Script:
#!/bin/bash # Search for API endpoint configuration and network implementation echo "Searching for API endpoint configuration..." rg -p "endpoint.*frequency" --type swift -A 5 echo "Searching for network layer implementation..." rg -p "class.*NetworkService|protocol.*NetworkService" --type swift -A 10 echo "Searching for NetworkErrorResponse definition..." rg -p "NetworkErrorResponse" --type swift -A 5Length of output: 66163
| // Fetch device info | ||
|
|
||
| let result = await devicesUseCase.getDeviceInfo(device: btDevice) | ||
| switch result { | ||
| case .success(let info): | ||
| // perform claim request | ||
| self.serialNumber = info?.devEUI | ||
| self.claimingKey = info?.claimingKey | ||
| case .failure(let error): | ||
| let failObj = self.getFailObject(for: error) { [weak self] in | ||
| self?.showLoading = false | ||
| } | ||
| loadingState = .fail(failObj) | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling consistency and loading state management.
The device info fetching segment needs improvements in state management and error handling:
- Loading state isn't updated after successful device info fetch
- Error handling pattern differs from other segments
- Missing step completion indicator for device info fetch
Consider applying this improvement:
let result = await devicesUseCase.getDeviceInfo(device: btDevice)
switch result {
case .success(let info):
// perform claim request
self.serialNumber = info?.devEUI
self.claimingKey = info?.claimingKey
+ steps[0].isCompleted = true // Mark device info step as complete
case .failure(let error):
let failObj = self.getFailObject(for: error) { [weak self] in
self?.showLoading = false
}
loadingState = .fail(failObj)
return
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 98-98: Return arrow and return type should be separated by a single space or on a separate line
(return_arrow_whitespace)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift (1)
190-196: Improve error message specificity and fix indentation.The generic error handling could be more informative and maintain consistent formatting.
func showGenericError() { let uiInfo = NetworkErrorResponse.UIInfo(title: LocalizableString.Error.genericMessage.localized, - description: nil) + description: LocalizableString.DeviceInfo.stationFrequencyChangeFailed.localized) let obj = uiInfo.defaultFailObject(type: .changeFrequency, - retryAction: { [weak self] in self?.dismissToggle.toggle() }) + retryAction: { [weak self] in self?.dismissToggle.toggle() }) state = .failed(obj) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift(3 hunks)PresentationLayer/UIComponents/ViewModelsFactory.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- PresentationLayer/UIComponents/ViewModelsFactory.swift
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: WeatherXM | Unit tests | Test - iOS
🔇 Additional comments (2)
PresentationLayer/UIComponents/Screens/ChangeFrequency/ChangeFrequencyViewModel.swift (2)
25-25: LGTM! Property and initializer changes look good.The new
meUseCaseproperty is correctly encapsulated, and the initializer is well-structured with proper parameter handling.Also applies to: 29-35
136-139: Verify state transitions during frequency updates.Please ensure that all possible state transitions are handled correctly when updating the frequency:
- Bluetooth frequency change → API update
- Failed Bluetooth change → Error state
- Failed API update → Error state
✅ Verification successful
State transitions during frequency updates are properly handled ✅
The codebase implements comprehensive error handling and state transitions for both Bluetooth and API frequency updates, including:
- Bluetooth frequency change failures → Error state with user feedback
- API update failures → Error state with descriptive messages
- Proper error propagation through all layers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all state transitions in the codebase echo "Searching for state transitions in ChangeFrequencyViewModel..." rg -A 2 "self\.state = \." --type swift echo "Searching for frequency-related error handling..." rg -A 5 "func.*Frequency.*Error" --type swiftLength of output: 16012
| func updateApiFrequency(frequency: Frequency) { | ||
| guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else { | ||
| self.showGenericError() | ||
| return | ||
| } | ||
|
|
||
| Task { @MainActor [weak self] in | ||
| do { | ||
| if let apiError = try await self?.meUseCase?.setFrequency(serialNumber, frequency: frequency) { | ||
| let uiInfo = apiError.uiInfo | ||
| let obj = uiInfo.defaultFailObject(type: .changeFrequency, | ||
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | ||
| self?.state = .failed(obj) | ||
| return | ||
| } | ||
| } catch { | ||
| self?.showGenericError() | ||
|
|
||
| return | ||
| } | ||
|
|
||
| let subtitle = LocalizableString.DeviceInfo.stationFrequencyChangedDescription(frequency.rawValue).localized | ||
| let obj = FailSuccessStateObject(type: .changeFrequency, | ||
| title: LocalizableString.DeviceInfo.stationFrequencyChanged.localized, | ||
| subtitle: subtitle.attributedMarkdown, | ||
| cancelTitle: nil, | ||
| retryTitle: LocalizableString.DeviceInfo.stationBackToSettings.localized, | ||
| contactSupportAction: nil, | ||
| cancelAction: nil, | ||
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | ||
| self?.state = .success(obj) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and add loading state.
The implementation could be improved in several areas:
- The catch block should preserve the error context
- Consider adding a loading state during the API call
- The success state object creation has inconsistent indentation
func updateApiFrequency(frequency: Frequency) {
guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else {
self.showGenericError()
return
}
Task { @MainActor [weak self] in
+ self?.state = .loading
do {
if let apiError = try await self?.meUseCase?.setFrequency(serialNumber, frequency: frequency) {
let uiInfo = apiError.uiInfo
let obj = uiInfo.defaultFailObject(type: .changeFrequency,
- retryAction: { [weak self] in self?.dismissToggle.toggle() })
+ retryAction: { [weak self] in self?.dismissToggle.toggle() })
self?.state = .failed(obj)
return
}
- } catch {
+ } catch let error {
self?.showGenericError()
+ Logger.error("Failed to update frequency: \(error)")
return
}
let subtitle = LocalizableString.DeviceInfo.stationFrequencyChangedDescription(frequency.rawValue).localized
let obj = FailSuccessStateObject(type: .changeFrequency,
- title: LocalizableString.DeviceInfo.stationFrequencyChanged.localized,
- subtitle: subtitle.attributedMarkdown,
- cancelTitle: nil,
- retryTitle: LocalizableString.DeviceInfo.stationBackToSettings.localized,
- contactSupportAction: nil,
- cancelAction: nil,
- retryAction: { [weak self] in self?.dismissToggle.toggle() })
+ title: LocalizableString.DeviceInfo.stationFrequencyChanged.localized,
+ subtitle: subtitle.attributedMarkdown,
+ cancelTitle: nil,
+ retryTitle: LocalizableString.DeviceInfo.stationBackToSettings.localized,
+ contactSupportAction: nil,
+ cancelAction: nil,
+ retryAction: { [weak self] in self?.dismissToggle.toggle() })
self?.state = .success(obj)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func updateApiFrequency(frequency: Frequency) { | |
| guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else { | |
| self.showGenericError() | |
| return | |
| } | |
| Task { @MainActor [weak self] in | |
| do { | |
| if let apiError = try await self?.meUseCase?.setFrequency(serialNumber, frequency: frequency) { | |
| let uiInfo = apiError.uiInfo | |
| let obj = uiInfo.defaultFailObject(type: .changeFrequency, | |
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | |
| self?.state = .failed(obj) | |
| return | |
| } | |
| } catch { | |
| self?.showGenericError() | |
| return | |
| } | |
| let subtitle = LocalizableString.DeviceInfo.stationFrequencyChangedDescription(frequency.rawValue).localized | |
| let obj = FailSuccessStateObject(type: .changeFrequency, | |
| title: LocalizableString.DeviceInfo.stationFrequencyChanged.localized, | |
| subtitle: subtitle.attributedMarkdown, | |
| cancelTitle: nil, | |
| retryTitle: LocalizableString.DeviceInfo.stationBackToSettings.localized, | |
| contactSupportAction: nil, | |
| cancelAction: nil, | |
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | |
| self?.state = .success(obj) | |
| } | |
| } | |
| func updateApiFrequency(frequency: Frequency) { | |
| guard let serialNumber = device.label?.replacingOccurrences(of: ":", with: "") else { | |
| self.showGenericError() | |
| return | |
| } | |
| Task { @MainActor [weak self] in | |
| self?.state = .loading | |
| do { | |
| if let apiError = try await self?.meUseCase?.setFrequency(serialNumber, frequency: frequency) { | |
| let uiInfo = apiError.uiInfo | |
| let obj = uiInfo.defaultFailObject(type: .changeFrequency, | |
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | |
| self?.state = .failed(obj) | |
| return | |
| } | |
| } catch let error { | |
| self?.showGenericError() | |
| Logger.error("Failed to update frequency: \(error)") | |
| return | |
| } | |
| let subtitle = LocalizableString.DeviceInfo.stationFrequencyChangedDescription(frequency.rawValue).localized | |
| let obj = FailSuccessStateObject(type: .changeFrequency, | |
| title: LocalizableString.DeviceInfo.stationFrequencyChanged.localized, | |
| subtitle: subtitle.attributedMarkdown, | |
| cancelTitle: nil, | |
| retryTitle: LocalizableString.DeviceInfo.stationBackToSettings.localized, | |
| contactSupportAction: nil, | |
| cancelAction: nil, | |
| retryAction: { [weak self] in self?.dismissToggle.toggle() }) | |
| self?.state = .success(obj) | |
| } | |
| } |
Why?
Post the selected frequency to our backend in the claim flow and the change frequency screen
How?
Testing
Ensure everything works as expected
Additional context
fixes fe-1492, fe-1567
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates
Frequencyenum to support concurrency.