diff --git a/openPathUpdateAll.py b/openPathUpdateAll.py index b042a79..7b5a1d4 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[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 = "" diff --git a/openPathUpdateSingle.py b/openPathUpdateSingle.py index af3ae24..cba2325 100644 --- a/openPathUpdateSingle.py +++ b/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''') diff --git a/openPathUtil.py b/openPathUtil.py index f6918aa..1635cfa 100644 --- a/openPathUtil.py +++ b/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}') diff --git a/tests/test_openPathUpdateAll.py b/tests/test_openPathUpdateAll.py index 3766de7..a1289c8 100644 --- a/tests/test_openPathUpdateAll.py +++ b/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), + ]) 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), + ])