BIT-2415: Send org events #739
Conversation
|
No New Or Fixed Issues Found |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #739 +/- ##
==========================================
- Coverage 88.18% 88.17% -0.01%
==========================================
Files 575 577 +2
Lines 28734 28789 +55
==========================================
+ Hits 25339 25385 +46
- Misses 3395 3404 +9 ☔ View full report in Codecov by Sentry. |
|
|
||
| /// Starts timer to send organization events regularly | ||
| private func startEventTimer() { | ||
| sendEventTimer = Timer.scheduledTimer(withTimeInterval: 5, repeats: true) { _ in |
There was a problem hiding this comment.
🤔 In MAUI this is configured to happen every 60seconds for iOS, 5min is for Android. Was this changed to unify the platforms?
There was a problem hiding this comment.
The ticket I was going off of indicated that it was five minutes, so I was going off of that. The Slack discussions I've been able to dig up don't provide any more illumination for me.
There was a problem hiding this comment.
Just asked in Slack to see what's the correct expected value.
BitwardenShared/Core/Platform/Services/API/E
8000
ventAPIService.swift
Outdated
Show resolved
Hide resolved
BitwardenShared/Core/Platform/Services/API/Requests/EventRequest.swift
Outdated
Show resolved
Hide resolved
The thing is, I'm doing that and it's not getting picked up. |
🎟️ Tracking
https://livefront.atlassian.net/browse/BIT-2415
📔 Objective
This adds a five-minute timer to the
AppProcessorthat regularly sends accumulated organization events (if there are any to send). As well, if the app is backgrounded, events are sent immediately and the timer is stopped, then restarted on foregrounding.There were issues with testing
AppProcessorparticularly. As near as I can tell, because theMockNotificationCenterServiceuses aCurrentValueSubjectfor its mock, when theTaskobjects inAppProcessor.init()are created and subscribe, the current value (Void) is emitted, triggering the foreground and background blocks to run.This means firstly that any tests that rely on e.g.
notificationCenterService.didEnterBackgroundSubject.send()can have that line taken out and have the test still pass, because in this exampledidEnterBackgroundSubjectis getting triggered on initialization. And secondly, that tests that rely on validating a series of events—namely, backgrounding then foregrounding the app—have nondeterministic results.I attempted several solutions to this—adding a
dropFirstto the subject, turning the subject into aPassthroughSubject, and various incarnations ofTaskmanipulation in the tests—to no avail. At best I could get nondeterministic results.As a result, I only have a test for the
initcurrently, and not other scenarios. Given that this is an effective singleton object, I'm not especially concerned, but it would be nice to find a way to test the app background/foregrounding in the future.The missing lines in the CodeCov report are, as near as I can tell, an issue with Xcode.
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes