Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Episode-entity relationship form #126

Merged
merged 71 commits into from
Sep 25, 2024
Merged

Conversation

lukavdplas
Copy link
Contributor

This adds forms for:

  • Editing related episodes on the agent form
  • Editing related agents, locations, letters, and gifts on the episode form

Close #125

Includes:

  • Some mutations and a query to handle the relationship between an episode and a related entity (i.e. an agent/location/gift/letter). Also adds some interfaces to work with shared properties.
  • GraphQL types are now explicit that name and description are non-nullable in the query output.
  • A EpisodeEntitiesComponent to be used in the episode form for agents, locations, gifts, and letters.
  • An AgentEpisodesComponent to be used in the agent form. This one is not directly reusable (yet).
  • An EpisodeLinkComponent and CollapsibleCardComponent as more basic building blocks.

Not (yet) included:

  • You can't yet edit related episodes on the forms for locations, letters, and gifts. This was harder to generalise than the other way around, but could still be done in the future. Or we could just copy the AgentEpisodesComponent.
  • The dropdown to add a new item includes a "create" button, but it's not yet implemented.

@lukavdplas lukavdplas marked this pull request as ready for review September 19, 2024 11:33
@lukavdplas lukavdplas added this to the Start logging milestone Sep 19, 2024
fields = [
"id",
"episode",
"agent",
"entity",
Copy link
Contributor

@XanderVertegaal XanderVertegaal Sep 22, 2024

Choose a reason for hiding this comment

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

"entity_type" does not exist on the model, so it does not belong here.

Copy link
Contributor

@XanderVertegaal XanderVertegaal left a comment

Choose a reason for hiding this comment

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

Very impressive how you managed to implement this despite the sparsity of the documentation. Well done!

I have a lot of nit-picky comments, but functionally, the code works great! Consider it approved after you've covered my remarks.

backend/event/types/EpisodeLetterType.py Outdated Show resolved Hide resolved
def resolve_episodes(
parent: AgentDescription, info: ResolveInfo
) -> QuerySet[EpisodeAgent]:
return EpisodeAgent.objects.filter(agent=parent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return EpisodeAgent.objects.filter(agent=parent)
return EpisodeAgentType.get_queryset(EpisodeAgent.objects, info).filter(agent=parent)

def resolve_episodes(
parent: LetterDescription, info: ResolveInfo
) -> QuerySet[EpisodeLetter]:
return EpisodeLetter.objects.filter(letter=parent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return EpisodeLetter.objects.filter(letter=parent)
return EpisodeLetterType.get_queryset(EpisodeLetter.objects, info).filter(letter=parent)

backend/event/types/EpisodeAgentType.py Outdated Show resolved Hide resolved
backend/event/types/EpisodeGiftType.py Outdated Show resolved Hide resolved
}

get linkedObjectName(): string {
return this.linkTo == 'episode' ? 'episode' : this.entityName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return this.linkTo == 'episode' ? 'episode' : this.entityName;
return this.linkTo == 'episode' ?? this.entityName;

(All of these getters would be sweet computed()s by the way, but that's something to consider when we're less strapped for time.)

actionIcons = actionIcons;

constructor(
query: DataEntryEpisodeEntityLinkGQL,
Copy link
Contributor

Choose a reason for hiding this comment

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

private? Also a better name than query would be helpful.

if (linkTo == 'episode') {
return 'episodes';
}
const routes = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This object is very similar to entityTypeNames in utils.ts. For consistency's sake, please put this object there as well. (You can maybe merge them as well, or derive one from the other). If you've done that, you can replace this function's body with return linkTo == 'episode' ? 'episodes' : entityTypeRoutes[entityType];

@lukavdplas lukavdplas merged commit 985c54d into develop Sep 25, 2024
2 checks passed
@lukavdplas lukavdplas deleted the feature/m2m-agent-episodes branch September 25, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Episode-entity relation form
2 participants