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 demo user #36

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add demo user #36

wants to merge 4 commits into from

Conversation

mlbiche
Copy link
Collaborator

@mlbiche mlbiche commented Feb 21, 2022

Add a demo user so demo are easier to run : you no longer have to fill in the whole form.

@mlbiche mlbiche requested a review from vinyll February 21, 2022 19:28
Copy link
Member

@vinyll vinyll left a comment

Choose a reason for hiding this comment

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

I was guessing that the purpose was to pre-fill the form for the demo user to get straight to the main interface.
However it doesn't happen on my localhost with the current branch and the form is still empty.
How can I experiment the proposal?


function onDemoMode() {
saveAuthenticatedUser({
email: 'john.doe@email.com',
Copy link
Member

Choose a reason for hiding this comment

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

@example.com, @example.org are protected domain dedicated for this kind of usage

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed 👍

src/js/utils.js Outdated
@@ -80,13 +80,12 @@ export function isPatientValid(patient) {
}

function isEmailValid(email) {
return email.match(/[a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z0-9-]+(\.[a-z0-9-]+)*/)
return email && email.match(/[a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z0-9-]+(\.[a-z0-9-]+)*/)
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to check the existance of email here before returning and why is the structure so difference than isIbanValid()?
I guess returning a boolean would make more sense as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At some point while testing, an error was popping in the console because the patient didn't have any email, so email was undefined and email.match was throwing an error. Even if, at the end, all patients have an email, I added this check to prevent any error.
You're right about IBAN. I think it is code evolution that led to this difference. It's now fixed and homogenized. For RCC validation too.

@mlbiche
Copy link
Collaborator Author

mlbiche commented Feb 28, 2022

I was guessing that the purpose was to pre-fill the form for the demo user to get straight to the main interface. However it doesn't happen on my localhost with the current branch and the form is still empty. How can I experiment the proposal?

When clicking on "Essayer en mode démo" on the authentication panel, the panel should close itself and "Cabinet de John" should be displayed in the upper left corner. When clicking on it, the user panel should show up with all the informations pre-filled. Tell me if it won't, we can check it together

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