diff --git a/common/djangoapps/student/management/commands/populate_is_marketable_user_attribute.py b/common/djangoapps/student/management/commands/populate_is_marketable_user_attribute.py index 0507dc414f..51fef174b4 100644 --- a/common/djangoapps/student/management/commands/populate_is_marketable_user_attribute.py +++ b/common/djangoapps/student/management/commands/populate_is_marketable_user_attribute.py @@ -2,10 +2,10 @@ import time -from django.conf import settings from django.contrib.auth import get_user_model from django.core.management.base import BaseCommand from django.db import IntegrityError +from django.db.models import Exists, OuterRef from common.djangoapps.student.models import UserAttribute from common.djangoapps.util.query import use_read_replica_if_available @@ -40,6 +40,20 @@ class Command(BaseCommand): default=10000, help='Batch size' ) + parser.add_argument( + '--start-user-id', + type=int, + dest='start_user_id', + default=10000, + help='Starting user ID' + ) + parser.add_argument( + '--start-userattribute-id', + type=int, + dest='start_userattribute_id', + default=10000, + help='Starting user attribute ID' + ) parser.add_argument( '--backfill-only', type=str, @@ -48,71 +62,99 @@ class Command(BaseCommand): help='Only backfill user attribute, renaming attribute is not required' ) - def _get_user_attribute_queryset(self, user_attribute_id, batch_size): - """ Fetches the user attributes in batches. """ + def _update_user_attribute(self, start_id, end_id): + """ Updates the user attributes in batches. """ self.stdout.write( - f'Fetching user attributes in batch starting from ID {user_attribute_id} with batch size {batch_size}.' + f'Updating user attribute starting from ID {start_id} till {end_id}.' ) - query_set = UserAttribute.objects.filter( - id__gt=user_attribute_id, + return UserAttribute.objects.filter( + id__gte=start_id, + id__lt=end_id, name=OLD_USER_ATTRIBUTE_NAME - ).order_by('id')[:batch_size] - return use_read_replica_if_available(query_set) + ).order_by('id').update(name=NEW_USER_ATTRIBUTE_NAME) - def _get_user_queryset(self, user_id, batch_size): + def _get_old_users_queryset(self, batch_size, marketing_opt_in_start_user_id, user_id=0): """ - Fetches users, in ascending order of id, that exist before a specified user id. + Fetches all the old users in batches, in ascending order of id, that exist before a specified user id. Returns queryset of tuples with 'id' and 'is_active' field values. """ - self.stdout.write(f'Fetching users in batch starting from ID {user_id} with batch size {batch_size}.') + self.stdout.write(f'Fetching old users in batch starting from ID {user_id} with batch size {batch_size}.') query_set = get_user_model().objects.filter( id__gt=user_id, - id__lt=settings.MARKETING_EMAILS_OPT_IN_FIRST_USER_ID + id__lt=marketing_opt_in_start_user_id ).values_list('id', 'is_active').order_by('id')[:batch_size] return use_read_replica_if_available(query_set) - def _backfill_is_marketable_user_attribute(self, batch_size, batch_delay): + def _get_recent_users_queryset(self, batch_size, user_id): """ - Backfills the is_marketable user attribute. Fetches user accounts, in ascending order of id, that are created - before a specified user id. + Fetches all the recent users in batches, in ascending order of id, that exist after a specified user id + and does not have 'is_marketable' user attribute set. + Returns queryset of tuples with 'id' and 'is_active' field values. """ + self.stdout.write(f'Fetching recent users in batch starting from ID {user_id} with batch size {batch_size}.') + user_attribute_qs = UserAttribute.objects.filter(user=OuterRef('pk'), name=NEW_USER_ATTRIBUTE_NAME) + user_query_set = get_user_model().objects.filter( + ~Exists(user_attribute_qs), + id__gt=user_id, + ).values_list('id', 'is_active').order_by('id')[:batch_size] + return use_read_replica_if_available(user_query_set) + + def _bulk_create_user_attributes(self, users): + """ Creates the UserAttribute records in bulk. """ last_user_id = 0 - users = self._get_user_queryset(last_user_id, batch_size) + user_attributes = [] + for user in users: + user_attributes.append(UserAttribute( + user_id=user[0], + name=NEW_USER_ATTRIBUTE_NAME, + value=str(user[1]).lower() + )) + last_user_id = user[0] + try: + UserAttribute.objects.bulk_create(user_attributes) + except IntegrityError: + # A UserAttribute object was already created. This could only happen if we try to create 'is_marketable' + # user attribute that is already created. Ignore it if it does. + self.stdout.write(f'IntegrityError raised during bulk_create having last user id: {last_user_id}.') + return last_user_id + + def _backfill_old_users_attribute(self, batch_size, batch_delay, marketing_opt_in_start_user_id): + """ + Backfills the is_marketable user attribute. Fetches all the old user accounts, in ascending order of id, + that were created before a specified user id. All the fetched users do not have 'is_marketable' + user attribute set. + """ + users = self._get_old_users_queryset(batch_size, marketing_opt_in_start_user_id) while users: - user_attributes = [] - for user in users: - user_attributes.append(UserAttribute( - user_id=user[0], - name=NEW_USER_ATTRIBUTE_NAME, - value=str(user[1]).lower() - )) - last_user_id = user[0] - - try: - UserAttribute.objects.bulk_create(user_attributes) - except IntegrityError: - # A UserAttribute object was already created. This could only happen if we try to create 'is_marketable' - # user attribute that is already created. Ignore it if it does. - pass - + last_user_id = self._bulk_create_user_attributes(users) time.sleep(batch_delay) - users = self._get_user_queryset(last_user_id, batch_size) + users = self._get_old_users_queryset(batch_size, marketing_opt_in_start_user_id, last_user_id) - def _rename_user_attribute_name(self, batch_size, batch_delay): + def _backfill_recent_users_attribute(self, batch_size, batch_delay, start_user_id): + """ + Backfills the is_marketable user attribute. Fetches all the recent user accounts, in ascending order of id, + that were created after a specified user id (start_user_id). + This method handles the backfill of all those users that have missing user attribute even after enabling + the MARKETING_EMAILS_OPT_IN flag. + """ + users = self._get_recent_users_queryset(batch_size, start_user_id) + while users: + last_user_id = self._bulk_create_user_attributes(users) + time.sleep(batch_delay) + users = self._get_recent_users_queryset(batch_size, last_user_id) + + def _rename_user_attribute_name(self, batch_size, batch_delay, start_user_attribute_id): """ Renames the old user attribute 'marketing_emails_opt_in' to 'is_marketable'. """ - last_user_attribute_id = 0 - user_attributes = self._get_user_attribute_queryset(last_user_attribute_id, batch_size) + updated_records_count = 0 + start_id = start_user_attribute_id + end_id = start_id + batch_size + total_user_attribute_count = UserAttribute.objects.filter(name=OLD_USER_ATTRIBUTE_NAME).count() - while user_attributes: - updates = [] - for user_attribute in user_attributes: - user_attribute.name = NEW_USER_ATTRIBUTE_NAME - last_user_attribute_id = user_attribute.id - updates.append(user_attribute) - - UserAttribute.objects.bulk_update(updates, ['name']) + while updated_records_count < total_user_attribute_count: + updated_records_count += self._update_user_attribute(start_id, end_id) + start_id = end_id + end_id = start_id + batch_size time.sleep(batch_delay) - user_attributes = self._get_user_attribute_queryset(last_user_attribute_id, batch_size) def handle(self, *args, **options): """ @@ -124,7 +166,8 @@ class Command(BaseCommand): self.stdout.write(f'Command execution started with options: {options}.') if not options['backfill_only']: - self._rename_user_attribute_name(batch_size, batch_delay) - self._backfill_is_marketable_user_attribute(batch_size, batch_delay) + self._rename_user_attribute_name(batch_size, batch_delay, options['start_userattribute_id']) + self._backfill_old_users_attribute(batch_size, batch_delay, options['start_user_id']) + self._backfill_recent_users_attribute(batch_size, batch_delay, options['start_user_id']) self.stdout.write('Command executed successfully.') diff --git a/common/djangoapps/student/management/tests/test_populate_is_marketable_user_attribute.py b/common/djangoapps/student/management/tests/test_populate_is_marketable_user_attribute.py index cc4da71aef..f692a09353 100644 --- a/common/djangoapps/student/management/tests/test_populate_is_marketable_user_attribute.py +++ b/common/djangoapps/student/management/tests/test_populate_is_marketable_user_attribute.py @@ -11,7 +11,7 @@ from common.djangoapps.student.models import UserAttribute from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangolib.testing.utils import skip_unless_lms -MARKETING_EMAILS_OPT_IN = 'is_marketable' +IS_MARKETABLE = 'is_marketable' @skip_unless_lms @@ -29,16 +29,24 @@ class TestPopulateMarketingOptInUserAttribute(TransactionTestCase): Test population of is_marketable attribute with an existing user. """ assert UserAttribute.objects.count() == 0 - call_command('populate_is_marketable_user_attribute') - assert UserAttribute.objects.filter(name=MARKETING_EMAILS_OPT_IN).count() == User.objects.count() + call_command( + 'populate_is_marketable_user_attribute', + start_user_id=1, + start_userattribute_id=1 + ) + assert UserAttribute.objects.filter(name=IS_MARKETABLE).count() == User.objects.count() def test_command_with_new_user(self): """ Test population of is_marketable attribute with a new user. """ user = UserFactory() - call_command('populate_is_marketable_user_attribute') - assert UserAttribute.objects.filter(name=MARKETING_EMAILS_OPT_IN).count() == User.objects.count() + call_command( + 'populate_is_marketable_user_attribute', + start_user_id=0, + start_userattribute_id=1 + ) + assert UserAttribute.objects.filter(name=IS_MARKETABLE).count() == User.objects.count() def test_command_rename_to_new_attribute(self): """ @@ -46,9 +54,13 @@ class TestPopulateMarketingOptInUserAttribute(TransactionTestCase): """ user = UserFactory() UserAttribute.objects.create(user=user, name='marketing_emails_opt_in', value='true') - call_command('populate_is_marketable_user_attribute') + call_command( + 'populate_is_marketable_user_attribute', + start_user_id=1, + start_userattribute_id=1 + ) assert UserAttribute.objects.filter(name='marketing_emails_opt_in').count() == 0 - assert UserAttribute.get_user_attribute(user, MARKETING_EMAILS_OPT_IN) == 'true' + assert UserAttribute.get_user_attribute(user, IS_MARKETABLE) == 'true' def test_command_with_invalid_argument(self): """ @@ -56,6 +68,8 @@ class TestPopulateMarketingOptInUserAttribute(TransactionTestCase): """ with pytest.raises(TypeError): call_command( - "populate_is_marketable_user_attribute", - batch_size='1000' + 'populate_is_marketable_user_attribute', + batch_size='1000', + start_user_id=1, + start_userattribute_id=1 ) diff --git a/lms/envs/common.py b/lms/envs/common.py index d5771dda21..8b9a4b1528 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1059,10 +1059,6 @@ RETRY_CALENDAR_SYNC_EMAIL_MAX_ATTEMPTS = 5 MARKETING_EMAILS_OPT_IN = False -# TODO: Remove this temporary flag after successfully running the management command. -# Ticket: https://openedx.atlassian.net/browse/VAN-966 -MARKETING_EMAILS_OPT_IN_FIRST_USER_ID = 10000 - # .. toggle_name: ENABLE_COPPA_COMPLIANCE # .. toggle_implementation: DjangoSetting # .. toggle_default: False diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index ef72e27673..fb06eca0c0 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -101,7 +101,7 @@ REGISTRATION_UTM_PARAMETERS = { 'utm_content': 'registration_utm_content', } REGISTRATION_UTM_CREATED_AT = 'registration_utm_created_at' -MARKETING_EMAILS_OPT_IN = 'is_marketable' +IS_MARKETABLE = 'is_marketable' # used to announce a registration # providing_args=["user", "registration"] REGISTER_USER = Signal() @@ -253,7 +253,8 @@ def create_account_with_params(request, params): # pylint: disable=too-many-sta except Exception: # pylint: disable=broad-except log.exception(f"Enable discussion notifications failed for user {user.id}.") - _track_user_registration(user, profile, params, third_party_provider, registration) + is_marketable = params.get('marketing_emails_opt_in') in ['true', '1'] + _track_user_registration(user, profile, params, third_party_provider, registration, is_marketable) # Announce registration REGISTER_USER.send(sender=None, user=user, registration=registration) @@ -275,7 +276,7 @@ def create_account_with_params(request, params): # pylint: disable=too-many-sta try: _record_registration_attributions(request, new_user) - _record_marketing_emails_opt_in_attribute(params.get('marketing_emails_opt_in'), new_user) + _record_is_marketable_attribute(is_marketable, new_user) # Don't prevent a user from registering due to attribution errors. except Exception: # pylint: disable=broad-except log.exception('Error while attributing cookies to user registration.') @@ -347,10 +348,9 @@ def _link_user_to_third_party_provider( return third_party_provider, running_pipeline -def _track_user_registration(user, profile, params, third_party_provider, registration): +def _track_user_registration(user, profile, params, third_party_provider, registration, is_marketable): """ Track the user's registration. """ if hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY: - is_marketable = params.get('marketing_emails_opt_in') in ['true', '1'] traits = { 'email': user.email, 'username': user.username, @@ -461,12 +461,12 @@ def _skip_activation_email(user, running_pipeline, third_party_provider): ) -def _record_marketing_emails_opt_in_attribute(opt_in, user): +def _record_is_marketable_attribute(is_marketable, user): """ Attribute this user's registration based on form data """ - if settings.MARKETING_EMAILS_OPT_IN and user and opt_in: - UserAttribute.set_user_attribute(user, MARKETING_EMAILS_OPT_IN, opt_in) + if settings.MARKETING_EMAILS_OPT_IN and user: + UserAttribute.set_user_attribute(user, IS_MARKETABLE, str(is_marketable).lower()) def _record_registration_attributions(request, user):