Allow runs with multiple prices to still be considered identical (#2563)

* improve handling for multiple prices

* update python version of identical runs func, too

* fix log typo

Author: ChristopherChudzicki

frontends/ol-utilities/src/learning-resources/learning-resources.test.ts

@@ -179,6 +179,84 @@ describe("allRunsAreIdentical", () => {
     expect(allRunsAreIdentical(resource)).toBe(true)
   })
 
+  test.each([
+    {
+      runPrices: [
+        [
+          { amount: "0", currency: "USD" },
+          { amount: "50", currency: "USD" },
+          { amount: "100", currency: "USD" },
+        ],
+        [
+          { amount: "0", currency: "USD" },
+          { amount: "50.00", currency: "USD" },
+          { amount: "100", currency: "USD" },
+        ],
+      ],
+      case: "2 runs, 3 prices each, all the same",
+      expected: true,
+    },
+    {
+      runPrices: [
+        [
+          { amount: "0", currency: "USD" },
+          { amount: "100", currency: "USD" },
+        ],
+        [
+          { amount: "0", currency: "USD" },
+          { amount: "50.00", currency: "USD" },
+          { amount: "100", currency: "USD" },
+        ],
+      ],
+      case: "2 runs, 2 vs 3 prices",
+      expected: false,
+    },
+    {
+      runPrices: [
+        [
+          { amount: "0", currency: "USD" },
+          { amount: "50.00", currency: "USD" },
+          { amount: "150", currency: "USD" },
+        ],
+        [
+          { amount: "0", currency: "USD" },
+          { amount: "50.00", currency: "USD" },
+          { amount: "100", currency: "USD" },
+        ],
+      ],
+      case: "2 runs, 3 prices each, some not all same",
+      expected: false,
+    },
+  ])(
+    "returns $expected if multiple runs ($case)",
+    ({ expected, runPrices: [prices1, prices2] }) => {
+      const resource = factories.learningResources.resource({
+        free: true,
+        certification: true,
+      })
+      const delivery = [
+        {
+          code: CourseResourceDeliveryInnerCodeEnum.InPerson,
+          name: "In person",
+        },
+      ]
+      const location = "New York"
+      resource.runs = [
+        makeRun({
+          resource_prices: prices1,
+          delivery: delivery,
+          location: location,
+        }),
+        makeRun({
+          resource_prices: prices2,
+          delivery: delivery,
+          location: location,
+        }),
+      ]
+      expect(allRunsAreIdentical(resource)).toBe(expected)
+    },
+  )
+
   test("returns false if delivery methods differ", () => {
     const resource = factories.learningResources.resource()
     const prices = [{ amount: "100", currency: "USD" }]

frontends/ol-utilities/src/learning-resources/learning-resources.ts

@@ -129,16 +129,15 @@ const allRunsAreIdentical = (resource: LearningResource) => {
   if (resource.runs.length <= 1) {
     return true
   }
-  const amounts = new Set<number>()
-  const currencies = new Set<string>()
+  const prices = new Set<string>()
   const deliveryMethods = new Set<string>()
   const locations = new Set<string>()
   for (const run of resource.runs) {
     if (run.resource_prices) {
-      run.resource_prices.forEach((price) => {
-        amounts.add(Number(price.amount))
-        currencies.add(price.currency)
-      })
+      const serialized = run.resource_prices
+        .map((price) => `${Number(price.amount).toFixed(2)}-${price.currency}`)
+        .join(":::")
+      prices.add(serialized)
     }
     if (run.delivery) {
       for (const dm of run.delivery) {
@@ -149,13 +148,11 @@ const allRunsAreIdentical = (resource: LearningResource) => {
       locations.add(run.location)
     }
   }
-  const expectedPrices = resource.free && resource.certification ? 2 : 1
   const hasInPerson = [...deliveryMethods].some(
     (dm) => dm === DeliveryEnum.InPerson,
   )
   return (
-    amounts.size === expectedPrices &&
-    currencies.size === 1 &&
+    prices.size <= 1 &&
     deliveryMethods.size === 1 &&
     (hasInPerson ? locations.size === 1 : locations.size === 0)
   )

learning_resources/management/commands/backpopulate_mitxonline_data.py

@@ -28,7 +28,7 @@ def handle(self, *args, **options):  # noqa: ARG002
         """Run Populate mitxonline courses"""
         if options["delete"]:
             self.stdout.write(
-                "Deleting all existing xPro courses from database and opensearch"
+                "Deleting all existing MITxOnline courses from database and opensearch"
             )
             for learning_resource in LearningResource.objects.filter(
                 etl_source=ETLSource.mitxonline.value

learning_resources/serializers.py

@@ -492,35 +492,27 @@ class LearningResourceMetadataDisplaySerializer(serializers.Serializer):
 
     def all_runs_are_identical(self, serialized_resource):
         """Check if all runs are identical"""
-        distinct_prices = set()
+        distinct_resource_prices = set()
         distinct_delivery_methods = set()
         resource_delivery = [
             delivery["code"] for delivery in serialized_resource["delivery"]
         ]
         has_in_person = (
             "in_person" in resource_delivery or "hybrid" in resource_delivery
         )
-        distinct_currencies = {
-            c["currency"] for c in serialized_resource["resource_prices"]
-        }
 
         distinct_locations = set()
         for run in serialized_resource.get("runs", []):
-            prices = run.get("prices", [])
-            if prices:
-                distinct_prices.update(prices)
+            resource_prices = run.get("resource_prices", [])
+            distinct_resource_prices.add(
+                tuple((price["currency"], price["amount"]) for price in resource_prices)
+            )
             distinct_delivery_methods.update(
                 [delivery["code"] for delivery in run.get("delivery", [])]
             )
             if run.get("location"):
                 distinct_locations.add(run["location"])
-        if (
-            serialized_resource.get("free")
-            and serialized_resource["certification"]
-            and len(distinct_prices) != 2  # noqa: PLR2004
-        ):
-            return False
-        if len(distinct_currencies) != 1:
+        if len(distinct_resource_prices) > 1:
             return False
         if len(distinct_delivery_methods) != 1:
             return False

learning_resources/serializers_test.py

@@ -665,7 +665,7 @@ def _create_identical_runs_resource():
         delivery=["in_person"],
     )
     learning_resource.resource_prices.set(
-        [LearningResourcePriceFactory.create(amount=Decimal("0.00"), currency="USD")]
+        [LearningResourcePriceFactory.create(amount=Decimal("1.00"), currency="USD")]
     )
     start_date = datetime.datetime.now(tz=datetime.UTC)
     run = LearningResourceRunFactory.create(
@@ -692,7 +692,6 @@ def _create_identical_runs_resource():
             LearningResourcePriceFactory.create(amount=Decimal("1.00"), currency="USD")
         ],
     )
-    run.prices = [Decimal("1.00"), Decimal("2.00")]
     run.save()
     return learning_resource
 
@@ -862,21 +861,32 @@ def test_metadata_display_serializer_all_runs_are_identical():
     assert (
         display_data_serializer.all_runs_are_identical(differing_runs_resource) is False
     )
-    # Test that all_runs_are_identical will return False if there is more than 1 unique currency
+    # Test that all_runs_are_identical will return False if not all runs have same prices
     differing_runs_resource = copy.deepcopy(identical_runs_resource)
-    differing_runs_resource["resource_prices"] = [
-        {"currency": "USD", "amount": Decimal("1.00")},
-        {"currency": "BTC", "amount": Decimal("1.00")},
+    differing_runs_resource["runs"][0]["resource_prices"] = [
+        {"amount": "10.00", "currency": "USD"},
+        {"amount": "20.00", "currency": "BTC"},
+    ]
+    differing_runs_resource["runs"][1]["resource_prices"] = [
+        {"amount": "10.00", "currency": "USD"},
+        {"amount": "21.00", "currency": "BTC"},
     ]
     assert (
         display_data_serializer.all_runs_are_identical(differing_runs_resource) is False
     )
 
-    # Test that all_runs_are_identical will return False if there is more than 1 unique price
+    # Test that all_runs_are_identical will return True if all runs DO have same prices
     differing_runs_resource = copy.deepcopy(identical_runs_resource)
-    differing_runs_resource["runs"][0]["prices"] = [Decimal(i) for i in range(5)]
+    differing_runs_resource["runs"][0]["resource_prices"] = [
+        {"amount": "10.00", "currency": "USD"},
+        {"amount": "20.00", "currency": "BTC"},
+    ]
+    differing_runs_resource["runs"][1]["resource_prices"] = [
+        {"amount": "10.00", "currency": "USD"},
+        {"amount": "20.00", "currency": "BTC"},
+    ]
     assert (
-        display_data_serializer.all_runs_are_identical(differing_runs_resource) is False
+        display_data_serializer.all_runs_are_identical(differing_runs_resource) is True
     )
     """
     Test that all_runs_are_identical will return False if there is more than 1 unique location