-
Notifications
You must be signed in to change notification settings - Fork 312
Conversation
* Add key revision columns to exposure table * This includes capturing the health authority ID (if provided) * Keys can only be revised by the same health authority ID * Implement setting of Exposure fields based on the claims from the verification certificate * If a ReportType is present, set it in the Exposure.ReportType * Optionally, backfill the transmission risk, based on the report type * Calculate the days +/- sypmtom onset * the symptom onset interval can be provided in the API or from the verification certificate * stub out key revision * existing keys are read from the DB if they match the input * TODO: merge existing and input keys * TODO: implement transactional key revision * IterateExposures * add ability to select revised keys More work on google#663
t.Run(tc.name, func(t *testing.T) { | ||
got := DaysFromSymptomOnset(tc.onset, tc.check) | ||
if tc.want != got { | ||
t.Fatalf("wrong day instance between %v and %v, want: %v got: %v", tc.onset, tc.check, tc.want, got) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got before want
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah... fixed this, but we have this problem everywhere.
TransmissionRisk int | ||
AppPackageName string | ||
Regions []string | ||
IntervalNumber int32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why int32? Why not uint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this has always been int32 - and aligns w/ the proto type in the export file.
|
||
// ExposureKeyBase64 returns the ExposuerKey property base64 encoded. | ||
func (e *Exposure) ExposureKeyBase64() string { | ||
return base64.StdEncoding.EncodeToString(e.ExposureKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
STD or RAW? Padded or non?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exposure keys have always been StdEncoding in our use in the database.
@@ -215,7 +215,7 @@ func TestPublishWithBypass(t *testing.T) { | |||
Error: "unable to validate diagnosis verification: token contains an invalid number of segments", | |||
}, | |||
{ | |||
Name: "valid HA certificate", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What’s different here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting / whitespace
|
||
-- health_authority_id is nullable to preserve backwards compatibility. | ||
ALTER TABLE exposure | ||
ADD COLUMN health_authority_id INT REFERENCES HealthAuthority(id), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Index this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't in any query at the moment.
will index later if that changes.
This will be used if a key is revised, we'll assert that it is being revised by the same health authority ID.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mikehelmick, sethvargo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* Work on v1.5 * Add key revision columns to exposure table * This includes capturing the health authority ID (if provided) * Keys can only be revised by the same health authority ID * Implement setting of Exposure fields based on the claims from the verification certificate * If a ReportType is present, set it in the Exposure.ReportType * Optionally, backfill the transmission risk, based on the report type * Calculate the days +/- sypmtom onset * the symptom onset interval can be provided in the API or from the verification certificate * stub out key revision * existing keys are read from the DB if they match the input * TODO: merge existing and input keys * TODO: implement transactional key revision * IterateExposures * add ability to select revised keys More work on google#663 * review comments * got/want fix * tabs
* Work on v1.5 * Add key revision columns to exposure table * This includes capturing the health authority ID (if provided) * Keys can only be revised by the same health authority ID * Implement setting of Exposure fields based on the claims from the verification certificate * If a ReportType is present, set it in the Exposure.ReportType * Optionally, backfill the transmission risk, based on the report type * Calculate the days +/- sypmtom onset * the symptom onset interval can be provided in the API or from the verification certificate * stub out key revision * existing keys are read from the DB if they match the input * TODO: merge existing and input keys * TODO: implement transactional key revision * IterateExposures * add ability to select revised keys More work on google#663 * review comments * got/want fix * tabs
* Work on v1.5 * Add key revision columns to exposure table * This includes capturing the health authority ID (if provided) * Keys can only be revised by the same health authority ID * Implement setting of Exposure fields based on the claims from the verification certificate * If a ReportType is present, set it in the Exposure.ReportType * Optionally, backfill the transmission risk, based on the report type * Calculate the days +/- sypmtom onset * the symptom onset interval can be provided in the API or from the verification certificate * stub out key revision * existing keys are read from the DB if they match the input * TODO: merge existing and input keys * TODO: implement transactional key revision * IterateExposures * add ability to select revised keys More work on google#663 * review comments * got/want fix * tabs
More work on #663