Bug fixes for refreshing and updating subscription state #2473
Conversation
miasma13
changed the title
miasma13
changed the title
github-actions
bot
added
bot: not in app board
| case .failure: | ||
| self.hasAccessToITR = false | ||
| } | ||
| await self.updateSubscription(with: .returnCacheDataElseLoad) |
There was a problem hiding this comment.
Is it ok for an init to have side effects like this one? I would generally avoid running operations in a function that is supposed to just initialise an object.
There was a problem hiding this comment.
I understand, the problem here was that those calls are used to setup the initial state of the VM but since they are async. I agree that it is smelly and screams for improvement.
|
|
||
| let subscriptionResult = await SubscriptionService.getSubscription(accessToken: token, cachePolicy: .reloadIgnoringLocalCacheData) | ||
| @MainActor |
There was a problem hiding this comment.
I have noticed the general approach to wrapping everything in @mainactor, but I don't see anything in this function that demands to be executed on the main thread. If any of the called function is @mainactor on it's own I would suggest to wrap in in a DisapatchQueue.main.async {}
There was a problem hiding this comment.
These actually were required since during runtime I got SwiftUI warnings about publishing changes not from main thread.
...Packages/SubscriptionUI/Sources/SubscriptionUI/Preferences/PreferencesSubscriptionView.swift
Show resolved
Hide resolved
Task/Issue URL: https://app.asana.com/0/1199230911884351/1206896697354395/f
Description:
Internal references:
Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation