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

Fix data donation key submission (EXPOSUREAPP-5814) #2626

Merged
merged 17 commits into from
Mar 17, 2021
Merged

Fix data donation key submission (EXPOSUREAPP-5814) #2626

merged 17 commits into from
Mar 17, 2021

Conversation

@chiljamgossow
Copy link
Contributor

@chiljamgossow chiljamgossow commented Mar 15, 2021 •
edited
Loading

remove data only when new test registered to allow donation after removal (this may be a new requirement)

calculate data on registration as data may be overwritten by the time the contribution is being made
check for order of dates
check for missing data, occurs when data donation is enabled after test registration and before test result

Default values should be -1 if they cannot be calculated.

@chiljamgossow chiljamgossow requested a review from a team March 15, 2021 17:59
@chiljamgossow chiljamgossow added maintainers Tag pull requests created by maintainers bug Something isn't working labels Mar 15, 2021
@ralfgehrer ralfgehrer added this to the 1.15.0 milestone Mar 15, 2021
@chiljamgossow
more tests
a818e96
@chiljamgossow
Merge branch 'release/1.15.x' into fix/data-donation-key-submission
2ef0852 
# Conflicts:
#	Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/keysubmission/AnalyticsKeySubmissionRepositoryTest.kt
@chiljamgossow
fix test
d7babb5
@ralfgehrer ralfgehrer added the prio PRs to review first. label Mar 16, 2021
@chiljamgossow chiljamgossow added the author merge PR to be merged by author label Mar 16, 2021
@LukasLechnerDev LukasLechnerDev self-assigned this Mar 16, 2021
@chiljamgossow chiljamgossow removed the author merge PR to be merged by author label Mar 16, 2021
@LukasLechnerDev
Copy link
Contributor

LukasLechnerDev commented Mar 16, 2021

I enabled data donation in the onboarding, and then scanned a positive test and submitted my keys.

This is the analytics data:

client_metadata {
  android_api_level: 30
  app_config_e_tag: "\"5357f74807272abd40b4aed12cdc0e9b\""
  cwa_version {
    major: 1
    minor: 15
  }
  enf_version: 18210613000
}
exposure_risk_metadata_set {
  date_changed_compared_to_previous_submission: true
  most_recent_date_at_risk_level: -1
  risk_level_changed_compared_to_previous_submission: true
}
test_result_metadata_set {
  days_since_most_recent_date_at_risk_level_at_test_registration: -1
  hours_since_high_risk_warning_at_test_registration: -1
  risk_level_at_test_registration: RISK_LEVEL_LOW
  risk_level_at_test_registration_value: 2
  test_result: TEST_RESULT_POSITIVE
  test_result_value: 3
}
user_metadata {
}

and this is stored in analytics_key_sumission_localdata.xml:

<map>
    <long name="analytics_key_submission_testRegisteredAt" value="1615890580496" />
    <int name="analytics_key_submission_daysSinceMostRecentDateAtRiskLevelAtTestRegistration" value="-1" />
    <int name="analytics_key_submission_lastSubmissionFlowScreen" value="2" />
    <boolean name="analytics_key_submission_submitted" value="true" />
    <long name="analytics_key_submission_submittedAt" value="1615890590310" />
    <int name="analytics_key_submission_riskLevelAtTestRegistration" value="2" />
</map>

kolyaopahle
kolyaopahle previously approved these changes Mar 16, 2021
View reviewed changes
Copy link
Contributor

@kolyaopahle kolyaopahle left a comment

Choose a reason for hiding this comment

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

one minor comment, nothing merge blocking, love the tests 🥇 👍

@chiljamgossow chiljamgossow added the author merge PR to be merged by author label Mar 16, 2021
@chiljamgossow chiljamgossow force-pushed the fix/data-donation-key-submission branch from 285a98c to 3a9af47 Compare March 16, 2021 14:32
@chiljamgossow
Copy link
Contributor Author

chiljamgossow commented Mar 16, 2021

I enabled data donation in the onboarding, and then scanned a positive test and submitted my keys.

This is the analytics data:

