diff --git a/matemat/db/facade.py b/matemat/db/facade.py index a94e420..67f1c5f 100644 --- a/matemat/db/facade.py +++ b/matemat/db/facade.py @@ -85,15 +85,17 @@ class MatematDatabase(object): def get_user(self, uid: int) -> User: """ - Return a user identified by its user ID - :param uid: The user's ID + Return a user identified by its user ID. + :param uid: The user's ID. """ with self.db.transaction(exclusive=False) as c: + # Fetch all values to construct the user c.execute('SELECT user_id, username, email, is_admin, is_member, balance FROM users WHERE user_id = ?', [uid]) row = c.fetchone() if row is None: raise ValueError(f'No user with user ID {uid} exists.') + # Unpack the row and construct the user user_id, username, email, is_admin, is_member, balance = row return User(user_id, username, balance, email, is_admin, is_member) @@ -230,12 +232,23 @@ class MatematDatabase(object): 'tkhash': tkhash }) - def change_user(self, user: User) -> None: + def change_user(self, user: User, **kwargs)\ + -> None: """ - Write changes in the User object to the database. - :param user: The user object to update in the database. + Commit changes to the user in the database. If writing the requested changes succeeded, the values are updated + in the provided user object. Otherwise the user object is left untouched. The user to update is identified by + the ID field in the provided user object. + + :param user: The user object to update and to identify the requested user by. + :param kwargs: The properties to change. :raises DatabaseConsistencyError: If the user represented by the object does not exist. """ + # Resolve the values to change + name: str = kwargs['name'] if 'name' in kwargs else user.name + email: str = kwargs['email'] if 'email' in kwargs else user.email + balance: int = kwargs['balance'] if 'balance' in kwargs else user.balance + is_admin: bool = kwargs['is_admin'] if 'is_admin' in kwargs else user.is_admin + is_member: bool = kwargs['is_member'] if 'is_member' in kwargs else user.is_member with self.db.transaction() as c: c.execute(''' UPDATE users SET @@ -248,16 +261,22 @@ class MatematDatabase(object): WHERE user_id = :user_id ''', { 'user_id': user.id, - 'username': user.name, - 'email': user.email, - 'balance': user.balance, - 'is_admin': user.is_admin, - 'is_member': user.is_member + 'username': name, + 'email': email, + 'balance': balance, + 'is_admin': is_admin, + 'is_member': is_member }) affected = c.execute('SELECT changes()').fetchone()[0] if affected != 1: raise DatabaseConsistencyError( f'change_user should affect 1 users row, but affected {affected}') + # Only update the actual user object after the changes in the database succeeded + user.name = name + user.email = email + user.balance = balance + user.is_admin = is_admin + user.is_member = is_member def delete_user(self, user: User) -> None: """ @@ -292,10 +311,11 @@ class MatematDatabase(object): def get_product(self, pid: int) -> Product: """ - Return a product identified by its product ID - :param pid: The products's ID + Return a product identified by its product ID. + :param pid: The products's ID. """ with self.db.transaction(exclusive=False) as c: + # Fetch all values to construct the product c.execute(''' SELECT product_id, name, price_member, price_non_member, stock FROM products @@ -304,6 +324,7 @@ class MatematDatabase(object): row = c.fetchone() if row is None: raise ValueError(f'No product with product ID {pid} exists.') + # Unpack the row and construct the product product_id, name, price_member, price_non_member, stock = row return Product(product_id, name, price_member, price_non_member, stock) @@ -333,7 +354,21 @@ class MatematDatabase(object): product_id = int(c.fetchone()[0]) return Product(product_id, name, price_member, price_non_member, 0) - def change_product(self, product: Product) -> None: + def change_product(self, product: Product, **kwargs) -> None: + """ + Commit changes to the product in the database. If writing the requested changes succeeded, the values are + updated in the provided product object. Otherwise the product object is left untouched. The product to update + is identified by the ID field in the provided product object. + + :param product: The product object to update and to identify the requested product by. + :param kwargs: The properties to change. + :raises DatabaseConsistencyError: If the product represented by the object does not exist. + """ + # Resolve the values to change + name: str = kwargs['name'] if 'name' in kwargs else product.name + price_member: int = kwargs['price_member'] if 'price_member' in kwargs else product.price_member + price_non_member: int = kwargs['price_non_member'] if 'price_non_member' in kwargs else product.price_non_member + stock: int = kwargs['stock'] if 'stock' in kwargs else product.stock with self.db.transaction() as c: c.execute(''' UPDATE products @@ -345,15 +380,20 @@ class MatematDatabase(object): WHERE product_id = :product_id ''', { 'product_id': product.id, - 'name': product.name, - 'price_member': product.price_member, - 'price_non_member': product.price_non_member, - 'stock': product.stock + 'name': name, + 'price_member': price_member, + 'price_non_member': price_non_member, + 'stock': stock }) affected = c.execute('SELECT changes()').fetchone()[0] if affected != 1: raise DatabaseConsistencyError( f'change_product should affect 1 products row, but affected {affected}') + # Only update the actual product object after the changes in the database succeeded + product.name = name + product.price_member = price_member + product.price_non_member = price_non_member + product.stock = stock def delete_product(self, product: Product) -> None: """ diff --git a/matemat/db/test/test_facade.py b/matemat/db/test/test_facade.py index b17262f..d15fa64 100644 --- a/matemat/db/test/test_facade.py +++ b/matemat/db/test/test_facade.py @@ -26,6 +26,19 @@ class DatabaseTest(unittest.TestCase): with self.assertRaises(ValueError): db.create_user('testuser', 'supersecurepassword2', 'testuser2@example.com') + def test_get_user(self) -> None: + with self.db as db: + with db.transaction(exclusive=False): + created = db.create_user('testuser', 'supersecurepassword', 'testuser@example.com', + admin=True, member=False) + user = db.get_user(created.id) + self.assertEqual('testuser', user.name) + self.assertEqual('testuser@example.com', user.email) + self.assertEqual(False, user.is_member) + self.assertEqual(True, user.is_admin) + with self.assertRaises(ValueError): + db.get_user(-1) + def test_list_users(self) -> None: with self.db as db: users = db.list_users() @@ -170,6 +183,17 @@ class DatabaseTest(unittest.TestCase): with self.assertRaises(ValueError): db.create_product('Club Mate', 250, 250) + def test_get_product(self) -> None: + with self.db as db: + with db.transaction(exclusive=False): + created = db.create_product('Club Mate', 150, 250) + product = db.get_product(created.id) + self.assertEqual('Club Mate', product.name) + self.assertEqual(150, product.price_member) + self.assertEqual(250, product.price_non_member) + with self.assertRaises(ValueError): + db.get_product(-1) + def test_list_products(self) -> None: with self.db as db: # Test empty list diff --git a/matemat/primitives/Product.py b/matemat/primitives/Product.py index 6208466..dd25738 100644 --- a/matemat/primitives/Product.py +++ b/matemat/primitives/Product.py @@ -3,6 +3,10 @@ from typing import Any class Product(object): + """ + Representation of a product offered by the Matemat, with a name, prices for users, and the number of items + currently in stock. + """ def __init__(self, product_id: int, @@ -10,6 +14,15 @@ class Product(object): price_member: int, price_non_member: int, stock: int) -> None: + """ + Create a new product instance with the given arguments. + + :param product_id: The product ID in the database. + :param name: The product's name. + :param price_member: The price of a unit of this product for users marked as "members". + :param price_non_member: The price of a unit of this product for users NOT marked as "members". + :param stock: The number of items of this product currently in stock. + """ self._product_id: int = product_id self._name: str = name self._price_member: int = price_member @@ -22,7 +35,8 @@ class Product(object): return self._product_id == other._product_id \ and self._name == other._name \ and self._price_member == other._price_member \ - and self._price_non_member == other._price_non_member + and self._price_non_member == other._price_non_member \ + and self._stock == other._stock @property def id(self) -> int: diff --git a/matemat/primitives/User.py b/matemat/primitives/User.py index 04b014a..45e026d 100644 --- a/matemat/primitives/User.py +++ b/matemat/primitives/User.py @@ -3,6 +3,11 @@ from typing import Optional, Any class User(object): + """ + Representation of a user registered with the Matemat, with a name, e-mail address (optional), whether the user is a + member of the organization the Matemat instance is used in, whether the user is an administrator, and the user's + account balance. + """ def __init__(self, user_id: int, @@ -11,6 +16,16 @@ class User(object): email: Optional[str] = None, admin: bool = False, member: bool = True) -> None: + """ + Create a new user instance with the given arguments. + + :param user_id: The user ID in the database. + :param username: The user's name. + :param balance: The balance of the user's account. + :param email: The user's e-mail address (optional). + :param admin: Whether the user is an administrator. + :param member: Whether the user is a member. + """ self._user_id: int = user_id self._username: str = username self._email: Optional[str] = email diff --git a/matemat/webserver/pagelets/admin.py b/matemat/webserver/pagelets/admin.py index 2759504..8918f94 100644 --- a/matemat/webserver/pagelets/admin.py +++ b/matemat/webserver/pagelets/admin.py @@ -49,15 +49,10 @@ def handle_change(args: RequestArguments, user: User, db: MatematDatabase, confi email = str(args.email) if len(email) == 0: email = None - oldname = user.name - oldmail = user.email try: - user.name = username - user.email = email - db.change_user(user) + db.change_user(user, name=username, email=email) except DatabaseConsistencyError: - user.name = oldname - user.email = oldmail + pass elif change == 'password': if 'oldpass' not in args or 'newpass' not in args or 'newpass2' not in args: diff --git a/matemat/webserver/pagelets/modproduct.py b/matemat/webserver/pagelets/modproduct.py index f8c4833..951c30e 100644 --- a/matemat/webserver/pagelets/modproduct.py +++ b/matemat/webserver/pagelets/modproduct.py @@ -62,21 +62,11 @@ def handle_change(args: RequestArguments, product: Product, db: MatematDatabase, price_member = int(str(args.pricemember)) price_non_member = int(str(args.pricenonmember)) stock = int(str(args.stock)) - oldname = product.name - oldprice_member = product.price_member - oldprice_non_member = product.price_non_member - oldstock = product.stock - product.name = name - product.price_member = price_member - product.price_non_member = price_non_member - product.stock = stock try: - db.change_product(product) + db.change_product(product, + name=name, price_member=price_member, price_non_member=price_non_member, stock=stock) except DatabaseConsistencyError: - product.name = oldname - product.email = oldprice_member - product.is_member = oldprice_non_member - product.stock = oldstock + pass if 'image' in args: image = bytes(args.image) if len(image) > 0: diff --git a/matemat/webserver/pagelets/moduser.py b/matemat/webserver/pagelets/moduser.py index f51ec91..eb12258 100644 --- a/matemat/webserver/pagelets/moduser.py +++ b/matemat/webserver/pagelets/moduser.py @@ -62,26 +62,14 @@ def handle_change(args: RequestArguments, user: User, db: MatematDatabase, confi email = str(args.email) password = str(args.password) balance = int(str(args.balance)) + is_member = 'ismember' in args + is_admin = 'isadmin' in args if len(email) == 0: email = None - oldname = user.name - oldmail = user.email - oldmember = user.is_member - oldadmin = user.is_admin - oldbalance = user.balance - user.name = username - user.email = email - user.is_member = 'ismember' in args - user.is_admin = 'isadmin' in args - user.balance = balance try: - db.change_user(user) + db.change_user(user, name=username, email=email, is_member=is_member, is_admin=is_admin, balance=balance) except DatabaseConsistencyError: - user.name = oldname - user.email = oldmail - user.is_member = oldmember - user.is_admin = oldadmin - user.balance = oldbalance + pass if len(password) > 0: db.change_password(user, '', password, verify_password=False) if 'avatar' in args: