From 877ec3f963f3ca38319e9a04db1a047df4bf3477 Mon Sep 17 00:00:00 2001
From: Alan Boudreault <boudreault.alan@gmail.com>
Date: Tue, 20 May 2014 09:50:41 -0400
Subject: [PATCH] some minor improvements to embargo

---
 cms/envs/common.py                             | 7 +++++--
 common/djangoapps/embargo/middleware.py        | 6 +++---
 common/djangoapps/embargo/models.py            | 2 +-
 common/djangoapps/embargo/tests/test_forms.py  | 8 ++++----
 common/djangoapps/embargo/tests/test_models.py | 2 +-
 lms/envs/common.py                             | 7 +++++--
 6 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/cms/envs/common.py b/cms/envs/common.py
index 58909c024a4..858de4e03cf 100644
--- a/cms/envs/common.py
+++ b/cms/envs/common.py
@@ -85,10 +85,10 @@ FEATURES = {
     # Hide any Personally Identifiable Information from application logs
     'SQUELCH_PII_IN_LOGS': False,
 
-    # Toggles embargo functionality
+    # Toggles the embargo functionality, which enable embargoing for particular courses
     'EMBARGO': False,
 
-    # Toggle embargo site functionality
+    # Toggles the embargo site functionality, which enable embargoing for the whole site
     'SITE_EMBARGOED': False,
 
     # Turn on/off Microsites feature
@@ -305,6 +305,9 @@ LOCALE_PATHS = (REPO_ROOT + '/conf/locale',)  # edx-platform/conf/locale/
 # Messages
 MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage'
 
+##### EMBARGO #####
+EMBARGO_SITE_REDIRECT_URL = None
+
 ############################### Pipeline #######################################
 
 STATICFILES_STORAGE = 'pipeline.storage.PipelineCachedStorage'
diff --git a/common/djangoapps/embargo/middleware.py b/common/djangoapps/embargo/middleware.py
index 6eb9af373a3..052c04bdbce 100644
--- a/common/djangoapps/embargo/middleware.py
+++ b/common/djangoapps/embargo/middleware.py
@@ -18,13 +18,13 @@ Usage:
 
 # Enable the middleware in your settings
 
-# To enable Embargoing by courses, add:
+# To enable Embargo for particular courses, set:
 FEATURES['EMBARGO'] = True # blocked ip will be redirected to /embargo
 
-# To enable Embargoing site:
+# To enable the Embargo feature for the whole site, set:
 FEATURES['SITE_EMBARGOED'] = True
 
-# With SITE_EMBARGOED, you can define an external to redirect with:
+# With SITE_EMBARGOED, you can define an external url to redirect with:
 EMBARGO_SITE_REDIRECT_URL = 'https://www.edx.org/'
 
 # if EMBARGO_SITE_REDIRECT_URL is missing, a HttpResponseForbidden is returned.
diff --git a/common/djangoapps/embargo/models.py b/common/djangoapps/embargo/models.py
index a48e39f1f24..677d174ccb7 100644
--- a/common/djangoapps/embargo/models.py
+++ b/common/djangoapps/embargo/models.py
@@ -92,7 +92,7 @@ class IPFilter(ConfigurationModel):
 
         def __iter__(self):
             for network in self.networks:
-                    yield network
+                yield network
 
         def __contains__(self, ip):
             try:
diff --git a/common/djangoapps/embargo/tests/test_forms.py b/common/djangoapps/embargo/tests/test_forms.py
index dc14515474e..21a6c37054d 100644
--- a/common/djangoapps/embargo/tests/test_forms.py
+++ b/common/djangoapps/embargo/tests/test_forms.py
@@ -171,16 +171,16 @@ class IPFilterFormTest(TestCase):
 
         # Network tests
         # ips not in whitelist network
-        for addr in '1.1.0.2, 1.0.1.0'.split(','):
+        for addr in ['1.1.0.2', '1.0.1.0']:
             self.assertNotIn(addr.strip(), whitelist)
         # ips in whitelist network
-        for addr in '1.1.0.1, 1.0.0.100'.split(','):
+        for addr in ['1.1.0.1', '1.0.0.100']:
             self.assertIn(addr.strip(), whitelist)
         # ips not in blacklist network
-        for addr in '2.0.0.0, 1.1.0.0'.split(','):
+        for addr in ['2.0.0.0', '1.1.0.0']:
             self.assertNotIn(addr.strip(), blacklist)
         # ips in blacklist network
-        for addr in '1.0.100.0, 1.0.0.10'.split(','):
+        for addr in ['1.0.100.0', '1.0.0.10']:
             self.assertIn(addr.strip(), blacklist)
 
         # Test clearing by adding an empty list is OK too
diff --git a/common/djangoapps/embargo/tests/test_models.py b/common/djangoapps/embargo/tests/test_models.py
index fb2e4c89c28..8f9dc5d9300 100644
--- a/common/djangoapps/embargo/tests/test_models.py
+++ b/common/djangoapps/embargo/tests/test_models.py
@@ -79,7 +79,7 @@ class EmbargoModelsTest(TestCase):
         cblacklist = IPFilter.current().blacklist_ips
         self.assertTrue(blacklist in cblacklist)
 
-        # network tests
+    def test_ip_network_blocking(self):
         whitelist = '1.0.0.0/24'
         blacklist = '1.1.0.0/16'
 
diff --git a/lms/envs/common.py b/lms/envs/common.py
index 24eafabc66a..9e6bf06714b 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -225,10 +225,10 @@ FEATURES = {
     # Hide any Personally Identifiable Information from application logs
     'SQUELCH_PII_IN_LOGS': False,
 
-    # Toggle embargo functionality
+    # Toggles the embargo functionality, which enable embargoing for particular courses
     'EMBARGO': False,
 
-    # Toggle embargo site functionality
+    # Toggles the embargo site functionality, which enable embargoing for the whole site
     'SITE_EMBARGOED': False,
 
     # Whether the Wiki subsystem should be accessible via the direct /wiki/ paths. Setting this to True means
@@ -680,6 +680,9 @@ ZENDESK_URL = None
 ZENDESK_USER = None
 ZENDESK_API_KEY = None
 
+##### EMBARGO #####
+EMBARGO_SITE_REDIRECT_URL = None
+
 ##### shoppingcart Payment #####
 PAYMENT_SUPPORT_EMAIL = 'payment@example.com'
 ##### Using cybersource by default #####
-- 
GitLab