CUser::loginRequiredAjaxResponse improvement

So, CUser::loginRequiredAjaxResponse is now a string, which allows us to set a message to be returned to the client when needed. I feel that this is somewhat limited, and would like the option to specify content type and headers as well. My case for this is as follows:

User::init:


$this->loginRequiredAjaxResponse = json_encode(array('ajaxError'=>array('code'=>401,'redirect'=>$this->loginUrl)));

User::login (snip, I added headers)


elseif(isset($this->loginRequiredAjaxResponse))

		{

                        header('HTTP/1.1 401 Unauthorized');

                        header('Content-type: application/json');

			echo $this->loginRequiredAjaxResponse;

			Yii::app()->end();

		}

We can then have the following in our JS to catch timeouts / required logins. This will hook on before our more specialized handlers (if any):


$.ajaxSetup({

    global: true,

    error:function(jqXHR, textStatus, errorThrown){


        var localJson = eval('('+jqXHR.responseText+')');


        if(!localJson.ajaxError || typeof localJson.ajaxError == 'undefined')

        {

            return;

        }


        if(typeof localJson.ajaxError.redirect == 'string')

        {

            window.location.replace(localJson.ajaxError.redirect);

        }

    }

});

This is especially usefull when we are loading ajax-tabs etc which earlier returned the actual login page.

To be able to hook on to .error we need to set a header that’s >= 300.

My suggestion is either to have CUser::loginRequiredAjaxResponse as string|function, or just make it a protected function which people can override. It’s not a huge change, but it’s way more convenient.

EDIT: Tracked at Google Code

Note that $.ajaxSetup() just sets default values for future ajax handlers - http://api.jquery.co…uery.ajaxSetup/

If you make an ajax request with a custom "error" option… this code would not be executed… and there are already Yii code that uses "error" (one is the CGridView update for example)

So it’s not a solution that would work in all situations… that’s why a simpler approach is taken

As I wrote on the issue… the idea of this parameter is to return a "known" value (json or normal string)… that can be checked by custom user code… this way in every success method you can add the check like

if(data="loginexpired") …

This example was somewhat simplified. Implementation of a global error-handler for ajaxError is not really the problem here, it’s a matter of flexibility. Let me go for a different angle:

A server-side “login required” should definitely set 401 Unauthorized. Returning a 200 will result in the success-handler being invoked, which does not make sense in my opinion. Also, since the $loginRequiredAjaxResponse can be json it would make sense to be able to set content type for it as well. :)

Problem is on how to handle this… global ajax events are handled after the local one… so first would be called a custom code from the ajax call only then the global event… and they can be disabled by the user too

http://docs.jquery.com/Ajax_Events

Following this thinking than this should be returned even for non ajax calls… but on that case is just redirected to the login page and the header status is 200.

Well, one does not need to do global events. There are also other options like dataFilter (only for success) which is invoked before anything. Another approach would be to create a class that is invoked in any custom .error which checks for this. I still believe that success is incorrect here…

Yeah, but the initial redirect is 302, which in turn leads to login with 200. This could be done with 401 for login required, then going to login as per normal?

Problem is to make this simple so that all users can understand it… and use it in all situations…

success make sense in an ajax thinking way… because the call is successful and gives a returned data…

I’m not sure on how to solve this better

All I’m requesting is allowing developers to do more than just echoing string/json with loginRequiredAjaxResponse. This does not mean that the current practice of returning a string needs to go away for other users, but rather that we have the option to actually set headers and content type if required.

Returning JSON without setting the proper content type is also not good since the JS then thinks it is a string until evaluated or similar, no?

I understand your request… but I would like to see a use case to evaluate if this is good to add or not… as I wrote above… we need to make a solution that will work in all or at least "almost all" cases… not a solution that will work only if you dont set the "error" option in the ajax call…

Problem with headers are:

  • how can a user decide if he wants the header be sent or not

  • how can a user decide which header to send

  • how can the user decide how many headers directive to send

NO… because in the ajax call you set the "dataType" to be returned.

Hi, I stumbled over this "issue" as well:

I use the AJAX-feature of CListView. I recognized that when the session timed out and the AJAX-code tries to update the list-view (and thus gets redirected to the login-form), the list-view instead is replaced with “no content”. So the list-view just disappeared :)

I solved it by overriding the CWebUser’s loginRequired()-method like the following:


    public function loginRequired()

    {

        $app=Yii::app();

        $request=$app->getRequest();


        if ($request->getIsAjaxRequest() && !isset($this->loginRequiredAjaxResponse)) {

            throw new CHttpException(403, Yii::t('general','Your session timed out!'));

        }


        parent::loginRequired();

    }

This way the default AJAX-error-handler of CListView (and probably other Yii/Zii-classes) is able to show the error using “alert()”. And it’s still possible to handle the error in custom AJAX-error-handlers using a XHR-status-check.

In my opinion it’s a good default behaviour. What do you think?