Don't store a detached User instance in user.dbuser

Fixes #220 and #313 on github.

user.database_user.dbuser is now a read-only attribute that returns a
User instance from the current ORM session.

Author: sde1000

quicktill/card.py

@@ -124,7 +124,6 @@ def enter(self):
 
         trans = td.s.get(Transaction, self.transid)
         user = ui.current_user().dbuser
-        td.s.add(user)
         if not trans or trans.closed:
             ui.infopopup(["The transaction was closed before the payment "
                           "could be recorded.  The payment has been "

quicktill/cash.py

@@ -39,7 +39,6 @@ def add_payment(self, transaction, description, amount):
         # Typically used by other payment drivers for cashback, or by
         # the register when deferring part-paid transactions
         user = ui.current_user().dbuser
-        td.s.add(user)
         p = Payment(transaction=transaction, paytype=self.paytype,
                     text=description, amount=amount, user=user,
                     source=tillconfig.terminal_name)

quicktill/managestock.py

@@ -248,19 +248,18 @@ def stock_purge_internal(source):
 
     # Mark all these stockitems as finished, removing them from being
     # on sale as we go
-    user = ui.current_user()
-    user = user.dbuser if user and hasattr(user, 'dbuser') else None
+    dbu = user.current_dbuser()
     for item in finished:
         if item.stockline:
             # Directly connected to a stockline
             td.s.add(StockAnnotation(
-                stockitem=item, atype="stop", user=user,
+                stockitem=item, atype="stop", user=dbu,
                 text=f"{item.stockline.name} (display stockline, {source})"))
         else:
             # Indirectly connected via a continuous stockline
             for sl in item.stocktype.stocklines:
                 td.s.add(StockAnnotation(
-                    stockitem=item, atype="stop", user=user,
+                    stockitem=item, atype="stop", user=dbu,
                     text=f"{sl.name} (continuous stockline, {source})"))
         item.finished = datetime.datetime.now()
         item.finishcode_id = 'empty'  # guaranteed to exist

quicktill/register.py

@@ -2631,7 +2631,6 @@ def clear_and_lock_register(self):
         # We set the user's current transaction to None and dismiss
         # the current register page, most likely returning to the lock
         # screen on the next time around the event loop.
-        self.user.dbuser = td.s.get(User, self.user.userid)
         self.user.dbuser.transaction = None
         td.s.flush()
         self.deselect()
@@ -2640,7 +2639,6 @@ def recalltrans(self, transid):
         # We refresh the user object as if in enter() here, but don't
         # bother with the full works because we're replacing the current
         # transaction anyway!
-        self.user.dbuser = td.s.get(User, self.user.userid)
         self._clear()
         self._redraw()
         if transid:
@@ -3202,14 +3200,6 @@ def entry(self):
         it isn't.  If False is returned, this function may have popped
         up a dialog box.
         """
-        # Fetch the current user database object.  We don't recreate
-        # the user.database_user object because that's unlikely to
-        # change often; we're just interested in the transaction and
-        # register fields.  The fetch from the database has probably
-        # already been done in hotkeypress() - here we are fetching
-        # from the sqlalchemy session's map
-        self.user.dbuser = td.s.get(User, self.user.userid)
-
         # This check has already been done in hotkeypress().  We
         # repeat it here because it is possible we may not have been
         # entered in response to a keypress - a timer event, for
@@ -3259,7 +3249,6 @@ def entry_noninteractive(self):
         If this returns False, the caller should clean up and then
         call the deselect() method.
         """
-        self.user.dbuser = td.s.get(User, self.user.userid)
         if self.user.dbuser.register_id != tillconfig.register_id:
             # User has logged in somewhere else
             return False
@@ -3369,13 +3358,13 @@ def hotkeypress(self, k):
         # the user.database_user object because that's unlikely to
         # change often; we're just interested in the transaction and
         # register fields.
-        self.user.dbuser = td.s.get(
+        dbuser = td.s.get(
             User, self.user.userid,
             options=[joinedload(User.transaction)])
 
         # Check that the user hasn't moved to another terminal.  If
         # they have, lock immediately.
-        if self.user.dbuser.register_id != tillconfig.register_id:
+        if dbuser.register_id != tillconfig.register_id:
             self.deselect()
             return super().hotkeypress(k)
 

quicktill/squareterminal.py

@@ -1563,7 +1563,6 @@ def _create_pending_payment(
                           "payment was started."], title="Error")
             return
         user = ui.current_user().dbuser
-        td.s.add(user)  # hack! See github issue #220
         p = Payment(transaction=reg_trans, amount=zero, paytype=pm,
                     text=pm.description, user=user,
                     source=tillconfig.terminal_name, pending=True)

quicktill/stock.py

@@ -124,10 +124,8 @@ def finish(self):
             ui.infopopup(["You can't add a blank annotation!"], title="Error")
             return
         annotation = self.annfield.f or ""
-        cu = ui.current_user()
-        user = cu.dbuser if cu and hasattr(cu, "dbuser") else None
         td.s.add(StockAnnotation(stockitem=item, type=anntype, text=annotation,
-                                 user=user))
+                                 user=user.current_dbuser()))
         td.s.flush()
         self.dismiss()
         ui.infopopup(

quicktill/user.py

@@ -287,13 +287,16 @@ class database_user(built_in_user):
     """
     def __init__(self, user):
         self.userid = user.id
-        self.dbuser = user
         super().__init__(
             user.fullname, user.shortname,
             permissions=[p.id for p in user.permissions],
             is_superuser=user.superuser)
         self.password_set = (user.password is not None)
 
+    @property
+    def dbuser(self):
+        return td.s.get(User, self.userid)
+
 
 def load_user(userid):
     """Load the specified user from the database
@@ -399,7 +402,7 @@ def token_login(t):
         ui.toast(f"User '{u.fullname}' is not active.")
         return
 
-    different_user = ui.current_user() and ui.current_user().dbuser != u
+    different_user = current_dbuser() != u
 
     if require_user_passwords() and not u.password:
         if different_user:
@@ -1013,10 +1016,6 @@ def save(self):
         u.webuser = wn if len(wn) > 0 else None
         u.enabled = self.actfield.read()
         self.dismiss()
-        # Update current_user().dbuser to ensure it is in the database
-        # session; it may be a detached instance in some circumstances
-        cu = ui.current_user()
-        cu.dbuser = td.s.get(User, cu.userid)
         log(f"Updated details for user {u.logref}")