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

Make the validator actually validate #3584

Merged
merged 1 commit into from Feb 17, 2023
Merged

Make the validator actually validate #3584

merged 1 commit into from Feb 17, 2023

Conversation

LudvigHz
Copy link
Member

Description

Previously it only checked if you had a user, and the qr did nothing at
all. With this change, we validate that the user is a member of Abakus
and the QR scanner also displays success or error messages for
validation status.

On error, it just gives an alert. I would make it nicer if I had time. But the alert works great tbh. It makes sure that the person validating needs to confirm the message before continuing. Not very fancy but effective.

All the other changed files are just related typing improvements.

Result

User is not member of Abakus:

Screencast.from.2023-02-16.18-11-01.webm

User does not exist (only makes sense for QR):

Screencast.from.2023-02-16.18-16-03.webm

Successful validation:

Screencast.from.2023-02-16.18-24-01.webm

Testing

  • I have thoroughly tested my changes.

@LudvigHz LudvigHz added enhancement Pull requests that make enhancements, instead of just purely new features flawless Pull requests with "flawless" code, e.g. formatting or simple renaming. Should be used with caution! priority:high Pull requests that have high priority, and should therefore be prioritized review-needed Pull requests that need review small-fix Pull requests that fix something small labels Feb 16, 2023
Copy link
Member

@norbye norbye left a comment

Choose a reason for hiding this comment

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

Reviewed on mobile and the videos didn't play in the app - so do with this as you'd like, but the code looked fine to me:)

sound.play();
showCompleted();
(user: User) => {
console.log(user);
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is only here for testing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ooops. Removed, thanks :)

Copy link
Member

@juniwbjerde juniwbjerde left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor

@erlingfn erlingfn left a comment

Choose a reason for hiding this comment

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

LGTM

Previously it only checked if you had a user, and the qr did nothing at
all. With this change, we validate that the user is a member of Abakus
and the QR scanner also displays success or error messages for
validation status.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pull requests that make enhancements, instead of just purely new features flawless Pull requests with "flawless" code, e.g. formatting or simple renaming. Should be used with caution! priority:high Pull requests that have high priority, and should therefore be prioritized review-needed Pull requests that need review small-fix Pull requests that fix something small
Projects
None yet
4 participants