Merge branch 'master' into NiedielnitsevIvan/FC-0047/feature/implemen…

…t-push-notifications-chanel

cms/djangoapps/contentstore/views/course.py

@@ -58,6 +58,7 @@
 from common.djangoapps.util.string_utils import _has_non_ascii_characters
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements
+from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course
 from openedx.core.djangoapps.models.course_details import CourseDetails
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangolib.js_utils import dump_js_escaped_json
@@ -302,6 +303,10 @@ def course_handler(request, course_key_string=None):
             else:
                 return HttpResponseBadRequest()
         elif request.method == 'GET':  # assume html
+            # Update course discussion settings, sometimes the course discussion settings are not updated
+            # when the course is created, so we need to update them here.
+            course_key = CourseKey.from_string(course_key_string)
+            update_discussions_settings_from_course(course_key)
             if course_key_string is None:
                 return redirect(reverse('home'))
             else:

cms/djangoapps/contentstore/views/tests/test_course_index.py

@@ -717,8 +717,8 @@ def test_number_of_calls_to_db(self):
         """
         Test to check number of queries made to mysql and mongo
         """
-        with self.assertNumQueries(29, table_ignorelist=WAFFLE_TABLES):
-            with check_mongo_calls(3):
+        with self.assertNumQueries(32, table_ignorelist=WAFFLE_TABLES):
+            with check_mongo_calls(5):
                 self.client.get_html(reverse_course_url('course_handler', self.course.id))
 
 

lms/djangoapps/discussion/rest_api/discussions_notifications.py

@@ -118,9 +118,10 @@ def send_new_comment_notification(self):
             self.parent_response and
             self.creator.id != int(self.thread.user_id)
         ):
+            author_name = f"{self.parent_response.username}'s"
             # use your if author of response is same as author of post.
             # use 'their' if comment author is also response author.
-            author_name = (
+            author_pronoun = (
                 # Translators: Replier commented on "your" response to your post
                 _("your")
                 if self._response_and_thread_has_same_creator()
@@ -129,10 +130,12 @@ def send_new_comment_notification(self):
                     _("their")
                     if self._response_and_comment_has_same_creator()
                     else f"{self.parent_response.username}'s"
+
                 )
             )
             context = {
                 "author_name": str(author_name),
+                "author_pronoun": str(author_pronoun),
             }
             self._send_notification([self.thread.user_id], "new_comment", extra_context=context)
 
@@ -189,10 +192,21 @@ def send_response_on_followed_post_notification(self):
         if not self.parent_id:
             self._send_notification(users, "response_on_followed_post")
         else:
+            author_name = f"{self.parent_response.username}'s"
+            # use 'their' if comment author is also response author.
+            author_pronoun = (
+                # Translators: Replier commented on "their" response in a post you're following
+                _("their")
+                if self._response_and_comment_has_same_creator()
+                else f"{self.parent_response.username}'s"
+            )
             self._send_notification(
                 users,
                 "comment_on_followed_post",
-                extra_context={"author_name": self.parent_response.username}
+                extra_context={
+                    "author_name": str(author_name),
+                    "author_pronoun": str(author_pronoun),
+                }
             )
 
     def _create_cohort_course_audience(self):

lms/djangoapps/discussion/rest_api/tests/test_tasks.py

@@ -338,6 +338,7 @@ def test_send_notification_to_parent_threads(self):
             'replier_name': self.user_3.username,
             'post_title': self.thread.title,
             'author_name': 'dummy\'s',
+            'author_pronoun': 'dummy\'s',
             'course_name': self.course.display_name,
             'sender_id': self.user_3.id
         }
@@ -399,7 +400,8 @@ def test_comment_creators_own_response(self):
         expected_context = {
             'replier_name': self.user_3.username,
             'post_title': self.thread.title,
-            'author_name': 'your',
+            'author_name': 'dummy\'s',
+            'author_pronoun': 'your',
             'course_name': self.course.display_name,
             'sender_id': self.user_3.id,
         }
@@ -441,7 +443,8 @@ def test_send_notification_to_followers(self, parent_id, notification_type):
             'sender_id': self.user_2.id,
         }
         if parent_id:
-            expected_context['author_name'] = 'dummy'
+            expected_context['author_name'] = 'dummy\'s'
+            expected_context['author_pronoun'] = 'dummy\'s'
         self.assertDictEqual(args.context, expected_context)
         self.assertEqual(
             args.content_url,
@@ -531,7 +534,8 @@ def test_new_comment_notification(self):
         send_response_notifications(thread.id, str(self.course.id), self.user_2.id, parent_id=response.id)
         handler.assert_called_once()
         context = handler.call_args[1]['notification_data'].context
-        self.assertEqual(context['author_name'], 'their')
+        self.assertEqual(context['author_name'], 'dummy\'s')
+        self.assertEqual(context['author_pronoun'], 'their')
 
 
 @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True)

