fix: change get_or_create_author() to upsert_author(). track username…

…/name/email changes (#1058)

services/repository.py

@@ -216,7 +216,7 @@ async def update_commit_from_provider_info(
             ),
         )
     else:
-        commit_author = get_or_create_author(
+        commit_author = upsert_author(
             db_session,
             commit.repository.service,
             author_info["id"],
@@ -281,37 +281,53 @@ async def update_commit_from_provider_info(
     db_session.commit()
 
 
-def get_or_create_author(
+def upsert_author(
     db_session, service, service_id, username, email=None, name=None
 ) -> Owner:
     query = db_session.query(Owner).filter(
         Owner.service == service, Owner.service_id == str(service_id)
     )
     author = query.first()
+
     if author:
-        return author
+        needs_update = False
+        db_session.begin(nested=True)
+        if author.username != username and username is not None:
+            author.username = username
+            needs_update = True
+        if author.name != name and name is not None:
+            author.name = name
+            needs_update = True
+        if author.email != email and email is not None:
+            author.email = email
+            needs_update = True
+
+        if needs_update:
+            db_session.commit()
+        else:
+            db_session.rollback()
+    else:
+        db_session.begin(nested=True)
+        try:
+            author = Owner(
+                service=service,
+                service_id=str(service_id),
+                username=username,
+                name=name,
+                email=email,
+                createstamp=datetime.now(),
+            )
+            db_session.add(author)
+            db_session.commit()
+        except IntegrityError:
+            log.warning(
+                "IntegrityError in upsert_author",
+                extra=dict(service=service, service_id=service_id, username=username),
+            )
+            db_session.rollback()
+            author = query.one()
 
-    db_session.begin(nested=True)
-    try:
-        author = Owner(
-            service=service,
-            service_id=str(service_id),
-            username=username,
-            name=name,
-            email=email,
-            createstamp=datetime.now(),
-        )
-        db_session.add(author)
-        db_session.commit()
-        return author
-    except IntegrityError:
-        log.warning(
-            "IntegrityError in get_or_create_author",
-            extra=dict(service=service, service_id=service_id, username=username),
-        )
-        db_session.rollback()
-        author = query.one()
-        return author
+    return author
 
 
 WEBHOOK_EVENTS = {
@@ -541,7 +557,7 @@ async def fetch_and_update_pull_request_information(
     pull.compared_to = compared_to
 
     if pull is not None and not pull.author:
-        pr_author = get_or_create_author(
+        pr_author = upsert_author(
             db_session,
             repository_service.service,
             pull_information["author"]["id"],

services/tests/test_repository_service.py

@@ -41,10 +41,10 @@
     fetch_and_update_pull_request_information_from_commit,
     fetch_appropriate_parent_for_commit,
     fetch_commit_yaml_and_possibly_store,
-    get_or_create_author,
     get_repo_provider_service,
     get_repo_provider_service_by_id,
     update_commit_from_provider_info,
+    upsert_author,
 )
 from tasks.notify import get_repo_provider_service_for_specific_commit
 
@@ -721,13 +721,13 @@ async def test_fetch_appropriate_parent_for_commit_multiple_parents(
 
 
 @freeze_time("2024-03-28T00:00:00")
-def test_get_or_create_author_doesnt_exist(dbsession):
+def test_upsert_author_doesnt_exist(dbsession):
     service = "github"
     author_id = "123"
     username = "username"
     email = "email"
     name = "name"
-    author = get_or_create_author(dbsession, service, author_id, username, email, name)
+    author = upsert_author(dbsession, service, author_id, username, email, name)
     dbsession.flush()
     assert author.free == 0
     assert author is not None
@@ -746,31 +746,71 @@ def test_get_or_create_author_doesnt_exist(dbsession):
     assert author.createstamp.isoformat() == "2024-03-28T00:00:00"
 
 
-def test_get_or_create_author_already_exists(dbsession):
+def test_upsert_author_already_exists(dbsession):
+    username = "username"
+    email = "[email protected]"
+    service = "bitbucket"
+    service_id = "975"
     owner = OwnerFactory.create(
-        service="bitbucket",
-        service_id="975",
-        email="different_email@email.com",
-        username="whoknew",
+        service=service,
+        service_id=service_id,
+        email=email,
+        username=username,
         yaml=dict(a=["12", "3"]),
     )
     dbsession.add(owner)
     dbsession.flush()
-    service = "bitbucket"
-    author_id = "975"
-    username = "username"
-    email = "email"
-    name = "name"
-    author = get_or_create_author(dbsession, service, author_id, username, email, name)
+
+    author = upsert_author(dbsession, service, service_id, username, None, None)
     dbsession.flush()
     assert author.ownerid == owner.ownerid
     assert author.free == 0
     assert author is not None
-    assert author.service == "bitbucket"
-    assert author.service_id == "975"
+    assert author.service == service
+    assert author.service_id == service_id
     assert author.name == owner.name
-    assert author.email == "[email protected]"
-    assert author.username == "whoknew"
+    assert author.email == email
+    assert author.username == username
+    assert author.plan_activated_users == []
+    assert author.admins == []
+    assert author.permission == []
+    assert author.integration_id is None
+    assert author.yaml == {"a": ["12", "3"]}
+    assert author.oauth_token == owner.oauth_token
+    assert author.bot_id == owner.bot_id
+
+
+def test_upsert_author_needs_update(dbsession):
+    username = "username"
+    email = "[email protected]"
+    service = "bitbucket"
+    service_id = "975"
+    owner = OwnerFactory.create(
+        service=service,
+        service_id=service_id,
+        email=email,
+        username=username,
+        yaml=dict(a=["12", "3"]),
+    )
+    dbsession.add(owner)
+    dbsession.flush()
+
+    new_name = "Newt Namenheim"
+    new_username = "new_username"
+    new_email = "[email protected]"
+    author = upsert_author(
+        dbsession, service, service_id, new_username, new_email, new_name
+    )
+    dbsession.flush()
+
+    assert author is not None
+    assert author.ownerid == owner.ownerid
+    assert author.free == 0
+    assert author.service == service
+    assert author.service_id == service_id
+    assert author.name == new_name
+    assert author.email == new_email
+    assert author.username == new_username
     assert author.plan_activated_users == []
     assert author.admins == []
     assert author.permission == []