fix(users): Incorrect UsersService injection #1283
Conversation
|
I can confirm that this is indeed a bug in latest master, thanks @t-jennings |
|
LGTM. Other than the commit message. @t-jennings Could you change it to something like Also, it doesn't look like we have coverage here for the Edit Profile route. Would it be possible to add something to this PR? I'm fine with merging this in as is, since it's a small change & fixes a bug. However, we should start building out our test coverage in these area's. |
|
@mleanos it would be nice to get a test for that to increase our coverage. |
t-jennings
changed the title
|
Updated title to follow convention. I won't be able to provide a test unfortunately, not familiar enough with testing yet. This is my first PR so it would be awesome if this got merged. |
|
@t-jennings That's understandable about the test. I think it's ok for this to be merged without an accommodating test. @ilanbiala I'm also not opposed to submitting a test myself to this branch. @t-jennings If I submitted a pull request to this branch, would you be able to merge it in? Also, we're looking for the actual git commit message to be updated. Not the title of the PR. |
…m/t-jennings/mean into fix-users-edit-profile-controller
|
Added commit with correct format for commit message. @mleanos , if you want to submit a PR to add testing I can merge it in. |
|
@mleanos adding a test to this via PR sounds good. |
|
@ilanbiala @t-jennings I've submitted a PR to this PR's branch. Another thing I noticed about the Edit Profile Controller, is that it's injecting the $location service, but not using it. We can probably get rid of it. @t-jennings Could you remove it? |
|
Any movement on this? |
|
@ilanbiala @t-jennings We can merge this in as is, and I'll add my test in a separate PR. This is a pretty simple PR, and fixes a bug. So it might be best to just get it merged. WDYT? See t-jennings#1 for the suggested test coverage. |
|
SGTM. |
Adds client-side tests for the Users Edit Profile client controller. 1) should have user context 2) should update the user profile 3) should set vm.error if error Related meanjs#1283
Adds client-side tests for the Users Edit Profile client controller. 1) should have user context 2) should update the user profile 3) should set vm.error if error Related #1283
Updated edit profile controller to use the users service correctly. Edit profile now working. Closes #1282.