Skip to content

E
edx-platform
Commit e4deef9d by Hasnain Naveed Committed by GitHub
Options

Merge pull request #15451 from edx/mjevtic/remove-waffle-switches 

LEARNER-1146 | Remove populate-multitenant-programs waffle switch
parents f42e6f1c bb33a3aa
Showing with 68 additions and 169 deletions
common/djangoapps/student/views.py
......@@ -208,11 +208,8 @@ def index(request, extra_context=None, user=AnonymousUser()):
# Insert additional context for use in the template
context.update(extra_context)
# Add marketable programs to the context if the multi-tenant programs switch is enabled.
if waffle.switch_is_active('get-multitenant-programs'):
programs_list = get_programs_with_type(request.site, include_hidden=False)
context['programs_list'] = programs_list
# Add marketable programs to the context.
context['programs_list'] = get_programs_with_type(request.site, include_hidden=False)
return render_to_response('index.html', context)
......
lms/djangoapps/branding/tests/test_page.py
......@@ -292,25 +292,18 @@ class IndexPageCourseCardsSortingTests(ModuleStoreTestCase):
self.assertEqual(context['courses'][2].id, self.course_with_default_start_date.id)
@ddt.ddt
@attr(shard=1)
class IndexPageProgramsTests(SiteMixin, ModuleStoreTestCase):
"""
Tests for Programs List in Marketing Pages.
"""
@ddt.data(True, False)
def test_get_programs_with_type_called(self, multitenant_programs_enabled):
def test_get_programs_with_type_called(self):
views = [
(reverse('root'), 'student.views.get_programs_with_type'),
(reverse('branding.views.courses'), 'courseware.views.views.get_programs_with_type'),
]
for url, dotted_path in views:
with patch(dotted_path) as mock_get_programs_with_type:
with override_switch('get-multitenant-programs', multitenant_programs_enabled):
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
if multitenant_programs_enabled:
mock_get_programs_with_type.assert_called_once()
else:
mock_get_programs_with_type.assert_not_called()
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
mock_get_programs_with_type.assert_called_once()
lms/djangoapps/courseware/views/views.py
......@@ -147,7 +147,6 @@ def courses(request):
Render "find courses" page. The course selection work is done in courseware.courses.
"""
courses_list = []
programs_list = []
course_discovery_meanings = getattr(settings, 'COURSE_DISCOVERY_MEANINGS', {})
if not settings.FEATURES.get('ENABLE_COURSE_DISCOVERY'):
courses_list = get_courses(request.user)
......@@ -158,9 +157,8 @@ def courses(request):
else:
courses_list = sort_by_announcement(courses_list)
# Add marketable programs to the context if the multi-tenant programs switch is enabled.
if waffle.switch_is_active('get-multitenant-programs'):
programs_list = get_programs_with_type(request.site, include_hidden=False)
# Add marketable programs to the context.
programs_list = get_programs_with_type(request.site, include_hidden=False)
return render_to_response(
"courseware/courses.html",
......
openedx/core/djangoapps/api_admin/tests/test_views.py
......@@ -4,7 +4,6 @@ import json
import ddt
import httpretty
import waffle
from django.conf import settings
from django.core.urlresolvers import reverse
from django.test import TestCase
......@@ -266,7 +265,6 @@ class CatalogSearchViewTest(CatalogTest):
self.assertEqual(response.status_code, 200)
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_post(self):
catalog_user = UserFactory()
self.mock_catalog_endpoint({'results': []})
......@@ -289,7 +287,6 @@ class CatalogListViewTest(CatalogTest):
self.url = reverse('api_admin:catalog-list', kwargs={'username': self.catalog_user.username})
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get(self):
catalog = CatalogFactory(viewers=[self.catalog_user.username])
self.mock_catalog_endpoint({'results': [catalog.attributes]})
......@@ -298,7 +295,6 @@ class CatalogListViewTest(CatalogTest):
self.assertIn(catalog.name, response.content.decode('utf-8'))
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get_no_catalogs(self):
"""Verify that the view works when no catalogs are set up."""
self.mock_catalog_endpoint({}, status_code=404)
......@@ -306,7 +302,6 @@ class CatalogListViewTest(CatalogTest):
self.assertEqual(response.status_code, 200)
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_post(self):
catalog_data = {
'name': 'test-catalog',
......@@ -321,7 +316,6 @@ class CatalogListViewTest(CatalogTest):
self.assertRedirects(response, reverse('api_admin:catalog-edit', kwargs={'catalog_id': catalog_id}))
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_post_invalid(self):
catalog = CatalogFactory(viewers=[self.catalog_user.username])
self.mock_catalog_endpoint({'results': [catalog.attributes]})
......@@ -347,7 +341,6 @@ class CatalogEditViewTest(CatalogTest):
self.url = reverse('api_admin:catalog-edit', kwargs={'catalog_id': self.catalog.id})
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get(self):
self.mock_catalog_endpoint(self.catalog.attributes, catalog_id=self.catalog.id)
response = self.client.get(self.url)
......@@ -355,7 +348,6 @@ class CatalogEditViewTest(CatalogTest):
self.assertIn(self.catalog.name, response.content.decode('utf-8'))
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_delete(self):
self.mock_catalog_endpoint(
self.catalog.attributes,
......@@ -372,7 +364,6 @@ class CatalogEditViewTest(CatalogTest):
self.assertEqual(len(httpretty.httpretty.latest_requests), 1)
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_edit(self):
self.mock_catalog_endpoint(self.catalog.attributes, method=httpretty.PATCH, catalog_id=self.catalog.id)
new_attributes = dict(self.catalog.attributes, **{'delete-catalog': 'off', 'name': 'changed'})
......@@ -381,7 +372,6 @@ class CatalogEditViewTest(CatalogTest):
self.assertRedirects(response, reverse('api_admin:catalog-edit', kwargs={'catalog_id': self.catalog.id}))
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_edit_invalid(self):
self.mock_catalog_endpoint(self.catalog.attributes, catalog_id=self.catalog.id)
new_attributes = dict(self.catalog.attributes, **{'delete-catalog': 'off', 'name': ''})
......@@ -401,7 +391,6 @@ class CatalogPreviewViewTest(CatalogTest):
self.url = reverse('api_admin:catalog-preview')
@httpretty.activate
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get(self):
data = {'count': 1, 'results': ['test data'], 'next': None, 'prev': None}
httpretty.register_uri(
......@@ -414,7 +403,6 @@ class CatalogPreviewViewTest(CatalogTest):
self.assertEqual(response.status_code, 200)
self.assertEqual(json.loads(response.content), data)
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get_without_query(self):
response = self.client.get(self.url)
self.assertEqual(response.status_code, 200)
......
openedx/core/djangoapps/catalog/cache.py
# Template used to create cache keys for individual programs.
PROGRAM_CACHE_KEY_TPL = 'program-{uuid}'
# Cache key used to locate an item containing a list of all program UUIDs.
# This has to be deleted when removing the waffle flags populate-multitenant-programs and get-multitenant-programs
# For more, see LEARNER-1146
PROGRAM_UUIDS_CACHE_KEY = 'program-uuids'
# Cache key used to locate an item containing a list of all program UUIDs for a site.
SITE_PROGRAM_UUIDS_CACHE_KEY_TPL = 'program-uuids-{domain}'
openedx/core/djangoapps/catalog/management/commands/cache_programs.py
import logging
import sys
import waffle
from django.contrib.auth import get_user_model
from django.contrib.sites.models import Site
from django.core.cache import cache
......@@ -9,7 +8,6 @@ from django.core.management import BaseCommand
from openedx.core.djangoapps.catalog.cache import (
PROGRAM_CACHE_KEY_TPL,
PROGRAM_UUIDS_CACHE_KEY,
SITE_PROGRAM_UUIDS_CACHE_KEY_TPL
)
from openedx.core.djangoapps.catalog.models import CatalogIntegration
......@@ -30,108 +28,51 @@ class Command(BaseCommand):
help = "Rebuild the LMS' cache of program data."
def handle(self, *args, **options):
if waffle.switch_is_active('populate-multitenant-programs'):
failure = False
logger.info('populate-multitenant-programs switch is ON')
failure = False
logger.info('populate-multitenant-programs switch is ON')
catalog_integration = CatalogIntegration.current()
username = catalog_integration.service_username
catalog_integration = CatalogIntegration.current()
username = catalog_integration.service_username
try:
user = User.objects.get(username=username)
except User.DoesNotExist:
logger.error(
'Failed to create API client. Service user {username} does not exist.'.format(username=username)
)
raise
programs = {}
for site in Site.objects.all():
site_config = getattr(site, 'configuration', None)
if site_config is None or not site_config.get_value('COURSE_CATALOG_API_URL'):
logger.info('Skipping site {domain}. No configuration.'.format(domain=site.domain))
cache.set(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), [], None)
continue
client = create_catalog_api_client(user, site=site)
uuids, program_uuids_failed = self.get_site_program_uuids(client, site)
new_programs, program_details_failed = self.fetch_program_details(client, uuids)
if program_uuids_failed or program_details_failed:
failure = True
programs.update(new_programs)
logger.info('Caching UUIDs for {total} programs for site {site_name}.'.format(
total=len(uuids),
site_name=site.domain,
))
cache.set(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), uuids, None)
successful = len(programs)
logger.info('Caching details for {successful} programs.'.format(successful=successful))
cache.set_many(programs, None)
if failure:
# This will fail a Jenkins job running this command, letting site
# operators know that there was a problem.
sys.exit(1)
else:
catalog_integration = CatalogIntegration.current()
username = catalog_integration.service_username
try:
user = User.objects.get(username=username)
except User.DoesNotExist:
logger.error(
'Failed to create API client. Service user {username} does not exist.'.format(username=username)
)
raise
try:
user = User.objects.get(username=username)
client = create_catalog_api_client(user)
except User.DoesNotExist:
logger.error(
'Failed to create API client. Service user {username} does not exist.'.format(username=username)
)
raise
programs = {}
for site in Site.objects.all():
site_config = getattr(site, 'configuration', None)
if site_config is None or not site_config.get_value('COURSE_CATALOG_API_URL'):
logger.info('Skipping site {domain}. No configuration.'.format(domain=site.domain))
cache.set(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), [], None)
continue
try:
querystring = {
'exclude_utm': 1,
'status': ('active', 'retired'),
'uuids_only': 1,
}
logger.info('Requesting program UUIDs.')
uuids = client.programs.get(**querystring)
except: # pylint: disable=bare-except
logger.error('Failed to retrieve program UUIDs.')
raise
total = len(uuids)
logger.info('Received {total} UUIDs.'.format(total=total))
programs = {}
failure = False
for uuid in uuids:
try:
logger.info('Requesting details for program {uuid}.'.format(uuid=uuid))
program = client.programs(uuid).get(exclude_utm=1)
cache_key = PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)
programs[cache_key] = program
except: # pylint: disable=bare-except
logger.exception('Failed to retrieve details for program {uuid}.'.format(uuid=uuid))
failure = True
continue
successful = len(programs)
logger.info('Caching details for {successful} programs.'.format(successful=successful))
cache.set_many(programs, None)
logger.info('Caching UUIDs for {total} programs.'.format(total=total))
cache.set(PROGRAM_UUIDS_CACHE_KEY, uuids, None)
if failure:
# This will fail a Jenkins job running this command, letting site
# operators know that there was a problem.
sys.exit(1)
client = create_catalog_api_client(user, site=site)
uuids, program_uuids_failed = self.get_site_program_uuids(client, site)
new_programs, program_details_failed = self.fetch_program_details(client, uuids)
if program_uuids_failed or program_details_failed:
failure = True
programs.update(new_programs)
logger.info('Caching UUIDs for {total} programs for site {site_name}.'.format(
total=len(uuids),
site_name=site.domain,
))
cache.set(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), uuids, None)
successful = len(programs)
logger.info('Caching details for {successful} programs.'.format(successful=successful))
cache.set_many(programs, None)
if failure:
# This will fail a Jenkins job running this command, letting site
# operators know that there was a problem.
sys.exit(1)
def get_site_program_uuids(self, client, site):
failure = False
......
openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py
import json
import httpretty
import waffle
from django.core.cache import cache
from django.core.management import call_command
from openedx.core.djangoapps.catalog.cache import (
PROGRAM_CACHE_KEY_TPL,
PROGRAM_UUIDS_CACHE_KEY,
SITE_PROGRAM_UUIDS_CACHE_KEY_TPL
)
from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory
......@@ -74,7 +72,6 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
content_type='application/json'
)
@waffle.testutils.override_switch('populate-multitenant-programs', True)
def test_handle(self):
"""
Verify that the command requests and caches program UUIDs and details.
......@@ -118,7 +115,6 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
for key, program in cached_programs.items():
self.assertEqual(program, programs[key])
@waffle.testutils.override_switch('populate-multitenant-programs', True)
def test_handle_missing_service_user(self):
"""
Verify that the command raises an exception when run without a service
......@@ -130,7 +126,6 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
cached_uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site_domain))
self.assertEqual(cached_uuids, None)
@waffle.testutils.override_switch('populate-multitenant-programs', True)
def test_handle_missing_uuids(self):
"""
Verify that the command raises an exception when it fails to retrieve
......@@ -145,7 +140,6 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
cached_uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site_domain))
self.assertEqual(cached_uuids, [])
@waffle.testutils.override_switch('populate-multitenant-programs', True)
def test_handle_missing_programs(self):
"""
Verify that a problem retrieving a program doesn't prevent the command
......
openedx/core/djangoapps/catalog/models.py
"""Models governing integration with the catalog service."""
import waffle
from config_models.models import ConfigurationModel
from django.conf import settings
from django.contrib.auth import get_user_model
......@@ -55,10 +54,7 @@ class CatalogIntegration(ConfigurationModel):
def get_internal_api_url(self):
""" Returns the internal Catalog API URL associated with the request's site. """
if waffle.switch_is_active("populate-multitenant-programs"):
return helpers.get_value('COURSE_CATALOG_API_URL', settings.COURSE_CATALOG_API_URL)
else:
return self.internal_api_url
return helpers.get_value('COURSE_CATALOG_API_URL', settings.COURSE_CATALOG_API_URL)
def get_service_user(self):
# NOTE: We load the user model here to avoid issues at startup time that result from the hacks
......
openedx/core/djangoapps/catalog/tests/test_models.py
"""Catalog model tests."""
import ddt
import mock
import waffle
from django.test import TestCase, override_settings
from openedx.core.djangoapps.catalog.tests import mixins
......@@ -32,7 +31,6 @@ class TestCatalogIntegration(mixins.CatalogIntegrationMixin, TestCase):
self.assertEqual(catalog_integration.is_cache_enabled, is_cache_enabled)
@override_settings(COURSE_CATALOG_API_URL=COURSE_CATALOG_API_URL)
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get_internal_api_url(self, _mock_cache):
""" Requests made without a microsite should return the value from settings. """
self.assert_get_internal_api_url_value(COURSE_CATALOG_API_URL)
......@@ -41,7 +39,6 @@ class TestCatalogIntegration(mixins.CatalogIntegrationMixin, TestCase):
@override_settings(COURSE_CATALOG_API_URL=COURSE_CATALOG_API_URL)
@with_site_configuration(configuration={})
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get_internal_api_url_without_microsite_override(self, _mock_cache):
""" Requests made to microsites that do not have COURSE_CATALOG_API_URL overridden should
return the default value from settings. """
......@@ -49,7 +46,6 @@ class TestCatalogIntegration(mixins.CatalogIntegrationMixin, TestCase):
@override_settings(COURSE_CATALOG_API_URL=COURSE_CATALOG_API_URL)
@with_site_configuration(configuration={'COURSE_CATALOG_API_URL': 'foo'})
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get_internal_api_url_with_microsite_override(self, _mock_cache):
""" If a microsite has overridden the value of COURSE_CATALOG_API_URL, the overridden
value should be returned. """
......
openedx/core/djangoapps/catalog/tests/test_utils.py
......@@ -9,7 +9,7 @@ from django.core.cache import cache
from django.test import TestCase, override_settings
from student.tests.factories import UserFactory
from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL, PROGRAM_UUIDS_CACHE_KEY
from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL, SITE_PROGRAM_UUIDS_CACHE_KEY_TPL
from openedx.core.djangoapps.catalog.models import CatalogIntegration
from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory, ProgramFactory, ProgramTypeFactory
from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
......@@ -53,7 +53,7 @@ class TestGetPrograms(CacheIsolationTestCase):
# Cache UUIDs for all 3 programs.
cache.set(
PROGRAM_UUIDS_CACHE_KEY,
SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site.domain),
[program['uuid'] for program in programs],
None
)
......
openedx/core/djangoapps/catalog/utils.py
......@@ -10,7 +10,6 @@ from edx_rest_api_client.client import EdxRestApiClient
from openedx.core.djangoapps.catalog.cache import (
PROGRAM_CACHE_KEY_TPL,
PROGRAM_UUIDS_CACHE_KEY,
SITE_PROGRAM_UUIDS_CACHE_KEY_TPL
)
from openedx.core.djangoapps.catalog.models import CatalogIntegration
......@@ -57,10 +56,7 @@ def get_programs(site, uuid=None):
logger.warning(missing_details_msg_tpl.format(uuid=uuid))
return program
if waffle.switch_is_active('get-multitenant-programs'):
uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), [])
else:
uuids = cache.get(PROGRAM_UUIDS_CACHE_KEY, [])
uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=site.domain), [])
if not uuids:
logger.warning('Failed to get program UUIDs from the cache.')
......
openedx/core/djangoapps/catalog/views.py
......@@ -3,6 +3,8 @@ from django.core.management import call_command
from django.http import Http404, HttpResponse
from django.views.decorators.http import require_GET
from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
@require_GET
def cache_programs(request):
......@@ -13,6 +15,19 @@ def cache_programs(request):
reached over HTTP (e.g., Selenium-based browser tests). The discovery service
API the management command attempts to call should be stubbed out first.
"""
# checks that does site has configuration if not then
# add a configuration with COURSE_CATALOG_API_URL parameter.
from common.test.acceptance.fixtures import CATALOG_STUB_URL
site_config = getattr(request.site, 'configuration', None)
if not site_config:
SiteConfiguration.objects.create(
site=request.site,
enabled=True,
values={"COURSE_CATALOG_API_URL": "{catalog_url}/api/v1/".format(catalog_url=CATALOG_STUB_URL)}
)
if settings.FEATURES.get('EXPOSE_CACHE_PROGRAMS_ENDPOINT'):
call_command('cache_programs')
......
openedx/core/lib/tests/test_edx_api_utils.py
......@@ -4,17 +4,12 @@ import json
import httpretty
import mock
import waffle
from django.core.cache import cache
from django.test.utils import override_settings
from edx_oauth2_provider.tests.factories import ClientFactory
from nose.plugins.attrib import attr
from provider.constants import CONFIDENTIAL
from openedx.core.djangoapps.catalog.models import CatalogIntegration
from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
from openedx.core.djangoapps.catalog.utils import create_catalog_api_client
from openedx.core.djangoapps.credentials.models import CredentialsApiConfig
from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from openedx.core.lib.edx_api_utils import get_edx_api_data
......@@ -74,7 +69,6 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
# Verify the API was actually hit (not the cache)
self._assert_num_requests(1)
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get_paginated_data(self):
"""Verify that paginated data can be retrieved."""
catalog_integration = self.create_catalog_integration()
......@@ -102,7 +96,6 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
self._assert_num_requests(len(expected_collection))
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get_paginated_data_do_not_traverse_pagination(self):
"""
Verify that pagination is not traversed if traverse_pagination=False is passed as argument.
......@@ -131,7 +124,6 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
self.assertEqual(actual_collection, expected_response)
self._assert_num_requests(1)
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get_specific_resource(self):
"""Verify that a specific resource can be retrieved."""
catalog_integration = self.create_catalog_integration()
......@@ -155,7 +147,6 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
self._assert_num_requests(1)
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_get_specific_resource_with_falsey_id(self):
"""
Verify that a specific resource can be retrieved, and pagination parsing is
......@@ -185,7 +176,6 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
self._assert_num_requests(1)
@waffle.testutils.override_switch("populate-multitenant-programs", True)
def test_cache_utilization(self):
"""Verify that when enabled, the cache is used."""
catalog_integration = self.create_catalog_integration(cache_ttl=5)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment