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

#159987721 List and update Users functionality #37

Merged
merged 1 commit into from
Sep 10, 2018

Conversation

CEOehis
Copy link
Contributor

@CEOehis CEOehis commented Sep 10, 2018

What does this PR do?

prevent user from updating other users profiles

Description of Task to be completed?

Users could previously update other users' profile. To prevent this
we simply check the user in the current session and compare it with the
record being updated.

  • Include relevant tests

How should this be manually tested?

Run npm run migrate
Run npm run start:dev
send a PUT request to api/users/<userId> to update user profile

What are the relevant pivotal tracker stories?

#159987721

Users could previously update other users' profile. To prevent this
we simply check the user in the current session and compare it with the
record being updated.

- Include relevant tests
@coveralls
Copy link

Pull Request Test Coverage Report for Build 256

  • 13 of 13 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 93.628%

Totals Coverage Status
Change from base Build 246: 0.06%
Covered Lines: 1085
Relevant Lines: 1135

💛 - Coveralls

error.status = 401;
return next(error);
}

if (!isValid) {
return res.status(400).json({ errors });
}
Copy link
Contributor

@d-beloved d-beloved Sep 10, 2018

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@emekafredy emekafredy left a comment

Choose a reason for hiding this comment

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

LGTM

@aknwosu aknwosu merged commit 8d2c5c6 into develop Sep 10, 2018
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

5 participants