Use confirmLogin and cancelLogin from broadcasted event rather than depending on a service

[christianrondeau]

Hi!

Here's the pull request for issue #57; I put together an implementation with as few changes and bells and whistles as possible. Let me know what you think. I also updated the README.md file, though again I didn't want to rewrite what didn't need to. It might be worth it though.

One last comment; I encourage you to use grunt or gulp to run jshint and maybe a few unit tests; it makes it easier to ensure someone doesn't break anything :)

Here is a simple example where myLoginService.promptLogin() is a custom service that shows a modal window and returns a promise.

$scope.$on('event:auth-loginRequired', function(e, args) {
    myLoginService.promptLogin().then(
        function() {
            args.confirmLogin();
        },
        function() {
            args.cancelLogin();
        });
});

I'll let you (or someone else) peer review the commit, especially the function names and event names. Since we're breaking backward compatibility, it would be a good timing to review them :) Let me know what you think!

[christianrondeau]

Merged into master, ready to go; any plans to merge this pull request?

[witoldsz]

What do confirmLogin() and cancelLogin() are supposed to do? Do they actually confirm or cancel anything?

[christianrondeau]

See #57, I'm not sure about your questions; are you suggesting another name, or do you ask about what these methods actually do?

In any case, these methods will do exactly the same thing as authService.loginConfirmed and authService.loginCancelled, but using the event instead of authService. I chose to rename the function names to an action (I would expect that function to be a verb in the present tense rather than in the past tense since it does something) but if you disagree, I can bring back the past tense.

Let me know.

[witoldsz]

The loginConfirmed method tells the auth module that (here it comes) login was confirmed, or login was cancelled. Your method name implies that the auth module, or whoever sits behind the callback, is about to confirm or cancel the login, which is totally confusing.

[christianrondeau]

That's all right with me... So are you saying I should create another pull request or are you dropping it?

[witoldsz]

As I said in #57, I will not pull it into my code now. I will leave it open not to forget it. There is no need to open another pull request. Whatever you commit into the branch updates this.

[christianrondeau]

Solves #53 by removing the service altogether :)

[vlapo]

Hi @christianrondeau. I was thinking about this change last days and this is my case (use in real app) where remove service will complicate all things.

I use some kind of dialog with own controller to get credentials and login

$rootScope.$on('event:auth-loginRequired', function() {
    // Open dialog popup to get credentials from user and login
});

Dialog controller, where authService is angular-http-auth service.

$scope.tryLogin = function() {
    $http.post('/auth/login', $scope.credentials, {skipAuthorization: true})
    .success(function(response) {
        // more my stuff here - not important
        authService.loginConfirmed('success', function(config) {
            // some mess I need to be done with config (using response)
            return config;
        });
    }).error(function(response) {
        // some stuff to handle wrong credentials
    });
}

IMHO this "public" service is core functionality of module and cannot be remove. Please feel free to criticize my case or way of thinking about this service :)