[GitHub] guacamole-client pull request #342: GUACAMOLE-598: Ignore if current user ha...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/342#discussion_r239300359
--- Diff:
guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js ---
@@ -95,13 +95,10 @@ angular.module('navigation').directive('guacUserMenu',
[function guacUserMenu()
*/
$scope.role = null;
-// Pull user data
+// Display user profile attributes if available
userService.getUser(authenticationService.getDataSource(),
$scope.username)
.then(function userRetrieved(user) {
-// Store retrieved user object
-$scope.user = user;
--- End diff --
> Just want to make sure this doesn't have any impact? Was this $scope.user
not used at all?
It wasn't used at all. I removed this unnecessary assignment to
`$scope.user` after a brief "how is this working at all???" moment. As that
assignment will only occur if `userService.getUser()` succeeds, anything which
depends on it would fail in cases where that failure previously resulted in a
warning, yet clearly those cases (CAS, OpenID, header auth) work just fine.
The relevant template only references `$scope.username`. Neither the
template nor the directive reference `$scope.user`.
The `$scope.user` assignment was introduced briefly via e9549fb, at which
point it should have been documented (but wasn't). The usefulness of that
assignment was removed via 06fb054. Both of these changes were due to
[GUACAMOLE-292](https://issues.apache.org/jira/browse/GUACAMOLE-292), part of
0.9.13-incubating.
---
[GitHub] guacamole-client pull request #342: GUACAMOLE-598: Ignore if current user ha...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/342#discussion_r239272644
--- Diff:
guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js ---
@@ -95,13 +95,10 @@ angular.module('navigation').directive('guacUserMenu',
[function guacUserMenu()
*/
$scope.role = null;
-// Pull user data
+// Display user profile attributes if available
userService.getUser(authenticationService.getDataSource(),
$scope.username)
.then(function userRetrieved(user) {
-// Store retrieved user object
-$scope.user = user;
--- End diff --
Ping @mike-jumper - just wanted to verify this and then I think I can push
forward with merging this PR.
---
[GitHub] guacamole-client pull request #342: GUACAMOLE-598: Ignore if current user ha...
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/342#discussion_r238886562
--- Diff:
guacamole/src/main/webapp/app/navigation/directives/guacUserMenu.js ---
@@ -95,13 +95,10 @@ angular.module('navigation').directive('guacUserMenu',
[function guacUserMenu()
*/
$scope.role = null;
-// Pull user data
+// Display user profile attributes if available
userService.getUser(authenticationService.getDataSource(),
$scope.username)
.then(function userRetrieved(user) {
-// Store retrieved user object
-$scope.user = user;
--- End diff --
Just want to make sure this doesn't have any impact? Was this $scope.user
not used at all?
---
