feat: Updates salesforce opportunity line item form validation (#502)

* feat: Updates salesforce opportunity line item form validation

* feat: Updates salesforce opportunity line item form validation

* feat: Updates salesforce opportunity line item form validation

license_manager/apps/subscriptions/forms.py

@@ -26,7 +26,10 @@
     SubscriptionPlan,
     SubscriptionPlanRenewal,
 )
-from license_manager.apps.subscriptions.utils import localized_utcnow
+from license_manager.apps.subscriptions.utils import (
+    localized_utcnow,
+    verify_sf_opportunity_product_line_item,
+)
 
 
 logger = logging.getLogger(__name__)
@@ -58,6 +61,14 @@ class SubscriptionPlanForm(forms.ModelForm):
         label="Reason for change",
     )
 
+    # Override the salesforce_opportunity_line_item help text to be more specific to the subscription plan
+    salesforce_opportunity_line_item = forms.CharField(
+        help_text=(
+            """18 character value that starts with '00k' --
+            Locate the appropriate Salesforce Opportunity Line Item record and copy it here."""
+        )
+    )
+
     def _validate_enterprise_catalog_uuid(self):
         """
         Verifies that the enterprise customer has a catalog with the given enterprise_catalog_uuid.
@@ -138,11 +149,16 @@ def is_valid(self):
             )
             return False
 
-        if product.plan_type.sf_id_required and self.cleaned_data.get('salesforce_opportunity_line_item') is None:
+        if (
+                product.plan_type.sf_id_required
+                and self.cleaned_data.get('salesforce_opportunity_line_item') is None
+                or not verify_sf_opportunity_product_line_item(self.cleaned_data.get(
+                'salesforce_opportunity_line_item'))
+        ):
             self._log_validation_error('no SF ID')
             self.add_error(
                 'salesforce_opportunity_line_item',
-                'You must specify Salesforce ID for selected product.',
+                'You must specify Salesforce ID for selected product. It must start with \'00k\'.',
             )
             return False
 
@@ -170,6 +186,14 @@ class SubscriptionPlanRenewalForm(forms.ModelForm):
         widget=forms.HiddenInput()
     )
 
+    salesforce_opportunity_id = forms.CharField(
+        help_text=(
+            "Locate the appropriate Salesforce Opportunity record and copy the Opportunity ID field "
+            "(18 characters and begin with '00k')."
+            " Note that this is not the same Salesforce Opportunity ID associated with the linked subscription."
+        )
+    )
+
     def is_valid(self):
         # Perform original validation and return if false
         if not super().is_valid():
@@ -180,6 +204,7 @@ def is_valid(self):
         # subscription renewal expiration date
         form_effective_date = self.cleaned_data.get('effective_date')
         form_renewed_expiration_date = self.cleaned_data.get('renewed_expiration_date')
+        form_future_salesforce_opportunity_line_item = self.cleaned_data.get('salesforce_opportunity_id')
 
         if form_effective_date < localized_utcnow():
             self.add_error(
@@ -203,6 +228,14 @@ def is_valid(self):
             )
             return False
 
+        if form_future_salesforce_opportunity_line_item is None or \
+                not verify_sf_opportunity_product_line_item(form_future_salesforce_opportunity_line_item):
+            self.add_error(
+                'salesforce_opportunity_id',
+                'You must specify Salesforce ID for the renewed product. It must start with \'00k\'.',
+            )
+            return False
+
         return True
 
     class Meta:

license_manager/apps/subscriptions/management/commands/seed_enterprise_devstack_data.py

@@ -108,7 +108,7 @@ def create_subscription_plan(self, customer_agreement, num_licenses=1):
             expiration_date=timestamp + timedelta(days=365),
             is_active=True,
             for_internal_use_only=True,
-            salesforce_opportunity_line_item=123456789123456789,
+            salesforce_opportunity_line_item='00k456789123456789',
             product=Product.objects.get(name="B2B Paid")
         )
         with transaction.atomic():

license_manager/apps/subscriptions/tests/factories.py

@@ -29,10 +29,9 @@
 
 def get_random_salesforce_id():
     """
