Adding links to T&A


Oct 31, 2022

SUPERSEDED

Peter Warner-Medley

#t&a

Technical Story: PT1-170

Context and Problem Statement¶

We need to add links to transfers, to activities (both effectively engagements at the DB and API level) at the time of writing, and to accommodations; consequently we need a decision on how to model them in the DB and in GraphQL. I shall refer to these as linkable entities throughout; a set of entities which may well expand in the future.

The requirements for links on each entity (transfer, activity or accommodation) are identical: a user must be able to add a link along with a plain text title to each type of entity, view a list of links and their titles on each entity and delete specific links from each entity. The UI is also identical.

As an implied decision of PT1-172, PT1-174 & PT1-182, we do not need to make any modelling decisions re: notes at this point as the team is aligned on adding them as a TEXT field on each entity updated with the usual updateWhatever mutation moving forwards.

Decision Drivers ¶

  • Easy to reverse (as we're unsure about future product requirements)
  • Avoid explicit polymorphism in GraphQL (due to recent burn)

Considered Options¶

It's important to note that, as usual with modelling decisions, we need to solve two problems:

  1. Storing links in the DB
  2. Presenting/editing links in the API

DB¶

For the DB, I see roughly three options (where n is the number of distinct linkable entities):

  • n tables of links (e.g. EngagementLinks and AccommodationLinks)
  • One table of links with a polymorphic join:
    • Either one foreign key column with a join type/discriminator field (e.g. url, title, entity enum and entityId)
    • Or Multiple nullable foreign key columns with a constraint enforcing exactly one non-null (e.g. url, title, accommodationId, engagementId)
  • One table of links with join table(s):
    • Either one join table with foreign key to link and foreign key to linkable entity expressed similarly to options above
    • Or n join tables with foreign keys to a linkable entity and a link (e.g. accommodationLinks with accommodationId and linkId; engagementLinks with engagementId and linkId)

API¶

For the API, there is IMO only one valid option: a Link type (with id, url and title) and a list of links on each linkable entity. I see no value in distinct types with identical fields for each linkable entity but am happy to discuss if someone wishes to present a meaningful solution.

For edits on the API, I see three solutions:

  • Include link additions/deletions on the usual updateWhatever mutation (e.g. editEngagement gains a links list which can be used to send an up-to-date list)
  • New addLinkToWhatever mutation and addLink input type (e.g. addLinkToEngagement which accepts an addLink type of id, url and title); plus complementary deleteLinkFromWhatever mutation and deleteLink input type which accepts an entity id and link id
  • New addLink mutation and addLink input type; we infer the linkable entity from the UUID. New deleteLink type which accepts a link id (or entity id and link id).

Options not considered¶

DB¶

  • Link composite type (url TEXT, title TEXT) and Link[] array on each linkable entity.

This is simply too exotic a model; few are familiar with it and it will add extra complexity for deletions especially (likely we would use the ARRAY_REMOVE function).

API¶

  • Distinct Link types (e.g. AccommodationLink, EngagementLink etc.) with same fields.

Discussed above

Decision Outcome¶

The following decision is accepted to move with expediency and clear the T&A project as quickly as Impossible.

A follow up ADR with will allow review and broader discussion once we've seen this in action.

DB model: table per linkable entity¶

Engagement {
    ...
    EngagementLink EngagementLink[]
}

model EngagementLink {
  id           String     @id @default(uuid()) @db.Uuid()
  Engagement   Engagement @relation(fields: [engagementId], references: [id])
  engagementId String
  url          String     @unique // Can't add duplicate URLs. Will use domain/regex constraint
  title        String

  @@unique([engagementId, url])
}

Accommodation {
    ...
    AccommodationLink AccommodationLink[]
}

model AccommodationLink {
  id              String        @id @default(uuid()) @db.Uuid()
  Accommodation   Accommodation @relation(fields: [accommodationId], references: [id])
  accommodationId String
  url             String        @unique // Can't add duplicate URLs. Will use domain/regex constraint
  title           String

  @@unique([accommodationId, url])
}
Mutation {
  addLinkToEngagement(engagementId: ID!, link: LinkInput!): Engagement!
  deleteLinkFromEngagement(engagementId!: ID!, linkId: ID!): Engagement!
  addLinkToAccommodation(accommodationId: ID!, link: LinkInput!): Accommodation!
  deleteLinkFromAccommodation(accommodationId!: ID!, linkId: ID!): Accommodation!
}

input LinkInput {
  url: URL!
  title: String!
}

N.B. We add a URL scalar which is simply a string with regex validation

scalar URL

Link {
    __typename
    id: ID!
    url: URL!
    title: String!
}

Accommodation {
    __typename
    id: ID!
    links: [Link!]!
    ...
}

Engagement {
    __typename
    id: ID!
    links: [Link!]!
    ...
}

Positive Consequences ¶

  • The DB model is highly reversible: at any point we can create a new generic Links table and insert from the various existing EntitlyLink tables
  • The DB model does not require us to reason about complex joins
  • The DB model plays well with Prisma
  • The DB model makes it easy to enforce data integrity and prohibit orphans
  • The client need only understand one object type and one input type from the API
  • The API mutations are explicit about types
  • The API model is highly reversible as the mutations do not rely on any assumptions about the DB model: field resolvers could easily be altered to fetch a different Prisma model and it's substantially easier to rationalise from two mutations (one taking engagementId and the other taking accommodationId) into one (simply taking an entityId UUID) than it is to do the opposite
  • The mutations do not rely on any assumptions about client-side caching or state updates (although they are demonstrated here with the parent list return idea, a refetch of the parent type could easily be added as could a subscription trigger)
  • Mutation resolvers colocated with other resolvers for parent linkable entity

Negative Consequences ¶

  • There are more tables than the minimum necessary which increases the length of our committed SQL and visual noise when browsing the schema
  • There are twice as many add/delete mutations as strictly necessary (one each for Accommodation and Engagement); this won't increase the number of tests (we should have an integration test covering happy path add/delete for each linkable entity either way) but does increase our overall code in graphql and server
  • Mutation resolvers will share identical code (although one can argue this is a strength)

Pros and Cons of the DB Options ¶

Table per linkable entity¶

model EngagementLink {
  id           String     @id @default(uuid()) @db.Uuid()
  Engagement   Engagement @relation(fields: [engagementId], references: [id])
  engagementId String
  url          String     @unique // Can't add duplicate URLs. Will use domain/regex constraint
  title        String

  @@unique([engagementId, url])
}

model AccommodationLink {
  id              String        @id @default(uuid()) @db.Uuid()
  Accommodation   Accommodation @relation(fields: [accommodationId], references: [id])
  accommodationId String
  url             String        @unique // Can't add duplicate URLs. Will use domain/regex constraint
  title           String

  @@unique([accommodationId, url])
}
  • Good, because looks like a normal one-to-many join
  • Good, because no decisions to make about how joins should work
  • Good, because easily reversible by inserting all into one Link table at a later date
  • Good, because plays very well with Prisma (which is highly opinionated about constraints and relations)
  • Bad, because extra tables with exact same schema holding exact same information for exact same entity (a link is a link is a link and we strongly believe that that will remain the case)
  • Bad, because leads to more generated Prisma types for us to work with
model Link {
  id           String     @id @default(uuid()) @db.Uuid()
  linkType     Linkable 
  linkableId   String
  url          String
  title        String
}

enum Linkable {
    ENGAGEMENT
    ACCOMMODATION
}

or

model Link {
  id              String         @id @default(uuid()) @db.Uuid()
  engagementId    String?
  Engagement      Engagement?    @relation(fields: [engagementId], references: [id])
  accommodationId String?
  Accommodation   Accommodation? @relation(fields: [accommodationId], references: [id])
  url             String
  title           String
}
  • Good, because represents a more 'pure' version of our mental model in the database (a link is a link is a link)
  • Good, because forces us to decide on how to represent polymorphic joins, giving us a pattern to use going forwards rather than doing a stoopid ADR each time
  • Bad, because either choice is in some way exotic or unfamiliar to someone
  • Bad, because requires further upfront choices pushing back tickets
  • Bad, because hard to predict the possible pains of either choice across the stack
model Link {
  id           String     @id @default(uuid()) @db.Uuid()
  url          String
  title        String
}

with either:

model Linkable {
  id           String       @id @default(uuid()) @db.Uuid()
  linkableId   String
  linkableType LinkableType
  linkId       String
  Link         Link         @relation(fields: [linkId], references: [id])
}

enum LinkableType {
  ENGAGEMENT
  ACCOMMODATION
}

or

model EngagementLink {
  id           String     @id @default(uuid()) @db.Uuid()
  engagementId String
  Engagement   Engagement @relation(fields: [engagementId], references: [id])
  linkId       String
  Link         Link       @relation(fields: [linkId], references: [id])
}

model AccommodationLink {
  id              String        @id @default(uuid()) @db.Uuid()
  accommodationId String
  Accommodation   Accommodation @relation(fields: [accommodationId], references: [id])
  linkId          String
  Link            Link          @relation(fields: [linkId], references: [id])
}
  • Good, because (as above) a better representation of our mental model
  • Good, because (as above) forces us to decide on a pattern which we can use for similar challenges going forwards
  • Good, because less unusual way of modelling joins than above (a join table is a pattern we're completely used to for many-to-many joins, which used here for a one-to-many join is less confusing than a straight up polymorphic join)
  • Bad, because more unusual join than one link table per linkable entity
  • Bad, because (as above) requires more upfront decisions making with less insight into consequences

 Pros and Cons of the API options¶

One add/delete mutation per linkable entity¶

Mutation {
  addLinkToEngagement(engagementId: ID!, link: LinkInput!): Engagement!
  deleteLinkFromEngagement(engagementId!: ID!, linkId: ID!): Engagement!
  addLinkToAccommodation(accommodationId: ID!, link: LinkInput!): Accommodation!
  deleteLinkFromAccommodation(accommodationId!: ID!, linkId: ID!): Accommodation!
}

input LinkInput {
  url: URL!
  title: String!
}
  • Good, because makes no assumptions about underlying DB model
  • Good, because makes no assumptions about client caching and state update (TBD in later headroom ticket once we have a better grasp of our estate)
  • Good, because by far easiest option to reverse:
    • addLinkToEngagement(engagementId: ID!, link: LinkInput!): Engagement! & addLinkToAccommodation(accommodationId: ID!, link: LinkInput!): Accommodation! → addLink(linkableId: ID!, link: LinkInput!): Link! is much easier than the opposite
  • Good, because very explicit (new engineer might be confused about the idea of a Linkable but addLinkToAccommodation says clearly what type is being added to what type)
  • Bad, because twice as many mutations as strictly needed
  • Bad, because wordy schema
Mutation {
  addLink(linkableId: ID!, link: LinkInput!): Link!
  deleteLink(linkableId!: ID!, linkId: ID!): Link!
}

input LinkInput {
  url: URL!
  title: String!
}
Mutation {
  addLink(linkableId: ID!, link: LinkInput!): Linkable!
  deleteLink(linkableId!: ID!, linkId: ID!): Linkable!
}

union Linkable = Accommodation | Engagement

input LinkInput {
  url: URL!
  title: String!
}
  • Good, because a clear representation of our mental model
  • Good, because fewest new mutations without requiring us to update existing ones which are inconsistent
  • Bad, because either pushes us towards a certain solution for client caching and state updates (by returning only a Link with id, we must either refetch or trigger subscription) or requires a union type to keep our options open (which may be harder to program around in TypeScript based on our experience of interfaces, although TBF we aren't dealing with overlap or similar looking entities here)
  • Bad, because implies a certain underlying DB model meaning it either forces our hand in the DB decision or we confuse future engineers who may make assumptions about our DB schema based on this
  • Bad, because less explicit about types and requires understanding of idea of Linkable

Add as an additional field in existing mutations¶

Mutation {
    editEngagement(engagement: EditEngagement!): Engagement!
    editAccommodation(accommodation: AccommodationInput!  accommodationId: Int!): Accommodation!
}

input EditEngagement {
  id: Int!
  notes: String
  title: String
  start: CustomerPlaceTimeInput
  end: CustomerPlaceTimeInput
  activity: ActivityInput
  transfer: EditTransfer
}
  • Good, because no new mutations or resolvers
  • Bad, because existing mutations follow no consistent pattern so essentially bifurcates decision (i.e. accommodations will likely need their own mutation(s) à la notes)
  • Bad, because hard to reason about logic for deletes in particular
  • Bad, because whole list will need to be sent on add/delete and diffed in/upserted by mutation resolver