Feature/epistolary event model updates #27
Conversation
| @@ -4,4 +4,4 @@ | |||
| class EventConfig(AppConfig): | |||
| default_auto_field = 'django.db.models.BigAutoField' | |||
| name = 'event' | |||
| verbose_name = 'epistolary episodes and actions' | |||
| verbose_name = 'events and actions' | |||
There was a problem hiding this comment.
I think this change is just for consistency, but in general, I think "episode/action" is clearer terminology than "event/action".
There was a problem hiding this comment.
My reasoning was that the term 'episodes' is otherwise not (yet) used, so it didn't make much sense to include it here. I agree that 'events' is a bit vague, so if you have any suggestions for terms that encapsulate both world and epistolary events, let me know!
backend/event/models.py
Outdated
| class EpistolaryEventTrigger(models.Model): | ||
| """ | ||
| A relationship between an epistolary event and a world event that triggered it. | ||
| """ | ||
|
|
||
| epistolary_event = models.ForeignKey( | ||
| to=EpistolaryEvent, | ||
| on_delete=models.CASCADE, | ||
| help_text="The epistolary event that was triggered by a world event", | ||
| ) | ||
|
|
||
| world_event = models.ForeignKey( | ||
| to=WorldEvent, | ||
| on_delete=models.CASCADE, | ||
| help_text="The world event that triggered an epistolary event", | ||
| ) | ||
|
|
||
| def __str__(self): | ||
| return f"{self.world_event} triggered {self.epistolary_event}" |
There was a problem hiding this comment.
Since this model doesn't contain extra metadata about the link, it looks like you don't need it; we can just use ManyToManyField without a through model.
That said, I think this should actually be an subclass of Field right? Then we would have extra properties.
backend/event/models.py
Outdated
| def __str__(self): | ||
| categories = self.categories.all() | ||
| category_names = [category.get_value_display() | ||
| for category in categories] | ||
| category_desc = ", ".join(category_names) | ||
| letters = ", ".join(letter.__str__() for letter in self.letters.all()) | ||
| return f"{category_desc} of {letters}" | ||
| return f"{category_desc} of {letters} ({self.date.display_date})" |
There was a problem hiding this comment.
If it's useful for the admin overview, we could also add the date to the admin list display.
There was a problem hiding this comment.
I'm interested, but I'm not sure what you mean. Would this be a dynamic label like the date ranges you have added to the Letter admin?
There was a problem hiding this comment.
Ah, I meant that you could configure the admin class of this model with something like list_display = ['description', 'display_date'] (if description and display_date are database fields or properties with @admin.display() decorators). Then the admin would show the date as a second column in the overview.
There was a problem hiding this comment.
Ah, very useful! Good idea.
backend/event/models.py
Outdated
| epistolary_events = models.ManyToManyField( | ||
| to=EpistolaryEvent, | ||
| through="EpistolaryEventTrigger", | ||
| related_name="triggers", | ||
| ) |
There was a problem hiding this comment.
Logically, epistolary events can also trigger world events, or epistolary events can trigger other epistolary events. Perhaps this requires a more complex relationship?
There was a problem hiding this comment.
Ah, good point. I'll make the relationship bidirectional.
This turned out to be a bit more complicated than I thought (but good practice with working with recursive ManyToManyFields). The new structure can be summarised as follows:
Let me know what you think! |
|
|
||
| class WorldEventsTriggeredWorldEventsInline(admin.StackedInline): | ||
| model = models.WorldEvent.triggered_world_events.through | ||
| fk_name = "triggering_world_event" |
There was a problem hiding this comment.
The fk_name is required if there are two references to the same model on the inline model.
| triggered_epistolary_events = models.ManyToManyField( | ||
| to="self", | ||
| through="EpistolaryEventSelfTrigger", | ||
| through_fields=("triggering_epistolary_event", "triggered_epistolary_event"), |
There was a problem hiding this comment.
Good to know (in case you didn't know this -- I didn't!):
- Without
symmetrical=False, Django will assume that any model's ManyToMany relationship with itself is symmetrical and will add two records in the database. through_fieldsmakes sure that when you callworld_event.triggered_world_events.add(...<another-world-event>...), thisanother-world-eventis mapped to the correct field in the through model. In other words, it marks which of the two foreign fields in the through model is the source and which is the target.
Resolves #9, #23.
This PR
Giftmodel, with foreign keys to an associated 'giver' and a letter action;WorldEventmodel, with a foreign key to an epistolary event. In the future, the team may decide to attach world events to letter actions, but for now, epistolary events feel like a good place to start.display_dateon the abstractLetterCraftDatemodel;instigatorto the list of possible Roles.