Check to make sure request has a user object

[doctormo]

Summary

Fixes #

The inkscape-web project has many tests (which run through django-autotest-command) which do not like this new signal because there's no request.user yet.

Links to related discussion

Proposed changes in this pull request

[czpython]

Hello @doctormo, I thought about this initially but decided against these checks. The reason being that adding these in, forces the app to workaround edge cases which are not present under normal circumstances. If under normal setup the user attribute is not present then is needed for this to fail loudly since that's a misconfiguration.

I recommend to disable the signal on your base test or remove app from your test installed apps.

[doctormo]

Hi czpython, If the code must fail loudly, then you should fail with an error message suitable to tell developers your thoughts.

This particular issue fails during tests during the "login" phase of every test, in every test suite. I suspect that the issue of the requests existence or the signals connecting during tests will crop up again in the future as more people use it.

[czpython]

That's a great idea, we can raise an ImproperlyConfigured error if the user attribute is not present on request. That said, handling the exception is outside of the scope for this app.

It would be cool if you can adapt your pr to add this exception :)

[doctormo]

I've updated the pull request, as needed.

But this decision means I must remove djangocms-history since I can't really have an app killing my test suite in a way that requires me to make special exceptions to it. Hacking django to bits just to undo this signal is not a good API for djangocms-history.

I understand you not wanting to take responsibility for issues outside of your own, but I don't even know how your own test suites are passing. Perhaps if you can point me to your test code where you use client login.

[czpython]

@doctormo Does client.login() not use the configured middlewares?

[doctormo]

Django 1.8.17 django/test/client.py lines 592-605:

        from django.contrib.auth import authenticate, login
        user = authenticate(**credentials)
        if (user and user.is_active and 
                apps.is_installed('django.contrib.sessions')):
            engine = import_module(settings.SESSION_ENGINE)

            # Create a fake request to store login details.
            request = HttpRequest()

            if self.session:
                request.session = self.session
            else:
                request.session = engine.SessionStore()
            login(request, user)

The last line login(...) is where your normal auth comes in, but you'll notice that it's creating a fake request object.

[czpython]

Thanks for the information, I didn't get the full picture till now. Would you mind adapting the pr once more and add back your hasatttr check to the archive_old_operations handler but with a comment as to why this is needed.

Really appreciate your patience :)

[czpython]

Fixed by https://github.com/divio/djangocms-history/commit/333ab3dd410821169af20b342d8950d0165251be

[doctormo]

Thanks for fixing!