-    Returns a random alpha-numeric string of the correct length for a salesforce opportunity id.
+    Returns a random alpha-numeric string of the correct length that starts with 00k for a salesforce opportunity line item.
     """
-    return ''.join(random.choice(string.ascii_uppercase + string.ascii_lowercase + string.digits)
-                   for _ in range(SALESFORCE_ID_LENGTH))
+    return '00k' + ''.join(random.choices(string.ascii_uppercase + string.digits, k=SALESFORCE_ID_LENGTH - 3))
 
 
 class CustomerAgreementFactory(factory.django.DjangoModelFactory):

license_manager/apps/subscriptions/tests/test_forms.py

@@ -133,15 +133,26 @@ def test_product_is_required(self):
         invalid_form = make_bound_subscription_form(has_product=False)
         assert invalid_form.is_valid() is False
 
-    def test_salesforce_opportunity_line_item(self):
+    @ddt.data(
+        {'salesforce_id': '00k123456789ABCdef', 'expected_value': True},
+        {'salesforce_id': None, 'expected_value': False},
+        {'salesforce_id': '00p123456789123456', 'expected_value': False},
+        {'salesforce_id': '00K123456789ABCdef', 'expected_value': False},
+        {'salesforce_id': '00K123456789ABCde', 'expected_value': False},
+        {'salesforce_id': '00K123456789ABCdeff', 'expected_value': False},
+        {'salesforce_id': '00K00k456789ABCdef', 'expected_value': False},
+    )
+    @ddt.unpack
+    def test_salesforce_opportunity_line_item(self, salesforce_id, expected_value):
         """
         Verify subscription plan form is invalid if salesforce_opportunity_id is None and the product requires it.
         """
-        invalid_form = make_bound_subscription_form(is_sf_id_required=True, salesforce_opportunity_line_item=None)
-        assert invalid_form.is_valid() is False
+        invalid_form = make_bound_subscription_form(is_sf_id_required=True, salesforce_opportunity_line_item=salesforce_id)
+        assert invalid_form.is_valid() is expected_value
 
 
 @mark.django_db
+@ddt.ddt
 class TestSubscriptionPlanRenewalForm(TestCase):
     """
     Unit tests for the SubscriptionPlanRenewalForm
@@ -182,6 +193,36 @@ def test_invalid_start_date_before_today(self):
         )
         assert not form.is_valid()
 
+    def test_empty_salesforce_opportunity_line_id(self):
+        prior_subscription_plan = SubscriptionPlanFactory.create()
+        form = make_bound_subscription_plan_renewal_form(
+            prior_subscription_plan=prior_subscription_plan,
+            effective_date=localized_utcnow() + timedelta(1),
+            renewed_expiration_date=prior_subscription_plan.expiration_date + timedelta(366),
+            salesforce_opportunity_id=None
+        )
+        assert form.has_error('salesforce_opportunity_id')
+        assert form.errors['salesforce_opportunity_id'] == ['This field is required.']
+        assert not form.is_valid()
+
+    @ddt.data(
+        {'salesforce_id': '00p123456789123456', 'expected_value': False},
+        {'salesforce_id': '00K123456789ABCdef', 'expected_value': False},
+        {'salesforce_id': '00K123456789ABCde', 'expected_value': False},
+        {'salesforce_id': '00K123456789ABCdeff', 'expected_value': False},
+        {'salesforce_id': '00K00k456789ABCdef', 'expected_value': False},
+    )
+    @ddt.unpack
+    def test_incorrect_salesforce_opportunity_line_id_format(self, salesforce_id, expected_value):
+        prior_subscription_plan = SubscriptionPlanFactory.create()
+        form = make_bound_subscription_plan_renewal_form(
+            prior_subscription_plan=prior_subscription_plan,
+            effective_date=localized_utcnow() + timedelta(1),
+            renewed_expiration_date=prior_subscription_plan.expiration_date + timedelta(366),
+            salesforce_opportunity_id=salesforce_id
+        )
+        assert form.is_valid() is expected_value
+
 
 @ddt.ddt
 @mark.django_db

license_manager/apps/subscriptions/utils.py

@@ -1,6 +1,7 @@
 """ Utility functions for the subscriptions app. """
 import hashlib
 import hmac
+import re
 from base64 import b64encode
 from datetime import datetime
 
@@ -125,3 +126,11 @@ def get_subsidy_checksum(lms_user_id, course_key, license_uuid):
 
     digest = hmac.digest(key, message, digest_function)
     return b64encode(digest).decode()
+
+
+def verify_sf_opportunity_product_line_item(salesforce_opportunity_line_item):
+    """
+    Returns boolean value to confirm if the passed salesforce_opportunity_line_item format
+    is correct
+    """
+    return re.search(r'^00k', salesforce_opportunity_line_item)