From 92ce8371e96ad272b6cfdd9bdf04a72206430bed Mon Sep 17 00:00:00 2001
From: Marcos Prieto <marcospri@gmail.com>
Date: Fri, 23 Aug 2024 10:53:59 +0200
Subject: [PATCH] Revert "Upsert LMSUser everytime we upsert User"

This reverts commit f524354c486bf6c5dad2d6463f75ac3b1396bc84.
---
 lms/services/user.py                 | 38 ----------------------------
 tests/unit/lms/services/user_test.py | 18 +------------
 2 files changed, 1 insertion(+), 55 deletions(-)

diff --git a/lms/services/user.py b/lms/services/user.py
index 747ca79daa..d9dfa116f4 100644
--- a/lms/services/user.py
+++ b/lms/services/user.py
@@ -10,8 +10,6 @@
     Assignment,
     AssignmentMembership,
     GroupingMembership,
-    LMSUser,
-    LMSUserApplicationInstance,
     LTIRole,
     LTIUser,
     RoleScope,
@@ -19,7 +17,6 @@
     User,
 )
 from lms.services.course import CourseService
-from lms.services.upsert import bulk_upsert
 
 
 class UserNotFound(Exception):
@@ -67,43 +64,8 @@ def upsert_user(self, lti_user: LTIUser) -> User:
             # We are only storing emails for teachers now.
             user.email = lti_user.email
 
-        self._upsert_lms_user(user)
-
         return user
 
-    def _upsert_lms_user(self, user: User) -> LMSUser:
-        """Upsert LMSUser based on a User object."""
-        self._db.flush()  # Make sure User has hit the DB on the current transaction
-
-        lms_user = bulk_upsert(
-            self._db,
-            LMSUser,
-            [
-                {
-                    "tool_consumer_instance_guid": user.application_instance.tool_consumer_instance_guid,
-                    "lti_user_id": user.user_id,
-                    "h_userid": user.h_userid,
-                    "email": user.email,
-                    "display_name": user.display_name,
-                }
-            ],
-            index_elements=["h_userid"],
-            update_columns=["updated", "display_name", "email"],
-        ).one()
-        bulk_upsert(
-            self._db,
-            LMSUserApplicationInstance,
-            [
-                {
-                    "application_instance_id": user.application_instance_id,
-                    "lms_user_id": lms_user.id,
-                }
-            ],
-            index_elements=["application_instance_id", "lms_user_id"],
-            update_columns=["updated"],
-        )
-        return lms_user
-
     @lru_cache(maxsize=128)
     def get(self, application_instance, user_id: str) -> User:
         """
diff --git a/tests/unit/lms/services/user_test.py b/tests/unit/lms/services/user_test.py
index 52d6ad49d5..d5a6d7c0d3 100644
--- a/tests/unit/lms/services/user_test.py
+++ b/tests/unit/lms/services/user_test.py
@@ -3,9 +3,8 @@
 
 import pytest
 from h_matchers import Any
-from sqlalchemy import select
 
-from lms.models import LMSUser, RoleScope, RoleType, User
+from lms.models import RoleScope, RoleType, User
 from lms.services import UserService
 from lms.services.user import UserNotFound, factory
 from tests import factories
@@ -31,7 +30,6 @@ def test_upsert_user(self, service, lti_user, db_session):
             }
         )
         assert saved_user == user
-        self.assert_lms_user(db_session, user)
 
     @pytest.mark.usefixtures("user_is_learner")
     def test_upsert_user_doesnt_save_email_for_students(
@@ -42,7 +40,6 @@ def test_upsert_user_doesnt_save_email_for_students(
         saved_user = db_session.query(User).order_by(User.id.desc()).first()
         assert saved_user.roles == lti_user.roles
         assert not saved_user.email
-        self.assert_lms_user(db_session, saved_user)
 
     @pytest.mark.usefixtures("user")
     def test_upsert_user_with_an_existing_user(self, service, lti_user, db_session):
@@ -52,7 +49,6 @@ def test_upsert_user_with_an_existing_user(self, service, lti_user, db_session):
         assert saved_user.id == user.id
         assert saved_user.roles == lti_user.roles
         assert user == saved_user
-        self.assert_lms_user(db_session, user)
 
     @pytest.mark.usefixtures("user")
     def test_upsert_user_doesnt_save_email_for_existing_students(
@@ -65,7 +61,6 @@ def test_upsert_user_doesnt_save_email_for_existing_students(
         saved_user = db_session.query(User).order_by(User.id.desc()).first()
         assert saved_user.roles == lti_user.roles
         assert not saved_user.email
-        self.assert_lms_user(db_session, saved_user)
 
     def test_get(self, user, service):
         db_user = service.get(user.application_instance, user.user_id)
@@ -174,17 +169,6 @@ def test_get_users_by_instructor_h_userid(
 
         assert db_session.scalars(query).all() == [student_in_assigment]
 
-    def assert_lms_user(self, db_session, user):
-        """Assert the corresponding LMSUser to user exists in the DB with the same attributes."""
-
-        lms_user = db_session.scalars(
-            select(LMSUser).where(LMSUser.h_userid == user.h_userid)
-        ).one()
-
-        assert lms_user.display_name == user.display_name
-        assert lms_user.email == user.email
-        assert lms_user.updated == user.updated
-
     @pytest.fixture
     def course(self, application_instance, db_session):
         course = factories.Course(application_instance=application_instance)