Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

[Discussion]: Future of relationships #2023

Open
6 tasks
machi1990 opened this issue Sep 9, 2020 · 6 comments
Open
6 tasks

[Discussion]: Future of relationships #2023

machi1990 opened this issue Sep 9, 2020 · 6 comments

Comments

@machi1990
Copy link
Contributor

machi1990 commented Sep 9, 2020

This is an issue that kind of wraps up all the things we can improve around relationship supports:

@machi1990
Copy link
Contributor Author

/cc @craicoverflow, @wtrocki
Automatically generated comment to notify maintainers

@craicoverflow
Copy link

Thank you for creating this issue @machi1990 - this refactor has been on the cards for a while now. The current implementation is pretty hard to extend and is preventing us from easily adding direct many-to-many relationships and implicit relationships.

@wtrocki
Copy link
Contributor

wtrocki commented Sep 9, 2020

Awesome work @machi1990 and @craicoverflow . Now we can make this work with context of all issues we see

@craicoverflow
Copy link

I've found that relationships caused issues with duplicate relationships, so could not complete #2057

In my attempt to allow implicit relationships in https://github.com/aerogear/graphback/pull/1873/files I had refactored relationships so that they would be stored in a config map, instead of an array. This gives many benefits, including:

  • No duplication of relationships since the key is unique in the config map
  • Easy to access the relationship you want (currently you have to perform a filter/find in an array)

Example:

interface ModelRelationshipConfigMap {
  [modelName: string]: ModelFieldRelationshipConfigMap
}

interface ModelFieldRelationshipConfigMap {
  [fieldName: string]: FieldRelationshipMetadata
}

const relationships: ModelRelationshipConfigMap = {
  Note: {
    comments: {...}
  },
  Comment: {
    note: {...}
  }
}

I think when this issue is actioned, this format shoulbe be implemented.

@machi1990
Copy link
Contributor Author

I've found that relationships caused issues with duplicate relationships, so could not complete #2057

In my attempt to allow implicit relationships in https://github.com/aerogear/graphback/pull/1873/files I had refactored relationships so that they would be stored in a config map, instead of an array. This gives many benefits, including:

  • No duplication of relationships since the key is unique in the config map
  • Easy to access the relationship you want (currently you have to perform a filter/find in an array)

Example:

interface ModelRelationshipConfigMap {
  [modelName: string]: ModelFieldRelationshipConfigMap
}

interface ModelFieldRelationshipConfigMap {
  [fieldName: string]: FieldRelationshipMetadata
}

const relationships: ModelRelationshipConfigMap = {
  Note: {
    comments: {...}
  },
  Comment: {
    note: {...}
  }
}

I think when this issue is actioned, this format shoulbe be implemented.

+1, a very good idea.

@wtrocki
Copy link
Contributor

wtrocki commented Sep 23, 2020

I have had chance to review relationships as I needed information form graphback that field is an relationship and sadly could not use it because of the way we do it (we build all relationships and there is no single helper to determine relationship for the model.

Ended up writing my own helper detached from the core.

@wtrocki wtrocki unpinned this issue Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants