Fixes #587 - Avoid unneeded migration for PhoneDevice

[claudep]

Also refs #469.

[codecov[bot]]

Codecov Report

Merging #596 (1621bdb) into master (6150a78) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #596   +/-   ##
=======================================
  Coverage   95.46%   95.47%           
=======================================
  Files          74       75    +1     
  Lines        3198     3204    +6     
  Branches      358      359    +1     
=======================================
+ Hits         3053     3059    +6     
  Misses        116      116           
  Partials       29       29           

Impacted Files	Coverage Δ
two_factor/plugins/phonenumber/apps.py	100.00% <100.00%> (ø)
two_factor/plugins/phonenumber/urls.py	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

[sevdog]

Also the migration 0004_auto_20160205_1827 does not work if phonenumbers is not installed:

  File "/opt/venv/lib/python3.9/site-packages/two_factor/migrations/0004_auto_20160205_1827.py", line 3, in <module>
    import phonenumber_field.modelfields
  File "/opt/venv/lib/python3.9/site-packages/phonenumber_field/modelfields.py", line 7, in <module>
    from phonenumber_field import formfields
  File "/opt/venv/lib/python3.9/site-packages/phonenumber_field/formfields.py", line 1, in <module>
    import phonenumbers

[claudep]

@sevdog, good catch. I tried avoiding this by squashing migrations in #597.

[claudep]

Tweaking the migrations is probably unneeded after 2ff98b514d. Patch needs rework.

[mgax]

Tweaking the migrations is probably unneeded after 2ff98b5. Patch needs rework.

On master, I've tried to simply comment out the phonenumber URLs from two_factor/urls.py and the migration is still being generated; stubbing out key_validator in migrations did the trick.

What is needed for the patch to be merged? I'd like to help.

[claudep]

I isolated migration part in #620. What's needed is an approval from a Jazzband member.

[claudep]

I guess that another less-invasive approach could be to conditionally include phonenumber urls in main urls by testing if the app is installed. I would be interested to collect opinions about this (the challenge being: avoid importing phonenumber stuff in main urls).

[dekkers]

There is also the option of keeping all the names of the urls of the phonenumber plugin in the two_factor namespace to avoid breaking backwards compatability. I am not really sure what the best way is to propose changes to an existing PR, but I did that in this commit and that seems to work:

https://github.com/dekkers/django-two-factor-auth/commit/70a45b8dc021125c71760390d1434723d92f1875

[claudep]

Nice! I merged your commit in my PR.