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

IEEE-209: Added acknowledgment form and participant onboarding #430

Merged
merged 14 commits into from Sep 22, 2022

Conversation

YuYing-Liang
Copy link
Member

Overview

  • Resolves #IEEE-209
  • Added user acceptance checking (get api thunk)
  • copied over acknowledgement form from old PR (Acknowledgement Form Connected to Formik #266) and converted to typescript
  • added create profile thunk
  • please include your feedback about the design!

Unit Tests Created

  • unit tests made for all new components and thunks

Steps to QA

  • create a normal user
  • login as them into the HSS with these situations:
    - when the user hasn't completed their application, the page should show an incomplete status message
    - when the user has been waitlisted (yellow alert) or rejected (red alert), they should see appropriate messages for those review statuses
    - when the user has been accepted they should see a green message box with a "Get Started" button
    - click that button and it should bring up the acknowledgement form
    - A popup should appear when you click "Waiver of Liability and Hold Harmless Agreement" which has the contents of the waiver
    - you need to check everything on the form & input an e signature to enable the "Continue" button
    - If the user hasn't RSVP'd to the event then an error should appear
    - If the user has RSVP'd then they should see a success message with their name and team code and a link to go to their dashboard page
  • please try to break this and find other edge cases

@YuYing-Liang YuYing-Liang changed the title Added acknowledgment form and participant onboarding IEEE-209: Added acknowledgment form and participant onboarding Aug 21, 2022
@YuYing-Liang
Copy link
Member Author

Email checks failing, pls ignore for now

Copy link
Contributor

@Leo6Leo Leo6Leo left a comment

Choose a reason for hiding this comment

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

create a normal user ✅
login as them into the HSS with these situations:

  • when the user hasn't completed their application, the page should show an incomplete status message ❌
  • when the user has been waitlisted (yellow alert) or rejected (red alert), they should see appropriate messages for those review statuses ✅
  • when the user has been accepted they should see a green message box with a "Get Started" button ✅
  • click that button and it should bring up the acknowledgement form ✅
  • A popup should appear when you click "Waiver of Liability and Hold Harmless Agreement" which has the contents of the waiver ✅
  • you need to check everything on the form & input an e signature to enable the "Continue" button ✅
  • If the user hasn't RSVP'd to the event then an error should appear ✅
  • If the user has RSVP'd then they should see a success message with their name and team code and a link to go to their dashboard page ✅

Haven't found any other buggs yet other than the first one.

Copy link
Contributor

@Leo6Leo Leo6Leo left a comment

Choose a reason for hiding this comment

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

Second round of review

import Typography from "@material-ui/core/Typography/Typography";
import { hackathonName } from "constants.js";

const Waver = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using MUI dialog? then we will have the transition animation without using react-transition-group

Copy link
Member Author

Choose a reason for hiding this comment

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

the waiver uses teh mui modal component and isn't attached to any transition group, the acknowledgement form is part of the transition though


export const UserHasBeenGrantedAccessMessage = () => (
<>
<Typography variant="h1">
Copy link
Contributor

Choose a reason for hiding this comment

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

Styling can be improved. I can make the Figma design if necessary.

<Grid container justifyContent="center">
<Grid item lg={6} xs={12}>
<Card style={{ padding: "15px 5px" }}>
<Typography style={{ fontSize: "25px" }} align="center">
Copy link
Contributor

Choose a reason for hiding this comment

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

Styling can be improved. I can make the Figma design if necessary.

Minor thing: we can capitalize the first letter of user's name so that it looks professional. Sometimes when user fills their application, they don't pay attention to the capital letter. (i.e I applied with my name as leo li, and on the website, it should display Leo there to make it look nice).

Copy link
Member Author

Choose a reason for hiding this comment

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

updated the naming

userAcceptance: {
user: null,
isLoading: false,
error: null,
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add a new field to record the application state? Bcz review_state will appear only if there is a review record in the database. If the user hasn't submitted their application, they won't be able to have the review_state

Copy link
Contributor

Choose a reason for hiding this comment

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

And that might be the reason why we are failing the first step as I mentioned in my comment

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed!

} catch (e: any) {
dispatch(
displaySnackbar({
message: `Failed to grant permissions to Hardware Signout Site: Error ${e.response.status}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

The message is kinda confusing from my view: something like "failed when granting permissions to *** to access HSS" would be a choice.

Copy link
Member Author

Choose a reason for hiding this comment

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

changed!

@YuYing-Liang
Copy link
Member Author

@Leo6Leo thanks for the review! I'm going to hold off on styling changes right now because of the urgency of this ticket, other feedback as been addressed though!

Also, try to be more descriptive when you say "Styling can be improved" I know you mention you can re-design it but maybe add more comments about the design to give the developer an idea of what they should change.

@Leo6Leo Leo6Leo self-requested a review September 21, 2022 16:14
Copy link
Contributor

@Leo6Leo Leo6Leo left a comment

Choose a reason for hiding this comment

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

Perfect Yuying!! The suggestions I have mentioned are all fixed! I will be more specific next time so that it will not confuse the developers!

@Leo6Leo Leo6Leo self-requested a review September 22, 2022 05:06
Copy link
Contributor

@Leo6Leo Leo6Leo left a comment

Choose a reason for hiding this comment

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

Great Work!

@YuYing-Liang YuYing-Liang merged commit 695ce1f into develop Sep 22, 2022
@YuYing-Liang YuYing-Liang deleted the IEEE-209-acknowledgement-page branch September 22, 2022 12:46
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.

None yet

2 participants