Skip to content

Commit

Permalink
Allow account name to be unset
Browse files Browse the repository at this point in the history
  • Loading branch information
jace committed Sep 5, 2023
1 parent 65ec038 commit 788680b
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 19 deletions.
23 changes: 13 additions & 10 deletions funnel/models/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -864,14 +864,11 @@ def do_delete(self):
# 6. Clear name (username), title (fullname) and stored password hash
self.name = None
self.title = ''
self.password = None

# 7. Unassign tickets assigned to the user
self.ticket_participants = [] # pylint: disable=attribute-defined-outside-init

# 8. Clear fullname and stored password hash
self.fullname = ''
self.password = None

@with_roles(call={'owner'})
@profile_state.transition(
profile_state.NOT_PUBLIC,
Expand Down Expand Up @@ -1193,14 +1190,20 @@ def is_available_name(cls, name: str) -> bool:
return cls.validate_name_candidate(name) is None

@sa.orm.validates('name')
def _validate_name(self, key: str, value: Optional[str]):
def _validate_name(self, key: str, value: Optional[str]) -> Optional[str]:
"""Validate the value of Account.name."""
if value and (
value.lower() in self.reserved_names or not valid_account_name(value)
):
if value is None:
return value

if not isinstance(value, str):
raise ValueError(f"Account name must be a string: {value}")

if not value.strip():
raise ValueError("Account name cannot be blank")

if value.lower() in self.reserved_names or not valid_account_name(value):
raise ValueError("Invalid account name: " + value)
if not value and self.name and not self.state.GONE:
raise ValueError("Account name cannot be unset")

# We don't check for existence in the db since this validator only
# checks for valid syntax. To confirm the name is actually available,
# the caller must call :meth:`is_available_name` or attempt to commit
Expand Down
21 changes: 12 additions & 9 deletions tests/unit/models/account_name_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,26 +84,29 @@ def test_cant_remove_username(db_session, user_twoflower) -> None:
user_twoflower.username = 'twoflower'
assert user_twoflower.username == 'twoflower'

# Can't be removed even though it was None to start with
with pytest.raises(ValueError, match='Account name cannot be unset'):
user_twoflower.username = None

# Can't be a blank value
with pytest.raises(ValueError, match='Account name cannot be unset'):
with pytest.raises(ValueError, match='Account name cannot be blank'):
user_twoflower.username = ' '

with pytest.raises(ValueError, match='Account name cannot be blank'):
user_twoflower.username = ''

# Can't be an invalid value
with pytest.raises(ValueError, match='Invalid account name'):
user_twoflower.username = ' '
with pytest.raises(ValueError, match='Account name must be a string'):
user_twoflower.username = []

with pytest.raises(ValueError, match='Account name must be a string'):
user_twoflower.username = False

with pytest.raises(ValueError, match='Account name must be a string'):
user_twoflower.username = True


def test_cant_remove_orgname(db_session, org_uu) -> None:
"""An org's name can be renamed but not removed."""
assert org_uu.name == 'UU'
org_uu.name = 'unseen'
assert org_uu.name == 'unseen'
with pytest.raises(ValueError, match='Account name cannot be unset'):
org_uu.name = None


def test_name_transfer(db_session, user_mort, user_rincewind) -> None:
Expand Down

0 comments on commit 788680b

Please sign in to comment.