Skip to content

Commit

Permalink
Remove description and banner from unverified profile pages (#2017)
Browse files Browse the repository at this point in the history
Resolves #2016.

---------
Co-authored-by: Kiran Jonnalagadda <[email protected]>
  • Loading branch information
vidya-ram authored Apr 15, 2024
1 parent 16da93e commit dea6dca
Show file tree
Hide file tree
Showing 15 changed files with 197 additions and 98 deletions.
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ repos:
- id: pyupgrade
args: ['--keep-runtime-typing', '--py311-plus']
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.3.5
rev: v0.3.7
hooks:
- id: ruff
args: ['--fix', '--exit-non-zero-on-fix']
Expand Down Expand Up @@ -102,7 +102,7 @@ repos:
additional_dependencies:
- tomli
- repo: https://github.com/psf/black
rev: 24.3.0
rev: 24.4.0
hooks:
- id: black
- repo: https://github.com/PyCQA/flake8
Expand Down
14 changes: 14 additions & 0 deletions funnel/assets/sass/pages/profile.scss
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,15 @@
}
}

.profile.profile--unverified {
.profile__banner__box {
padding-bottom: 70px;
.profile__banner__box__img {
display: none;
}
}
}

.profile-create-btn {
margin-right: 8px;
margin-top: 0;
Expand Down Expand Up @@ -188,6 +197,11 @@
.profile-details {
padding: 2 * $mui-grid-padding 0 0;
}
.profile.profile--unverified {
.profile__banner__box {
padding-bottom: 45px;
}
}
}

.profile-subheader {
Expand Down
36 changes: 21 additions & 15 deletions funnel/forms/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from collections.abc import Iterable, Sequence
from hashlib import sha1
from typing import Any
from typing import Any, NoReturn

import requests
from flask import url_for
Expand All @@ -19,6 +19,7 @@
PASSWORD_MAX_LENGTH,
PASSWORD_MIN_LENGTH,
Account,
AccountNameProblem,
Anchor,
User,
check_password_strength,
Expand Down Expand Up @@ -384,21 +385,26 @@ def validate_old_password(self, field: forms.Field) -> None:
raise forms.validators.ValidationError(_("Incorrect password"))


def raise_username_error(reason: str) -> str:
def raise_username_error(reason: AccountNameProblem) -> NoReturn:
"""Provide a user-friendly error message for a username field error."""
if reason == 'blank':
raise forms.validators.ValidationError(_("This is required"))
if reason == 'long':
raise forms.validators.ValidationError(_("This is too long"))
if reason == 'invalid':
raise forms.validators.ValidationError(
_("Usernames can only have alphabets, numbers and underscores")
)
if reason == 'reserved':
raise forms.validators.ValidationError(_("This username is reserved"))
if reason in ('user', 'org'):
raise forms.validators.ValidationError(_("This username has been taken"))
raise forms.validators.ValidationError(_("This username is not available"))
match reason:
case AccountNameProblem.BLANK:
raise forms.validators.ValidationError(_("This is required"))
case AccountNameProblem.LONG:
raise forms.validators.ValidationError(_("This is too long"))
case AccountNameProblem.INVALID:
raise forms.validators.ValidationError(
_("Usernames can only have alphabets, numbers and underscores")
)
case AccountNameProblem.RESERVED:
raise forms.validators.ValidationError(_("This username is reserved"))
case (
AccountNameProblem.ACCOUNT
| AccountNameProblem.USER
| AccountNameProblem.ORG
| AccountNameProblem.PLACEHOLDER
):
raise forms.validators.ValidationError(_("This username is taken"))


@Account.forms('main')
Expand Down
69 changes: 33 additions & 36 deletions funnel/forms/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from baseframe import _, __, forms

from ..models import Account, Team, User
from ..models import Account, AccountNameProblem, Team, User

__all__ = ['OrganizationForm', 'TeamForm']

Expand Down Expand Up @@ -51,46 +51,43 @@ class OrganizationForm(forms.Form):
)

def validate_name(self, field: forms.Field) -> None:
"""Validate name is valid and available for this organization."""
reason = Account.validate_name_candidate(field.data)
"""Validate name is valid and available for this account."""
if self.edit_obj:
reason = self.edit_obj.validate_new_name(field.data)
else:
reason = Account.validate_name_candidate(field.data)
if not reason:
return # name is available
if reason == 'invalid':
raise forms.validators.ValidationError(
_("Names can only have alphabets, numbers and underscores")
)
if reason == 'reserved':
raise forms.validators.ValidationError(_("This name is reserved"))
if (
self.edit_obj
and self.edit_obj.name
and field.data.lower() == self.edit_obj.name.lower()
):
# Name has only changed case from previous name. This is a validation pass
return
if reason == 'user':
if (
self.edit_user.username
and field.data.lower() == self.edit_user.username.lower()
):
match reason:
case AccountNameProblem.INVALID:
raise forms.validators.ValidationError(
Markup(
_(
"This is <em>your</em> current username."
' You must change it first from <a href="{account}">your'
" account</a> before you can assign it to an organization"
).format(account=url_for('account'))
_("Names can only have alphabets, numbers and underscores")
)
case AccountNameProblem.RESERVED:
raise forms.validators.ValidationError(_("This name is reserved"))
case AccountNameProblem.USER:
if self.edit_user.name_is(field.data):
raise forms.validators.ValidationError(
Markup(
_(
'This is <em>your</em> current username.'
' You must change it first from <a href="'
'{account}">your account</a> before you can assign it'
' to an organization'
).format(account=url_for('account'))
)
)
raise forms.validators.ValidationError(
_("This name has been taken by another user")
)
case AccountNameProblem.ORG:
raise forms.validators.ValidationError(
_("This name has been taken by another organization")
)
case _:
raise forms.validators.ValidationError(
_("This name has been taken by another account")
)
raise forms.validators.ValidationError(
_("This name has been taken by another user")
)
if reason == 'org':
raise forms.validators.ValidationError(
_("This name has been taken by another organization")
)
# We're not supposed to get an unknown reason. Flag error to developers.
raise ValueError(f"Unknown account name validation failure reason: {reason}")


@Team.forms('main')
Expand Down
4 changes: 4 additions & 0 deletions funnel/forms/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ class ProfileForm(OrganizationForm):
def __post_init__(self) -> None:
"""Prepare form for use."""
self.logo_url.profile = self.account.name or self.account.buid
if self.account.is_user_profile:
self.make_for_user()
if not self.account.is_verified:
del self.description

def make_for_user(self) -> None:
"""Customise form for a user account."""
Expand Down
2 changes: 2 additions & 0 deletions funnel/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"AccountEmailClaim",
"AccountExternalId",
"AccountMembership",
"AccountNameProblem",
"AccountOldId",
"AccountPasswordNotification",
"AccountPhone",
Expand Down Expand Up @@ -199,6 +200,7 @@
"getextid",
"getuser",
"helpers",
"hybrid_method",
"hybrid_property",
"label",
"login_session",
Expand Down
4 changes: 4 additions & 0 deletions funnel/models/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ from .account import (
AccountEmail,
AccountEmailClaim,
AccountExternalId,
AccountNameProblem,
AccountOldId,
AccountPhone,
Anchor,
Expand Down Expand Up @@ -93,6 +94,7 @@ from .base import (
db,
declarative_mixin,
declared_attr,
hybrid_method,
hybrid_property,
postgresql,
relationship,
Expand Down Expand Up @@ -248,6 +250,7 @@ __all__ = [
"AccountEmailClaim",
"AccountExternalId",
"AccountMembership",
"AccountNameProblem",
"AccountOldId",
"AccountPasswordNotification",
"AccountPhone",
Expand Down Expand Up @@ -424,6 +427,7 @@ __all__ = [
"getextid",
"getuser",
"helpers",
"hybrid_method",
"hybrid_property",
"label",
"login_session",
Expand Down
69 changes: 45 additions & 24 deletions funnel/models/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import itertools
from collections.abc import Iterable, Iterator, Sequence
from datetime import datetime
from enum import Enum
from typing import (
TYPE_CHECKING,
Any,
Expand Down Expand Up @@ -59,6 +60,7 @@
UrlType,
UuidMixin,
db,
hybrid_method,
hybrid_property,
relationship,
sa,
Expand All @@ -80,6 +82,7 @@

__all__ = [
'ACCOUNT_STATE',
'AccountNameProblem',
'Account',
'deleted_account',
'removed_account',
Expand All @@ -98,6 +101,17 @@
]


class AccountNameProblem(Enum):
BLANK = 'blank' # Name is blank
RESERVED = 'reserved' # Name is a reserved keyword
INVALID = 'invalid' # Name has invalid syntax
LONG = 'long' # Name is too long
USER = 'user' # Name is taken by a user account
ORG = 'org' # Name is taken by an organization account
PLACEHOLDER = 'placeholder' # Name is taken by a placeholder account
ACCOUNT = 'account' # Name is taken by an account of unknown type


class ACCOUNT_STATE(LabeledEnum): # noqa: N801
"""State codes for accounts."""

Expand Down Expand Up @@ -944,6 +958,7 @@ def ticket_followers(self) -> Query[Account]:
'timezone',
'description',
'website',
'tagline',
'logo_url',
'banner_image_url',
'joined_at',
Expand Down Expand Up @@ -1531,8 +1546,18 @@ def _uuid_zbase32_comparator(cls) -> ZBase32Comparator:
"""Return SQL comparator for :prop:`uuid_zbase32`."""
return ZBase32Comparator(cls.uuid) # type: ignore[arg-type]

@hybrid_method
def name_is(self, name: str) -> bool:
if name.startswith('~'):
return self.uuid_zbase32 == name[1:]
return (
self.name is not None
and self.name.lower() == name.replace('-', '_').lower()
)

@name_is.expression
@classmethod
def name_is(cls, name: str) -> ColumnElement:
def _name_is_expr(cls, name: str) -> ColumnElement:
"""Generate query filter to check if name is matching (case insensitive)."""
if name.startswith('~'):
return cls.uuid_zbase32 == name[1:]
Expand Down Expand Up @@ -1751,46 +1776,42 @@ def autocomplete(cls, prefix: str) -> list[Self]:
return users

@classmethod
def validate_name_candidate(cls, name: str) -> str | None:
def validate_name_candidate(cls, name: str) -> AccountNameProblem | None:
"""
Validate an account name candidate.
Returns one of several error codes, or `None` if all is okay:
* ``blank``: No name supplied
* ``reserved``: Name is reserved
* ``invalid``: Invalid characters in name
* ``long``: Name is longer than allowed size
* ``user``: Name is assigned to a user
* ``org``: Name is assigned to an organization
Returns ``None`` if all is okay, or :enum:`AccountNameProblem`.
"""
if not name:
return 'blank'
if not name or not name.strip():
return AccountNameProblem.BLANK
if name.lower() in cls.reserved_names:
return 'reserved'
return AccountNameProblem.RESERVED
if not valid_account_name(name):
return 'invalid'
return AccountNameProblem.INVALID
if len(name) > cls.__name_length__:
return 'long'
return AccountNameProblem.LONG
# Look for existing on the base Account model, not the subclass, as SQLAlchemy
# will add a filter condition on subclasses to restrict the query to that type.
existing = (
Account.query.filter(sa.func.lower(Account.name) == sa.func.lower(name))
Account.query.filter(Account.name_is(name))
.options(sa_orm.load_only(cls.id, cls.uuid, cls.type_))
.one_or_none()
)
if existing is not None:
if isinstance(existing, Placeholder):
return 'reserved'
if isinstance(existing, User):
return 'user'
if isinstance(existing, Organization):
return 'org'
match existing:
case User():
return AccountNameProblem.USER
case Organization():
return AccountNameProblem.ORG
case Placeholder():
return AccountNameProblem.PLACEHOLDER
case _:
return AccountNameProblem.ACCOUNT
return None

def validate_new_name(self, name: str) -> str | None:
def validate_new_name(self, name: str) -> AccountNameProblem | None:
"""Validate a new name for this account, returning an error code or None."""
if self.name and name.lower() == self.name.lower():
if self.name_is(name):
return None
return self.validate_name_candidate(name)

Expand Down
Loading

0 comments on commit dea6dca

Please sign in to comment.