-
Notifications
You must be signed in to change notification settings - Fork 75
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
fix(auth): ownerfield as pk in relational models #1389
Conversation
packages/amplify-graphql-auth-transformer/src/resolvers/query.ts
Outdated
Show resolved
Hide resolved
#set( $result = { | ||
\\"items\\": [] | ||
} ) | ||
#return($result) | ||
#else | ||
#set( $limit = $util.defaultIfNull($context.args.limit, 100) ) | ||
#set( $sortKeyValue0 = $util.defaultIfNull($ctx.stash.connectionAttibutes.get(\\"title\\"), $ctx.source.title) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, we are defining the sort key value, but not using it anywhere? Are we missing some step to set the sort key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is expected as the CPK feature flag is not enabled. The sort key field will not be used in the query. This is kept for not breaking old projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test statement for this snapshot: should not generate sort key field in implicit hasMany relation when CPK feature is disabled
#set( $result = { | ||
\\"items\\": [] | ||
} ) | ||
#return($result) | ||
#else | ||
#set( $limit = $util.defaultIfNull($context.args.limit, 100) ) | ||
#set( $sortKeyValue0 = $util.defaultIfNull($ctx.stash.connectionAttibutes.get(\\"title\\"), $ctx.source.title) ) | ||
#set( $sortKeyValue1 = $util.defaultIfNull($ctx.stash.connectionAttibutes.get(\\"likes\\"), $ctx.source.likes) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double check on usage of these sort key values please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above as CPK feature flag is false
@@ -299,7 +299,7 @@ test('it generates expected resources', () => { | |||
{ | |||
Ref: Match.anyValue(), | |||
}, | |||
'"))\n$util.qr($ctx.stash.put("endpoint", "https://', | |||
'"))\n$util.qr($ctx.stash.put("connectionAttributes", {}))\n$util.qr($ctx.stash.put("endpoint", "https://', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an expected change for Searchable
resolver? Can you confirm whether we changed the corresponding querying logic to use 'connctionAttributes' variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that the search query does consume the connectionAttributes
. The code block here is to have it initialized as it is applied to all init
slot
* fix(auth): ownerfield as pk in relational models * fix hasone belongsto relation of owner pk auth * fix: use stash for sort key value * fix unit test * fix: unit tests * rm yarn lock change * add e2e test * change snapshot
* fix(auth): ownerfield as pk in relational models * fix hasone belongsto relation of owner pk auth * fix: use stash for sort key value * fix unit test * fix: unit tests * rm yarn lock change * add e2e test * change snapshot
* fix(auth): ownerfield as pk in relational models * fix hasone belongsto relation of owner pk auth * fix: use stash for sort key value * fix unit test * fix: unit tests * rm yarn lock change * add e2e test * change snapshot
Description of changes
Use
stash
instead ofsource
for owner auth resolver when the owner field of related model is part of the key schema. The connection fields will be put into$ctx.stash.connectionAttributes
in auth resolver and consumed by the latter data resolver.CDK / CloudFormation Parameters Changed
Issue #, if available
Fix #1383
Description of how you validated changes
Unit & e2e tests
Team Bug bash
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.