Merge pull request #18756 from edx/colelrogers/LEARNER-6075-pathway-caching-pages

Fix pathway caching to work with multiple pages returned from endpoint

common/test/acceptance/fixtures/catalog.py

@@ -49,7 +49,7 @@ class CatalogFixture(object):
         """
         requests.put(
             '{}/set_config'.format(CATALOG_STUB_URL),
-            data={'catalog.credit_pathways': json.dumps({'results': credit_pathways})}
+            data={'catalog.credit_pathways': json.dumps({'results': credit_pathways, 'next': None})}
         )
 
     def install_program_types(self, program_types):

openedx/core/djangoapps/catalog/management/commands/cache_programs.py

@@ -138,11 +138,16 @@ class Command(BaseCommand):
         """
         Get all pathways for the current client
         """
-        pathways = {}
+        pathways = []
         failure = False
         logger.info('Requesting credit pathways for {domain}.'.format(domain=site.domain))
         try:
-            pathways = client.credit_pathways.get(exclude_utm=1)['results']
+            next_page = 1
+            while next_page:
+                new_pathways = client.credit_pathways.get(exclude_utm=1, page=next_page)
+                pathways.extend(new_pathways['results'])
+                next_page = next_page + 1 if new_pathways['next'] else None
+
         except:  # pylint: disable=bare-except
             logger.error('Failed to retrieve credit pathways for site: {domain}.'.format(domain=site.domain))
             failure = True

openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py

@@ -86,7 +86,7 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
             content_type='application/json'
         )
 
-    def mock_pathways(self):
+    def mock_pathways(self, pathways, page_number=1, final=True):
         """
         Mock the data for discovery's credit pathways endpoint
         """
@@ -94,18 +94,31 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
             """
             Mocks response
             """
+
             expected = {
                 'exclude_utm': ['1'],
+                'page': [str(page_number)],
             }
             self.assertEqual(request.querystring, expected)
 
-            return (200, headers, json.dumps({'results': self.pathways}))
+            body = {
+                'count': len(pathways),
+                'next': None if final else 'more',  # we don't actually parse this value
+                'prev': None,
+                'results': pathways
+            }
 
+            return (200, headers, json.dumps(body))
+
+        # NOTE: httpretty does not properly match against query strings here (using match_querystring arg)
+        # as such, it does not actually look at the query parameters (for page num), but returns items in a LIFO order.
+        # this means that for multiple pages, you must call this function starting from the last page.
+        # we do assert the page number query param above, however
         httpretty.register_uri(
             httpretty.GET,
             self.pathway_url,
             body=pathways_callback,
-            content_type='application/json'
+            content_type='application/json',
         )
 
     def test_handle_programs(self):
@@ -123,7 +136,7 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
         }
 
         self.mock_list()
-        self.mock_pathways()
+        self.mock_pathways(self.pathways)
 
         for uuid in self.uuids:
             program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)]
@@ -170,7 +183,7 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
         }
 
         self.mock_list()
-        self.mock_pathways()
+        self.mock_pathways(self.pathways)
 
         for uuid in self.uuids:
             program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)]
@@ -202,6 +215,60 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase, SiteMix
 
             self.assertEqual(pathway, pathways[key])
 
+    def test_pathways_multiple_pages(self):
+        """
+        Verify that the command properly caches credit pathways when multiple pages are returned from its endpoint
+        """
+        UserFactory(username=self.catalog_integration.service_username)
+        new_pathways = CreditPathwayFactory.create_batch(40)
+        for new_pathway in new_pathways:
+            new_pathway['programs'] = []
+        pathways = self.pathways + new_pathways
+
+        programs = {
+            PROGRAM_CACHE_KEY_TPL.format(uuid=program['uuid']): program for program in self.programs
+        }
+
+        self.mock_list()
+        for uuid in self.uuids:
+            program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)]
+            self.mock_detail(uuid, program)
+
+        # mock 3 pages of credit pathways, starting at the last
+        self.mock_pathways(pathways[40:], page_number=3, final=True)
+        self.mock_pathways(pathways[20:40], page_number=2, final=False)
+        self.mock_pathways(pathways[:20], page_number=1, final=False)
+
+        call_command('cache_programs')
+
+        pathways_dict = {
+            CREDIT_PATHWAY_CACHE_KEY_TPL.format(id=pathway['id']): pathway for pathway in pathways
+        }
+        pathway_keys = pathways_dict.keys()
+
+        cached_pathway_keys = cache.get(SITE_CREDIT_PATHWAY_IDS_CACHE_KEY_TPL.format(domain=self.site_domain))
+        self.assertEqual(
+            set(cached_pathway_keys),
+            set(pathway_keys)
+        )
+
+        cached_pathways = cache.get_many(pathway_keys)
+        self.assertEqual(
+            set(cached_pathways),
+            set(pathways_dict)
+        )
+
+        # We can't use a set comparison here because these values are dictionaries
+        # and aren't hashable. We've already verified that all pathways came out
+        # of the cache above, so all we need to do here is verify the accuracy of
+        # the data itself.
+        for key, pathway in cached_pathways.items():
+            # cached pathways store just program uuids instead of the full programs, transform before comparing
+            pathways_dict[key]['program_uuids'] = [program['uuid'] for program in pathways_dict[key]['programs']]
+            del pathways_dict[key]['programs']
+
+            self.assertEqual(pathway, pathways_dict[key])
+
     def test_handle_missing_service_user(self):
         """
         Verify that the command raises an exception when run without a service