Skip to content

Commit

Permalink
chore: removed obsolete feature flags (openedx#34942)
Browse files Browse the repository at this point in the history
  • Loading branch information
AhtishamShahid authored Jun 27, 2024
1 parent 8c40057 commit 15e2834
Show file tree
Hide file tree
Showing 13 changed files with 57 additions and 172 deletions.
8 changes: 4 additions & 4 deletions lms/djangoapps/discussion/rest_api/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
from django.contrib.auth import get_user_model
from edx_django_utils.monitoring import set_code_owner_attribute
from opaque_keys.edx.locator import CourseKey

from lms.djangoapps.courseware.courses import get_course_with_access
from lms.djangoapps.discussion.rest_api.discussions_notifications import DiscussionNotificationSender
from openedx.core.djangoapps.django_comment_common.comment_client import Comment
from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS, ENABLE_COURSEWIDE_NOTIFICATIONS
from lms.djangoapps.discussion.rest_api.discussions_notifications import DiscussionNotificationSender

from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS

User = get_user_model()

Expand All @@ -22,7 +22,7 @@ def send_thread_created_notification(thread_id, course_key_str, user_id):
Send notification when a new thread is created
"""
course_key = CourseKey.from_string(course_key_str)
if not (ENABLE_NOTIFICATIONS.is_enabled(course_key) and ENABLE_COURSEWIDE_NOTIFICATIONS.is_enabled(course_key)):
if not ENABLE_NOTIFICATIONS.is_enabled(course_key):
return
thread = Thread(id=thread_id).retrieve()
user = User.objects.get(id=user_id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@
from unittest.mock import MagicMock, patch

import pytest
from edx_toggles.toggles.testutils import override_waffle_flag

from lms.djangoapps.discussion.rest_api.discussions_notifications import DiscussionNotificationSender
from lms.djangoapps.discussion.toggles import ENABLE_REPORTED_CONTENT_NOTIFICATIONS


@patch('lms.djangoapps.discussion.rest_api.discussions_notifications.DiscussionNotificationSender'
Expand All @@ -22,7 +20,6 @@ class TestDiscussionNotificationSender(unittest.TestCase):
Tests for the DiscussionNotificationSender class
"""

@override_waffle_flag(ENABLE_REPORTED_CONTENT_NOTIFICATIONS, True)
def setUp(self):
self.thread = MagicMock()
self.course = MagicMock()
Expand Down
10 changes: 5 additions & 5 deletions lms/djangoapps/discussion/rest_api/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@
import httpretty
from django.conf import settings
from edx_toggles.toggles.testutils import override_waffle_flag
from openedx_events.learning.signals import USER_NOTIFICATION_REQUESTED, COURSE_NOTIFICATION_REQUESTED
from openedx_events.learning.signals import COURSE_NOTIFICATION_REQUESTED, USER_NOTIFICATION_REQUESTED

from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.tests.factories import StaffFactory, UserFactory
from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory
from lms.djangoapps.discussion.rest_api.tasks import (
send_response_endorsed_notifications,
send_response_notifications,
send_thread_created_notification,
send_response_endorsed_notifications)
send_thread_created_notification
)
from lms.djangoapps.discussion.rest_api.tests.utils import ThreadMock, make_minimal_cs_thread
from openedx.core.djangoapps.course_groups.models import CohortMembership, CourseCohortsSettings
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
Expand All @@ -28,7 +29,7 @@
FORUM_ROLE_STUDENT,
CourseDiscussionSettings
)
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_COURSEWIDE_NOTIFICATIONS, ENABLE_NOTIFICATIONS
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory

Expand All @@ -47,7 +48,6 @@ def _get_mfe_url(course_id, post_id):
@httpretty.activate
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
@override_waffle_flag(ENABLE_NOTIFICATIONS, active=True)
@override_waffle_flag(ENABLE_COURSEWIDE_NOTIFICATIONS, active=True)
class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""
Test cases related to new_discussion_post and new_question_post notification types
Expand Down
12 changes: 4 additions & 8 deletions lms/djangoapps/discussion/signals/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,17 @@
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryLocator

from lms.djangoapps.discussion.rest_api.discussions_notifications import DiscussionNotificationSender
from lms.djangoapps.discussion.toggles import ENABLE_REPORTED_CONTENT_NOTIFICATIONS
from xmodule.modulestore.django import SignalHandler, modulestore

from lms.djangoapps.discussion import tasks
from lms.djangoapps.discussion.rest_api.discussions_notifications import DiscussionNotificationSender
from lms.djangoapps.discussion.rest_api.tasks import (
send_response_endorsed_notifications,
send_response_notifications,
send_thread_created_notification,
send_response_endorsed_notifications
send_thread_created_notification
)
from openedx.core.djangoapps.django_comment_common import signals
from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
from openedx.core.djangoapps.theming.helpers import get_current_site
from xmodule.modulestore.django import SignalHandler, modulestore

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -101,8 +99,6 @@ def send_reported_content_notification(sender, user, post, **kwargs):
Sends notification for reported content.
"""
course_key = CourseKey.from_string(post.course_id)
if not ENABLE_REPORTED_CONTENT_NOTIFICATIONS.is_enabled(course_key):
return
course = modulestore().get_course(course_key)
DiscussionNotificationSender(post, course, user).send_reported_content_notification()

Expand Down
12 changes: 0 additions & 12 deletions lms/djangoapps/discussion/toggles.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,3 @@
# .. toggle_creation_date: 2021-11-05
# .. toggle_target_removal_date: 2022-12-05
ENABLE_DISCUSSIONS_MFE = CourseWaffleFlag(f'{WAFFLE_FLAG_NAMESPACE}.enable_discussions_mfe', __name__)

# .. toggle_name: discussions.enable_reported_content_notifications
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable reported content notifications.
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 18-Jan-2024
# .. toggle_target_removal_date: 18-Feb-2024
ENABLE_REPORTED_CONTENT_NOTIFICATIONS = CourseWaffleFlag(
f'{WAFFLE_FLAG_NAMESPACE}.enable_reported_content_notifications',
__name__
)
40 changes: 0 additions & 40 deletions openedx/core/djangoapps/notifications/config/waffle.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,46 +18,6 @@
# .. toggle_tickets: INF-866
ENABLE_NOTIFICATIONS = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_notifications', __name__)

# .. toggle_name: notifications.show_notifications_tray
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to show notifications tray
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 2023-06-07
# .. toggle_target_removal_date: 2023-12-07
# .. toggle_tickets: INF-902
SHOW_NOTIFICATIONS_TRAY = CourseWaffleFlag(f"{WAFFLE_NAMESPACE}.show_notifications_tray", __name__)

# .. toggle_name: notifications.enable_notifications_filters
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable filters in notifications task
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 2023-06-07
# .. toggle_target_removal_date: 2024-06-01
# .. toggle_tickets: INF-902
ENABLE_NOTIFICATIONS_FILTERS = CourseWaffleFlag(f"{WAFFLE_NAMESPACE}.enable_notifications_filters", __name__)

# .. toggle_name: notifications.enable_coursewide_notifications
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable coursewide notifications
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 2023-10-25
# .. toggle_target_removal_date: 2024-06-01
# .. toggle_tickets: INF-1145
ENABLE_COURSEWIDE_NOTIFICATIONS = CourseWaffleFlag(f"{WAFFLE_NAMESPACE}.enable_coursewide_notifications", __name__)

# .. toggle_name: notifications.enable_ora_staff_notifications
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable ORA staff notifications
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 2024-04-04
# .. toggle_target_removal_date: 2024-06-04
# .. toggle_tickets: INF-1304
ENABLE_ORA_STAFF_NOTIFICATION = CourseWaffleFlag(f"{WAFFLE_NAMESPACE}.enable_ora_staff_notifications", __name__)

# .. toggle_name: notifications.enable_email_notifications
# .. toggle_implementation: WaffleFlag
# .. toggle_default: False
Expand Down
17 changes: 6 additions & 11 deletions openedx/core/djangoapps/notifications/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@
from django.dispatch import receiver
from openedx_events.learning.signals import (
COURSE_ENROLLMENT_CREATED,
COURSE_UNENROLLMENT_COMPLETED,
USER_NOTIFICATION_REQUESTED,
COURSE_NOTIFICATION_REQUESTED,
COURSE_UNENROLLMENT_COMPLETED,
USER_NOTIFICATION_REQUESTED
)

from common.djangoapps.student.models import CourseEnrollment
from openedx.core.djangoapps.notifications.audience_filters import (
ForumRoleAudienceFilter,
EnrollmentAudienceFilter,
TeamAudienceFilter,
CohortAudienceFilter,
CourseRoleAudienceFilter,
EnrollmentAudienceFilter,
ForumRoleAudienceFilter,
TeamAudienceFilter
)
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS, ENABLE_ORA_STAFF_NOTIFICATION
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
from openedx.core.djangoapps.notifications.models import CourseNotificationPreference

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -108,11 +108,6 @@ def generate_course_notifications(signal, sender, course_notification_data, meta
"""
Watches for COURSE_NOTIFICATION_REQUESTED signal and calls send_notifications task
"""
if (
course_notification_data.notification_type == 'ora_staff_notification'
and not ENABLE_ORA_STAFF_NOTIFICATION.is_enabled(course_notification_data.course_key)
):
return

from openedx.core.djangoapps.notifications.tasks import send_notifications
course_notification_data = course_notification_data.__dict__
Expand Down
3 changes: 1 addition & 2 deletions openedx/core/djangoapps/notifications/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
get_notification_channels, get_additional_notification_channel_settings
)
from .base_notification import COURSE_NOTIFICATION_APPS, COURSE_NOTIFICATION_TYPES, EmailCadence
from .utils import filter_course_wide_preferences, remove_preferences_with_no_access
from .utils import remove_preferences_with_no_access


def add_info_to_notification_config(config_obj):
Expand Down Expand Up @@ -73,7 +73,6 @@ def to_representation(self, instance):
course_id = self.context['course_id']
user = self.context['user']
preferences = add_info_to_notification_config(preferences)
preferences = filter_course_wide_preferences(course_id, preferences)
preferences = remove_preferences_with_no_access(preferences, user)
return preferences

Expand Down
11 changes: 5 additions & 6 deletions openedx/core/djangoapps/notifications/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
get_default_values_of_preference,
get_notification_content
)
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS, ENABLE_NOTIFICATIONS_FILTERS
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
from openedx.core.djangoapps.notifications.events import notification_generated_event
from openedx.core.djangoapps.notifications.filters import NotificationFilter
from openedx.core.djangoapps.notifications.models import (
CourseNotificationPreference,
Notification,
get_course_notification_preference_config_version,
get_course_notification_preference_config_version
)
from openedx.core.djangoapps.notifications.utils import clean_arguments, get_list_in_batches

Expand Down Expand Up @@ -137,10 +137,9 @@ def send_notifications(user_ids, course_key: str, app_name, notification_type, c
generated_notification_audience = []

for batch_user_ids in get_list_in_batches(user_ids, batch_size):
if ENABLE_NOTIFICATIONS_FILTERS.is_enabled(course_key):
logger.info(f'Sending notifications to {len(batch_user_ids)} users in {course_key}')
batch_user_ids = NotificationFilter().apply_filters(batch_user_ids, course_key, notification_type)
logger.info(f'After applying filters, sending notifications to {len(batch_user_ids)} users in {course_key}')
logger.info(f'Sending notifications to {len(batch_user_ids)} users in {course_key}')
batch_user_ids = NotificationFilter().apply_filters(batch_user_ids, course_key, notification_type)
logger.info(f'After applying filters, sending notifications to {len(batch_user_ids)} users in {course_key}')

# check if what is preferences of user and make decision to send notification or not
preferences = CourseNotificationPreference.objects.filter(
Expand Down
17 changes: 8 additions & 9 deletions openedx/core/djangoapps/notifications/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,19 @@
Tests for notifications tasks.
"""

