Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 116 additions & 0 deletions catalog/migrations/0003_people_itempeoplerelation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
# Generated by Django 5.2.6 on 2025-09-07 23:02

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("catalog", "0002_externalresource_catalog_extres_lookup_ids_gin"),
]

operations = [
migrations.CreateModel(
name="People",
fields=[
(
"item_ptr",
models.OneToOneField(
auto_created=True,
on_delete=django.db.models.deletion.CASCADE,
parent_link=True,
primary_key=True,
serialize=False,
to="catalog.item",
),
),
(
"people_type",
models.CharField(
choices=[
("person", "Person"),
("organization", "Organization"),
],
default="person",
max_length=20,
verbose_name="type",
),
),
],
options={
"abstract": False,
"base_manager_name": "objects",
},
bases=("catalog.item",),
),
migrations.CreateModel(
name="ItemPeopleRelation",
fields=[
(
"id",
models.BigAutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
(
"role",
models.CharField(
choices=[
("author", "Author"),
("translator", "Translator"),
("performer", "Performer"),
("actor", "Actor"),
("director", "Director"),
("composer", "Composer"),
("artist", "Artist"),
("voice_actor", "Voice Actor"),
("host", "Host"),
("publisher", "Publisher"),
("distributor", "Distributor"),
("production_company", "Production Company"),
("record_label", "Record Label"),
("developer", "Developer"),
("studio", "Studio"),
],
max_length=50,
verbose_name="role",
),
),
(
"metadata",
models.JSONField(
blank=True, default=dict, null=True, verbose_name="metadata"
),
),
("created_time", models.DateTimeField(auto_now_add=True)),
("edited_time", models.DateTimeField(auto_now=True)),
(
"item",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name="people_relations",
to="catalog.item",
),
),
(
"people",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name="item_relations",
to="catalog.people",
),
),
],
options={
"indexes": [
models.Index(
fields=["item", "role"], name="catalog_ite_item_id_df3230_idx"
)
],
"unique_together": {("item", "people", "role")},
},
),
]
14 changes: 14 additions & 0 deletions catalog/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@
)
from .movie import Movie, MovieInSchema, MovieSchema
from .music import Album, AlbumInSchema, AlbumSchema
from .people import (
ItemPeopleRelation,
People,
PeopleInSchema,
PeopleRole,
PeopleSchema,
PeopleType,
)
from .performance import (
Performance,
PerformanceProduction,
Expand Down Expand Up @@ -137,4 +145,10 @@ def init_catalog_audit_log():
"TVShowInSchema",
"TVShowSchema",
"init_catalog_audit_log",
"People",
"PeopleInSchema",
"PeopleSchema",
"PeopleType",
"PeopleRole",
"ItemPeopleRelation",
]
4 changes: 2 additions & 2 deletions catalog/models/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class ItemType(models.TextChoices):
Collection = "collection", _("Collection")
# Person = "person", _("Person")
# Organization = "organization", _("Organization")
# People = "people", _("Person / Organization")
People = "people", _("Person / Organization")


class ItemCategory(models.TextChoices):
Expand All @@ -133,7 +133,7 @@ class ItemCategory(models.TextChoices):
Performance = "performance", _("Performance")
# FanFic = "fanfic", _("FanFic")
# Exhibition = "exhibition", _("Exhibition")
# People = "people", _("Person / Organization")
People = "people", _("Person / Organization")
Collection = "collection", _("Collection")


Expand Down
28 changes: 28 additions & 0 deletions catalog/models/item.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
from users.models import User

from ..common import ResourceContent
from .people import ItemPeopleRelation, PeopleRole


class PrimaryLookupIdDescriptor(object): # TODO make it mixin of Field
Expand Down Expand Up @@ -143,6 +144,7 @@ class Item(PolymorphicModel):
merged_from_items: QuerySet["Item"]
merged_to_item_id: int
mark: "Mark"
people_relations: QuerySet["ItemPeopleRelation"]
schema = ItemSchema
category: ItemCategory # subclass must specify this
type: ItemType # subclass must specify this
Expand Down Expand Up @@ -323,6 +325,24 @@ def log_action(self, changes: dict[str, Any]):
self, action=LogEntry.Action.UPDATE, changes=changes
)

def merge_people_relations(self, to_item: Self) -> bool:
"""Merge people relations from this item to the target item"""
updated = False
for relation in self.people_relations.all():
existing_relation = to_item.people_relations.filter(
people=relation.people, role=relation.role
).first()
if existing_relation:
if relation.metadata and not existing_relation.metadata:
existing_relation.metadata = relation.metadata
existing_relation.save()
Comment on lines +336 to +338

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The logic for merging an existing ItemPeopleRelation only considers the metadata field. It's missing the logic to merge the character field. The new test test_item_merge_with_duplicate_people_relations correctly expects the character information to be preserved, but it will fail with the current implementation. The merge logic should be updated to handle both character and metadata to avoid data loss on merge.

Suggested change
if relation.metadata and not existing_relation.metadata:
existing_relation.metadata = relation.metadata
existing_relation.save()
relation_updated = False
if relation.character and not existing_relation.character:
existing_relation.character = relation.character
relation_updated = True
if relation.metadata and not existing_relation.metadata:
existing_relation.metadata = relation.metadata
relation_updated = True
if relation_updated:
existing_relation.save()

relation.delete()
else:
relation.item = to_item
relation.save()
updated = True
return updated

def merge_to(self, to_item: Self | None):
if to_item is None:
if self.merged_to_item is not None:
Expand Down Expand Up @@ -358,6 +378,7 @@ def merge_to(self, to_item: Self | None):
to_item.cover = self.cover
updated = True
updated |= to_item.normalize_metadata()
updated |= self.merge_people_relations(to_item)
to_item.log_action({"!merged_from": [str(self.merged_to_item), str(to_item)]})
if updated:
to_item.save()
Expand Down Expand Up @@ -674,6 +695,13 @@ def process_fetched_item(
def is_editable(self):
return not self.is_deleted and self.merged_to_item is None

def get_people_by_role(self, role: "PeopleRole"):
from .people import People

return People.objects.filter(
item_relations__item=self, item_relations__role=role
)

@property
def mark_count(self):
from journal.models import Mark
Expand Down
Loading