diff --git a/scripts/user_retirement/docs/driver_setup.rst b/scripts/user_retirement/docs/driver_setup.rst index 334b99ba68d9..f419405f466e 100644 --- a/scripts/user_retirement/docs/driver_setup.rst +++ b/scripts/user_retirement/docs/driver_setup.rst @@ -55,14 +55,15 @@ of execution for each environment. Each item is a list in the form of: #. Start state name #. End state name -#. IDA to call against (LMS, ECOMMERCE, or CREDENTIALS currently) +#. Service key to call against (for example, LMS or LICENSE_MANAGER) #. Method name to call in `edx_api.py `_ -For example: ``['RETIRING_CREDENTIALS', 'CREDENTIALS_COMPLETE', 'CREDENTIALS', -'retire_learner']`` will set the user's state to ``RETIRING_CREDENTIALS``, call -a pre-instantiated ``retire_learner`` method in the ``CredentialsApi``, then set -the user's state to ``CREDENTIALS_COMPLETE``. +For example: ``['RETIRING_ENROLLMENTS', 'ENROLLMENTS_COMPLETE', 'LMS', +'retirement_unenroll']`` will set the user's state to +``RETIRING_ENROLLMENTS``, call a pre-instantiated ``retirement_unenroll`` +method in the ``LmsApi``, then set the user's state to +``ENROLLMENTS_COMPLETE``. Examples ******** diff --git a/scripts/user_retirement/retire_one_learner.py b/scripts/user_retirement/retire_one_learner.py index 87cb2e4ad270..5b0bfae32633 100755 --- a/scripts/user_retirement/retire_one_learner.py +++ b/scripts/user_retirement/retire_one_learner.py @@ -11,8 +11,6 @@ ecommerce: http://localhost:18130/ credentials: http://localhost:18150/ retirement_pipeline: - - ['RETIRING_CREDENTIALS', 'CREDENTIALS_COMPLETE', 'CREDENTIALS', 'retire_learner'] - - ['RETIRING_ECOM', 'ECOM_COMPLETE', 'ECOMMERCE', 'retire_learner'] - ['RETIRING_LICENSE_MANAGER', 'LICENSE_MANAGER_COMPLETE', 'LICENSE_MANAGER', 'retire_learner'] - ['RETIRING_FORUMS', 'FORUMS_COMPLETE', 'LMS', 'retirement_retire_forum'] - ['RETIRING_EMAIL_LISTS', 'EMAIL_LISTS_COMPLETE', 'LMS', 'retirement_retire_mailings'] @@ -129,21 +127,6 @@ def _get_learner_and_state_index_or_exit(config, username): FAIL_EXCEPTION(ERR_SETUP_FAILED, 'Unexpected error fetching user state!', str(exc)) -def _get_ecom_segment_id(config, learner): - """ - Calls Ecommerce to get the ecom-specific Segment tracking id that we need to retire. - This is only available from Ecommerce, unfortunately, and makes more sense to handle - here than to pass all of the config down to SegmentApi. - """ - try: - return config['ECOMMERCE'].get_tracking_key(learner) - except HttpDoesNotExistException: - LOG('Learner {} not found in Ecommerce. Setting Ecommerce Segment ID to None'.format(learner)) # noqa: UP032 - return None - except Exception as exc: # pylint: disable=broad-except - FAIL_EXCEPTION(ERR_SETUP_FAILED, 'Unexpected error fetching Ecommerce tracking id!', str(exc)) - - @click.command("retire_learner") @click.option( '--username', @@ -172,9 +155,6 @@ def retire_learner( learner, learner_state_index = _get_learner_and_state_index_or_exit(config, username) - if config.get('fetch_ecommerce_segment_id', False): - learner['ecommerce_segment_id'] = _get_ecom_segment_id(config, learner) - start_state = None try: for start_state, end_state, service, method in config['retirement_pipeline']: diff --git a/scripts/user_retirement/tests/retirement_helpers.py b/scripts/user_retirement/tests/retirement_helpers.py index 17e76c40e7d9..8c7ce06ccf64 100644 --- a/scripts/user_retirement/tests/retirement_helpers.py +++ b/scripts/user_retirement/tests/retirement_helpers.py @@ -53,7 +53,7 @@ def flatten_partner_list(partner_list): return [partner for sublist in partner_list for partner in sublist] -def fake_config_file(f, orgs=None, fetch_ecom_segment_id=False): +def fake_config_file(f, orgs=None): """ Create a config file for a single test. Combined with CliRunner.isolated_filesystem() to ensure the file lifetime is limited to the test. See _call_script for usage. @@ -85,9 +85,6 @@ def fake_config_file(f, orgs=None, fetch_ecom_segment_id=False): 'segment_auth_token': 'fakeauthtoken', } - if fetch_ecom_segment_id: - config['fetch_ecommerce_segment_id'] = True - yaml.safe_dump(config, f) diff --git a/scripts/user_retirement/tests/test_retire_one_learner.py b/scripts/user_retirement/tests/test_retire_one_learner.py index 9299da27d5fd..3f45e82ea5cc 100644 --- a/scripts/user_retirement/tests/test_retire_one_learner.py +++ b/scripts/user_retirement/tests/test_retire_one_learner.py @@ -19,7 +19,7 @@ from scripts.user_retirement.utils.exception import HttpDoesNotExistException -def _call_script(username, fetch_ecom_segment_id=False): +def _call_script(username): """ Call the retired learner script with the given username and a generic, temporary config file. Returns the CliRunner.invoke results @@ -27,7 +27,7 @@ def _call_script(username, fetch_ecom_segment_id=False): runner = CliRunner() with runner.isolated_filesystem(): with open('test_config.yml', 'w') as f: - fake_config_file(f, fetch_ecom_segment_id=fetch_ecom_segment_id) + fake_config_file(f) result = runner.invoke(retire_learner, args=['--username', username, '--config_file', 'test_config.yml']) print(result) print(result.output) @@ -35,7 +35,6 @@ def _call_script(username, fetch_ecom_segment_id=False): @patch('scripts.user_retirement.utils.edx_api.BaseApiClient.get_access_token') -@patch('scripts.user_retirement.utils.edx_api.EcommerceApi.get_tracking_key') @patch.multiple( 'scripts.user_retirement.utils.edx_api.LmsApi', get_learner_retirement_state=DEFAULT, @@ -48,7 +47,7 @@ def _call_script(username, fetch_ecom_segment_id=False): def test_successful_retirement(*args, **kwargs): username = 'test_username' - mock_get_access_token = args[1] + mock_get_access_token = args[0] mock_get_retirement_state = kwargs['get_learner_retirement_state'] mock_update_learner_state = kwargs['update_learner_retirement_state'] mock_retire_forum = kwargs['retirement_retire_forum'] @@ -59,7 +58,7 @@ def test_successful_retirement(*args, **kwargs): mock_get_access_token.return_value = ('THIS_IS_A_JWT', None) mock_get_retirement_state.return_value = get_fake_user_retirement(original_username=username) - result = _call_script(username, fetch_ecom_segment_id=True) + result = _call_script(username) # Called once per API we instantiate (LMS, ECommerce, Credentials) assert mock_get_access_token.call_count == 3 @@ -297,116 +296,3 @@ def test_skipping_states(*args, **kwargs): assert required_output in result.output -@patch('scripts.user_retirement.utils.edx_api.BaseApiClient.get_access_token') -@patch('scripts.user_retirement.utils.edx_api.EcommerceApi.get_tracking_key') -@patch.multiple( - 'scripts.user_retirement.utils.edx_api.LmsApi', - get_learner_retirement_state=DEFAULT, - update_learner_retirement_state=DEFAULT, - retirement_retire_forum=DEFAULT, - retirement_retire_mailings=DEFAULT, - retirement_unenroll=DEFAULT, - retirement_lms_retire=DEFAULT -) -def test_get_segment_id_success(*args, **kwargs): - username = 'test_username' - - mock_get_tracking_key = args[0] - mock_get_access_token = args[1] - mock_get_retirement_state = kwargs['get_learner_retirement_state'] - mock_retirement_retire_forum = kwargs['retirement_retire_forum'] - - mock_get_access_token.return_value = ('THIS_IS_A_JWT', None) - mock_get_tracking_key.return_value = {'id': 1, 'ecommerce_tracking_id': 'ecommerce-1'} - - # The learner starts off with these values, 'ecommerce_segment_id' is added during script - # startup - mock_get_retirement_state.return_value = get_fake_user_retirement( - original_username=username, - ) - - _call_script(username, fetch_ecom_segment_id=True) - mock_get_tracking_key.assert_called_once_with(mock_get_retirement_state.return_value) - - config_after_get_segment_id = mock_get_retirement_state.return_value - config_after_get_segment_id['ecommerce_segment_id'] = 'ecommerce-1' - - mock_retirement_retire_forum.assert_called_once_with(config_after_get_segment_id) - - -@patch('scripts.user_retirement.utils.edx_api.BaseApiClient.get_access_token') -@patch('scripts.user_retirement.utils.edx_api.EcommerceApi.get_tracking_key') -@patch.multiple( - 'scripts.user_retirement.utils.edx_api.LmsApi', - get_learner_retirement_state=DEFAULT, - update_learner_retirement_state=DEFAULT, - retirement_retire_forum=DEFAULT, - retirement_retire_mailings=DEFAULT, - retirement_unenroll=DEFAULT, - retirement_lms_retire=DEFAULT -) -def test_get_segment_id_not_found(*args, **kwargs): - username = 'test_username' - - mock_get_tracking_key = args[0] - mock_get_access_token = args[1] - mock_get_retirement_state = kwargs['get_learner_retirement_state'] - - mock_get_access_token.return_value = ('THIS_IS_A_JWT', None) - mock_get_tracking_key.side_effect = HttpDoesNotExistException('{} not found'.format(username)) # noqa: UP032 - - mock_get_retirement_state.return_value = get_fake_user_retirement( - original_username=username, - ) - - result = _call_script(username, fetch_ecom_segment_id=True) - mock_get_tracking_key.assert_called_once_with(mock_get_retirement_state.return_value) - assert 'Setting Ecommerce Segment ID to None' in result.output - - # Reset our call counts for the next test - mock_get_access_token.reset_mock() - mock_get_retirement_state.reset_mock() - - -@patch('scripts.user_retirement.utils.edx_api.BaseApiClient.get_access_token') -@patch('scripts.user_retirement.utils.edx_api.EcommerceApi.get_tracking_key') -@patch.multiple( - 'scripts.user_retirement.utils.edx_api.LmsApi', - get_learner_retirement_state=DEFAULT, - update_learner_retirement_state=DEFAULT, - retirement_retire_forum=DEFAULT, - retirement_retire_mailings=DEFAULT, - retirement_unenroll=DEFAULT, - retirement_lms_retire=DEFAULT -) -def test_get_segment_id_error(*args, **kwargs): - username = 'test_username' - - mock_get_tracking_key = args[0] - mock_get_access_token = args[1] - mock_get_retirement_state = kwargs['get_learner_retirement_state'] - mock_update_learner_state = kwargs['update_learner_retirement_state'] - - mock_get_access_token.return_value = ('THIS_IS_A_JWT', None) - - test_exception_message = 'Test Exception!' - mock_get_tracking_key.side_effect = Exception(test_exception_message) - - mock_get_retirement_state.return_value = get_fake_user_retirement( - original_username=username, - ) - - mock_get_retirement_state.return_value = { - 'original_username': username, - 'current_state': { - 'state_name': 'PENDING' - } - } - - result = _call_script(username, fetch_ecom_segment_id=True) - mock_get_tracking_key.assert_called_once_with(mock_get_retirement_state.return_value) - mock_update_learner_state.assert_not_called() - - assert result.exit_code == ERR_SETUP_FAILED - assert 'Unexpected error fetching Ecommerce tracking id!' in result.output - assert test_exception_message in result.output diff --git a/scripts/user_retirement/tests/utils/test_edx_api.py b/scripts/user_retirement/tests/utils/test_edx_api.py index c23cad5b8b03..cdddc389c974 100644 --- a/scripts/user_retirement/tests/utils/test_edx_api.py +++ b/scripts/user_retirement/tests/utils/test_edx_api.py @@ -351,39 +351,6 @@ def tearDown(self): super().tearDown() responses.reset() - @patch.object(edx_api.EcommerceApi, 'retire_learner') - def test_retirement_partner_report(self, mock_method): - json_data = { - 'username': FAKE_ORIGINAL_USERNAME, - } - responses.add( - POST, - urljoin(self.lms_base_url, 'api/v2/user/retire/'), - match=[matchers.json_params_matcher(json_data)] - ) - self.ecommerce_api.retire_learner( - learner=get_fake_user_retirement(original_username=FAKE_ORIGINAL_USERNAME) - ) - mock_method.assert_called_once_with( - learner=get_fake_user_retirement(original_username=FAKE_ORIGINAL_USERNAME) - ) - - @patch.object(edx_api.EcommerceApi, 'retire_learner') - def get_tracking_key(self, mock_method): - original_username = { - 'original_username': get_fake_user_retirement(original_username=FAKE_ORIGINAL_USERNAME) - } - responses.add( - GET, - urljoin(self.lms_base_url, f"api/v2/retirement/tracking_id/{original_username}/"), - ) - self.ecommerce_api.get_tracking_key( - learner=get_fake_user_retirement(original_username=FAKE_ORIGINAL_USERNAME) - ) - mock_method.assert_called_once_with( - learner=get_fake_user_retirement(original_username=FAKE_ORIGINAL_USERNAME) - ) - @patch.object(edx_api.EcommerceApi, 'replace_usernames') def test_replace_usernames(self, mock_method): json_data = { @@ -424,23 +391,6 @@ def tearDown(self): super().tearDown() responses.reset() - @patch.object(edx_api.CredentialsApi, 'retire_learner') - def test_retire_learner(self, mock_method): - json_data = { - 'username': FAKE_ORIGINAL_USERNAME - } - responses.add( - POST, - urljoin(self.credentials_base_url, 'user/retire/'), - match=[matchers.json_params_matcher(json_data)] - ) - self.credentials_api.retire_learner( - learner=get_fake_user_retirement(original_username=FAKE_ORIGINAL_USERNAME) - ) - mock_method.assert_called_once_with( - learner=get_fake_user_retirement(original_username=FAKE_ORIGINAL_USERNAME) - ) - @patch.object(edx_api.CredentialsApi, 'replace_usernames') def test_replace_usernames(self, mock_method): json_data = { diff --git a/scripts/user_retirement/utils/edx_api.py b/scripts/user_retirement/utils/edx_api.py index 42ab3f137ffa..f068b8416e6a 100644 --- a/scripts/user_retirement/utils/edx_api.py +++ b/scripts/user_retirement/utils/edx_api.py @@ -398,24 +398,6 @@ class EcommerceApi(BaseApiClient): Ecommerce API client with convenience methods for making API calls. """ - @_retry_lms_api() - def retire_learner(self, learner): - """ - Performs the learner retirement step for Ecommerce - """ - data = {"username": learner["original_username"]} - api_url = self.get_api_url("api/v2/user/retire") - return self._request("POST", api_url, json=data) - - @_retry_lms_api() - def get_tracking_key(self, learner): - """ - Fetches the ecommerce tracking id used for Segment tracking when - ecommerce doesn't have access to the LMS user id. - """ - api_url = self.get_api_url(f"api/v2/retirement/tracking_id/{learner['original_username']}") - return self._request("GET", api_url)["ecommerce_tracking_id"] - def replace_usernames(self, username_mappings): """ Calls the ecommerce API to replace usernames. @@ -434,15 +416,6 @@ class CredentialsApi(BaseApiClient): Credentials API client with convenience methods for making API calls. """ - @_retry_lms_api() - def retire_learner(self, learner): - """ - Performs the learner retirement step for Credentials - """ - data = {"username": learner["original_username"]} - api_url = self.get_api_url("user/retire") - return self._request("POST", api_url, json=data) - def replace_usernames(self, username_mappings): """ Calls the credentials API to replace usernames.