import datetime
from unittest.mock import patch

import datetime
import ddt
from django.core.exceptions import ValidationError
from django.conf import settings
from django.core.exceptions import ValidationError
from edx_toggles.toggles.testutils import override_waffle_flag

from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory

from .utils import create_notification
from ..config.waffle import ENABLE_NOTIFICATIONS
from ..models import CourseNotificationPreference, Notification
from ..tasks import (
Expand All @@ -24,6 +23,7 @@
send_notifications,
update_user_preference
)
from .utils import create_notification


@patch('openedx.core.djangoapps.notifications.models.COURSE_NOTIFICATION_CONFIG_VERSION', 1)
Expand Down Expand Up @@ -225,7 +225,6 @@ def test_notification_not_created_when_context_is_incomplete(self):


@ddt.ddt
@patch('openedx.core.djangoapps.notifications.tasks.ENABLE_NOTIFICATIONS_FILTERS.is_enabled', lambda x: False)
class SendBatchNotificationsTest(ModuleStoreTestCase):
"""
Test that notification and notification preferences are created in batches
Expand Down Expand Up @@ -255,9 +254,9 @@ def _create_users(self, num_of_users):

@override_waffle_flag(ENABLE_NOTIFICATIONS, active=True)
@ddt.data(
(settings.NOTIFICATION_CREATION_BATCH_SIZE, 1, 2),
(settings.NOTIFICATION_CREATION_BATCH_SIZE + 10, 2, 4),
(settings.NOTIFICATION_CREATION_BATCH_SIZE - 10, 1, 2),
(settings.NOTIFICATION_CREATION_BATCH_SIZE, 7, 3),
(settings.NOTIFICATION_CREATION_BATCH_SIZE + 10, 9, 6),
(settings.NOTIFICATION_CREATION_BATCH_SIZE - 10, 7, 3),
)
@ddt.unpack
def test_notification_is_send_in_batch(self, creation_size, prefs_query_count, notifications_query_count):
Expand Down Expand Up @@ -307,7 +306,7 @@ def test_preference_not_created_for_default_off_preference(self):
"username": "Test Author"
}
with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True):
with self.assertNumQueries(1):
with self.assertNumQueries(7):
send_notifications(user_ids, str(self.course.id), notification_app, notification_type,
context, "http://test.url")

Expand All @@ -326,7 +325,7 @@ def test_preference_created_for_default_on_preference(self):
"replier_name": "Replier Name"
}
with override_waffle_flag(ENABLE_NOTIFICATIONS, active=True):
with self.assertNumQueries(3):
with self.assertNumQueries(9):
send_notifications(user_ids, str(self.course.id), notification_app, notification_type,
context, "http://test.url")

Expand Down
Loading

0 comments on commit 15e2834

Please sign in to comment.