diff --git a/completion/api/v1/views.py b/completion/api/v1/views.py index bf3c6bdc..2ed30a2b 100644 --- a/completion/api/v1/views.py +++ b/completion/api/v1/views.py @@ -33,7 +33,6 @@ except ImportError: pass -from completion import waffle from completion.api.permissions import IsStaffOrOwner, IsUserInUrl from completion.models import BlockCompletion @@ -79,11 +78,6 @@ def _validate_and_parse(self, batch_object): ObjectDoesNotExist: If a database object cannot be found an ObjectDoesNotExist is raised. """ - if not waffle.ENABLE_COMPLETION_TRACKING_SWITCH.is_enabled(): - raise ValidationError( - _("BlockCompletion.objects.submit_batch_completion should not be called when the feature is disabled.") - ) - for key in self.REQUIRED_KEYS: if key not in batch_object: raise ValidationError(_("Key '{key}' not found.").format(key=key)) diff --git a/completion/handlers.py b/completion/handlers.py index a4ca741e..2e3bb769 100644 --- a/completion/handlers.py +++ b/completion/handlers.py @@ -10,7 +10,6 @@ from xblock.completable import XBlockCompletionMode from xblock.core import XBlock -from . import waffle from .models import BlockCompletion log = logging.getLogger(__name__) @@ -21,8 +20,6 @@ def scorable_block_completion(sender, **kwargs): # pylint: disable=unused-argum """ When a problem is scored, submit a new BlockCompletion for that block. """ - if not waffle.ENABLE_COMPLETION_TRACKING_SWITCH.is_enabled(): - return try: block_key = UsageKey.from_string(kwargs['usage_id']) except InvalidKeyError: diff --git a/completion/models.py b/completion/models.py index 338fdc14..4307ad37 100644 --- a/completion/models.py +++ b/completion/models.py @@ -16,8 +16,6 @@ from eventtracking import tracker -from . import waffle - log = logging.getLogger(__name__) User = auth.get_user_model() @@ -93,47 +91,39 @@ def submit_completion(self, user, block_key, completion): "block_key = block_key.replace(course_key=modulestore().fill_in_run(block_key.course_key))" ) - if waffle.ENABLE_COMPLETION_TRACKING_SWITCH.is_enabled(): - try: - with transaction.atomic(): - obj, is_new = self.get_or_create( # pylint: disable=unpacking-non-sequence - user=user, - context_key=context_key, - block_key=block_key, - defaults={ - 'completion': completion, - 'block_type': block_type, - }, - ) - except IntegrityError: - # The completion was created concurrently by another process - log.info( - "An IntegrityError was raised when trying to create a BlockCompletion for %s:%s:%s. " - "Falling back to get().", - user, - context_key, - block_key, - ) - obj = self.get( + try: + with transaction.atomic(): + obj, is_new = self.get_or_create( # pylint: disable=unpacking-non-sequence user=user, context_key=context_key, block_key=block_key, + defaults={ + 'completion': completion, + 'block_type': block_type, + }, ) - is_new = False - - if not is_new and obj.completion != completion: - obj.completion = completion - obj.full_clean() - obj.save(update_fields={'completion', 'modified'}) - - obj.emit_tracking_log() - else: - # If the feature is not enabled, this method should not be called. - # Error out with a RuntimeError. - raise RuntimeError( - "BlockCompletion.objects.submit_completion should not be \ - called when the feature is disabled." + except IntegrityError: + # The completion was created concurrently by another process + log.info( + "An IntegrityError was raised when trying to create a BlockCompletion for %s:%s:%s. " + "Falling back to get().", + user, + context_key, + block_key, ) + obj = self.get( + user=user, + context_key=context_key, + block_key=block_key, + ) + is_new = False + + if not is_new and obj.completion != completion: + obj.completion = completion + obj.full_clean() + obj.save(update_fields={'completion', 'modified'}) + + obj.emit_tracking_log() return obj, is_new diff --git a/completion/services.py b/completion/services.py index 14174e28..886512f0 100644 --- a/completion/services.py +++ b/completion/services.py @@ -7,7 +7,6 @@ from xblock.completable import XBlockCompletionMode from .models import BlockCompletion -from . import waffle User = auth.get_user_model() @@ -31,13 +30,11 @@ def __init__(self, user, context_key): def completion_tracking_enabled(self): """ - Exposes ENABLE_COMPLETION_TRACKING waffle switch to XModule runtime - Return value: bool -> True if completion tracking is enabled. """ - return waffle.ENABLE_COMPLETION_TRACKING_SWITCH.is_enabled() + return True def get_completions(self, candidates): """ diff --git a/completion/test_utils.py b/completion/test_utils.py index 0f9a5ef4..496bdd39 100644 --- a/completion/test_utils.py +++ b/completion/test_utils.py @@ -3,12 +3,10 @@ """ -from contextlib import contextmanager from datetime import datetime from django.contrib import auth from django.test.utils import override_settings -from edx_toggles.toggles.testutils import override_waffle_switch from eventtracking import tracker from django.test import TestCase from eventtracking.django import DjangoTracker @@ -17,7 +15,6 @@ from opaque_keys.edx.keys import UsageKey from pytz import UTC -from . import waffle from .models import BlockCompletion User = auth.get_user_model() @@ -60,45 +57,11 @@ class Meta: date_joined = datetime(2011, 1, 1, tzinfo=UTC) -class CompletionWaffleTestMixin: - """ - Mixin to provide waffle switch overriding ability to child TestCase classes. - """ - - def override_waffle_switch(self, override): - """ - Override the setting of the ENABLE_COMPLETION_TRACKING waffle switch - for the course of the test. - Parameters: - override (bool): True if tracking should be enabled. - """ - _waffle_overrider = override_waffle_switch( - waffle.ENABLE_COMPLETION_TRACKING_SWITCH, override - ) - _waffle_overrider.__enter__() # pylint: disable=unnecessary-dunder-call - self.addCleanup(_waffle_overrider.__exit__, None, None, None) - - class CompletionSetUpMixin: """ Mixin to provide set_up_completion() function to child TestCase classes. """ - COMPLETION_SWITCH_ENABLED = False - - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.waffle_patcher = override_waffle_switch( - waffle.ENABLE_COMPLETION_TRACKING_SWITCH, cls.COMPLETION_SWITCH_ENABLED - ) - cls.waffle_patcher.__enter__() # pylint: disable=unnecessary-dunder-call - - @classmethod - def tearDownClass(cls): - super().tearDownClass() - cls.waffle_patcher.__exit__(None, None, None) - def setUp(self): super().setUp() self.block_key = UsageKey.from_string('block-v1:edx+test+run+type@video+block@doggos') @@ -117,14 +80,6 @@ def set_up_completion(self): completion=0.5, ) - @contextmanager - def override_completion_switch(self, enabled): - """ - Overrides the completion-enabled waffle switch value within a context. - """ - with override_waffle_switch(waffle.ENABLE_COMPLETION_TRACKING_SWITCH, enabled): - yield - IN_MEMORY_BACKEND_CONFIG = { 'mem': { diff --git a/completion/tests/test_models.py b/completion/tests/test_models.py index 69f292d2..a892f083 100644 --- a/completion/tests/test_models.py +++ b/completion/tests/test_models.py @@ -36,7 +36,6 @@ class SubmitCompletionTestCase(CompletionSetUpMixin, EventTrackingTestCase, Test Test that BlockCompletion.objects.submit_completion has the desired semantics. """ - COMPLETION_SWITCH_ENABLED = True def setUp(self): super().setUp() @@ -118,38 +117,11 @@ def test_submit_batch_completion_with_same_block_new_completion_value(self): self.assertEqual(model.completion, 1.0) -class CompletionDisabledTestCase(CompletionSetUpMixin, TestCase): - """ - Test that completion is not track when the feature switch is disabled. - """ - COMPLETION_SWITCH_ENABLED = False - - def setUp(self): - super().setUp() - self.set_up_completion() - - def test_cannot_call_submit_completion(self): - self.assertEqual(models.BlockCompletion.objects.count(), 1) - with self.assertRaises(RuntimeError): - models.BlockCompletion.objects.submit_completion( - user=self.user, - block_key=self.block_key, - completion=0.9, - ) - self.assertEqual(models.BlockCompletion.objects.count(), 1) - - def test_submit_batch_completion_without_waffle(self): - with self.assertRaises(RuntimeError): - blocks = [(self.block_key, 1.0)] - models.BlockCompletion.objects.submit_batch_completion(self.user, blocks) - - class CompletionFetchingTestCase(CompletionSetUpMixin, TestCase): """ Test model methods: latest completion per course, and all completions per course """ - COMPLETION_SWITCH_ENABLED = True def setUp(self): super().setUp() @@ -223,7 +195,6 @@ class CompletionClearingTestCase(CompletionSetUpMixin, TestCase): """ Tests for clear_learning_context_completion """ - COMPLETION_SWITCH_ENABLED = True BLOCKS_PER_CONTEXT = 3 def setUp(self): diff --git a/completion/tests/test_services.py b/completion/tests/test_services.py index 8eb5d2cd..50691a59 100644 --- a/completion/tests/test_services.py +++ b/completion/tests/test_services.py @@ -22,7 +22,6 @@ class CompletionServiceTestCase(CompletionSetUpMixin, TestCase): """ Test the data returned by the CompletionService. """ - COMPLETION_SWITCH_ENABLED = True def setUp(self): super().setUp() @@ -131,10 +130,8 @@ def test_get_completions_block_keys_missing_run(self): expected_completions = dict(zip(expected_block_keys, [1.0, 0.8, 0.6, 0.0, 0.0])) self.assertEqual(expected_completions, actual_completions) - @ddt.data(True, False) - def test_enabled_honors_waffle_switch(self, enabled): - with self.override_completion_switch(enabled): - self.assertEqual(self.completion_service.completion_tracking_enabled(), enabled) + def test_completion_tracking_always_enabled(self): + self.assertTrue(self.completion_service.completion_tracking_enabled()) @ddt.data( (XBlockCompletionMode.COMPLETABLE, False, False, True), diff --git a/completion/tests/test_utilities.py b/completion/tests/test_utilities.py index 705d9d9e..1f5e6420 100644 --- a/completion/tests/test_utilities.py +++ b/completion/tests/test_utilities.py @@ -17,8 +17,6 @@ class TestCompletionUtilities(CompletionSetUpMixin, TestCase): Tests methods in completion's external-facing API. """ - COMPLETION_SWITCH_ENABLED = True - def setUp(self): super().setUp() self.user = UserFactory.create() diff --git a/completion/waffle.py b/completion/waffle.py deleted file mode 100644 index a2cecf9f..00000000 --- a/completion/waffle.py +++ /dev/null @@ -1,22 +0,0 @@ -""" -This module contains various configuration settings via -waffle switches for the completion app. -""" - - -from edx_toggles.toggles import WaffleSwitch - -# The switch and namespace names variables are preserved for backward compatibility -WAFFLE_NAMESPACE = "completion" -ENABLE_COMPLETION_TRACKING = "enable_completion_tracking" # pylint: disable=annotation-missing-token -# .. toggle_name: completion.enable_completion_tracking -# .. toggle_implementation: WaffleSwitch -# .. toggle_default: False -# .. toggle_description: Indicates whether or not to track completion of individual blocks. Keeping this disabled -# will prevent creation of BlockCompletion objects in the database, as well as preventing completion-related -# network access by certain xblocks. -# .. toggle_use_cases: open_edx -ENABLE_COMPLETION_TRACKING_SWITCH = WaffleSwitch( - f"{WAFFLE_NAMESPACE}.{ENABLE_COMPLETION_TRACKING}", - module_name=__name__, -)