refactor: remove dead code and cover untested branches - #144
Open
CuriousLearner wants to merge 1 commit into
Open
refactor: remove dead code and cover untested branches#144CuriousLearner wants to merge 1 commit into
CuriousLearner wants to merge 1 commit into
Conversation
Replace the `ugettext_lazy`/`gettext_lazy` try/except shims in `models.py` and `serializers.py` with a direct `gettext_lazy` import. `gettext_lazy` predates the project's Django floor, so the fallback was unreachable and the shim preferred the deprecated alias wherever it still existed. Drop the unused `logging` import and module-level `logger` from `serializers.py`, and reduce the always-true `hasattr`/`callable` guard in `PhoneVerificationService._generate_message` to a single `getattr` lookup that still supports duck-typed backends. Parse `Accept-Language` by quality value in `VerificationViewSet.register` so the highest priority language wins instead of the first one listed, per RFC 7231. Entries with equal or absent `q` keep their header order, malformed entries are skipped, and a header with no usable language still yields `None` so the message is sent unlocalized. Build the parametrized backend lists in `conftest.py` as lists rather than sets, so test ordering no longer depends on per-process string hashing. Add coverage for `phone_verify/admin.py`, the expiration default in `get_security_code_expiration`, and the truncated dry-run preview in the `cleanup_phone_verifications` command. Testing the admin requires `django.contrib.admin` in the test settings, since the module cannot be imported without it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Accept-Languageparsing took the first language in the header and ignored quality values, sode;q=0.5,en;q=0.9selected German even though the client ranked English higher. It now ranks entries by theirqweight per RFC 7231, treatsq=0as "not acceptable", ignores malformed entries instead of raising, and keeps header order for ties. An absent or unusable header still yields no language, preserving the documented behavior that the message is sent unlocalized when no language is set.The
ugettext_lazyimport shim inmodels.pyandserializers.pywas both dead and backwards.gettext_lazyhas existed since Django 2.0, below the project's floor of 2.1.5, so the fallback never ran; meanwhile on versions whereugettext_lazystill existed the shim selected the deprecated alias, which Django removed in 4.0. Both are replaced with a direct import.Also removes an unused logger in
serializers.pyand reduces an always-truehasattr/callableguard inservices.pyto a singlegetattr, keeping a minimal guard so duck-typed backends that do not inherit fromBaseBackendstill work.conftest.pybuilt its backend parametrization from set literals, so collection order changed between runs under Python's per-process string hash randomization, making-x,--lf, and xdist ordering irreproducible. They are now lists; every consumer only does membership tests.Test plan
phone_verify/admin.pygoes from 0% to 100% coverage, the largest gap in the project. Exercising it required addingdjango.contrib.adminto the test settings, since the@admin.registerdecorator resolves the default admin site at import time.constants.pyand the cleanup command each had one uncovered branch, both now covered. Total coverage 96% to 99%.PYTHONHASHSEEDvalues.