guanacone / fullstack_app

BSD Zero Clause License
0 stars 0 forks source link

User edit #48

Closed guanacone closed 3 years ago

guanacone commented 3 years ago

I separated the functionality to update user data and password. To update the password I created a new route and controller.

The idea is to have the user enter his old password and have it checked with UserSchema.methods.isValidPassword defined in server/models/user.js like it is used in ../auth/index.js in the login strategy. Unfortunately I get an error Error: data and hash arguments required when trying to compare the entered password with the user method. Any hint on how to debug?

edwmurph commented 3 years ago

since a lot of this code is in your other PR, i'd recommend us holding off on this until that's merged so we dont have to review the same code twice. alternatively since this PR is dependent on the other PR, you could rebase this branch off the email-confirmation branch for the review and then rebase back to master when the other PR is merged, but that gets into some risky git commands we should probably go over together if you're interested in pushing this forward in parallel

guanacone commented 3 years ago

Did what you said I shouldn't do (merge email-conf to master and the rebased this branch from master. Will discuss that on thursday)... Anyways, The problem is still the same isValidPassword() that is defined in user model is raising an error. I don't understand why I can't use it my controller the same way as in login strategy of auth

edwmurph commented 3 years ago

a lot of the commits and the diff of this PR are already in master so ya something is off about git here. the simplest way to unblock this would be to create a fresh branch/PR by doing something like:

git checkout master
git pull
git checkout -b user-edit-2

and then manually make all the new changes you wanted for this PR in a new commit on that new branch and create a new PR. the git commit history in the new PR shouldn't have all these extra changes. and then this PR/branch can be deleted. or i can walk you through how to rectify this in this branch in our next meeting