From 3a750e9ce3d1ac0c97f7de3b4191ba3a26d5fbf3 Mon Sep 17 00:00:00 2001 From: robz Date: Sun, 29 Mar 2026 12:02:04 -0500 Subject: [PATCH 1/6] Add logging for response errors, skip updating after an error, and explicitly check reverse external_id map --- openPathUpdateAll.py | 27 ++++++++++++++++++++++++--- openPathUtil.py | 12 +++++++----- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/openPathUpdateAll.py b/openPathUpdateAll.py index b042a79..e729c20 100644 --- a/openPathUpdateAll.py +++ b/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[str(externalId)] = opUser + subscriberCount = 0 ceramicsCount = 0 facilityUserCount = 0 @@ -43,6 +50,19 @@ def openPathUpdateAll(neonAccounts, mailSummary = False): paidCeramics = 0 for account in neonAccounts: + if not neonAccounts[account].get("OpenPathID"): + neonId = neonAccounts[account].get("Account ID") + if neonId and str(neonId) in opUsersByExternalId: + opUser = opUsersByExternalId[str(neonId)] + logging.info( + "Reconciling OpenPathID %s for Neon account %s (%s)", + opUser.get("id"), + neonId, + neonAccounts[account].get("Email 1"), + ) + neonAccounts[account]["OpenPathID"] = opUser.get("id") + neonUtil.updateOpenPathID(neonAccounts[account]) + if not neonAccounts[account].get("paidRegular") and not neonAccounts[account].get("paidCeramics") and not neonUtil.accountIsType(neonAccounts[account], neonUtil.STAFF_TYPE): #Accounts that are neither paid nor staff might still have access if neonUtil.accountIsType(neonAccounts[account], neonUtil.LEAD_TYPE): @@ -80,9 +100,10 @@ def openPathUpdateAll(neonAccounts, mailSummary = False): 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]) + if neonAccounts[account].get("OpenPathID"): + 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"): diff --git a/openPathUtil.py b/openPathUtil.py index f6918aa..b00b3a1 100644 --- a/openPathUtil.py +++ b/openPathUtil.py @@ -406,9 +406,10 @@ 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 @@ -527,8 +528,9 @@ def updateOpenPathByNeonId(neonId): 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) + if account.get("OpenPathID"): + 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}') From 1c677377820b03307e4548dd4e70965cad0f2c2d Mon Sep 17 00:00:00 2001 From: robz Date: Sun, 29 Mar 2026 12:08:54 -0500 Subject: [PATCH 2/6] clean up account loop --- openPathUpdateAll.py | 90 ++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/openPathUpdateAll.py b/openPathUpdateAll.py index e729c20..5077b93 100644 --- a/openPathUpdateAll.py +++ b/openPathUpdateAll.py @@ -32,7 +32,7 @@ def openPathUpdateAll(neonAccounts, mailSummary = False): for opUser in opUsers.values(): externalId = opUser.get("externalId") if externalId: - opUsersByExternalId[str(externalId)] = opUser + opUsersByExternalId[externalId] = opUser subscriberCount = 0 ceramicsCount = 0 @@ -49,71 +49,69 @@ def openPathUpdateAll(neonAccounts, mailSummary = False): paidRegulars = 0 paidCeramics = 0 - for account in neonAccounts: - if not neonAccounts[account].get("OpenPathID"): - neonId = neonAccounts[account].get("Account ID") - if neonId and str(neonId) in opUsersByExternalId: - opUser = opUsersByExternalId[str(neonId)] - logging.info( - "Reconciling OpenPathID %s for Neon account %s (%s)", - opUser.get("id"), - neonId, - neonAccounts[account].get("Email 1"), - ) - neonAccounts[account]["OpenPathID"] = opUser.get("id") - neonUtil.updateOpenPathID(neonAccounts[account]) - - 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]) - if neonAccounts[account].get("OpenPathID"): - openPathUtil.updateGroups(neonAccounts[account], + if not account.get("WaiverDate"): + warningUsers.append(f'''{account.get("fullName")} ({account.get("Email 1")})''') + elif neonUtil.accountHasFacilityAccess(account): + openPathUtil.createUser(account) + if account.get("OpenPathID"): + openPathUtil.updateGroups(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}''' + 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 = "" From 2473650421a220c4a83eec1ad91680a2d151d8d5 Mon Sep 17 00:00:00 2001 From: robz Date: Sun, 29 Mar 2026 12:14:32 -0500 Subject: [PATCH 3/6] Add unit test --- tests/test_openPathUpdateAll.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/test_openPathUpdateAll.py b/tests/test_openPathUpdateAll.py index 3766de7..6cdb474 100644 --- a/tests/test_openPathUpdateAll.py +++ b/tests/test_openPathUpdateAll.py @@ -210,3 +210,34 @@ 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]} From e8d673b384a2b282c00309966fded3e1312a2d35 Mon Sep 17 00:00:00 2001 From: robz Date: Sun, 29 Mar 2026 12:40:10 -0500 Subject: [PATCH 4/6] tests and early return --- openPathUpdateSingle.py | 3 +++ openPathUtil.py | 12 ++++++------ tests/test_openPathUpdateAll.py | 20 ++++++++++++++++++++ tests/test_openPathUpdateSingle.py | 19 +++++++++++++++++++ 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/openPathUpdateSingle.py b/openPathUpdateSingle.py index af3ae24..0cd502e 100644 --- a/openPathUpdateSingle.py +++ b/openPathUpdateSingle.py @@ -23,6 +23,9 @@ def openPathUpdateSingle(neonID): neonUtil.accountIsType(account, neonUtil.INSTRUCTOR_TYPE) or neonUtil.accountIsType(account, neonUtil.ONDUTY_TYPE)): account = openPathUtil.createUser(account) + if not account.get("OpenPathID"): + logging.error(f'Failed to create Alta user for Neon user {neonID}') + return openPathUtil.updateGroups(account, openPathGroups=[]) #pass empty groups list to skip the http get openPathUtil.createMobileCredential(account) elif account.get("validMembership"): diff --git a/openPathUtil.py b/openPathUtil.py index b00b3a1..002140c 100644 --- a/openPathUtil.py +++ b/openPathUtil.py @@ -528,9 +528,9 @@ def updateOpenPathByNeonId(neonId): neonUtil.accountIsType(account, neonUtil.ONDUTY_TYPE_CERAMICS)): logging.info(f'Creating account for Neon user {neonId}') account = createUser(account) - if account.get("OpenPathID"): - 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}') + if not account.get("OpenPathID"): + logging.error(f'Failed to create Alta user for Neon user {neonId}') + return + 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}') diff --git a/tests/test_openPathUpdateAll.py b/tests/test_openPathUpdateAll.py index 6cdb474..a1289c8 100644 --- a/tests/test_openPathUpdateAll.py +++ b/tests/test_openPathUpdateAll.py @@ -241,3 +241,23 @@ def test_reconciles_missing_openpath_id(requests_mock): } } 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), + ]) diff --git a/tests/test_openPathUpdateSingle.py b/tests/test_openPathUpdateSingle.py index 228574a..150cb0a 100644 --- a/tests/test_openPathUpdateSingle.py +++ b/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), + ]) From ebeb17967f6879401b311e5da0bbb9e9ac120576 Mon Sep 17 00:00:00 2001 From: robz Date: Sun, 29 Mar 2026 12:59:00 -0500 Subject: [PATCH 5/6] delete dead code --- openPathUtil.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/openPathUtil.py b/openPathUtil.py index 002140c..5867d2a 100644 --- a/openPathUtil.py +++ b/openPathUtil.py @@ -510,27 +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) - if not account.get("OpenPathID"): - logging.error(f'Failed to create Alta user for Neon user {neonId}') - return - 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}') From 48303263b6202999022e244f65a4ee4a5b60829d Mon Sep 17 00:00:00 2001 From: robz Date: Sun, 29 Mar 2026 13:09:42 -0500 Subject: [PATCH 6/6] clean up return value for createUser --- openPathUpdateAll.py | 3 +-- openPathUpdateSingle.py | 9 +++------ openPathUtil.py | 4 ++-- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/openPathUpdateAll.py b/openPathUpdateAll.py index 5077b93..7b5a1d4 100644 --- a/openPathUpdateAll.py +++ b/openPathUpdateAll.py @@ -97,8 +97,7 @@ def openPathUpdateAll(neonAccounts, mailSummary = False): if not account.get("WaiverDate"): warningUsers.append(f'''{account.get("fullName")} ({account.get("Email 1")})''') elif neonUtil.accountHasFacilityAccess(account): - openPathUtil.createUser(account) - if account.get("OpenPathID"): + if openPathUtil.createUser(account): openPathUtil.updateGroups(account, openPathGroups=[]) #pass empty groups list to skip the http get openPathUtil.createMobileCredential(account) diff --git a/openPathUpdateSingle.py b/openPathUpdateSingle.py index 0cd502e..cba2325 100644 --- a/openPathUpdateSingle.py +++ b/openPathUpdateSingle.py @@ -22,12 +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) - if not account.get("OpenPathID"): - logging.error(f'Failed to create Alta user for Neon user {neonID}') - return - 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''') diff --git a/openPathUtil.py b/openPathUtil.py index 5867d2a..1635cfa 100644 --- a/openPathUtil.py +++ b/openPathUtil.py @@ -411,7 +411,7 @@ def createUser(neonAccount): pformat(data), response.text, ) - return neonAccount + return False # openPath times are in UTC opUser = response.json().get("data") @@ -455,7 +455,7 @@ def createUser(neonAccount): else: logging.warning("DryRun in openPathUtil.createUser()") - return neonAccount + return True #################################################################################