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.

Sign up for GitHub

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

Jump to bottom

fix(graphql-model-transformer): override resource logical id to fix v1 to v2 transformer migration #8597

Merged
merged 1 commit into from Oct 31, 2021
Merged

fix(graphql-model-transformer): override resource logical id to fix v1 to v2 transformer migration #8597

merged 1 commit into from Oct 31, 2021

Conversation

lazpavel
Copy link
Contributor

@lazpavel lazpavel commented Oct 29, 2021 •
edited

Description of changes

  • sets resolver and data source logical IDs as V1 to allow proper migration
  • adds migration test for @model
  • fixes auth resolver lookup when queries are renamed

Description of how you validated changes

  • yarn test passes
  • manual testing
  • e2e migration for @model with key timestamps and queries override from v1 to v2 passes

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@lazpavel lazpavel marked this pull request as ready for review October 30, 2021 03:19
@lazpavel lazpavel requested a review from a team as a code owner October 30, 2021 03:19
SwaySway
SwaySway approved these changes Oct 30, 2021
View reviewed changes
Copy link
Contributor

@SwaySway SwaySway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm aside from nits

packages/amplify-graphql-model-transformer/src/wrappers/object-definition-wrapper.ts Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Oct 30, 2021

This pull request introduces 2 alerts when merging 4fd46c8 into 622b17d - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

@lazpavel lazpavel merged commit e3a2afb into aws-amplify:master Oct 31, 2021
ammarkarachi pushed a commit to ammarkarachi/amplify-cli that referenced this pull request Nov 3, 2021
@lazpavel
fix(graphql-model-transformer): override resource logical id to fix v…
51fc2c6 
…1 to v2 transformer migration (aws-amplify#8597)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig left review comments

@edwardfoyle edwardfoyle edwardfoyle left review comments

@SwaySway SwaySway SwaySway approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lazpavel @cjihrig @SwaySway @edwardfoyle