From c9fecb110325fc310b14c2cafe36428dffe4958d Mon Sep 17 00:00:00 2001 From: Guruprasad Lakshmi Narayanan Date: Fri, 3 Jan 2020 11:52:14 +0530 Subject: [PATCH] Update the existing 'create_site_configuration' management command Updates the existing command, after renaming it, to allow: * Specifying the site ID instead of the domain. * Updating the existing site configuration parameters. * Enabling and disabling the SiteConfiguration instance. --- .../create_or_update_site_configuration.py | 114 ++++++++++++ .../commands/create_site_configuration.py | 50 ----- .../commands/tests/fixtures/config1.json | 4 + ...est_create_or_update_site_configuration.py | 176 ++++++++++++++++++ .../tests/test_create_site_configuration.py | 52 ------ 5 files changed, 294 insertions(+), 102 deletions(-) create mode 100644 openedx/core/djangoapps/site_configuration/management/commands/create_or_update_site_configuration.py delete mode 100644 openedx/core/djangoapps/site_configuration/management/commands/create_site_configuration.py create mode 100644 openedx/core/djangoapps/site_configuration/management/commands/tests/fixtures/config1.json create mode 100644 openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_or_update_site_configuration.py delete mode 100644 openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_site_configuration.py diff --git a/openedx/core/djangoapps/site_configuration/management/commands/create_or_update_site_configuration.py b/openedx/core/djangoapps/site_configuration/management/commands/create_or_update_site_configuration.py new file mode 100644 index 0000000000..7c57b10e2a --- /dev/null +++ b/openedx/core/djangoapps/site_configuration/management/commands/create_or_update_site_configuration.py @@ -0,0 +1,114 @@ +""" +Create or updates the SiteConfiguration for a site. +""" +from __future__ import absolute_import, unicode_literals + +import codecs +import json +import logging + +from django.contrib.sites.models import Site +from django.core.management import BaseCommand + + +from openedx.core.djangoapps.site_configuration.models import SiteConfiguration + +LOG = logging.getLogger(__name__) + + +def load_json_from_file(filename): + return json.load(codecs.open(filename, encoding='utf-8')) + + +class Command(BaseCommand): + """ + Management command to create or update SiteConfiguration. + """ + help = 'Create or update SiteConfiguration' + + def add_arguments(self, parser): + args_group1 = parser.add_mutually_exclusive_group(required=True) + args_group1.add_argument( + '--site-id', + action='store', + dest='site_id', + type=int, + help='ID of the Site whose SiteConfiguration has to be updated.' + ) + args_group1.add_argument('domain', nargs='?', default='') + + args_group2 = parser.add_mutually_exclusive_group() + args_group2.add_argument( + '--configuration', + type=json.loads, + help="Enter JSON site configuration", + required=False + ) + args_group2.add_argument( + '-f', + '--configuration-file', + type=load_json_from_file, + dest='config_file_data', + help="Enter the path to the JSON file containing the site configuration", + required=False + ) + + args_group3 = parser.add_mutually_exclusive_group() + args_group3.add_argument( + '--enabled', + action='store_true', + dest='enabled', + default=None, + help='Enable the SiteConfiguration.' + ) + args_group3.add_argument( + '--disabled', + action='store_false', + dest='enabled', + help='Disable the SiteConfiguration.' + ) + + def handle(self, *args, **options): + site_id = options.get('site_id') + domain = options.get('domain') + configuration = options.get('configuration') + config_file_data = options.get('config_file_data') + + enabled = options.get('enabled') + + if site_id is not None: + site, created = Site.objects.get_or_create(id=site_id) + else: + site, created = Site.objects.get_or_create( + domain=domain, + name=domain, + ) + if created: + LOG.info("Site does not exist. Created new site '{site_name}'".format(site_name=site.domain)) + else: + LOG.info("Found existing site for '{site_name}'".format(site_name=site.domain)) + + site_configuration, created = SiteConfiguration.objects.get_or_create(site=site) + if created: + LOG.info( + "Site configuration does not exist. Created new instance for '{site_name}'".format( + site_name=site.domain + ) + ) + else: + LOG.info( + "Found existing site configuration for '{site_name}'. Updating it.".format(site_name=site.domain) + ) + + site_configuration_values = configuration or config_file_data + + if site_configuration_values: + if site_configuration.values: + site_configuration.values.update(site_configuration_values) + else: + site_configuration.values = site_configuration_values + + if enabled is not None: + site_configuration.enabled = enabled + + site_configuration.save() diff --git a/openedx/core/djangoapps/site_configuration/management/commands/create_site_configuration.py b/openedx/core/djangoapps/site_configuration/management/commands/create_site_configuration.py deleted file mode 100644 index c35ba87c78..0000000000 --- a/openedx/core/djangoapps/site_configuration/management/commands/create_site_configuration.py +++ /dev/null @@ -1,50 +0,0 @@ -""" -Run by ansible to setup a single site configuration in sandbox environments -""" -import json -import logging -from textwrap import dedent - -from django.contrib.sites.models import Site -from django.core.management.base import BaseCommand - -from openedx.core.djangoapps.site_configuration.models import SiteConfiguration - -LOG = logging.getLogger(__name__) - - -class Command(BaseCommand): - """ - Command to create a configuration for a single site. If the site does not - already exist one will be created. - - Example: - ./manage.py lms create_configuration uox.sandbox.edx.org - --configuration="{'COURSE_CATALOG_API_URL':'https://discovery-uox.sandbox.edx.org/api/v1'}" - """ - help = dedent(__doc__).strip() - - def add_arguments(self, parser): - parser.add_argument('domain') - parser.add_argument( - '--configuration', - type=json.loads, - help="Enter JSON site configuration", - required=False, - default='' - ) - - def handle(self, *args, **options): - domain = options['domain'] - configuration = options['configuration'] - site, created = Site.objects.get_or_create( - domain=domain, - name=domain, - ) - if created: - LOG.info(u"Site does not exist. Created new site '{site_name}'".format(site_name=domain)) - else: - LOG.info(u"Found existing site for '{site_name}'".format(site_name=domain)) - - LOG.info(u"Creating '{site_name}' SiteConfiguration".format(site_name=domain)) - SiteConfiguration.objects.create(site=site, values=configuration, enabled=True) diff --git a/openedx/core/djangoapps/site_configuration/management/commands/tests/fixtures/config1.json b/openedx/core/djangoapps/site_configuration/management/commands/tests/fixtures/config1.json new file mode 100644 index 0000000000..3b7b75bb8e --- /dev/null +++ b/openedx/core/djangoapps/site_configuration/management/commands/tests/fixtures/config1.json @@ -0,0 +1,4 @@ +{ + "ABC": 123, + "XYZ": "789" +} diff --git a/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_or_update_site_configuration.py b/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_or_update_site_configuration.py new file mode 100644 index 0000000000..0a0d4974ad --- /dev/null +++ b/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_or_update_site_configuration.py @@ -0,0 +1,176 @@ +""" +Tests for the create_or_update_site_configuration management command. +""" +from __future__ import absolute_import, unicode_literals + +import codecs +import json + +import ddt +from django.contrib.sites.models import Site +from django.core.management import call_command, CommandError +from django.test import TestCase +from path import Path + +from openedx.core.djangoapps.site_configuration.models import SiteConfiguration + + +@ddt.ddt +class CreateOrUpdateSiteConfigurationTest(TestCase): + """ + Test for the create_or_update_site_configuration management command. + """ + command = 'create_or_update_site_configuration' + + def setUp(self): + super(CreateOrUpdateSiteConfigurationTest, self).setUp() + self.site_id = 1 + self.site_id_arg = ['--site-id', str(self.site_id)] + self.json_file_path = Path(__file__).parent / "fixtures/config1.json" + self.input_configuration = { + 'FEATURE_FLAG': True, + 'SERVICE_URL': 'https://foo.bar', + 'ABC': 123, + } + + @property + def site(self): + """ + Return the fixture site for this test class. + """ + return Site.objects.get(id=self.site_id) + + def assert_site_configuration_does_not_exist(self): + """ + Assert that the site configuration for the fixture site does not exist. + """ + with self.assertRaises(SiteConfiguration.DoesNotExist): + SiteConfiguration.objects.get(site=self.site) + + def get_site_configuration(self): + """ + Return the site configuration for the fixture site. + """ + return SiteConfiguration.objects.get(site=self.site) + + def create_fixture_site_configuration(self, enabled): + SiteConfiguration.objects.update_or_create( + site=self.site, + defaults={'enabled': enabled, 'values': {'ABC': 'abc', 'B': 'b'}} + ) + + def test_command_no_args(self): + """ + Verify the error on the command with no arguments. + """ + with self.assertRaises(CommandError) as error: + call_command(self.command) + self.assertIn('Error: one of the arguments --site-id domain is required', str(error.exception)) + + def test_site_created_when_site_id_non_existent(self): + """ + Verify that a new site is created when given a site ID that doesn't exist. + """ + non_existent_site_id = 999 + with self.assertRaises(Site.DoesNotExist): + Site.objects.get(id=non_existent_site_id) + + call_command(self.command, '--site-id', non_existent_site_id) + Site.objects.get(id=non_existent_site_id) + + def test_site_created_when_domain_non_existent(self): + """ + Verify that a new site is created when given a domain name that does not have an existing site.. + """ + domain = 'nonexistent.com' + with self.assertRaises(Site.DoesNotExist): + Site.objects.get(domain=domain) + call_command(self.command, domain) + Site.objects.get(domain=domain) + + def test_both_site_id_domain_given(self): + """ + Verify that an error is thrown when both site_id and the domain name are provided. + """ + with self.assertRaises(CommandError) as error: + call_command(self.command, 'domain.com', '--site-id', '1') + + self.assertIn('not allowed with argument', str(error.exception)) + + def test_site_configuration_created_when_non_existent(self): + """ + Verify that a SiteConfiguration instance is created if it doesn't exist. + """ + self.assert_site_configuration_does_not_exist() + + call_command(self.command, *self.site_id_arg) + site_configuration = SiteConfiguration.objects.get(site=self.site) + self.assertFalse(site_configuration.values) + self.assertFalse(site_configuration.enabled) + + def test_both_enabled_disabled_flags(self): + """ + Verify the error on providing both the --enabled and --disabled flags. + """ + with self.assertRaises(CommandError) as error: + call_command(self.command, '--enabled', '--disabled', *self.site_id_arg) + self.assertIn('argument --disabled: not allowed with argument --enabled', str(error.exception)) + + @ddt.data(('enabled', True), + ('disabled', False)) + @ddt.unpack + def test_site_configuration_enabled_disabled(self, flag, enabled): + """ + Verify that the SiteConfiguration instance is enabled/disabled as per the flag used. + """ + self.assert_site_configuration_does_not_exist() + call_command(self.command, '--{}'.format(flag), *self.site_id_arg) + site_configuration = SiteConfiguration.objects.get(site=self.site) + self.assertFalse(site_configuration.values) + self.assertEqual(enabled, site_configuration.enabled) + + def test_site_configuration_created_with_parameters(self): + """ + Verify that a SiteConfiguration instance is created with the provided values if it does not exist. + """ + self.assert_site_configuration_does_not_exist() + call_command(self.command, '--configuration', json.dumps(self.input_configuration), *self.site_id_arg) + site_configuration = self.get_site_configuration() + self.assertDictEqual(site_configuration.values, self.input_configuration) + + def test_site_configuration_created_with_json_file_parameters(self): + """ + Verify that a SiteConfiguration instance is created with the provided values if it does not exist. + """ + self.assert_site_configuration_does_not_exist() + call_command(self.command, '-f', str(self.json_file_path.abspath()), *self.site_id_arg) + site_configuration = self.get_site_configuration() + self.assertEqual(site_configuration.values, {'ABC': 123, 'XYZ': '789'}) + + @ddt.data(True, False) + def test_site_configuration_updated_with_parameters(self, enabled): + """ + Verify that the existing parameters are updated when provided in the command. + """ + self.create_fixture_site_configuration(enabled) + call_command(self.command, '--configuration', json.dumps(self.input_configuration), *self.site_id_arg) + site_configuration = self.get_site_configuration() + self.assertEqual( + site_configuration.values, + {'ABC': 123, 'B': 'b', 'FEATURE_FLAG': True, 'SERVICE_URL': 'https://foo.bar'} + ) + self.assertEqual(site_configuration.enabled, enabled) + + @ddt.data(True, False) + def test_site_configuration_updated_from_json_file(self, enabled): + """ + Verify that the existing parameteres are updated when provided through a YAML file. + """ + self.create_fixture_site_configuration(enabled) + call_command(self.command, '-f', str(self.json_file_path.abspath()), *self.site_id_arg) + site_configuration = self.get_site_configuration() + expected_site_configuration = {'ABC': 'abc', 'B': 'b'} + with codecs.open(self.json_file_path, encoding='utf-8') as f: + expected_site_configuration.update(json.load(f)) + self.assertEqual(site_configuration.values, expected_site_configuration) + self.assertEqual(site_configuration.enabled, enabled) diff --git a/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_site_configuration.py b/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_site_configuration.py deleted file mode 100644 index 95f3ce612e..0000000000 --- a/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_site_configuration.py +++ /dev/null @@ -1,52 +0,0 @@ -""" -Test create_site_configuration management command -""" -import json - -from django.contrib.sites.models import Site -from django.core.management import call_command -from django.test import TestCase -from openedx.core.djangoapps.site_configuration.models import SiteConfiguration - - -class TestCreateSiteConfiguration(TestCase): - """ Test create_site_configuration command """ - def setUp(self): - super(TestCreateSiteConfiguration, self).setUp() - self.site_domain = 'example.com' - self.input_configuration = { - 'FEATURE_FLAG': True, - 'SERVICE_URL': 'https://foo.bar' - } - - def _validate_site_configuration(self, site): - site_configuration = SiteConfiguration.objects.get(site_id=site.id) - self.assertDictEqual(site_configuration.values, self.input_configuration) - - def test_create_site_and_config(self): - call_command( - 'create_site_configuration', - self.site_domain, - '--configuration', json.dumps(self.input_configuration), - ) - - site = Site.objects.get(domain__contains=self.site_domain) - self.assertEqual(site.name, self.site_domain) - self.assertEqual(site.domain, self.site_domain) - - self._validate_site_configuration(site) - - def test_create_for_existing_site(self): - site, created = Site.objects.get_or_create( # pylint: disable=unused-variable - name=self.site_domain, - domain=self.site_domain, - ) - - call_command( - 'create_site_configuration', - self.site_domain, - '--configuration', json.dumps(self.input_configuration), - ) - - self.assertEqual(len(Site.objects.filter(domain__contains=self.site_domain)), 1) - self._validate_site_configuration(site)