Skip to content

Commit

Permalink
ref: fix typing for sentry.web.forms.accounts (take 2) (#83597)
Browse files Browse the repository at this point in the history
this time with a fix for username being missing

<!-- Describe your PR here. -->
  • Loading branch information
asottile-sentry authored and andrewshie-sentry committed Jan 22, 2025
1 parent 426aa3a commit d26db81
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 15 deletions.
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@ module = [
"sentry.testutils.cases",
"sentry.utils.auth",
"sentry.utils.committers",
"sentry.web.forms.accounts",
"sentry.web.frontend.auth_login",
"sentry.web.frontend.auth_organization_login",
"sentry.web.frontend.base",
Expand Down
30 changes: 16 additions & 14 deletions src/sentry/web/forms/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ class AuthenticationForm(forms.Form):
"inactive": _("This account is inactive."),
}

def __init__(self, request: HttpRequest | None = None, *args: Any, **kwargs: Any) -> None:
def __init__(self, request: HttpRequest, *args: Any, **kwargs: Any) -> None:
"""
If request is passed in, the form will validate that cookies are
enabled. Note that the request (a HttpRequest object) must have set a
cookie with the key TEST_COOKIE_NAME and value TEST_COOKIE_VALUE before
running this validation.
"""
self.request = request
self.user_cache = None
self.user_cache: User | None = None
super().__init__(*args, **kwargs)

# Set the label for the "username" field.
Expand Down Expand Up @@ -128,11 +128,10 @@ def clean(self) -> dict[str, Any] | None:
return self.cleaned_data

def check_for_test_cookie(self):
if self.request:
if not self.request.session.test_cookie_worked():
raise forms.ValidationError(self.error_messages["no_cookies"])
else:
self.request.session.delete_test_cookie()
if not self.request.session.test_cookie_worked():
raise forms.ValidationError(self.error_messages["no_cookies"])
else:
self.request.session.delete_test_cookie()

def get_user_id(self):
if self.user_cache:
Expand Down Expand Up @@ -170,7 +169,7 @@ class PasswordlessRegistrationForm(forms.ModelForm):

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if not newsletter.is_enabled():
if not newsletter.backend.is_enabled():
del self.fields["subscribe"]
else:
# NOTE: the text here is duplicated within the ``NewsletterConsent`` component
Expand Down Expand Up @@ -206,8 +205,8 @@ def save(self, commit=True):
if commit:
user.save()
if self.cleaned_data.get("subscribe"):
newsletter.create_or_update_subscriptions(
user, list_ids=newsletter.get_default_list_ids()
newsletter.backend.create_or_update_subscriptions(
user, list_ids=newsletter.backend.get_default_list_ids()
)
return user

Expand All @@ -219,9 +218,12 @@ class RegistrationForm(PasswordlessRegistrationForm):

def clean_password(self):
password = self.cleaned_data["password"]
password_validation.validate_password(
password, user=User(username=self.cleaned_data.get("username"))
user = (
User(username=self.cleaned_data["username"])
if "username" in self.cleaned_data
else None
)
password_validation.validate_password(password, user=user)
return password

def save(self, commit=True):
Expand All @@ -230,8 +232,8 @@ def save(self, commit=True):
if commit:
user.save()
if self.cleaned_data.get("subscribe"):
newsletter.create_or_update_subscriptions(
user, list_ids=newsletter.get_default_list_ids()
newsletter.backend.create_or_update_subscriptions(
user, list_ids=newsletter.backend.get_default_list_ids()
)
return user

Expand Down
6 changes: 6 additions & 0 deletions tests/sentry/web/forms/test_accounts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from sentry.web.forms.accounts import RegistrationForm


def test_valid_does_not_crash_without_username() -> None:
form = RegistrationForm({"password": "watwatwatwatwatwatawtataw"})
assert form.is_valid() is False

0 comments on commit d26db81

Please sign in to comment.