3
\$\begingroup\$

Our application needed a function to sign out of Google. Unfortunately, there is no such API and I was forced to use an iframe to: http://accounts.google.com/Logout and a polling mechanism to see when the operation is complete.

I had a lot of doubts whether to put this code in a directive or in a service.

On one hand it has DOM manipulation, so the instinct is to put it in a directive.

On the other hand:

  • The directive is completely invisible. Most of the time it doesn't create any DOM, and only during logout it creates a hidden iframe.
  • The usage of this directive, isn't by invoking an event in event in its template, since, as I said, it is invisible. The logout logic will be invoked in a logic in a controller/service so I will be basically calling a function inside the directive.

I would appreciate a code review regarding this points, and of course, anything else that comes to your mind:

// This directive allows a controller to log-out easily from Google
// Usage:
// <tg-google-logout logout-control="googleLogoutControl"></tg-google-logout>
// The given parameter must be an object. This object, is assigned by a directive by a single "logout" function.
// When logout is needed, it can be invoked using this object, e.g in this case in the controller you would write:
//  $scope.googleLogoutControl.logout()
// This function returns a promise. This promise gets resolved if logout was successful and rejected otherwise.
// If the user isn't logged out in MAX_LOGOUT_TIME_SECONDS
angular.module('codereview').directive('tgGoogleLogout', function () {
    'use strict';

    return {
        restrict: 'E',
        replace: true,
        template: '<iframe class="hidden" ng-if="showLogoutFrame" src="https://www.google.com/accounts/Logout"></iframe>',
        scope: {
            logoutControl: "="
        },
        controller: function ($scope, $q, $timeout, $log, googleAuthService) {

            var MAX_LOGOUT_TIME_SECONDS = 5;

            $scope.showLogoutFrame = false;

            // Polls Google until user is completely logged out
            // maxSecondsToWait - The maximum number of seconds for a logout to happen.
            // Returns a promise. Resolves when logout is detected. Rejects if logout was not detected in time.
            function waitUntilLogout(maxSecondsToWait) {
                // Default - Time to try before giving up on logout
                if (angular.isUndefined(maxSecondsToWait)) {
                    maxSecondsToWait = 5;
                }

                return googleAuthService.getAuthorizedUserEmail()
                    .then(function (email) {
                        $log.debug("getAuthorizedUserEmail - Got email: {0}".format(email));
                        if (email) {
                            maxSecondsToWait--;

                            if (maxSecondsToWait > 0) {
                                // Retry after 1000ms. The actual function happens on the next "then".
                                // NOTE: The total amount of seconds we will retry, is set by 'maxSecondsToLogout'
                                //       This controls only how often we try.
                                $log.debug("getAuthorizedUserEmail - Retrying {0} seconds left".format(maxSecondsToWait));

                                return $timeout(function () {
                                }, 1000).then(function () {
                                    return waitUntilLogout(maxSecondsToWait)
                                });
                            } else {
                                return $q.reject("Failed to logout");
                            }
                        }
                    });
            }

            if (!angular.isObject($scope.logoutControl)) {
                throw new Error("logoutControl - A control object is required to use this directive. It can be an empty object.")
            }

            if (angular.isDefined($scope.logoutControl.logout)) {
                throw new Error("logoutControl - logout property must not be already defined - Overwrite risk")
            }

            // See directive docs for usage.
            //noinspection JSPrimitiveTypeWrapperUsage
            $scope.logoutControl.logout = function () {
                // Showing the frame starts the logout process
                $scope.showLogoutFrame = true;

                return waitUntilLogout(MAX_LOGOUT_TIME_SECONDS)
                    .finally(function () {
                        // Hide the frame regardless of success/failure of logout
                        $scope.showLogoutFrame = false;
                    });
            }
        }
    };
});
\$\endgroup\$
1
\$\begingroup\$

Funny question,

your code is of commendable quality, the only thing I would have said is put it in a service.

From a once over:

  • Check for missing semicolons to make jshint.com happy.
  • 'use strict' <- Good
  • Overall logic <- Good
  • Comments <- Very good
\$\endgroup\$
  • \$\begingroup\$ Thank you for the comments and the kind words. After some pondering, I completely agree. Despite the DOM manipulations, it should be a service. \$\endgroup\$ – VitalyB Aug 25 '14 at 15:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.