Feature/sup 2663

[tladendo]

fyi @nlitwin @parthshah @vikasrohit

[nlitwin]

Do we need data: {}?

[parthshah]

No, we can remove that line.
-P

On Thu, Dec 10, 2015 at 10:04 AM, Nicholas Litwin [email protected]
wrote:

In app/services/tcAuth.service.js
#606 (comment)
:

@@ -162,6 +163,19 @@
}

 function logout() {

•  $http({
  

•    url: apiUrl + '/authorizations/1',
  

•    method: 'DELETE',
  

•    headers: {
  

•      'Authorization': "Bearer " + AuthTokenService.getV3Token()
  

•    },
  

•    data: {}
  

Do we need data: {}?

—
Reply to this email directly or view it on GitHub
https://github.com/appirio-tech/topcoder-app/pull/606/files#r47260868.

Thanks
Parth Shah

[parthshah]

I don't think this will quite work. $http call is a promise so even before it's resolved AuthTokenService would be called. I think the solution is to chain these promises

[nlitwin]

Would this work?

return $http({
    ...
}).then(function(res) {
    AuthTokenService.removeTokens();
    $rootScope.$broadcast(CONSTANTS.EVENT_USER_LOGGED_OUT);
}).catch(function(err) {
   // handle error
});

[parthshah]

I think so :). As long we log exceptions.

[nlitwin]

I think we should move more toward naming variables and passing them in. To me, this would be preferable:

function logout() {
      var logoutOptions = {
        url: apiUrl + '/authorizations/1',
        method: 'DELETE',
        headers: {
          Authorization: 'Bearer ' + AuthTokenService.getV3Token()
        }
      };

      var removeTokens = function() {
            $log.log('Logout successful.');

            AuthTokenService.removeTokens();
            $rootScope.$broadcast(CONSTANTS.EVENT_USER_LOGGED_OUT);
      };

      // Name below according to what it does
      var describesWhatHappensOnError = function(error) {
            $log.error(error);
      };

      $http(logoutOptions)
      .then(removeTokens)
      .catch(describesWhatHappensOnError);
}

What do you guys think?

[tladendo]

@parthshah @nlitwin -- the way I'm doing it now works. These don't need to be chained, since it's fine if the tokens get removed before the DELETE request goes through. I can do the logoutOptions thing to make this cleaner, but I think it's fine to have these calls be independent.

[nlitwin]

If it is indeed ok to remove the tokens before the DELETE request is finished, do we need $q? I think we might be able to get rid of $q either way since there is nothing asynchronous to wait for with AuthTokenService.removeTokens(); and $rootScope.$broadcast(CONSTANTS.EVENT_USER_LOGGED_OUT); Maybe I'm just not seeing something