fix(user): hide delete button for self #1535
Conversation
|
Coverage remained the same at 73.224% when pulling 16626bd94cae345aa5f9cff1c3e6e71865fe12b2 on sujeethk:fixes1531 into e3eafa6 on meanjs:master. |
| @@ -8,7 +8,7 @@ <h1 ng-bind="vm.user.username"></h1> | |||
| <a class="btn btn-primary" ui-sref="admin.user-edit({userId: vm.user._id})"> | |||
| <i class="glyphicon glyphicon-edit"></i> | |||
| </a> | |||
| <a class="btn btn-primary" ng-click="vm.remove()" ng-if="vm.user._id !== vm.authentication.user._id"> | |||
| <a class="btn btn-primary" ng-click="vm.remove()" ng-if="vm.user.username !== vm.authentication.user.username"> | |||
There was a problem hiding this comment.
Using username here is OK. It always feels weird to me though, to compare using a field like this, vs the actual _id or some other more intentional field.
Instead of doing the comparison in the template, how about we create a flag that we manage in the controller using vm? Something like: vm.isCurrentUserSelf = isSelf(); in the controller. In this case, I feel better about using username.
By doing it this way, we can avoid the need to update the template if we ever change how we're determined what "self" means in this context.
There was a problem hiding this comment.
Or maybe a better name for the view-model field would be vm.isContextUserSelf or vm.isEditedUserSelf.. "isCurrentUserSelf" seems confusing.
There was a problem hiding this comment.
@mleanos did you mean client controller or server controller? reason I ask is the existing 'isCurrentUserOwner' is done on the server controller. I believe you mean client controller as you referred vm and I am ok with that approach. Just wanted to make sure you are ok if it is not consistent across
There was a problem hiding this comment.
Yep. I meant client-side. Just to keep the logic out of the view.
|
Updated per comments. Hoping coveralls doesn't drop. |
Remove delete button for self Fixes #1531
|
Coverage remained the same at 73.224% when pulling c3abe6d3728d134e0214c30d42da674909f87761 on sujeethk:fixes1531 into e3eafa6 on meanjs:master. |
fix(user): hide delete button for self
Remove delete button for self
Fixes #1531