Skip to content

Conversation

@jaap3
Copy link

@jaap3 jaap3 commented Nov 15, 2022

Description

Restore the ability to override the success_url in urls.py (it was added in 3d0d92f)

Motivation and Context

We use a simple subclass to redirect the users to the backup token screen once setup is completed

class TwoFactorSetupView(SetupView):
    # Redirect to the "backup tokens" instead of the "setup complete" screen
    success_url = "two_factor:backup_tokens"

I believe this doesn't work anymore after a (recent) change to SetupView.

How Has This Been Tested?

I have not tested this, I was just reading the code when I noticed this change.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented Nov 15, 2022

Codecov Report

Merging #569 (1bad97c) into master (864d696) will decrease coverage by 7.95%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #569      +/-   ##
==========================================
- Coverage   95.66%   87.71%   -7.96%     
==========================================
  Files          65       65              
  Lines        2952     2889      -63     
  Branches      331      198     -133     
==========================================
- Hits         2824     2534     -290     
- Misses        103      337     +234     
+ Partials       25       18       -7     
Impacted Files Coverage Δ
tests/test_views_setup.py 100.00% <100.00%> (ø)
two_factor/views/core.py 96.10% <100.00%> (-0.52%) ⬇️
two_factor/plugins/webauthn/urls.py 0.00% <0.00%> (-100.00%) ⬇️
two_factor/plugins/webauthn/admin.py 0.00% <0.00%> (-100.00%) ⬇️
two_factor/plugins/webauthn/models.py 0.00% <0.00%> (-91.67%) ⬇️
two_factor/plugins/webauthn/forms.py 0.00% <0.00%> (-83.55%) ⬇️
two_factor/plugins/webauthn/method.py 0.00% <0.00%> (-80.56%) ⬇️
two_factor/plugins/webauthn/apps.py 0.00% <0.00%> (-75.00%) ⬇️
two_factor/plugins/webauthn/tests/test_utils.py 30.76% <0.00%> (-69.24%) ⬇️
two_factor/plugins/webauthn/utils.py 0.00% <0.00%> (-67.86%) ⬇️
... and 19 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@claudep
Copy link
Contributor

claudep commented Nov 15, 2022

Would be great to have a test preventing such a regression.

@jaap3
Copy link
Author

jaap3 commented Nov 15, 2022

@claudep I agree, sadly I don't currently have the time to do so (I'd have to dive into this project much more than I already have) as I'm on a tight deadline :-(

@jaap3
Copy link
Author

jaap3 commented Nov 16, 2022

Seems like this is the second regression from PR #500

@claudep
Copy link
Contributor

claudep commented Nov 16, 2022

I would appreciate that people introducing regressions would help working on solving them. My experience tells that it doesn't happen most of the time, but we can always hope :-)

@claudep
Copy link
Contributor

claudep commented Dec 3, 2022

What if I merge this and in exchange you review #568 😊 ?!?

@jaap3
Copy link
Author

jaap3 commented Dec 4, 2022

I'll be done with a project on Wednesday, so will have time to write a test and review that PR by then. Does that work with your planned release schedule?

@claudep
Copy link
Contributor

claudep commented Dec 4, 2022

Absolutely, thanks a lot!

@jaap3
Copy link
Author

jaap3 commented Dec 5, 2022

Not sure what to do with the test failures, it's caused by selenium.common.exceptions.TimeoutException: Message

Restore the ability to override the success_url in urls.py
@claudep
Copy link
Contributor

claudep commented Dec 8, 2022

Thanks a lot!! Test failure is unrelated. Some new webauthn tests are a bit flaky and are known to fail for obscure reasons (selenium tests).

@claudep claudep merged commit d7abfd7 into jazzband:master Dec 8, 2022
@jaap3 jaap3 deleted the patch-1 branch December 8, 2022 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants