-
Notifications
You must be signed in to change notification settings - Fork 64
feat: moving retirement code to enterprise #2559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
de66e26
feat: moving retirement code to enterprise
kiram15 006dd65
fix: version bump
kiram15 e0e7f24
fix: whoopsie quality fixes
kiram15 b7db665
Merge branch 'master' into kiram15/ENT-11475
kiram15 591f3fc
fix: PR requests
kiram15 8cc4314
fix: lint changes
kiram15 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| """ | ||
| Django signal handlers for the consent module. | ||
| """ | ||
| from logging import getLogger | ||
|
|
||
| from consent.models import DataSharingConsent | ||
|
|
||
| logger = getLogger(__name__) | ||
|
|
||
| try: | ||
| from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL | ||
| except ImportError: | ||
| USER_RETIRE_LMS_CRITICAL = None | ||
|
|
||
|
|
||
| def retire_users_data_sharing_consent(sender, user, retired_username, **kwargs): # pylint: disable=unused-argument | ||
| """ | ||
| Handle USER_RETIRE_LMS_CRITICAL signal: retire DataSharingConsent username records. | ||
|
|
||
| Idempotent: only updates records where username hasn't already been changed to the retired value. | ||
| """ | ||
| if user.username != retired_username: | ||
| logger.info( | ||
| "Updating username %s to retired username %s on DataSharingConsent records", | ||
| user.username, | ||
| retired_username, | ||
| ) | ||
| DataSharingConsent.objects.filter( | ||
| username=user.username, | ||
| ).exclude( | ||
| username=retired_username, | ||
| ).update(username=retired_username) | ||
|
|
||
|
|
||
| if USER_RETIRE_LMS_CRITICAL is not None: | ||
| USER_RETIRE_LMS_CRITICAL.connect(retire_users_data_sharing_consent) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,4 +2,4 @@ | |
| Your project description goes here. | ||
| """ | ||
|
|
||
| __version__ = "6.6.7" | ||
| __version__ = "6.6.8" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,3 +11,11 @@ class IntegratedChannelConfig(AppConfig): | |
| """ | ||
| name = 'integrated_channels.integrated_channel' | ||
| verbose_name = "Enterprise Integrated Channels" | ||
|
|
||
| # TODO: We should move these integrated-channel-specific retirement handlers to the edx-integrated-channels repo | ||
| # and use `channel_integrations.integrated_channel` instead of `integrated_channels.integrated_channel` | ||
| def ready(self): | ||
| """ | ||
| Perform one-time initialization: connect signal handlers. | ||
| """ | ||
| import integrated_channels.integrated_channel.signals # noqa: F401 pylint: disable=import-outside-toplevel,unused-import | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Looks like claude version of this PR got this slightly wrong by doing the connect() call inside ready() which technically works but is less clean. |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| """ | ||
| Django signal handlers for integrated channels user retirement. | ||
| """ | ||
| from logging import getLogger | ||
|
|
||
| from enterprise.models import EnterpriseCourseEnrollment, EnterpriseCustomerUser | ||
| from integrated_channels.degreed.models import DegreedLearnerDataTransmissionAudit | ||
| from integrated_channels.sap_success_factors.models import SapSuccessFactorsLearnerDataTransmissionAudit | ||
|
|
||
| logger = getLogger(__name__) | ||
|
|
||
| try: | ||
| from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL | ||
| except ImportError: | ||
| USER_RETIRE_LMS_CRITICAL = None | ||
|
|
||
|
|
||
| def retire_sapsf_data_transmission(sender, user, **kwargs): # pylint: disable=unused-argument | ||
kiram15 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| """ | ||
| Handle USER_RETIRE_LMS_CRITICAL signal: clear sapsf_user_id on audit records. | ||
|
|
||
| Idempotent: only updates records where sapsf_user_id is not already empty. | ||
| """ | ||
| logger.info(f'Retiring sapsf_user_id {user.id} on audit records') | ||
| for ent_user in EnterpriseCustomerUser.objects.filter(user_id=user.id): | ||
| for enrollment in EnterpriseCourseEnrollment.objects.filter(enterprise_customer_user=ent_user): | ||
| logger.info(f'Updating enrollment {enrollment.id} to clear sapsf_user_id {user.id}') | ||
| SapSuccessFactorsLearnerDataTransmissionAudit.objects.filter( | ||
| enterprise_course_enrollment_id=enrollment.id, | ||
| ).exclude( | ||
| sapsf_user_id="", | ||
| ).update(sapsf_user_id="") | ||
|
|
||
|
|
||
| def retire_degreed_data_transmission(sender, user, **kwargs): # pylint: disable=unused-argument | ||
| """ | ||
| Handle USER_RETIRE_LMS_CRITICAL signal: clear degreed_user_email on audit records. | ||
|
|
||
| Idempotent: only updates records where degreed_user_email is not already empty. | ||
| """ | ||
| logger.info(f'Retiring degreed_user_email on audit records for user {user.id}') | ||
| for ent_user in EnterpriseCustomerUser.objects.filter(user_id=user.id): | ||
| for enrollment in EnterpriseCourseEnrollment.objects.filter(enterprise_customer_user=ent_user): | ||
| logger.info(f'Updating enrollment {enrollment.id} to clear degreed_user_email for ECU {ent_user}') | ||
| DegreedLearnerDataTransmissionAudit.objects.filter( | ||
| enterprise_course_enrollment_id=enrollment.id, | ||
| ).exclude( | ||
| degreed_user_email="", | ||
| ).update(degreed_user_email="") | ||
|
|
||
|
|
||
| if USER_RETIRE_LMS_CRITICAL is not None: | ||
| USER_RETIRE_LMS_CRITICAL.connect(retire_sapsf_data_transmission) | ||
| USER_RETIRE_LMS_CRITICAL.connect(retire_degreed_data_transmission) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| """ | ||
| Tests for consent signal handlers. | ||
| """ | ||
| import unittest | ||
|
|
||
| from pytest import mark | ||
|
|
||
| from consent.models import DataSharingConsent | ||
| from consent.signals import retire_users_data_sharing_consent | ||
| from test_utils.factories import DataSharingConsentFactory, EnterpriseCustomerFactory, UserFactory | ||
|
|
||
|
|
||
| @mark.django_db | ||
| class TestRetireUsersDataSharingConsent(unittest.TestCase): | ||
| """ | ||
| Tests for the retire_users_data_sharing_consent signal handler. | ||
| """ | ||
|
|
||
| def setUp(self): | ||
| super().setUp() | ||
| self.user = UserFactory() | ||
| self.retired_username = f'retired__{self.user.username}' | ||
| self.enterprise_customer = EnterpriseCustomerFactory() | ||
| self.consent = DataSharingConsentFactory( | ||
| username=self.user.username, | ||
| enterprise_customer=self.enterprise_customer, | ||
| ) | ||
|
|
||
| def _send_signal(self): | ||
| """Helper to invoke the handler as the signal would.""" | ||
| retire_users_data_sharing_consent( | ||
| sender=None, | ||
| user=self.user, | ||
| retired_username=self.retired_username, | ||
| retired_email=f'retired__{self.user.email}', | ||
| ) | ||
|
|
||
| def test_retires_username_on_consent_records(self): | ||
| self._send_signal() | ||
|
|
||
| self.consent.refresh_from_db() | ||
| assert self.consent.username == self.retired_username | ||
|
|
||
| def test_no_consent_records_with_original_username_remain(self): | ||
| original_username = self.user.username | ||
| self._send_signal() | ||
|
|
||
| assert not DataSharingConsent.objects.filter(username=original_username).exists() | ||
|
|
||
| def test_idempotent_when_already_retired(self): | ||
| """Calling the handler twice does not raise and leaves data in the correct state.""" | ||
| self._send_signal() | ||
| self._send_signal() | ||
|
|
||
| self.consent.refresh_from_db() | ||
| assert self.consent.username == self.retired_username |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, integrated_channels is deprecated, we migrated to channel_integrations. Confusingly, BOTH are still installed and active:
Your hunch was correct to migrate these because they are indeed functions we currently call during retirement, but unfortunately I think they're no-ops since the old tables just don't get updated anymore.
Please just add a code comment that we should really move these integrated-channel-specific retirement handlers again to the edx-integrated-channels repository and update the handlers to import the new models of the same name.