Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add recovered/vaccinated option to checkin #355

Merged
merged 7 commits into from
Oct 14, 2021

Conversation

stringparser
Copy link
Contributor

Ticket: railslove/recover-backlog#179

In progress still. Adding you guys for review so you can check it or say if there something is missing.

At the moment what I have to see is if we need something extra on the checkin page.

@stringparser stringparser self-assigned this Oct 7, 2021
@stringparser stringparser changed the title add 2G option to checkin add recovered/vaccinated option to checkin Oct 7, 2021
@stringparser
Copy link
Contributor Author

Seems we already have a checkbox for confirming vaccination (see screenshot below).

@juanmendi could you test here https://rcvr-app-9zukytc9o-railslove.vercel.app/ for the modal setup and let me know if that's enough? I've updated the locales and added the new option.

image

@bitboxer
Copy link
Contributor

bitboxer commented Oct 8, 2021

@stringparser did you check if the data exporter part shows a correct value with this?

@stringparser
Copy link
Contributor Author

stringparser commented Oct 8, 2021

@bitboxer did you check if the data exporter part shows a correct value with this?

I haven't worked yet with the exporter. Could you help with this?

Comment on lines +8 to +19
switch (value) {
case 0:
case 1:
case 24:
case 48: {
return CoronaTestOptions[value]
}
default: {
return null
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried value in CoronaTestOptions and if (CoronaTestOptions[value]) but return value still showed any. This is what I could come up with without using typecasting.

@stringparser
Copy link
Contributor Author

stringparser commented Oct 8, 2021

Ok, could make it work locally (see screenshot below were the Onboarding component doesn't show the provide test option radio button).

@bitboxer not sure if we need some other adjustment. Let me know.

Here localhost checkin url for testing:

http://localhost:3000/checkin?a=a4ad1d77-e900-4626-a785-8ae945bf2c7e&k=Ox9VVDPRqhUKO9hp%2FzZo%2BBiSF%2FG%2BbFfDGqWLntVWXl4%3D

Simply start the server as normal and as far as I can tell should work for you too.

image

@stringparser stringparser marked this pull request as ready for review October 8, 2021 14:50
0: 'NO_TEST',
1: 'RECOVERED_OR_VACCINATED',
24: '24_HOUR_TEST_NEEDED',
48: '48_HOUR_TEST_NEEDED',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's time to rewrite this into propper enums?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is saved as number on the database. Actually tried to use recovered as key first but didn't work. So somehow we need the keys to be a number.

@bitboxer
Copy link
Contributor

@stringparser you just need to login as business owner and go to the export part. There you can request a new data dump and after that you can see the table there. I could show it to you via screen share if you want.

@stringparser
Copy link
Contributor Author

@stringparser you just need to login as business owner and go to the export part. There you can request a new data dump and after that you can see the table there. I could show it to you via screen share if you want.

ah I see. will check and if all looks good I'll merge

@stringparser
Copy link
Contributor Author

@stringparser you just need to login as business owner and go to the export part. There you can request a new data dump and after that you can see the table there. I could show it to you via screen share if you want.

can't figure it out. will merge since this was needed a while ago and check out with you separately. in case we need a fix we can deploy it separately.

@stringparser stringparser merged commit 9630a9c into master Oct 14, 2021
@stringparser stringparser deleted the feature/recovered-option branch October 14, 2021 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants