Merge pull request #418 from openedx/asheehan-edx/reverting-problems

Asheehan edx/reverting problems

enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py

@@ -41,11 +41,10 @@
 from enterprise_access.apps.subsidy_access_policy.tests.factories import (
     AssignedLearnerCreditAccessPolicyFactory,
     PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory,
-    PerLearnerSpendCapLearnerCreditAccessPolicyFactory,
-    PolicyGroupAssociationFactory
+    PerLearnerSpendCapLearnerCreditAccessPolicyFactory
 )
 from enterprise_access.apps.subsidy_access_policy.utils import create_idempotency_key_for_transaction
-from test_utils import TEST_ENTERPRISE_GROUP_UUID, TEST_USER_RECORD, APITestWithMocks
+from test_utils import TEST_USER_RECORD, APITestWithMocks
 
 SUBSIDY_ACCESS_POLICY_LIST_ENDPOINT = reverse('api:v1:subsidy-access-policies-list')
 
@@ -1067,16 +1066,11 @@ def setup_mocks(self):
         self.addCleanup(contains_key_patcher.stop)
         self.addCleanup(get_content_metadata_patcher.stop)
 
-    @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient')
     @mock.patch('enterprise_access.apps.subsidy_access_policy.models.get_and_cache_transactions_for_learner')
-    def test_redeem_policy(self, mock_transactions_cache_for_learner, mock_oauth):  # pylint: disable=unused-argument
+    def test_redeem_policy(self, mock_transactions_cache_for_learner):  # pylint: disable=unused-argument
         """
         Verify that SubsidyAccessPolicyRedeemViewset redeem endpoint works as expected
         """
-        PolicyGroupAssociationFactory(
-            enterprise_group_uuid=TEST_ENTERPRISE_GROUP_UUID,
-            subsidy_access_policy=self.redeemable_policy
-        )
         self.mock_get_content_metadata.return_value = {'content_price': 123}
         mock_transaction_record = {
             'uuid': str(uuid4()),
@@ -1622,12 +1616,6 @@ def setUp(self):
         )
         self.non_redeemable_policy = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory()
 
