mpauly commented 2 years ago

Description

I modularized parts of the LoginView, as far as I can tell without modifying existing behavior, but providing more hooks to custromize the LoginView via inheritance.

Notice how I split the constants defining the steps into AUTH_STEP and FIRST_STEP. I assume the clearing of the state in post() is meant to happen at the first step of the wizard, which does not necessarily have to be the 'auth' step.

Motivation and Context

We use a custom version of the LoginView that adds an additional step in the login flow before the actual 'auth' step. At the moment this requires duplicating large amounts of code. By merging this change it would become a lot easier to accommodate such customization without large changes.

How Has This Been Tested?

We are currently integrating my fork and it allows large parts of the customization we require.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] 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.

claudep commented 2 years ago

Could you please make a separate PR with your first commit?

claudep commented 1 year ago

I extracted the first commit and pushed to #604.

claudep commented 1 year ago

@mpauly, your first commit is now in master, thanks a lot! Would you like to rebase this patch (and merge commits)?

mpauly commented 1 year ago

I just rebased against the current master and squashed the commits.

codecov[bot] commented 1 year ago

Codecov Report

Merging #559 (1e34cd6) into master (008dd92) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #559   +/-   ##
=======================================
  Coverage   95.42%   95.43%           
=======================================
  Files          74       74           
  Lines        3193     3195    +2     
  Branches      358      358           
=======================================
+ Hits         3047     3049    +2     
  Misses        117      117           
  Partials       29       29

Impacted Files	Coverage Δ
two_factor/views/core.py	`96.66% <100.00%> (+0.01%)`	:arrow_up:

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

jazzband / django-two-factor-auth

Modularise parts of the `LoginView` #559