client_metadata {
  android_api_level: 30
  app_config_e_tag: "\"5357f74807272abd40b4aed12cdc0e9b\""
  cwa_version {
    major: 1
    minor: 15
  }
  enf_version: 18210613000
}
exposure_risk_metadata_set {
  date_changed_compared_to_previous_submission: true
  most_recent_date_at_risk_level: -1
  risk_level_changed_compared_to_previous_submission: true
}
test_result_metadata_set {
  days_since_most_recent_date_at_risk_level_at_test_registration: -1
  hours_since_high_risk_warning_at_test_registration: -1
  risk_level_at_test_registration: RISK_LEVEL_LOW
  risk_level_at_test_registration_value: 2
  test_result: TEST_RESULT_POSITIVE
  test_result_value: 3
}
user_metadata {
}

and this is stored in analytics_key_sumission_localdata.xml:

<map>
    <long name="analytics_key_submission_testRegisteredAt" value="1615890580496" />
    <int name="analytics_key_submission_daysSinceMostRecentDateAtRiskLevelAtTestRegistration" value="-1" />
    <int name="analytics_key_submission_lastSubmissionFlowScreen" value="2" />
    <boolean name="analytics_key_submission_submitted" value="true" />
    <long name="analytics_key_submission_submittedAt" value="1615890590310" />
    <int name="analytics_key_submission_riskLevelAtTestRegistration" value="2" />
</map>

Please test again. The key_submission_metadata_set must be present if data donation is turned on before receiving a positive test result and keys are submitted.

@chiljamgossow chiljamgossow removed the author merge PR to be merged by author label Mar 16, 2021
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 17, 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

82.5% 82.5% Coverage
0.0% 0.0% Duplication

@LukasLechnerDev
Copy link
Contributor

LukasLechnerDev commented Mar 17, 2021

Tested it again:

analytics_key_sumission_localdata.xml:

<map>
    <long name="analytics_key_submission_testRegisteredAt" value="1615969209626" />
    <int name="analytics_key_submission_daysSinceMostRecentDateAtRiskLevelAtTestRegistration" value="0" />
    <boolean name="analytics_key_submission_submitted" value="true" />
    <long name="analytics_key_submission_submittedAt" value="1615969225440" />
    <int name="analytics_key_submission_lastSubmissionFlowScreen" value="2" />
    <long name="analytics_key_submission_testResultReceivedAt" value="1615969209623" />
    <int name="analytics_key_submission_riskLevelAtTestRegistration" value="2" />
</map>

AnalyticsData:

# de.rki.coronawarnapp.server.protocols.internal.ppdd.PpaData$PPADataAndroid@5f6d86be
client_metadata {
  android_api_level: 30
  app_config_e_tag: "\"5357f74807272abd40b4aed12cdc0e9b\""
  cwa_version {
    major: 1
    minor: 15
  }
  enf_version: 18210613000
}
exposure_risk_metadata_set {
  date_changed_compared_to_previous_submission: true
  most_recent_date_at_risk_level: -1
  risk_level_changed_compared_to_previous_submission: true
}
key_submission_metadata_set {
  hours_since_high_risk_warning_at_test_registration: -1
  last_submission_flow_screen: SUBMISSION_FLOW_SCREEN_TEST_RESULT
  last_submission_flow_screen_value: 2
  submitted: true
}
test_result_metadata_set {
  hours_since_high_risk_warning_at_test_registration: -1
  risk_level_at_test_registration: RISK_LEVEL_LOW
  risk_level_at_test_registration_value: 2
  test_result: TEST_RESULT_POSITIVE
  test_result_value: 3
}
user_metadata {
}

key_submission_metadata_set is now included.

@chiljamgossow chiljamgossow merged commit e94169b into release/1.15.x Mar 17, 2021
@chiljamgossow chiljamgossow deleted the fix/data-donation-key-submission branch March 17, 2021 13:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@LukasLechnerDev LukasLechnerDev LukasLechnerDev approved these changes

@kolyaopahle kolyaopahle kolyaopahle approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@LukasLechnerDev LukasLechnerDev

@kolyaopahle kolyaopahle

Labels

bug Something isn't working maintainers Tag pull requests created by maintainers prio PRs to review first.

Projects

None yet

Milestone

1.15.0

Development

Successfully merging this pull request may close these issues.

5 participants

@chiljamgossow @LukasLechnerDev @kolyaopahle @ralfgehrer