Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Adjust deadman notification scheduler (EXPOSUREAPP-6600) #2993

Merged

Conversation

LukasLechnerDev
Copy link
Contributor

@LukasLechnerDev LukasLechnerDev commented Apr 28, 2021

Refactor the Deadman Notification scheduler to our new way of how workers work. This way, we don't schedule work from several places within the app, but only from DeadmanNotificationScheduler

Basically, deadmanNotificationScheduler.setup() is called at app start and then the scheduler observes if

  • onBoarding was done
  • a positive test result is registered
  • enf tracing is enabled

=> We schedule the Deadman Notification if onBoarding was done, enf tracing is activated and no positive test result is registered.
=> I didn't change the worker itself, so it should behave as before.

@LukasLechnerDev LukasLechnerDev added the maintainers Tag pull requests created by maintainers label Apr 28, 2021
@LukasLechnerDev LukasLechnerDev added this to the 2.1.0 milestone Apr 28, 2021
@LukasLechnerDev LukasLechnerDev requested a review from a team April 28, 2021 14:06
@jurajkusnier jurajkusnier self-assigned this Apr 28, 2021
jurajkusnier
jurajkusnier previously approved these changes Apr 28, 2021
Copy link
Contributor

@jurajkusnier jurajkusnier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Tested on Pixel 4 (Android 11)

@d4rken d4rken self-assigned this Apr 28, 2021
@d4rken d4rken self-requested a review April 28, 2021 15:26
@BMItr BMItr assigned BMItr and d4rken and unassigned d4rken Apr 28, 2021
Copy link
Member

@d4rken d4rken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it, removing all that code from different places 👌.

Small question? 👇

coronaTestRepository.coronaTests,
enfClient.isTracingEnabled
) { isOnboarded, coronaTests, isTracingEnabled ->
val noPositiveTestRegistered = coronaTests.none { it.isSubmissionAllowed }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use isPositive? Because isSubmissionAllowed will be false again after submission.

    override val isSubmissionAllowed: Boolean
        get() = isPositive && !isSubmitted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Thanks! Changed it to use isPositive now.

BMItr
BMItr previously approved these changes Apr 28, 2021
Copy link
Contributor

@BMItr BMItr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!
tested on oneplus 8

@LukasLechnerDev LukasLechnerDev dismissed stale reviews from BMItr and jurajkusnier via 2302ba8 April 28, 2021 15:49
@sonarcloud
Copy link

sonarcloud bot commented Apr 28, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

71.4% 71.4% Coverage
0.0% 0.0% Duplication

@harambasicluka harambasicluka merged commit d8a4226 into release/2.1.x Apr 28, 2021
@harambasicluka harambasicluka deleted the feature/6600-Review-DeadmanNotificationScheduler branch April 28, 2021 16:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintainers Tag pull requests created by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants