Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove description and banner from unverified profile pages #2017

Merged
merged 7 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading