Handle missing openpath ids

openPathUpdateAll.py

@@ -27,6 +27,13 @@ def getWarningText(warningUsers):
 def openPathUpdateAll(neonAccounts, mailSummary = False):
     opUsers = openPathUtil.getAllUsers()
 
+    # Build externalId->opUser lookup to reconcile Neon accounts missing their OpenPathID
+    opUsersByExternalId = {}
+    for opUser in opUsers.values():
+        externalId = opUser.get("externalId")
+        if externalId:
+            opUsersByExternalId[externalId] = opUser
+
     subscriberCount = 0
     ceramicsCount = 0
     facilityUserCount = 0
@@ -42,57 +49,68 @@ def openPathUpdateAll(neonAccounts, mailSummary = False):
     paidRegulars = 0
     paidCeramics = 0
 
-    for account in neonAccounts:
-        if not neonAccounts[account].get("paidRegular") and not neonAccounts[account].get("paidCeramics") and not neonUtil.accountIsType(neonAccounts[account], neonUtil.STAFF_TYPE):
+    for accountId, account in neonAccounts.items():
+        if not account.get("OpenPathID") and accountId in opUsersByExternalId:
+            opUser = opUsersByExternalId[accountId]
+            logging.info(
+                "Reconciling OpenPathID %s for Neon account %s (%s)",
+                opUser.get("id"),
+                accountId,
+                account.get("Email 1"),
+            )
+            account["OpenPathID"] = opUser.get("id")
+            neonUtil.updateOpenPathID(account)
+
+        if not account.get("paidRegular") and not account.get("paidCeramics") and not neonUtil.accountIsType(account, neonUtil.STAFF_TYPE):
             #Accounts that are neither paid nor staff might still have access
-            if neonUtil.accountIsType(neonAccounts[account], neonUtil.LEAD_TYPE):
-                compedLeaders.append(f'''{neonAccounts[account].get("fullName")} ({neonAccounts[account].get("Email 1")})''')
-            elif neonAccounts[account].get("compedRegular") or neonAccounts[account].get("compedCeramics"):
-                compedSubscribers.append(f'''{neonAccounts[account].get("fullName")} ({neonAccounts[account].get("Email 1")})''')
+            if neonUtil.accountIsType(account, neonUtil.LEAD_TYPE):
+                compedLeaders.append(f'''{account.get("fullName")} ({account.get("Email 1")})''')
+            elif account.get("compedRegular") or account.get("compedCeramics"):
+                compedSubscribers.append(f'''{account.get("fullName")} ({account.get("Email 1")})''')
 
-        if neonAccounts[account].get("validMembership"):
+        if account.get("validMembership"):
             subscriberCount += 1
-            if neonAccounts[account].get("ceramicsMembership"):
+            if account.get("ceramicsMembership"):
                 ceramicsCount += 1
 
             #accounts with concurrent paid regular and ceramics memberships are most likely
             #upgrades that should be only counted as ceramics members
-            if neonAccounts[account].get("paidCeramics"):
+            if account.get("paidCeramics"):
                 paidCeramics += 1
-            elif neonAccounts[account].get("paidRegular"):
+            elif account.get("paidRegular"):
                 paidRegulars += 1
 
-        if neonAccounts[account].get("paidRegular") and neonAccounts[account].get("paidCeramics"):
-            logging.info(f'''{neonAccounts[account].get("fullName")} ({neonAccounts[account].get("Email 1")}) has concurrent paid memberships.''')
+        if account.get("paidRegular") and account.get("paidCeramics"):
+            logging.info(f'''{account.get("fullName")} ({account.get("Email 1")}) has concurrent paid memberships.''')
 
-        if neonUtil.subscriberHasFacilityAccess(neonAccounts[account]):
+        if neonUtil.subscriberHasFacilityAccess(account):
             facilityUserCount += 1
 
-        if neonUtil.subscriberHasCeramicsAccess(neonAccounts[account]):
+        if neonUtil.subscriberHasCeramicsAccess(account):
             ceramicsFacilityCount += 1
 
-        if neonAccounts[account].get("OpenPathID"):
-            openPathUtil.updateGroups(neonAccounts[account], 
-                                        openPathGroups=opUsers.get(int(neonAccounts[account].get("OpenPathID"))).get("groups"))
+        if account.get("OpenPathID"):
+            openPathUtil.updateGroups(account,
+                                        openPathGroups=opUsers.get(int(account.get("OpenPathID"))).get("groups"))
             #note that this isn't necessarily 100% accurate, because we have Neon users with provisioned OpenPath IDs and no access groups
             #assuming that typical users who gained and lost openPath access have a signed waiver
-            if not neonAccounts[account].get("WaiverDate"):
-                warningUsers.append(f'''{neonAccounts[account].get("fullName")} ({neonAccounts[account].get("Email 1")})''')
-        elif neonUtil.accountHasFacilityAccess(neonAccounts[account]):
-            neonAccounts[account] = openPathUtil.createUser(neonAccounts[account])
-            openPathUtil.updateGroups(neonAccounts[account],
-                                        openPathGroups=[]) #pass empty groups list to skip the http get
-            openPathUtil.createMobileCredential(neonAccounts[account])
-        elif neonAccounts[account].get("validMembership"):
-            startDate = neonAccounts[account].get("Membership Start Date")
-            if not neonAccounts[account].get("WaiverDate"):
-                missingWaiverSubscribers[account] = f'''{neonAccounts[account].get("fullName")} ({neonAccounts[account].get("Email 1")}) - since {startDate}'''
-            if not neonAccounts[account].get("FacilityTourDate"):
-                missingTourSubscribers[account] = f'''{neonAccounts[account].get("fullName")} ({neonAccounts[account].get("Email 1")}) - since {startDate}'''
+            if not account.get("WaiverDate"):
+                warningUsers.append(f'''{account.get("fullName")} ({account.get("Email 1")})''')
+        elif neonUtil.accountHasFacilityAccess(account):
+            if openPathUtil.createUser(account):
+                openPathUtil.updateGroups(account,
+                                            openPathGroups=[]) #pass empty groups list to skip the http get
+                openPathUtil.createMobileCredential(account)
+        elif account.get("validMembership"):
+            startDate = account.get("Membership Start Date")
+            if not account.get("WaiverDate"):
+                missingWaiverSubscribers[accountId] = f'''{account.get("fullName")} ({account.get("Email 1")}) - since {startDate}'''
+            if not account.get("FacilityTourDate"):
+                missingTourSubscribers[accountId] = f'''{account.get("fullName")} ({account.get("Email 1")}) - since {startDate}'''
 
         #an account might be missing CSI but still have regular facility access stuff handled
-        if neonAccounts[account].get("ceramicsMembership") and not neonAccounts[account].get("CsiDate"):
-            missingCsiSubscribers[account] = f'''{neonAccounts[account].get("fullName")} ({neonAccounts[account].get("Email 1")}) - since {neonAccounts[account].get("Ceramics Start Date")}'''
+        if account.get("ceramicsMembership") and not account.get("CsiDate"):
+            missingCsiSubscribers[accountId] = f'''{account.get("fullName")} ({account.get("Email 1")}) - since {account.get("Ceramics Start Date")}'''
 
     list_separator = '\n            '
     compedSubscriberString = ""

openPathUpdateSingle.py

@@ -22,9 +22,9 @@ def openPathUpdateSingle(neonID):
     elif ( neonUtil.accountHasFacilityAccess(account) or 
            neonUtil.accountIsType(account, neonUtil.INSTRUCTOR_TYPE) or
            neonUtil.accountIsType(account, neonUtil.ONDUTY_TYPE)):
-        account = openPathUtil.createUser(account)
-        openPathUtil.updateGroups(account, openPathGroups=[]) #pass empty groups list to skip the http get
-        openPathUtil.createMobileCredential(account)
+        if openPathUtil.createUser(account):
+            openPathUtil.updateGroups(account, openPathGroups=[]) #pass empty groups list to skip the http get
+            openPathUtil.createMobileCredential(account)
     elif account.get("validMembership"):
         if not account.get("WaiverDate"):
             logging.info(f'''{account.get("fullName")} ({account.get("Email 1")} is missing the Waiver''')

openPathUtil.py

@@ -406,11 +406,12 @@ def createUser(neonAccount):
         response = requests.post(url, json=data, headers=O_headers)
         if response.status_code != 201:
             logging.error(
-                "Status %s (expected 201) creating OpenPath User %s",
+                "Status %s (expected 201) creating OpenPath User %s\nResponse: %s",
                 response.status_code,
                 pformat(data),
+                response.text,
             )
-            return neonAccount
+            return False
 
         # openPath times are in UTC
         opUser = response.json().get("data")
@@ -454,7 +455,7 @@ def createUser(neonAccount):
     else:
         logging.warning("DryRun in openPathUtil.createUser()")
 
-    return neonAccount
+    return True
 
 
 #################################################################################
@@ -509,26 +510,3 @@ def createMobileCredential(neonAccount):
         raise ValueError(
             f"Post {url} returned status code {response.status_code}; expected 204"
         )
