feat(pg) single backward relations with arguments if primary key covered partially#567
feat(pg) single backward relations with arguments if primary key covered partially#567ab-pm wants to merge 2 commits intographile:masterfrom
Conversation
|
Does this need to go behind a feature flag? I figured this might break existing schemas that a) have such foreign references on parts of their primary keys and b) have a |
50fabfa to
f3da297
Compare
|
@ab-pm Thanks for the PR. I'm going to leave this hanging for a while because I want to figure out what I'm doing with the existing core plugins in v5 before extending the codebase further. |
benjie
left a comment
There was a problem hiding this comment.
I appreciate this PR and it's a cool feature, but I think we're going to be changing so much of the fundamentals in v5 that I can't currently merge it. I'm going to leave it open and merge parts of it into my v5 works when I'm ready. There's not much point changing it more right now because some of the interfaces it currently uses are going to be removed in v5.
| ? primaryKeys.filter(attr => !keys.includes(attr)) | ||
| : []; | ||
| const parameterKeys = uncoveredPrimaryKeys.map(key => ({ | ||
| ...key, |
There was a problem hiding this comment.
There's less risk of conflicts here if we don't splat this out:
| ...key, | |
| key, |
| ) | ||
| : uncoveredPrimaryKeys.length | ||
| ? inflection.rowByRelationBackwardsAndUniqueKeys( | ||
| uncoveredPrimaryKeys, |
There was a problem hiding this comment.
| uncoveredPrimaryKeys, | |
| uncoveredPrimaryKeys.map(details => details.key), |
| primaryKeyConstraint && | ||
| !omit(table, "single") && | ||
| !omit(primaryKeyConstraint, "single") && | ||
| !omit(constraint, "single")); |
There was a problem hiding this comment.
This is a lot of potential omits. I'd like to really tighten up how omit works in V5, I think in this case it should be something like !omit(constraint, "backward")
Integrating the plugin I had posted on Discord into core.
Basic tests (creates the fields it should, queries work as expected, can be omitted using new
@omit single#460) included based on the existing test schemac.Not sure if separate (smaller) schema is needed to go over all edge cases as well, including renaming and custom inflection?