-        group_uuid = uuid4()
-        PolicyGroupAssociationFactory(
-            enterprise_group_uuid=group_uuid,
-            subsidy_access_policy=self.redeemable_policy
-        )
-
     def test_can_redeem_policy_missing_params(self):
         """
         Test that the can_redeem endpoint returns an access policy when one is redeemable.

enterprise_access/apps/subsidy_access_policy/constants.py

@@ -105,7 +105,6 @@ class MissingSubsidyAccessReasonUserMessages:
 REASON_SUBSIDY_EXPIRED = "subsidy_expired"
 REASON_CONTENT_NOT_IN_CATALOG = "content_not_in_catalog"
 REASON_LEARNER_NOT_IN_ENTERPRISE = "learner_not_in_enterprise"
-REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP = "learner_not_in_enterprise_group"
 REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY = "not_enough_value_in_subsidy"
 REASON_LEARNER_MAX_SPEND_REACHED = "learner_max_spend_reached"
 REASON_POLICY_SPEND_LIMIT_REACHED = "policy_spend_limit_reached"

enterprise_access/apps/subsidy_access_policy/models.py

@@ -30,7 +30,6 @@
     REASON_LEARNER_MAX_SPEND_REACHED,
     REASON_LEARNER_NOT_ASSIGNED_CONTENT,
     REASON_LEARNER_NOT_IN_ENTERPRISE,
-    REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP,
     REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY,
     REASON_POLICY_EXPIRED,
     REASON_POLICY_SPEND_LIMIT_REACHED,
@@ -592,29 +591,10 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False):
 
         # learner not associated to enterprise
         if not skip_customer_user_check:
-            learner_record = self.lms_api_client.get_enterprise_user(self.enterprise_customer_uuid, lms_user_id)
-            if not learner_record:
+            if self.lms_api_client.get_enterprise_user(self.enterprise_customer_uuid, lms_user_id) is None:
                 self._log_redeemability(False, REASON_LEARNER_NOT_IN_ENTERPRISE, lms_user_id, content_key)
                 return (False, REASON_LEARNER_NOT_IN_ENTERPRISE, [])
 
-            associated_group_uuids = set(  # pylint: disable=consider-using-set-comprehension
-                [str(group["uuid"]) for group in learner_record['user']['enterprise_group']]
-            )
-
-            policy_groups = PolicyGroupAssociation.objects.filter(subsidy_access_policy=self).all()
-            policy_groups_uuids = set(  # pylint: disable=consider-using-set-comprehension
-                [str(group.enterprise_group_uuid) for group in policy_groups]
-            )
-
-            # if there are groups associated with the policy, if not we want to be backwards compatible with
-            # policies which haven't implemented groups yet
-            if len(policy_groups) > 0:
-                uuid_overlap = associated_group_uuids.intersection(policy_groups_uuids)
-                # if any of the user's associated groups are connected to the policy
-                if len(uuid_overlap) == 0:
-                    self._log_redeemability(False, REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP, lms_user_id, content_key)
-                    return (False, REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP, [])
-
         # no content key in catalog
         if not self.catalog_contains_content_key(content_key):
             self._log_redeemability(False, REASON_CONTENT_NOT_IN_CATALOG, lms_user_id, content_key)

enterprise_access/apps/subsidy_access_policy/tests/test_models.py

@@ -28,7 +28,6 @@
     REASON_LEARNER_MAX_SPEND_REACHED,
     REASON_LEARNER_NOT_ASSIGNED_CONTENT,
     REASON_LEARNER_NOT_IN_ENTERPRISE,
-    REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP,
     REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY,
     REASON_POLICY_EXPIRED,
     REASON_POLICY_SPEND_LIMIT_REACHED,
@@ -50,7 +49,7 @@
     PolicyGroupAssociationFactory
 )
 from enterprise_access.cache_utils import request_cache
-from test_utils import TEST_ENTERPRISE_GROUP_UUID, TEST_USER_RECORD, TEST_USER_RECORD_NO_GROUPS
+from test_utils import TEST_USER_RECORD
 
 from ..constants import AccessMethods
 from ..exceptions import PriceValidationError
@@ -205,9 +204,9 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args):
 
     @ddt.data(
         {
-            # Happy path: content in catalog, learner in enterprise, learner in group,
-            # subsidy has value, existing transactions for learner and policy below
-            # the policy limits. Expected can_redeem result: True
+            # Happy path: content in catalog, learner in enterprise, subsidy has value,
+            # existing transactions for learner and policy below the policy limits.
+            # Expected can_redeem result: True
             'policy_active_type': 'active',
             'catalog_contains_content': True,
             'get_enterprise_user': TEST_USER_RECORD,
@@ -242,19 +241,6 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args):
             'expect_content_metadata_fetch': False,
             'expect_transaction_fetch': False,
         },
-        {
-            # Learner is not in the enterprise group, every other check would succeed.
-            # Expected can_redeem result: False
-            'policy_active_type': 'active',
-            'catalog_contains_content': True,
-            'get_enterprise_user': TEST_USER_RECORD_NO_GROUPS,
-            'subsidy_is_redeemable': {'can_redeem': True, 'active': True},
-            'transactions_for_learner': {'transactions': [], 'aggregates': {}},
-            'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}},
-            'expected_policy_can_redeem': (False, REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP, []),
-            'expect_content_metadata_fetch': False,
-            'expect_transaction_fetch': False,
-        },
         {
             # The subsidy is not redeemable, every other check would succeed.
             # Expected can_redeem result: False
@@ -375,11 +361,6 @@ def test_learner_enrollment_cap_policy_can_redeem(
         elif policy_active_type == 'non_redeemable':
             policy_record = self.active_non_redeemable_per_learner_enroll_policy
 
-        PolicyGroupAssociationFactory(
-            enterprise_group_uuid=TEST_ENTERPRISE_GROUP_UUID,
-            subsidy_access_policy=policy_record
-        )
-
         can_redeem_result = policy_record.can_redeem(self.lms_user_id, self.course_id)
 
         self.assertEqual(can_redeem_result, expected_policy_can_redeem, [])
@@ -438,19 +419,6 @@ def test_learner_enrollment_cap_policy_can_redeem(
             'expect_content_metadata_fetch': False,
             'expect_transaction_fetch': False,
         },
-        {
-            # Learner is not in the enterprise group, every other check would succeed.
-            # Expected can_redeem result: False
-            'policy_is_active': True,
-            'catalog_contains_content': True,
-            'get_enterprise_user': TEST_USER_RECORD_NO_GROUPS,
-            'subsidy_is_redeemable': {'can_redeem': True, 'active': True},
-            'transactions_for_learner': {'transactions': [], 'aggregates': {}},
-            'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}},
-            'expected_policy_can_redeem': (False, REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP, []),
-            'expect_content_metadata_fetch': False,
-            'expect_transaction_fetch': False,
-        },
         {
             # The subsidy is not redeemable, every other check would succeed.
             # Expected can_redeem result: False
@@ -556,11 +524,6 @@ def test_learner_spend_cap_policy_can_redeem(
         if policy_is_active:
             policy_record = self.per_learner_spend_policy
 
-        PolicyGroupAssociationFactory(
-            enterprise_group_uuid=TEST_ENTERPRISE_GROUP_UUID,
-            subsidy_access_policy=policy_record
-        )
-
         can_redeem_result = policy_record.can_redeem(self.lms_user_id, self.course_id)
 
         self.assertEqual(can_redeem_result, expected_policy_can_redeem)