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

thread test date all the way through #883

Merged
merged 3 commits into from
Oct 22, 2020

Conversation

mikehelmick
Copy link
Contributor

Fixes #882

Proposed Changes

  • Add test date into verify API return
  • Thread this all the way through the verification token

Release Note

API CHANGE : /api/verify now returns the testDate in addition to symptom date if present. When the verification certificate is issue only one interval is inserted: symptomDate if present, testDate if not.

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikehelmick

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-cla google-cla bot added the cla: yes Auto: added by CLA bot when all committers have signed a CLA. label Oct 22, 2020
@mikehelmick
Copy link
Contributor Author

/hold
for verification w/ clinical team

@@ -77,8 +85,8 @@ func (s *Subject) SymptomInterval() uint32 {

func ParseSubject(sub string) (*Subject, error) {
parts := strings.Split(sub, ".")
if length := len(parts); length < 1 || length > 2 {
return nil, fmt.Errorf("subject must contain 2 parts, got: %v", length)
if length := len(parts); length < 2 || length > 3 {
Copy link
Member

Choose a reason for hiding this comment

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

Won't think break existing JWT tokens that have the old subject?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the old if statement was wrong - it should have always been 2 parts before.

Why I didn't write length != 2 the first time, we'll never know.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not that I doubt your code, but this feels like a test.

Copy link
Member

Choose a reason for hiding this comment

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

^ what Jeremy said

Copy link
Contributor Author

Choose a reason for hiding this comment

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

funny enough - the 2 segment subject was the only thing under test before.
test coverage has been boosted to cover all scenarios.

@jeremyfaller
Copy link
Contributor

/lgtm

still hold, pending clinicians.

@mikehelmick
Copy link
Contributor Author

/unhold

@google-oss-robot google-oss-robot merged commit 4af80e8 into google:main Oct 22, 2020
@mikehelmick mikehelmick deleted the issue882 branch October 23, 2020 16:58
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Auto: added by CLA bot when all committers have signed a CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Store and return testDate on verify code API
4 participants