Merge pull request #186 from ferbncode/fix-reviewed-deleted

CB-269: Handle reviewed entities deleted in MB database

critiquebrainz/db/review.py

@@ -628,3 +628,26 @@ def distinct_entities():
               FROM review
         """))
         return {row[0] for row in results.fetchall()}
+
+
+def reviewed_entities(*, entity_ids, entity_type):
+    """Check if an entity has been reviewed.
+
+    Args:
+        entity_ids: List of ID(s) of the entities.
+        entity_type: Type of the entities.
+    Returns:
+        List of entity ID(s) that have a review in the database.
+    """
+    with db.engine.connect() as connection:
+        results = connection.execute(sqlalchemy.text("""
+            SELECT entity_id
+              FROM review
+             WHERE entity_type = :entity_type
+               AND entity_id IN :entity_ids
+        """), {
+            "entity_type": entity_type,
+            "entity_ids": tuple(entity_ids),
+        })
+        reviewed_ids = [str(row[0]) for row in results.fetchall()]
+    return reviewed_ids

critiquebrainz/db/review_test.py

@@ -269,3 +269,24 @@ def test_distinct_entities(self):
         )
         entities = db_review.distinct_entities()
         self.assertEqual(len(entities), 1)
+
+    def test_reviewed_entities(self):
+        review = db_review.create(
+            user_id=self.user.id,
+            entity_id="e7aad618-fa86-3983-9e77-405e21796eca",
+            entity_type="release_group",
+            text="Awesome",
+            is_draft=False,
+            license_id=self.license["id"],
+        )
+        reviewed_entities = db_review.reviewed_entities(
+            entity_ids=["e7aad618-fa86-3983-9e77-405e21796eca", "deae6fc2-a675-4f35-9565-d2aaea4872c7"],
+            entity_type="release_group",
+        )
+        self.assertListEqual(reviewed_entities, ["e7aad618-fa86-3983-9e77-405e21796eca"])
+        db_review.delete(review["id"])
+        reviewed_entities = db_review.reviewed_entities(
+            entity_ids=["e7aad618-fa86-3983-9e77-405e21796eca"],
+            entity_type="release_group",
+        )
+        self.assertListEqual(reviewed_entities, [])

critiquebrainz/frontend/external/musicbrainz_db/serialize.py

@@ -125,7 +125,7 @@ def to_dict_release_groups(release_group, includes=None):
     if 'artist-credit-phrase' in includes:
         data['artist-credit-phrase'] = includes['artist-credit-phrase']
 
-    if 'meta' in includes and includes['meta'].first_release_date_year:
+    if 'meta' in includes and includes['meta'] and includes['meta'].first_release_date_year:
         data['first-release-year'] = includes['meta'].first_release_date_year
 
     if 'artist-credit-names' in includes:

critiquebrainz/frontend/external/musicbrainz_db/special_entities.py

@@ -0,0 +1,29 @@
+from mbdata import models
+
+# Unknown entities
+unknown_artist = models.Artist()
+unknown_artist.gid = '125ec42a-7229-4250-afc5-e057484327fe'
+unknown_artist.id = 97546
+unknown_artist.name = '[unknown]'
+unknown_artist.sort_name = '[unknown]'
+
+unknown_artist_credit_name = models.ArtistCreditName()
+unknown_artist_credit_name.name = '[unknown]'
+unknown_artist_credit_name.artist = unknown_artist
+
+unknown_artist_credit = models.ArtistCredit()
+unknown_artist_credit.name = '[unknown]'
+unknown_artist_credit.artists = [unknown_artist_credit_name]
+
+unknown_release_group = models.ReleaseGroup()
+unknown_release_group.artist_credit = unknown_artist_credit
+unknown_release_group.id = 0
+unknown_release_group.name = '[Unknown Release Group]'
+
+unknown_place = models.Place()
+unknown_place.id = 0
+unknown_place.name = '[Unknown Place]'
+
+unknown_event = models.Place()
+unknown_event.id = 0
+unknown_event.name = '[Unknown Event]'

critiquebrainz/frontend/external/musicbrainz_db/tests/event_test.py

@@ -3,6 +3,7 @@
 from critiquebrainz.frontend.external.musicbrainz_db import event as mb_event
 from critiquebrainz.frontend.external.musicbrainz_db.test_data import taubertal_festival_2004, event_ra_hall_uk
 from critiquebrainz.frontend.external.musicbrainz_db.tests import setup_cache
+import critiquebrainz.frontend.external.musicbrainz_db.utils as mb_utils
 
 
 class EventTestCase(TestCase):
@@ -30,3 +31,10 @@ def test_fetch_multiple_events(self):
                          'Taubertal-Festival 2004, Day 1')
         self.assertEqual(events['40e6153d-a042-4c95-a0a9-b0a47e3825ce']['name'],
                          '1996-04-17: Royal Albert Hall, London, England, UK')
+
+    def test_unknown_place(self):
+        self.event_query.return_value = []
+        mb_utils.reviewed_entities = MagicMock()
+        mb_utils.reviewed_entities.return_value = ['40e6153d-a042-4c95-a0a9-b0a47e3825df']
+        event = mb_event.get_event_by_id('40e6153d-a042-4c95-a0a9-b0a47e3825df')
+        self.assertEqual(event['name'], '[Unknown Event]')

critiquebrainz/frontend/external/musicbrainz_db/tests/place_test.py

@@ -3,6 +3,7 @@
 from critiquebrainz.frontend.external.musicbrainz_db import place as mb_place
 from critiquebrainz.frontend.external.musicbrainz_db.test_data import place_suisto, place_verkatehdas
 from critiquebrainz.frontend.external.musicbrainz_db.tests import setup_cache
+import critiquebrainz.frontend.external.musicbrainz_db.utils as mb_utils
 
 
 class PlaceTestCase(TestCase):
@@ -32,3 +33,10 @@ def test_fetch_multiple_places(self):
         places = mb_place.fetch_multiple_places(['f9587914-8505-4bd1-833b-16a3100a4948', 'd71ffe38-5eaf-426b-9a2e-e1f21bc84609'])
         self.assertEqual(places['d71ffe38-5eaf-426b-9a2e-e1f21bc84609']['name'], 'Suisto')
         self.assertEqual(places['f9587914-8505-4bd1-833b-16a3100a4948']['name'], 'Verkatehdas')
+
+    def test_unknown_place(self):
+        self.place_query.return_value = []
+        mb_utils.reviewed_entities = MagicMock()
+        mb_utils.reviewed_entities.return_value = ['d71ffe38-5eaf-426b-9a2e-e1f21bc846df']
+        place = mb_place.get_place_by_id('d71ffe38-5eaf-426b-9a2e-e1f21bc846df')
+        self.assertEqual(place['name'], '[Unknown Place]')

critiquebrainz/frontend/external/musicbrainz_db/tests/release_group_test.py

@@ -3,6 +3,7 @@
 from critiquebrainz.frontend.external.musicbrainz_db import release_group as mb_release_group
 from critiquebrainz.frontend.external.musicbrainz_db.test_data import releasegroup_numb_encore, releasegroup_collision_course
 from critiquebrainz.frontend.external.musicbrainz_db.tests import setup_cache
+import critiquebrainz.frontend.external.musicbrainz_db.utils as mb_utils
 
 
 class ReleaseGroupTestCase(TestCase):
@@ -78,3 +79,18 @@ def test_fetch_browse_release_groups(self):
             }
         ])
         self.assertEqual(release_groups[1], 2)
+
+    def test_unknown_release_group(self):
+        self.release_group_query.return_value = []
+        mb_utils.reviewed_entities = MagicMock()
+        mb_utils.reviewed_entities.return_value = ['8ef859e3-feb2-4dd1-93da-22b91280d7df']
+        release_group = mb_release_group.get_release_group_by_id('8ef859e3-feb2-4dd1-93da-22b91280d7df')
+        self.assertEqual(release_group['title'], '[Unknown Release Group]')
+        self.assertListEqual(release_group['artist-credit'], [{
+            'name': '[unknown]',
+            'artist': {
+                'id': '125ec42a-7229-4250-afc5-e057484327fe',
+                'name': '[unknown]',
+                'sort_name': '[unknown]'
+            }
+        }])

critiquebrainz/frontend/external/musicbrainz_db/utils.py

@@ -1,5 +1,7 @@
 from mbdata import models
 import critiquebrainz.frontend.external.musicbrainz_db.exceptions as mb_exceptions
+from critiquebrainz.frontend.external.musicbrainz_db import special_entities
+from critiquebrainz.db.review import reviewed_entities, ENTITY_TYPES as CB_ENTITIES
 
 
 # Entity models
@@ -24,6 +26,25 @@
 }
 
 
+def unknown_entity(entity_gid, entity_type):
+    """Returns special unknown entities (in case, they are not present in MB).
+
+    Args:
+        entity_gid: MBID of the unknown entity.
+        entity_type: Type of the unknown entity.
+    Returns:
+        Special entity object (unknown) of specified entity_type.
+    """
+    if entity_type == 'release_group':
+        entity = special_entities.unknown_release_group
+    elif entity_type == 'place':
+        entity = special_entities.unknown_place
+    elif entity_type == 'event':
+        entity = special_entities.unknown_event
+    entity.gid = entity_gid
+    return entity
+
+
 def get_entities_by_gids(*, query, entity_type, mbids):
     """Get entities using their MBIDs.
 
@@ -52,6 +73,16 @@ def get_entities_by_gids(*, query, entity_type, mbids):
         for entity, redirect_obj in results:
             entities[redirect_obj.gid] = entity
         remaining_gids = list(set(remaining_gids) - {redirect_obj.gid for entity, redirect_obj in results})
+
+    if remaining_gids and entity_type in CB_ENTITIES:
+        reviewed_gids = reviewed_entities(
+            entity_ids=remaining_gids,
+            entity_type=entity_type,
+        )
+        for entity_id in reviewed_gids:
+            entities[entity_id] = unknown_entity(entity_id, entity_type)
+        remaining_gids = list(set(remaining_gids) - set(reviewed_gids))
+
     if remaining_gids:
         raise mb_exceptions.NoDataFoundException("Couldn't find entities with IDs: {mbids}".format(mbids=remaining_gids))
     return entities