From 4c3dcfe0e5ce300da58b1087e0d7cc397ee95c4a Mon Sep 17 00:00:00 2001
From: Tim Van Baak <tim.vanbaak@gmail.com>
Date: Thu, 6 May 2021 11:14:33 -0700
Subject: [PATCH] Improve user creation logic

---
 amanuensis/backend/user.py |  6 ++++--
 tests/test_db.py           | 17 +++++++++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/amanuensis/backend/user.py b/amanuensis/backend/user.py
index 98c4c8b..a9e210b 100644
--- a/amanuensis/backend/user.py
+++ b/amanuensis/backend/user.py
@@ -5,6 +5,8 @@ User query interface
 import re
 import uuid
 
+from sqlalchemy import select, func
+
 from amanuensis.db import DbContext, User
 from amanuensis.errors import ArgumentError
 
@@ -30,11 +32,11 @@ def create_user(
     if not password:
         raise ArgumentError('Password must be provided')
     # If display name is not provided, use the username
-    if not display_name.strip():
+    if not display_name or not display_name.strip():
         display_name = username
 
     # Query the db to make sure the username isn't taken
-    if db.session.query(User.username == username).count() > 0:
+    if db.session.query(func.count(User.id)).filter(User.username == username).scalar() > 0:
         raise ArgumentError('Username is already taken')
 
     new_user = User(
diff --git a/tests/test_db.py b/tests/test_db.py
index eb05f44..866e408 100644
--- a/tests/test_db.py
+++ b/tests/test_db.py
@@ -27,7 +27,7 @@ def test_create(db):
 
 
 def test_create_user(db):
-    """New user creation"""
+    """Test new user creation."""
     kwargs = {
         'username': 'username',
         'password': 'password',
@@ -36,16 +36,29 @@ def test_create_user(db):
         'is_site_admin': False
     }
 
+    # Test length constraints
+    with pytest.raises(ArgumentError):
+        userq.create_user(db, **{**kwargs, 'username': 'me'})
+    with pytest.raises(ArgumentError):
+        userq.create_user(db, **{**kwargs, 'username': 'the right honorable user-name, esquire'})
+    # Test allowed characters
     with pytest.raises(ArgumentError):
         userq.create_user(db, **{**kwargs, 'username': 'user name'})
-
+    # No password
     with pytest.raises(ArgumentError):
         userq.create_user(db, **{**kwargs, 'password': None})
 
+    # Valid creation works and populates fields
     new_user = userq.create_user(db, **kwargs)
     assert new_user
     assert new_user.id is not None
     assert new_user.created is not None
 
+    # No duplicate usernames
     with pytest.raises(ArgumentError):
         duplicate = userq.create_user(db, **kwargs)
+
+    # Missing display name populates with username
+    user2_kw = {**kwargs, 'username': 'user2', 'display_name': None}
+    user2 = userq.create_user(db, **user2_kw)
+    assert user2.display_name is not None