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

Draft: 346: account management #384

Open
wants to merge 68 commits into
base: develop
Choose a base branch
from
Open

Draft: 346: account management #384

wants to merge 68 commits into from

Conversation

wendy-wm-wu
Copy link
Collaborator

Checklist

  • Add description
  • Reference the open issue that the pull request addresses
  • Pass code quality checks
    • spin up docker docker-compose up -d --build
    • enter api container docker-compose exec api /bin/bash
    • run api tests make validate
    • exit container ctrl/command+D or exit
    • enter web container docker-compose exec web /bin/sh
    • run front-end tests npm run test or npx jest
    • lint npm run lint-fix
    • exit container as above
  • Request code review
    • Please allow 36 hours from opening a pull request before merging a pull request- even if it has already received an approving review.
  • Address comments on code and resolve requested changes
  • Merge own code

Description

Issue: #346

Brief description of solution
This is a POC for account management. This ticket will register, log-in, load, and log-out a user; it also verifies authentication and generates a JWT token.

The password reset feature is not complete and will be taken up in a different story. We also will include further CSS changes as a separate story.

@TangoYankee
Copy link
Member

@wendy-wm-wu , would you be open to splitting this into two separate PRs- one for the front end and another for the backend? It should make it simpler review each one individually.

@wendy-wm-wu
Copy link
Collaborator Author

wendy-wm-wu commented Dec 13, 2021

@TangoYankee - Sorry for the late reply. I keep forgetting to check my other email that's linked to this GitHub. Since this is a PR with so many commits between both Alana and myself, I think it would be easier to leave as is if that's okay. In the future, we won't have a large MR like this and will split into frontend/backend.

Also, thank you so much for reviewing and for your thoughtful comments. I'll take a look soon.

@wendy-wm-wu
Copy link
Collaborator Author

@TangoYankee - Did you check your email? I think you have to click the url provided in the email, which will take you back to the site. There should be a button (still requires some CSS changes) on the page that you'll then click to activate your account.

@TangoYankee
Copy link
Member

@TangoYankee - Did you check your email? I think you have to click the url provided in the email, which will take you back to the site. There should be a button (still requires some CSS changes) on the page that you'll then click to activate your account.

Even if we are utilizing email as part of the account creation process, the initial request should not return an error.

@sribbleinc
Copy link
Collaborator

@TangoYankee Both Wendy and I have never seen this error before. Would you be able to record yourself producing the error to send to us? Or let us know if you want to schedule a time to go over it together.

@exchrotek exchrotek added this to In Review in Sprint 6: Ending 1/31/22 Jan 26, 2022
@exchrotek exchrotek moved this from In Review to In progress in Sprint 6: Ending 1/31/22 Jan 26, 2022
@exchrotek exchrotek moved this from In progress to In Review in Sprint 6: Ending 1/31/22 Jan 26, 2022
@TangoYankee
Copy link
Member

@wendy-wm-wu I was able to sign in successfully. However, after I signed in, attempting to upload or view data was prevented by 401 errors.

@wendy-wm-wu
Copy link
Collaborator Author

wendy-wm-wu commented Jan 31, 2022

@TangoYankee - I looked into the issue, and I think we need to send "JWT token" with every request that we make. This will also require some backend changes that I'll need to dig into.

@exchrotek exchrotek added this to in Review in Sprint 7: ending 2/28/22 Feb 2, 2022
@TangoYankee
Copy link
Member

@wendy-wm-wu The upload page is working. It seems the map page needs the same treatment.

@sribbleinc
Copy link
Collaborator

Hey @TangoYankee Wendy and I have found an issue with token persistence that we need to look into. We'll let you know when you can review again.

@wendy-wm-wu
Copy link
Collaborator Author

@TangoYankee - Oops, that MR was not ready for review yet. I pushed up the initial change, so Alana could work on it. I've been using my company laptop to develop, and last week, I had to remove Docker desktop since it won't be free for commercial use anymore. Unfortunately, the Docker alternatives that I looked into do not support docker-compose. I'm going to try to set this up on my very slow personal computer, so I can start developing again. Like Alana mentioned, we'll let you know when this MR will be ready for review. Thanks for your patience.

@wendy-wm-wu wendy-wm-wu changed the title 346: account management Draft: 346: account management Feb 12, 2022
@sribbleinc sribbleinc force-pushed the 346_signin branch 2 times, most recently from 25ade88 to 7f8bbc9 Compare February 12, 2022 03:38
@exchrotek exchrotek added this to In Review in Sprint 8: Ending 3/28/22 Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants