Merge pull request #294 from NYPL/HOTFIX_ItemMediaTypeSorting

HOTFIX_ItemSorting
NYPL · Feb 23, 2024 · 6d76454 · 6d76454
2 parents d752486 + 59ce63b
commit 6d76454
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@
 - NYPL records not being added due to SQLAlchemy error
 - Bardo CCE API and Hathi DataFiles URL updated
 - Deleted Tugboat configuration as Tugboat is no longer used and no longer builds
+- Adjusted sorting for item source and media type priority
 
 ## 2023-09-05 version -- v0.12.3
 ## Removed

diff --git a/api/utils.py b/api/utils.py
@@ -366,7 +366,7 @@ def formatEdition(
             editionDict['items'].append(itemDict)
 
         editionDict['items']\
-            .sort(key=lambda x: cls.SOURCE_PRIORITY[x['source']])
+            .sort(key=lambda x: (cls.SOURCE_PRIORITY[x['source']], cls.sortByMediaType(x['links'][0])))
 
         if records is not None:
             itemsByLink = {}
@@ -391,12 +391,12 @@ def formatEdition(
     def sortByMediaType(link):
         scores = {
             'application/webpub+json': 1,
-            'text/html': 2,
-            'application/pdf': 3,
-            'application/html+edd': 4,
-            'application/x.html+edd': 4,
-            'application/epub+xml': 5,
-            'application/epub+zip': 5,
+            'application/pdf': 2,
+            'application/epub+xml': 3,
+            'application/epub+zip': 3,
+            'text/html': 4,
+            'application/html+edd': 5,
+            'application/x.html+edd': 5,
             'application/html+catalog': 6
         }
 

diff --git a/tests/unit/test_api_utils.py b/tests/unit/test_api_utils.py
@@ -101,6 +101,15 @@ def testWebpubLink(self, MockDBObject):
             url='testURI',
             flags={'test': True}
         )
+
+    @pytest.fixture
+    def testPDFLink(self, MockDBObject):
+        return MockDBObject(
+            id='li2',
+            media_type='application/pdf',
+            url='testURI',
+            flags={'test': True}
+        )
 
     @pytest.fixture
     def testRights(self, MockDBObject):
@@ -130,6 +139,16 @@ def testWebpubItem(self, MockDBObject, testWebpubLink):
             physical_location={},
             source='gutenberg'
         )
+
+    @pytest.fixture
+    def testPDFItem(self, MockDBObject, testPDFLink):
+        return MockDBObject(
+            id='it3',
+            links=[testPDFLink],
+            rights=[],
+            physical_location={},
+            source='gutenberg'
+        )
 
     @pytest.fixture
     def testEdition(self, MockDBObject, testItem, mocker):
@@ -491,16 +510,19 @@ def test_formatEdition_v1_reader_flag(self, testEdition, testWebpubItem):
         assert formattedEdition['items'][1]['links'][0]['mediaType'] ==\
             'application/epub+xml'
 
-    def test_formatEdition_v2_reader_flag(self, testEdition, testWebpubItem):
+    def test_formatEdition_v2_reader_flag(self, testEdition, testWebpubItem, testPDFItem):
+        testEdition.items.append(testPDFItem)
         testEdition.items.append(testWebpubItem)
 
         formattedEdition = APIUtils.formatEdition(testEdition, reader='v2')
 
-        assert len(formattedEdition['items']) == 2
+        assert len(formattedEdition['items']) == 3
         assert formattedEdition['items'][0]['item_id'] == 'it2'
         assert formattedEdition['items'][0]['links'][0]['mediaType'] ==\
             'application/webpub+json'
-        assert formattedEdition['items'][1]['links'][0]['flags']['reader'] is\
+        assert formattedEdition['items'][1]['links'][0]['mediaType'] ==\
+            'application/pdf'
+        assert formattedEdition['items'][2]['links'][0]['flags']['reader'] is\
             False
 
     def test_formatRecord(self, testRecord, mocker):
@@ -646,21 +668,21 @@ def test_addWorkMeta(self):
 
     def test_sortByMediaType(self):
         testList = [
-            {'id': 2, 'mediaType': 'text/html'},
-            {'id': 1, 'mediaType': 'application/epub+xml'},
-            {'id': 4, 'mediaType': 'application/html+edd'},
-            {'id': 1, 'mediaType': 'application/epub+zip'},
-            {'id': 5, 'mediaType': 'application/webpub+json'},
-            {'id': 3, 'mediaType': 'application/pdf'}
+            {'id': 4, 'mediaType': 'text/html'},
+            {'id': 3, 'mediaType': 'application/epub+xml'},
+            {'id': 5, 'mediaType': 'application/html+edd'},
+            {'id': 3, 'mediaType': 'application/epub+zip'},
+            {'id': 1, 'mediaType': 'application/webpub+json'},
+            {'id': 2, 'mediaType': 'application/pdf'}
         ]
 
         shuffle(testList)
         testList.sort(key=APIUtils.sortByMediaType)
-        assert [i['id'] for i in testList] == [5, 2, 3, 4, 1, 1]
+        assert [i['id'] for i in testList] == [1, 2, 3, 3, 4, 5]
 
         shuffle(testList)
         testList.sort(key=APIUtils.sortByMediaType)
-        assert [i['id'] for i in testList] == [5, 2, 3, 4, 1, 1]
+        assert [i['id'] for i in testList] == [1, 2, 3, 3, 4, 5]
 
     def test_getPresignedUrlFromObjectUrl(self, mocker):
         mockGenerateUrl = mocker.patch.object(APIUtils, 'generate_presigned_url')