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

Converted risk level transaction to task (EXPOSUREAPP-2842) #1412

Merged
merged 61 commits into from Nov 2, 2020
Merged

Converted risk level transaction to task (EXPOSUREAPP-2842) #1412

merged 61 commits into from Nov 2, 2020

Conversation

chris-cwa
Copy link
Contributor

@chris-cwa chris-cwa commented Oct 15, 2020 •
edited

Using CWAs new task controller for risk level calculation.

Change to user experience of the app is not intended and should keep this PR from being approved.

@chris-cwa chris-cwa added the maintainers Tag pull requests created by maintainers label Oct 15, 2020
@chris-cwa chris-cwa added this to the 1.6.0 milestone Oct 15, 2020
d4rken
d4rken suggested changes Oct 16, 2020
View reviewed changes
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.

Looking good

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/DefaultRiskLevels.kt Outdated Show resolved Hide resolved
Corona-Warn-App/src/main/java/de/rki/coronawarnapp/risk/RiskLevelTask.kt Outdated Show resolved Hide resolved
@d4rken
Copy link
Member

d4rken commented Oct 16, 2020

Didn't do an indepth review yet, just short lookthrough.

@kolyaopahle
Added additional checks to notification of exposure/test-result to ch…
b29d9a5 
…eck for finished submission

Signed-off-by: Kolya Opahle <k.opahle@sap.com>
@kolyaopahle kolyaopahle mentioned this pull request Oct 16, 2020
Merged
@BMItr
Copy link
Contributor

BMItr commented Oct 29, 2020

It's really a pleasure not to see the old Transactions anymore! Great Work! ❤️

@chris-cwa
Copy link
Contributor Author

It's really a pleasure not to see the old Transactions anymore! Great Work! ❤️

Thanks! 😃

harambasicluka
harambasicluka previously approved these changes Oct 30, 2020
View reviewed changes
chris-cwa and others added 5 commits October 30, 2020 14:10
@chris-cwa
reduced static access
1a16472
@chris-cwa
Merge remote-tracking branch 'origin/release/1.7.x' into feature/2909…
5e42700 
…_refactor_risk_level_transaction

# Conflicts:
#	Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/ExposureStateUpdateWorker.kt
@chris-cwa
Saint Pipeline, please give me the green light
fc1977a
@chris-cwa
Merge remote-tracking branch 'origin/release/1.7.x' into feature/2909…
1bf9119 
…_refactor_risk_level_transaction

# Conflicts:
#	Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RiskLevelTransactionTest.kt
@chris-cwa
Merge branch 'release/1.7.x' into feature/2909_refactor_risk_level_tr…
cf2dbe0 
…ansaction
@AlexanderAlferov
Copy link
Contributor

AlexanderAlferov commented Nov 2, 2020 •
edited

Tested. Looks good.
But I confirm Luka's comment regarding manual mode and risk auto-update when navigating from one screen to another.
Update: could not confirm same behaviour in 1.7.x - will double check with Luka.

d4rken
d4rken previously approved these changes Nov 2, 2020
View reviewed changes
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.

Two cases where we can inject instead of static access, but otherwise looks good.

Successfully tested on a Pixel 3 @ Android 11

Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/ExposureStateUpdateWorker.kt Outdated Show resolved Hide resolved
Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/TracingRepository.kt Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Nov 2, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

13.8% 13.8% Coverage
0.0% 0.0% Duplication

d4rken
d4rken approved these changes Nov 2, 2020
View reviewed changes
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.

lgtm

@harambasicluka harambasicluka merged commit 46aaaf1 into release/1.7.x Nov 2, 2020
@harambasicluka harambasicluka deleted the feature/2909_refactor_risk_level_transaction branch November 2, 2020 17:09
@ralfgehrer
Copy link
Contributor

Successfully tested on Pixel (Android 10) and Pixel 5 (Android 11)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ralfgehrer ralfgehrer ralfgehrer left review comments

@SamuraiKek SamuraiKek SamuraiKek left review comments

@harambasicluka harambasicluka harambasicluka left review comments

@kolyaopahle kolyaopahle kolyaopahle left review comments

@d4rken d4rken d4rken approved these changes

@BMItr BMItr BMItr approved these changes

@pwoessner pwoessner Awaiting requested review from pwoessner

@mlenkeit mlenkeit Awaiting requested review from mlenkeit

Assignees
No one assigned
Labels
maintainers Tag pull requests created by maintainers prio PRs to review first.
Projects
None yet
Milestone
1.7.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@chris-cwa @d4rken @kolyaopahle @BMItr @AlexanderAlferov @ralfgehrer @SamuraiKek @harambasicluka