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

feat: Enable customization through a relationship filter #70

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mgagliardo91
Copy link

I had originally opened an issue, but thought it would be better to POC the change I was hoping for and see if this is something worth getting into the main branch.

The problem

We would like to use the pg-many-to-many plugin to enable relationships between linker tables. While the plugin does work for this, it also ends up creating a lot of relationships based on constraints across all the tables we have which result in very verbose resolver names, such as entityByEntityAIdAndEntityBIdAndEntityCId, etc.

The @omit manyToMany option does allow us to remove these resolvers, but because its opt-out first, we are having to do this on most constraints and it makes the growing schema difficult to manage, as it becomes a requirement that we can't forget to do.

Proposal

Allow consumers to pass a filter function that can let them decide when the derived relationship should result in a valid manyToMany connection. This function, here called pgManyToManyFilter, is backwards compatible and opt-in, so it has no downstream effects on existing usages.

An example of the usage, in the case of only wanting to enable for relationships where the linker table has a composite primary key, would look like:

...
graphileBuildOptions: {
    pgManyToManyFilter(
      leftTable: PostgraphileIntrospection,
      { junctionTable }: { junctionTable: PostgraphileIntrospection },
    ) {
      return (
        junctionTable.primaryKeyConstraint !== undefined &&
        junctionTable.primaryKeyConstraint.keyAttributes.length > 1
      )
    },
  },
...

I did not add any tests to validate this proposal, as I wasn't sure if it would be accepted as is, but I'd be happy to do so. We are currently consuming my fork, but I would prefer to get this change (or something similar) into the primary repo so we can point back to it.

Thanks again for contributing to OSS!

@mgagliardo91
Copy link
Author

@mattbretl are you still the current maintainer? Would it be possible to give this one a review or leave a comment on whether it can be achieved in a different way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant