From 1f7b12a38d6f252a6f897d6c8014277737e3aa17 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 23 Jul 2019 14:57:48 -0400 Subject: [PATCH] Remove references to push notifications. Push notifications was only ever setup to connect to Parse.com a service that has been discontinued. Since we haven't replaced the functionality for a few years now, remove this dead code. In cms/templates/js/course_info_update.underscore we're allowing content to not be escaped because this is by design according to the tests. Long term there should be a better fix for this but for now this is intended behavior. --- cms/djangoapps/contentstore/admin.py | 3 +- .../contentstore/course_info_model.py | 3 - cms/djangoapps/contentstore/models.py | 8 -- .../contentstore/push_notification.py | 87 ------------------- cms/djangoapps/contentstore/tasks.py | 5 +- cms/djangoapps/contentstore/tests/tests.py | 14 --- cms/djangoapps/contentstore/views/course.py | 2 - .../views/tests/test_course_updates.py | 40 +-------- cms/static/js/factories/course_info.js | 5 +- cms/static/js/models/course_update.js | 4 +- cms/static/js/spec/views/course_info_spec.js | 53 +---------- cms/static/js/views/course_info_edit.js | 3 +- cms/static/js/views/course_info_update.js | 29 +------ cms/templates/course_info.html | 3 +- .../js/course_info_update.underscore | 25 ++---- common/djangoapps/util/tests/test_db.py | 1 + requirements/edx/base.txt | 1 - requirements/edx/development.txt | 1 - requirements/edx/github.in | 1 - requirements/edx/testing.txt | 1 - 20 files changed, 25 insertions(+), 264 deletions(-) delete mode 100644 cms/djangoapps/contentstore/push_notification.py diff --git a/cms/djangoapps/contentstore/admin.py b/cms/djangoapps/contentstore/admin.py index a04928882c..54adb440b8 100644 --- a/cms/djangoapps/contentstore/admin.py +++ b/cms/djangoapps/contentstore/admin.py @@ -7,7 +7,6 @@ from __future__ import absolute_import from config_models.admin import ConfigurationModelAdmin from django.contrib import admin -from contentstore.models import PushNotificationConfig, VideoUploadConfig +from contentstore.models import VideoUploadConfig admin.site.register(VideoUploadConfig, ConfigurationModelAdmin) -admin.site.register(PushNotificationConfig, ConfigurationModelAdmin) diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py index 099de0255d..e5610cc03a 100644 --- a/cms/djangoapps/contentstore/course_info_model.py +++ b/cms/djangoapps/contentstore/course_info_model.py @@ -20,7 +20,6 @@ import re from django.http import HttpResponseBadRequest from django.utils.translation import ugettext as _ -from cms.djangoapps.contentstore.push_notification import enqueue_push_course_update from openedx.core.lib.xblock_utils import get_course_update_items from xmodule.html_module import CourseInfoModule from xmodule.modulestore.django import modulestore @@ -49,7 +48,6 @@ def update_course_updates(location, update, passed_id=None, user=None): Either add or update the given course update. Add: If the passed_id is absent or None, the course update is added. - If push_notification_selected is set in the update, a celery task for the push notification is created. Update: It will update it if it has a passed_id which has a valid value. Until updates have distinct values, the passed_id is the location url + an index into the html structure. @@ -79,7 +77,6 @@ def update_course_updates(location, update, passed_id=None, user=None): "status": CourseInfoModule.STATUS_VISIBLE } course_update_items.append(course_update_dict) - enqueue_push_course_update(update, location.course_key) # update db record save_course_update_items(location, course_updates, course_update_items, user) diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index 0100b6e12d..9ab2e4ad1b 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -23,11 +23,3 @@ class VideoUploadConfig(ConfigurationModel): def get_profile_whitelist(cls): """Get the list of profiles to include in the encoding download""" return [profile for profile in cls.current().profile_whitelist.split(",") if profile] - - -class PushNotificationConfig(ConfigurationModel): - """ - Configuration for mobile push notifications. - - .. no_pii: - """ diff --git a/cms/djangoapps/contentstore/push_notification.py b/cms/djangoapps/contentstore/push_notification.py deleted file mode 100644 index b626e3aed5..0000000000 --- a/cms/djangoapps/contentstore/push_notification.py +++ /dev/null @@ -1,87 +0,0 @@ -""" -Helper methods for push notifications from Studio. -""" - -from __future__ import absolute_import - -from logging import exception as log_exception -from uuid import uuid4 - -import six -from django.conf import settings -from parse_rest.connection import register -from parse_rest.core import ParseError -from parse_rest.installation import Push -from six import text_type - -from contentstore.models import PushNotificationConfig -from contentstore.tasks import push_course_update_task -from xmodule.modulestore.django import modulestore - - -def push_notification_enabled(): - """ - Returns whether the push notification feature is enabled. - """ - return PushNotificationConfig.is_enabled() - - -def enqueue_push_course_update(update, course_key): - """ - Enqueues a task for push notification for the given update for the given course if - (1) the feature is enabled and - (2) push_notification is selected for the update - """ - if push_notification_enabled() and update.get("push_notification_selected"): - course = modulestore().get_course(course_key) - if course: - push_course_update_task.delay( - six.text_type(course_key), - course.clean_id(padding_char='_'), - course.display_name - ) - - -def send_push_course_update(course_key_string, course_subscription_id, course_display_name): - """ - Sends a push notification for a course update, given the course's subscription_id and display_name. - """ - if settings.PARSE_KEYS: - try: - register( - settings.PARSE_KEYS["APPLICATION_ID"], - settings.PARSE_KEYS["REST_API_KEY"], - ) - push_payload = { - "action": "course.announcement", - "notification-id": six.text_type(uuid4()), - - "course-id": course_key_string, - "course-name": course_display_name, - } - push_channels = [course_subscription_id] - - # Push to all Android devices - Push.alert( - data=push_payload, - channels={"$in": push_channels}, - where={"deviceType": "android"}, - ) - - # Push to all iOS devices - # With additional payload so that - # 1. The push is displayed automatically - # 2. The app gets it even in the background. - # See http://stackoverflow.com/questions/19239737/silent-push-notification-in-ios-7-does-not-work - push_payload.update({ - "alert": "", - "content-available": 1 - }) - Push.alert( - data=push_payload, - channels={"$in": push_channels}, - where={"deviceType": "ios"}, - ) - - except ParseError as error: - log_exception(text_type(error)) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index f715d4e36b..fe3bf88dfa 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -555,9 +555,8 @@ def push_course_update_task(course_key_string, course_subscription_id, course_di """ Sends a push notification for a course update. """ - # TODO Use edx-notifications library instead (MA-638). - from .push_notification import send_push_course_update - send_push_course_update(course_key_string, course_subscription_id, course_display_name) + # TODO Delete once we've done a deploy where nothing is using this. DEPR-41 + pass class CourseExportTask(UserTask): # pylint: disable=abstract-method diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 0b12e2451e..128672b93b 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -8,7 +8,6 @@ import time import mock import pytest -from contentstore.models import PushNotificationConfig from contentstore.tests.test_course_settings import CourseTestCase from contentstore.tests.utils import AjaxEnabledTestClient, parse_json, registration, user from ddt import data, ddt, unpack @@ -417,16 +416,3 @@ class CourseKeyVerificationTestCase(CourseTestCase): ) resp = self.client.get_html(url) self.assertEqual(resp.status_code, status_code) - - -class PushNotificationConfigTestCase(TestCase): - """ - Tests PushNotificationConfig. - """ - - def test_notifications_defaults(self): - self.assertFalse(PushNotificationConfig.is_enabled()) - - def test_notifications_enabled(self): - PushNotificationConfig(enabled=True).save() - self.assertTrue(PushNotificationConfig.is_enabled()) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index f4ef8c320f..5b5aa03264 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -39,7 +39,6 @@ from contentstore.course_group_config import ( ) from contentstore.course_info_model import delete_course_update, get_course_updates, update_course_updates from contentstore.courseware_index import CoursewareSearchIndexer, SearchIndexingError -from contentstore.push_notification import push_notification_enabled from contentstore.tasks import rerun_course as rerun_course_task from contentstore.utils import ( add_instructor, @@ -966,7 +965,6 @@ def course_info_handler(request, course_key_string): 'updates_url': reverse_course_url('course_info_update_handler', course_key), 'handouts_locator': course_key.make_usage_key('course_info', 'handouts'), 'base_asset_url': StaticContent.get_base_url_path_for_course_assets(course_module.id), - 'push_notification_enabled': push_notification_enabled() } ) else: diff --git a/cms/djangoapps/contentstore/views/tests/test_course_updates.py b/cms/djangoapps/contentstore/views/tests/test_course_updates.py index 3ebc8a0fd2..1950a340a2 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_updates.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_updates.py @@ -9,7 +9,6 @@ from django.test.utils import override_settings from mock import patch from opaque_keys.edx.keys import UsageKey -from contentstore.models import PushNotificationConfig from contentstore.tests.test_course_settings import CourseTestCase from contentstore.utils import reverse_course_url, reverse_usage_url from xmodule.modulestore.django import modulestore @@ -239,7 +238,7 @@ class CourseUpdateTest(CourseTestCase): payload = json.loads(resp.content) self.assertEqual(len(payload), 1) - def post_course_update(self, send_push_notification=False): + def post_course_update(self): """ Posts an update to the course """ @@ -250,8 +249,6 @@ class CourseUpdateTest(CourseTestCase): content = u"Sample update" payload = {'content': content, 'date': 'January 8, 2013'} - if send_push_notification: - payload['push_notification_selected'] = True resp = self.client.ajax_post(course_update_url, payload) # check that response status is 200 not 400 @@ -260,16 +257,12 @@ class CourseUpdateTest(CourseTestCase): payload = json.loads(resp.content) self.assertHTMLEqual(payload['content'], content) - @patch("contentstore.push_notification.send_push_course_update") - def test_post_course_update(self, mock_push_update): + def test_post_course_update(self): """ Test that a user can successfully post on course updates and handouts of a course """ self.post_course_update() - # check that push notifications are not sent - self.assertFalse(mock_push_update.called) - updates_location = self.course.id.make_usage_key('course_info', 'updates') self.assertTrue(isinstance(updates_location, UsageKey)) self.assertEqual(updates_location.block_id, u'updates') @@ -287,32 +280,3 @@ class CourseUpdateTest(CourseTestCase): payload = json.loads(resp.content) self.assertHTMLEqual(payload['data'], content) - - @patch("contentstore.push_notification.send_push_course_update") - def test_notifications_enabled_but_not_requested(self, mock_push_update): - PushNotificationConfig(enabled=True).save() - self.post_course_update() - self.assertFalse(mock_push_update.called) - - @patch("contentstore.push_notification.send_push_course_update") - def test_notifications_enabled_and_sent(self, mock_push_update): - PushNotificationConfig(enabled=True).save() - self.post_course_update(send_push_notification=True) - self.assertTrue(mock_push_update.called) - - @override_settings(PARSE_KEYS={"APPLICATION_ID": "TEST_APPLICATION_ID", "REST_API_KEY": "TEST_REST_API_KEY"}) - @patch("contentstore.push_notification.Push") - def test_notifications_sent_to_parse(self, mock_parse_push): - PushNotificationConfig(enabled=True).save() - self.post_course_update(send_push_notification=True) - self.assertEquals(mock_parse_push.alert.call_count, 2) - - @override_settings(PARSE_KEYS={"APPLICATION_ID": "TEST_APPLICATION_ID", "REST_API_KEY": "TEST_REST_API_KEY"}) - @patch("contentstore.push_notification.log_exception") - @patch("contentstore.push_notification.Push") - def test_notifications_error_from_parse(self, mock_parse_push, mock_log_exception): - PushNotificationConfig(enabled=True).save() - from parse_rest.core import ParseError - mock_parse_push.alert.side_effect = ParseError - self.post_course_update(send_push_notification=True) - self.assertTrue(mock_log_exception.called) diff --git a/cms/static/js/factories/course_info.js b/cms/static/js/factories/course_info.js index 4629859919..e0f5fb7c9e 100644 --- a/cms/static/js/factories/course_info.js +++ b/cms/static/js/factories/course_info.js @@ -3,7 +3,7 @@ define([ 'js/models/course_info', 'js/views/course_info_edit' ], function($, CourseUpdateCollection, ModuleInfoModel, CourseInfoModel, CourseInfoEditView) { 'use strict'; - return function(updatesUrl, handoutsLocator, baseAssetUrl, push_notification_enabled) { + return function(updatesUrl, handoutsLocator, baseAssetUrl) { var course_updates = new CourseUpdateCollection(), course_handouts, editor; @@ -18,8 +18,7 @@ define([ updates: course_updates, base_asset_url: baseAssetUrl, handouts: course_handouts - }), - push_notification_enabled: push_notification_enabled + }) }); editor.render(); }; diff --git a/cms/static/js/models/course_update.js b/cms/static/js/models/course_update.js index 9bd642aad6..164ae840d5 100644 --- a/cms/static/js/models/course_update.js +++ b/cms/static/js/models/course_update.js @@ -3,9 +3,7 @@ define(['backbone', 'jquery', 'jquery.ui'], function(Backbone, $) { var CourseUpdate = Backbone.Model.extend({ defaults: { date: $.datepicker.formatDate('MM d, yy', new Date()), - content: '', - push_notification_enabled: false, - push_notification_selected: false + content: '' }, validate: function(attrs) { var date_exists = (attrs.date !== null && attrs.date !== ''); diff --git a/cms/static/js/spec/views/course_info_spec.js b/cms/static/js/spec/views/course_info_spec.js index fdfd834b7a..8d3dca7fb4 100644 --- a/cms/static/js/spec/views/course_info_spec.js +++ b/cms/static/js/spec/views/course_info_spec.js @@ -25,7 +25,7 @@ define(["js/views/course_info_handout", "js/views/course_info_update", "js/model delete window.course_location_analytics; }); - describe("Course Updates without Push notification", function() { + describe("Course Updates", function() { const courseInfoTemplate = readFixtures('course_info_update.underscore'); beforeEach(function() { @@ -136,10 +136,7 @@ define(["js/views/course_info_handout", "js/views/course_info_update", "js/model this.courseInfoEdit.$el.find('.save-button').click(); expect(model.save).toHaveBeenCalled(); - // Verify push_notification_selected is set to false. const requestSent = JSON.parse(requests[requests.length - 1].requestBody); - expect(requestSent.push_notification_selected).toEqual(false); - // Verify the link is not rewritten when saved. expect(requestSent.content).toEqual('/static/image.jpg'); @@ -190,54 +187,6 @@ define(["js/views/course_info_handout", "js/views/course_info_update", "js/model }); - describe("Course Updates WITH Push notification", function() { - const courseInfoTemplate = readFixtures('course_info_update.underscore'); - - beforeEach(function() { - setFixtures($("