fix: restore autocomplete regression test + simplify communication preferences JS#197
Merged
Merged
Conversation
PR #195 accidentally deleted the def line for test_register_page_confirm_password_has_autocomplete, collapsing its body into test_register_page_shows_password_requirements and demoting its docstring to a no-op string statement. This restored the issue #156 autocomplete check as its own test. Co-authored-by: Cursor <cursoragent@cursor.com>
The syncSubPreferences helper was only invoked from the master checkbox's change handler (disable branch) and never on init, where initial visibility is already set server-side via Jinja. Inlining it removes the indirection and the asymmetry between the enable and disable paths while preserving behavior: enabling reveals the sub-preferences and checks updates; disabling hides them and clears both sub-preferences. Co-authored-by: Cursor <cursoragent@cursor.com>
chriscarrollsmith
changed the title
2 tasks
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
Two small, related cleanups to the communication-preferences feature added in #195:
Restore the confirm-password autocomplete regression test. add communication preferences #195 accidentally deleted the
defline fortest_register_page_confirm_password_has_autocomplete, collapsing its body intotest_register_page_shows_password_requirementsand demoting its docstring to a no-op string statement. The issue "Register" page must document the security policy #156 autocomplete check is restored as its own test. The comm-preference assertions remain intest_register_page_shows_password_requirements, where they correctly verify the register page renders the new fields.Drop the
syncSubPreferenceshelper. Incommunication_preferences_fields.htmlthe helper was only ever called from the master checkbox'schangehandler (disable branch) and never on init — initial visibility is already set server-side via the Jinjastyle="display: none;"guard. Inlining it removes the indirection and the enable/disable asymmetry. Behavior is preserved exactly: enabling reveals the sub-preferences and checkscomm_updates; disabling hides them and clears both sub-preferences.Test plan
uv run pytest tests/routers/core/test_account.py(91 passed)uv run pytest tests/test_templates.py(30 passed)test_register_page_shows_password_requirementsandtest_register_page_confirm_password_has_autocompleteare collected as separate testsuv run ty check .clean