openedx/core/djangoapps/notifications/base_notification.py

@@ -7,6 +7,7 @@
 from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
 from .utils import find_app_in_normalized_apps, find_pref_in_normalized_prefs
 from ..django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA
+from .notification_content import get_notification_type_content_function
 
 FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE = 'filter_audit_expired_users_with_no_role'
 
@@ -109,8 +110,9 @@
         'is_core': True,
         'info': '',
         'non_editable': [],
-        'content_template': _('<{p}><{strong}>{replier_name}</{strong}> commented on {author_name}\'s response in '
-                              'a post you’re following <{strong}>{post_title}</{strong}></{p}>'),
+        'content_template': _('<{p}><{strong}>{replier_name}</{strong}> commented on <{strong}>{author_name}'
+                              '</{strong}> response in a post you’re following <{strong}>{post_title}'
+                              '</{strong}></{p}>'),
         'content_context': {
             'post_title': 'Post title',
             'author_name': 'author name',
@@ -465,10 +467,13 @@ def get_notification_content(notification_type, context):
         'strong': 'strong',
         'p': 'p',
     }
+    content_function = get_notification_type_content_function(notification_type)
     if notification_type == 'course_update':
         notification_type = 'course_updates'
     notification_type = NotificationTypeManager().notification_types.get(notification_type, None)
     if notification_type:
+        if content_function:
+            return content_function(notification_type, context)
         notification_type_content_template = notification_type.get('content_template', None)
         if notification_type_content_template:
             return notification_type_content_template.format(**context, **html_tags_context)

openedx/core/djangoapps/notifications/email/tasks.py

@@ -70,18 +70,19 @@ def get_user_preferences_for_courses(course_ids, user):
     return new_preferences
 
 
-def send_digest_email_to_user(user, cadence_type, course_language='en', courses_data=None):
+def send_digest_email_to_user(user, cadence_type, start_date, end_date, course_language='en', courses_data=None):
     """
     Send [cadence_type] email to user.
     Cadence Type can be EmailCadence.DAILY or EmailCadence.WEEKLY
+    start_date: Datetime object
+    end_date: Datetime object
     """
     if cadence_type not in [EmailCadence.DAILY, EmailCadence.WEEKLY]:
         raise ValueError('Invalid cadence_type')
     logger.info(f'<Email Cadence> Sending email to user {user.username} ==Temp Log==')
     if not is_email_notification_flag_enabled(user):
         logger.info(f'<Email Cadence> Flag disabled for {user.username} ==Temp Log==')
         return
-    start_date, end_date = get_start_end_date(cadence_type)
     notifications = Notification.objects.filter(user=user, email=True,
                                                 created__gte=start_date, created__lte=end_date)
     if not notifications:
@@ -115,6 +116,7 @@ def send_digest_email_to_all_users(cadence_type):
     logger.info(f'<Email Cadence> Sending cadence email of type {cadence_type}')
     users = get_audience_for_cadence_email(cadence_type)
     courses_data = {}
+    start_date, end_date = get_start_end_date(cadence_type)
     logger.info(f'<Email Cadence> Email Cadence Audience {len(users)}')
     for user in users:
-        send_digest_email_to_user(user, cadence_type, courses_data=courses_data)
+        send_digest_email_to_user(user, cadence_type, start_date, end_date, courses_data=courses_data)

openedx/core/djangoapps/notifications/email/tests/test_tasks.py

@@ -16,6 +16,7 @@
     send_digest_email_to_all_users,
     send_digest_email_to_user
 )
+from openedx.core.djangoapps.notifications.email.utils import get_start_end_date
 from openedx.core.djangoapps.notifications.models import CourseNotificationPreference
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -42,8 +43,9 @@ def test_email_is_not_sent_if_no_notifications(self, mock_func):
         """
         Tests email is sent iff waffle flag is enabled
         """
+        start_date, end_date = get_start_end_date(EmailCadence.DAILY)
         with override_waffle_flag(ENABLE_EMAIL_NOTIFICATIONS, True):
-            send_digest_email_to_user(self.user, EmailCadence.DAILY)
+            send_digest_email_to_user(self.user, EmailCadence.DAILY, start_date, end_date)
         assert not mock_func.called
 
     @ddt.data(True, False)
@@ -54,31 +56,56 @@ def test_email_is_sent_iff_flag_enabled(self, flag_value, mock_func):
         """
         created_date = datetime.datetime.now() - datetime.timedelta(days=1)
         create_notification(self.user, self.course.id, created=created_date)
+        start_date, end_date = get_start_end_date(EmailCadence.DAILY)
         with override_waffle_flag(ENABLE_EMAIL_NOTIFICATIONS, flag_value):
-            send_digest_email_to_user(self.user, EmailCadence.DAILY)
+            send_digest_email_to_user(self.user, EmailCadence.DAILY, start_date, end_date)
         assert mock_func.called is flag_value
 
     @patch('edx_ace.ace.send')
     def test_notification_not_send_if_created_on_next_day(self, mock_func):
         """
         Tests email is not sent if notification is created on next day
         """
-        create_notification(self.user, self.course.id)
+        start_date, end_date = get_start_end_date(EmailCadence.DAILY)
+        create_notification(self.user, self.course.id, created=end_date + datetime.timedelta(minutes=2))
         with override_waffle_flag(ENABLE_EMAIL_NOTIFICATIONS, True):
-            send_digest_email_to_user(self.user, EmailCadence.DAILY)
+            send_digest_email_to_user(self.user, EmailCadence.DAILY, start_date, end_date)
         assert not mock_func.called
 
     @patch('edx_ace.ace.send')
     def test_notification_not_send_if_created_day_before_yesterday(self, mock_func):
         """
         Tests email is not sent if notification is created day before yesterday
         """
-        created_date = datetime.datetime.now() - datetime.timedelta(days=2)
+        start_date, end_date = get_start_end_date(EmailCadence.DAILY)
+        created_date = datetime.datetime.now() - datetime.timedelta(days=1, minutes=18)
         create_notification(self.user, self.course.id, created=created_date)
         with override_waffle_flag(ENABLE_EMAIL_NOTIFICATIONS, True):
-            send_digest_email_to_user(self.user, EmailCadence.DAILY)
+            send_digest_email_to_user(self.user, EmailCadence.DAILY, start_date, end_date)
         assert not mock_func.called
 
+    @ddt.data(
+        (EmailCadence.DAILY, datetime.datetime.now() - datetime.timedelta(days=1, minutes=30), False),
+        (EmailCadence.DAILY, datetime.datetime.now() - datetime.timedelta(minutes=10), True),
+        (EmailCadence.DAILY, datetime.datetime.now() - datetime.timedelta(days=1), True),
+        (EmailCadence.DAILY, datetime.datetime.now() + datetime.timedelta(minutes=20), False),
+        (EmailCadence.WEEKLY, datetime.datetime.now() - datetime.timedelta(days=7, minutes=30), False),
+        (EmailCadence.WEEKLY, datetime.datetime.now() - datetime.timedelta(days=7), True),
+        (EmailCadence.WEEKLY, datetime.datetime.now() - datetime.timedelta(minutes=20), True),
+        (EmailCadence.WEEKLY, datetime.datetime.now() + datetime.timedelta(minutes=20), False),
+    )
+    @ddt.unpack
+    @patch('edx_ace.ace.send')
+    def test_notification_content(self, cadence_type, created_time, notification_created, mock_func):
+        """
+        Tests email only contains notification created within date
+        """
+        start_date, end_date = get_start_end_date(cadence_type)
+        create_notification(self.user, self.course.id, created=created_time)
+        with override_waffle_flag(ENABLE_EMAIL_NOTIFICATIONS, True):
+            send_digest_email_to_user(self.user, EmailCadence.DAILY, start_date, end_date)
+        assert mock_func.called is notification_created
+
 
 @ddt.ddt
 class TestEmailDigestAudience(ModuleStoreTestCase):
@@ -146,10 +173,11 @@ def test_digest_should_contain_email_enabled_notifications(self, email_value, mo
         """
         Tests email is sent only when notifications with email=True exists
         """
-        created_date = datetime.datetime.now() - datetime.timedelta(days=1)
+        start_date, end_date = get_start_end_date(EmailCadence.DAILY)
+        created_date = datetime.datetime.now() - datetime.timedelta(hours=23, minutes=59)
         create_notification(self.user, self.course.id, created=created_date, email=email_value)
         with override_waffle_flag(ENABLE_EMAIL_NOTIFICATIONS, True):
-            send_digest_email_to_user(self.user, EmailCadence.DAILY)
+            send_digest_email_to_user(self.user, EmailCadence.DAILY, start_date, end_date)
             assert mock_func.called is email_value
 
 
@@ -166,21 +194,22 @@ def setUp(self):
         self.user = UserFactory()
         self.course = CourseFactory.create(display_name='test course', run="Testing_course")
         self.preference = CourseNotificationPreference.objects.create(user=self.user, course_id=self.course.id)
-        created_date = datetime.datetime.now() - datetime.timedelta(days=1)
+        created_date = datetime.datetime.now() - datetime.timedelta(hours=23)
         create_notification(self.user, self.course.id, notification_type='new_discussion_post', created=created_date)
 
     @patch('edx_ace.ace.send')
     def test_email_send_for_digest_preference(self, mock_func):
         """
         Tests email is send for digest notification preference
         """
+        start_date, end_date = get_start_end_date(EmailCadence.DAILY)
         config = self.preference.notification_preference_config
         types = config['discussion']['notification_types']
         types['new_discussion_post']['email_cadence'] = EmailCadence.DAILY
         types['new_discussion_post']['email'] = True
         self.preference.save()
         with override_waffle_flag(ENABLE_EMAIL_NOTIFICATIONS, True):
-            send_digest_email_to_user(self.user, EmailCadence.DAILY)
+            send_digest_email_to_user(self.user, EmailCadence.DAILY, start_date, end_date)
         assert mock_func.called
 
     @ddt.data(True, False)
@@ -189,24 +218,26 @@ def test_email_send_for_email_preference_value(self, pref_value, mock_func):
         """
         Tests email is sent iff preference value is True
         """
+        start_date, end_date = get_start_end_date(EmailCadence.DAILY)
         config = self.preference.notification_preference_config
         types = config['discussion']['notification_types']
         types['new_discussion_post']['email_cadence'] = EmailCadence.DAILY
         types['new_discussion_post']['email'] = pref_value
         self.preference.save()
         with override_waffle_flag(ENABLE_EMAIL_NOTIFICATIONS, True):
-            send_digest_email_to_user(self.user, EmailCadence.DAILY)
+            send_digest_email_to_user(self.user, EmailCadence.DAILY, start_date, end_date)
         assert mock_func.called is pref_value
 
     @patch('edx_ace.ace.send')
     def test_email_not_send_if_different_digest_preference(self, mock_func):
         """
         Tests email is not send if digest notification preference doesnot match
         """
+        start_date, end_date = get_start_end_date(EmailCadence.DAILY)
         config = self.preference.notification_preference_config
         types = config['discussion']['notification_types']
         types['new_discussion_post']['email_cadence'] = EmailCadence.WEEKLY
         self.preference.save()
         with override_waffle_flag(ENABLE_EMAIL_NOTIFICATIONS, True):
-            send_digest_email_to_user(self.user, EmailCadence.DAILY)
+            send_digest_email_to_user(self.user, EmailCadence.DAILY, start_date, end_date)
         assert not mock_func.called

openedx/core/djangoapps/notifications/email/utils.py

@@ -171,13 +171,10 @@ def get_start_end_date(cadence_type):
     """
     if cadence_type not in [EmailCadence.DAILY, EmailCadence.WEEKLY]:
         raise ValueError('Invalid cadence_type')
-    date_today = datetime.datetime.now()
-    yesterday = date_today - datetime.timedelta(days=1)
-    end_date = datetime.datetime.combine(yesterday, datetime.time.max)
-    start_date = end_date
+    end_date = datetime.datetime.now()
+    start_date = end_date - datetime.timedelta(days=1, minutes=15)
     if cadence_type == EmailCadence.WEEKLY:
-        start_date = end_date - datetime.timedelta(days=6)
-    start_date = datetime.datetime.combine(start_date, datetime.time.min)
+        start_date = start_date - datetime.timedelta(days=6)
     return utc.localize(start_date), utc.localize(end_date)
 
 

openedx/core/djangoapps/notifications/notification_content.py

@@ -0,0 +1,35 @@
+"""
+Helper functions for overriding notification content for given notification type.
+"""
+
+
+def get_notification_type_content_function(notification_type):
+    """
+        Returns the content function for the given notification if it exists.
+    """
+    try:
+        return globals()[f"get_{notification_type}_notification_content"]
+    except KeyError:
+        return None
+
+
+def get_notification_content_with_author_pronoun(notification_type, context):
+    """
+        Helper function to get notification content with author's pronoun.
+    """
+    html_tags_context = {
+        'strong': 'strong',
+        'p': 'p',
+    }
+    notification_type_content_template = notification_type.get('content_template', None)
+    if 'author_pronoun' in context:
+        context['author_name'] = context['author_pronoun']
+    if notification_type_content_template:
+        return notification_type_content_template.format(**context, **html_tags_context)
+    return ''
+
+
+# Returns notification content for the new_comment notification.
+get_new_comment_notification_content = get_notification_content_with_author_pronoun
+# Returns notification content for the comment_on_followed_post notification.
+get_comment_on_followed_post_notification_content = get_notification_content_with_author_pronoun