-
-
-#################################################################################
-# Given a single Neon ID, perform necessary OpenPath updates
-#################################################################################
-def updateOpenPathByNeonId(neonId):
-    logging.info("Updating Neon ID %s", neonId)
-    account = neonUtil.getMemberById(neonId)
-    # logging.debug(account)
-    if account.get("OpenPathID"):
-        updateGroups(account, email=True)
-    #instructors and on-duty volunteers might need OP credentials without having facility access
-    elif ( neonUtil.accountHasFacilityAccess(account) or 
-           neonUtil.accountIsType(account, neonUtil.INSTRUCTOR_TYPE) or
-           neonUtil.accountIsType(account, neonUtil.ONDUTY_TYPE) or
-           neonUtil.accountIsType(account, neonUtil.ONDUTY_TYPE_CERAMICS)):
-        logging.info(f'Creating account for Neon user {neonId}')
-        account = createUser(account)
-        updateGroups(
-            account, openPathGroups=[]
-        )  # pass empty groups list to skip the http get
-        createMobileCredential(account)
-    logging.info(f'Successfully updated Alta groups for Neon user {neonID}')

tests/test_openPathUpdateAll.py

@@ -210,3 +210,54 @@ def test_creates_user(requests_mock):
         "mobile": {"name": "Automatic Mobile Credential"},
         "credentialTypeId": 1,
     }
+
+
+def test_reconciles_missing_openpath_id(requests_mock):
+    """Account missing OpenPathID is reconciled via externalId and updated normally."""
+    rm = requests_mock
+
+    account = NeonUserMock(waiver_date=start, facility_tour_date=tour)\
+        .add_membership(REGULAR, start, end, fee=100.0)
+
+    get_all_users = mock_get_all_users(rm, [
+        {"id": ALTA_ID, "externalId": str(account.account_id), "groups": []},
+    ])
+    update_neon = rm.patch(f'{N_baseURL}/accounts/{account.account_id}', status_code=200)
+    update_groups = rm.put(f'{O_baseURL}/users/{ALTA_ID}/groupIds', status_code=204)
+
+    accounts = {str(account.account_id): account.mock(rm)}
+
+    assert_history(rm, lambda: openPathUpdateAll(accounts), [
+        (get_all_users._method, get_all_users._url),
+        (update_neon._method, update_neon._url),
+        (update_groups._method, update_groups._url),
+    ])
+
+    assert update_neon.last_request.json() == {
+        "individualAccount": {
+            "accountCustomFields": [
+                {"id": str(ACCOUNT_FIELD_OPENPATH_ID), "name": "OpenPathID", "value": str(ALTA_ID)}
+            ]
+        }
+    }
+    assert update_groups.last_request.json() == {"groupIds": [GROUP_SUBSCRIBERS]}
+
+
+def test_handles_failed_user_creation(requests_mock):
+    """When OpenPath returns 400 for user creation, skip group and credential setup."""
+    rm = requests_mock
+
+    account = NeonUserMock(waiver_date=start, facility_tour_date=tour)\
+        .add_membership(REGULAR, start, end, fee=100.0)
+
+    get_all_users = mock_get_all_users(rm, [])
+    create_alta = rm.post(f'{O_baseURL}/users', status_code=400, json={
+        "message": "This user is already active in this organization."
+    })
+
+    accounts = {str(account.account_id): account.mock(rm)}
+
+    assert_history(rm, lambda: openPathUpdateAll(accounts), [
+        (get_all_users._method, get_all_users._url),
+        (create_alta._method, create_alta._url),
+    ])

tests/test_openPathUpdateSingle.py

@@ -155,3 +155,22 @@ def test_creates_user_with_correct_group(requests_mock, mocker):
             "mobile": {"name": "Automatic Mobile Credential"},
             "credentialTypeId": 1,
         }
+
+
+def test_handles_failed_user_creation(requests_mock):
+    """When OpenPath returns 400 for user creation, log error and don't proceed."""
+    rm = requests_mock
+
+    account = NeonUserMock(waiver_date=start, facility_tour_date=tour)\
+        .add_membership(REGULAR, start, end, fee=100.0)
+    account.mock(rm)
+
+    create_alta = rm.post(f'{O_baseURL}/users', status_code=400, json={
+        "message": "This user is already active in this organization."
+    })
+
+    assert_history(rm, lambda: openPathUpdateSingle(account.account_id), [
+        ('GET', f'{N_baseURL}/accounts/{account.account_id}'),
+        ('GET', f'{N_baseURL}/accounts/{account.account_id}/memberships'),
+        (create_alta._method, create_alta._url),
+    ])