Use the call expiration timestamp to define the ringing window #4652
Conversation
BillCarsonFr
changed the title
BillCarsonFr
force-pushed
the
valere/call/notification_lifetime_2
branch
from
29e86cf to
8d797df
Compare
| private var declineListenerHandle: TaskHandle? | ||
|
|
||
| override init() { | ||
| init(callProvider: CXProviderProtocol? = nil, timeProvider: TimeProvider? = nil) { |
There was a problem hiding this comment.
Where is this new init being called? I don't see its usage anywhere outside of the test
| self.timeProvider = timeProvider ?? TimeProvider(clock: ContinuousClock(), now: Date.init) | ||
|
|
||
| if let callProvider { | ||
| self.callProvider = callProvider | ||
| } else { | ||
| let configuration = CXProviderConfiguration() | ||
| configuration.supportsVideo = true | ||
| configuration.includesCallsInRecents = true | ||
|
|
||
| if let callKitIcon = UIImage(named: "images/app-logo") { | ||
| configuration.iconTemplateImageData = callKitIcon.pngData() | ||
| } | ||
|
|
||
| // https://stackoverflow.com/a/46077628/730924 | ||
| configuration.supportedHandleTypes = [.generic] | ||
|
|
||
| self.callProvider = CXProvider(configuration: configuration) | ||
| } |
There was a problem hiding this comment.
Okay I see...
However I am not a huge fan of this approach, I would rather have the init() have non optional values but have always concrete values passed inside the init for the protocols.
There was a problem hiding this comment.
Let's wrap both the CXProvider and TimeProvider into a single mockable class maybe called ElementCallProviderProtocol and implement concretely what happens in the init right now by default into an ElementCallProvider that implements such protocol.
In the tests we would then use the ElementCallProviderProtocolMock
Thanks to this would not need then to use a TimeProvider struct , that uses that weird now closure and the any Clock, and neither to create a specific protocol for the CXProvider just to mock its behaviour, the wrapping class can take care of all of this.
| // Keep this class testable | ||
| struct TimeProvider { | ||
| var clock: any Clock<Duration> | ||
| var now: () -> Date | ||
| } |
There was a problem hiding this comment.
Not a fan, I think we had a unified class/protocol that can be mocked these could just be simple functions of such class that could be mocked.
# Conflicts: # ElementX.xcodeproj/project.pbxproj
pixlwave
force-pushed
the
valere/call/notification_lifetime_2
branch
from
8d797df to
4cb6883
Compare
pixlwave
left a comment
pixlwave
left a comment
There was a problem hiding this comment.
Sorry @BillCarsonFr, you've had 3 separate reviews with different comments on this one. I've rebased and added two small commits onto the end, otherwise it looks good to me.
So we'll merge this and then argue amongst ourselves about whether we want to change anything.
|
❌ 2 Tests Failed:
View the full list of 2 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
pixlwave
changed the title
pixlwave
changed the title
pixlwave
changed the title
Pull Request Checklist
Re-edition of #4521 rebased and cherry-picked
UI changes have been tested with: