jazzband / django-invitations

Generic invitations app for Django
GNU General Public License v3.0
559 stars 166 forks source link

Pass through redirect query arg #233

Closed blag closed 9 months ago

blag commented 10 months ago

Fixes #231.

Pass through a next=... query parameter to the signup and login views.

The docs for SIGNUP_REDIRECT and LOGIN_REDIRECT indicate they should be a URL name, the default values for these settings are URL names, and the tests for the allauth backend do not override those options and so expect a URL name.

But the tests for the basic backend override that option with a direct URL:

        settings.INVITATIONS_LOGIN_REDIRECT = "/login-url/"
        ...
        settings.INVITATIONS_LOGIN_REDIRECT = "/login-url/"
        ...
        settings.INVITATIONS_SIGNUP_REDIRECT = "/signup-url/"
        ...
        settings.INVITATIONS_SIGNUP_REDIRECT = "/non-existent-url/"
        ...
        settings.INVITATIONS_SIGNUP_REDIRECT = "/non-existent-url/"
        ...
    @override_settings(INVITATIONS_SIGNUP_REDIRECT="/non-existent-url/")
    ...
        settings.INVITATIONS_LOGIN_REDIRECT = "/login-url/"

Which is why I do this in this PR:

        try:
            signup_redirect = reverse(app_settings.SIGNUP_REDIRECT)
        except NoReverseMatch:
            signup_redirect = app_settings.SIGNUP_REDIRECT
        ...
        try:
            login_redirect = reverse(app_settings.LOGIN_REDIRECT)
        except NoReverseMatch:
            login_redirect = app_settings.LOGIN_REDIRECT

Fixing this may not be backwards compatible with existing users. It is also best left for a separate discussion, so for this PR I allowed for both options.

I also removed the walrus operator in a separate commit, so once we drop support for Python 3.7 we can revert that single commit to use the walrus operator.

codecov[bot] commented 10 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (dde3307) 64.34% compared to head (51e481e) 65.71%.

Files Patch % Lines
invitations/views.py 82.60% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #233 +/- ## ========================================== + Coverage 64.34% 65.71% +1.36% ========================================== Files 18 18 Lines 474 490 +16 ========================================== + Hits 305 322 +17 + Misses 169 168 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

blag commented 10 months ago

pre-commit.ci autofix

valberg commented 9 months ago

Hi @blag

Nice work! :clap:

Python 3.7 has been dropped in 2.1.0 - so we can revert the mentioned commit :blush:

Also there is a conflict.

I'll approve and merge when those issues are resolved :+1:

blag commented 9 months ago

Removed commit and fixed conflict.

blag commented 8 months ago

Hey @valberg, thanks for the merge.

Do you mind tagging a release? I have a project that needs my fix and I'd like to specify a version instead of a commit hash for it. 😄