search

WOEIP / truck-tracker

trucktracker.net
1 stars 0 forks source link

Ajay account changes #70

Closed Ajay-Vishwanath closed 3 years ago

Ajay-Vishwanath commented 3 years ago

This Pull Request implements a few changes:

  1. Changes 'Register' to 'Sign Up'
  2. Logs in users after registration and pushes them to the Report page where they see a confirmation message
  3. Adds basic frontend validations for User Registration

To be ready for MVP Account Registration, we still need a little backend work that:

  1. Adds a uniqueness check to the username field in the user table (currently in the create-users-table migration .unique() is only called on the email field and not on the username field.
  2. Passes the appropriate error forward in the users.post route. Then we can display that error on the frontend rather than having a generic 500 level error.

Happy to try and implement both those changes after another discussion on the backend dev environment/refamiliarizing the SSH workflow. A lot of the changes showing in the PR are because I linted the Report.js and RegistrationPage.js components which I can avoid doing for further work.

motching commented 3 years ago

Hi, I'm not too apt in using this github workflow, do you get my mail if I reply without adding you manually? :)

Anyway. I added some comments, nothing complex. Once those are fixed or ditched you could merge this. If you find an easy fix for the automatic login post-registration, feel free to push, otherwise I'll look at it. Thanks!

On Tue, Jan 5, 2021 at 4:01 PM Ajay Vishwanath notifications@github.com wrote:

@Ajay-Vishwanath https://github.com/Ajay-Vishwanath requested your review on: #70 https://github.com/WOEIP/truck-tracker/pull/70 Ajay account changes.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/WOEIP/truck-tracker/pull/70#event-4172592807, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ54WONNTIEOPI43NRVHPTSYOR4BANCNFSM4VWTZAJA .

Ajay-Vishwanath commented 3 years ago

Hi, I'm not too apt in using this github workflow, do you get my mail if I reply without adding you manually? :) Anyway. I added some comments, nothing complex. Once those are fixed or ditched you could merge this. If you find an easy fix for the automatic login post-registration, feel free to push, otherwise I'll look at it. Thanks! … On Tue, Jan 5, 2021 at 4:01 PM Ajay Vishwanath @.***> wrote: @Ajay-Vishwanath https://github.com/Ajay-Vishwanath requested your review on: #70 <#70> Ajay account changes. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#70 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ54WONNTIEOPI43NRVHPTSYOR4BANCNFSM4VWTZAJA .

I am getting mail notified about your replies! Thanks for looking the PR over - I will implement those changes, push, and merge. I have 2 questions :

  1. What is happening when you register an account with this branch? The login API call isn't working on your end?
  2. Once the branch is merged to master, will that automatically reflect in Trucktracker.net or will there need to be more action?

Thanks!

motching commented 3 years ago

The login API call works, just not immediately after registration. I'm suspecting some flaw in logic or maybe a DB race condition? I'll have to look at that (if you don't happen to spot it).

There was some autodeploy process but it doesn't kick in any more. You're right though, it's better to be a 100% what we're doing, I'll polish the deploy process tonight. If your changes are in by then, I'll merge, if not, you can do it in the upcoming days.

Good work with the messaging component, it's coming together! I know it's hard to build something with this little established guidance, I appreciate it. :)

The password reset backend part and sending out the token via email works now, I'm working on the landing page of that, that should be relatively quick. After that, I really get down to write some documentation, I promise.

On Wed, Jan 6, 2021 at 1:27 PM Ajay Vishwanath notifications@github.com wrote:

Hi, I'm not too apt in using this github workflow, do you get my mail if I reply without adding you manually? :) Anyway. I added some comments, nothing complex. Once those are fixed or ditched you could merge this. If you find an easy fix for the automatic login post-registration, feel free to push, otherwise I'll look at it. Thanks! … <#m-4690577018270606906> On Tue, Jan 5, 2021 at 4:01 PM Ajay Vishwanath @.***> wrote: @Ajay-Vishwanath https://github.com/Ajay-Vishwanath https://github.com/Ajay-Vishwanath requested your review on: #70 https://github.com/WOEIP/truck-tracker/pull/70 <#70 https://github.com/WOEIP/truck-tracker/pull/70> Ajay account changes. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#70 (comment) https://github.com/WOEIP/truck-tracker/pull/70#event-4172592807>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ54WONNTIEOPI43NRVHPTSYOR4BANCNFSM4VWTZAJA .

I am getting mail notified about your replies! Thanks for looking the PR over - I will implement those changes, push, and merge. I have 2 questions :

  1. What is happening when you register an account with this branch? The login API call isn't working on your end?
  2. Once the branch is merged to master, will that automatically reflect in Trucktracker.net or will there need to be more action?

Thanks!

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/WOEIP/truck-tracker/pull/70#issuecomment-755724785, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ54WKKUXBMIR3R3RQQBC3SYTIVFANCNFSM4VWTZAJA .

Ajay-Vishwanath commented 3 years ago

@motching Interesting - it works immediately on my end, that will need to be fixed. We'll see how it works when deployed and go from there.

And np happy to help and I will definitely have time to help build out the UI for the forgot password / photo upload features, that's exciting you have the backend for that setup. On Friday, we can talk about the remaining very small backend work needed to complete the account registration MVP, perhaps I could hop in and work on that after documentation or a development environment refresher call.

Thank you!