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

fix(appsync-modelgen-plugin): non model type with id field in java #260

Merged
merged 1 commit into from
Oct 19, 2021
Merged

fix(appsync-modelgen-plugin): non model type with id field in java #260

merged 1 commit into from
Oct 19, 2021

Conversation

AaronZyLee
Copy link
Contributor

@AaronZyLee AaronZyLee commented Oct 13, 2021

Description of changes

Generate correct output for non-model type with id field defined. id is currently reserved field in model types but not in non-model types

Issue #, if available

fix #231

Description of how you validated changes

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • Breaking changes to existing customers are released behind a feature flag or major version update
  • Changes are tested using sample applications for all relevant platforms (iOS/android/flutter/Javascript) that use the feature added/modified

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

@AaronZyLee AaronZyLee requested a review from a team as a code owner October 13, 2021 22:41
@AaronZyLee AaronZyLee requested a review from a team October 13, 2021 22:41
Copy link
Contributor Author

@AaronZyLee AaronZyLee left a comment

Choose a reason for hiding this comment

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

Have the lint changes from commit hook. Marked as following. Please ignore them.

selectedType?: string,
generate: CodeGenGenerateEnum = CodeGenGenerateEnum.code,
usePipelinedTransformer: boolean = false,
) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

isTimestampFieldsAdded: true,
handleListNullabilityTransparently: true,
usePipelinedTransformer: usePipelinedTransformer,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

schema: string,
selectedType?: string,
generate: CodeGenGenerateEnum = CodeGenGenerateEnum.code,
) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

@@ -98,7 +115,7 @@ describe('AppSyncModelVisitor', () => {
it('Should generate a class a model with all optional fields except id field', () => {
const schema = /* GraphQL */ `
type SimpleModel @model {
id: ID!,
id: ID!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

author: String
book: String
}
`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

author: String
book: String
}
`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

author: String
book: String
}
`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

type ListContainer
@model
{
type ListContainer @model {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

@@ -383,11 +398,11 @@ describe('AppSyncModelVisitor', () => {

it('should generate class with non-default providers', () => {
const schema = /* GraphQL */ `
type Employee @model @auth(rules: [{ allow: owner }, { allow: private, provider:"iam" } ]) {
type Employee @model @auth(rules: [{ allow: owner }, { allow: private, provider: "iam" }]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

id: ID!
name: String!
address: String!
ssn: String @auth(rules: [{ allow: groups, provider:"oidc", groups: ["Admins"] }])
ssn: String @auth(rules: [{ allow: groups, provider: "oidc", groups: ["Admins"] }])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lint change

@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2021

Codecov Report

Merging #260 (6d62ba9) into master (1f5e883) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #260      +/-   ##
==========================================
- Coverage   85.53%   85.53%   -0.01%     
==========================================
  Files         144      144              
  Lines        6609     6614       +5     
  Branches     1517     1553      +36     
==========================================
+ Hits         5653     5657       +4     
- Misses        875      883       +8     
+ Partials       81       74       -7     
Impacted Files Coverage Δ
...delgen-plugin/src/visitors/appsync-java-visitor.ts 93.04% <100.00%> (+0.01%) ⬆️
...elgen-plugin/src/visitors/appsync-swift-visitor.ts 95.43% <0.00%> (-0.34%) ⬇️
...kages/graphql-types-generator/src/swift/helpers.ts 94.06% <0.00%> (ø)
...c-modelgen-plugin/src/utils/process-connections.ts 92.13% <0.00%> (ø)
...odelgen-plugin/src/utils/process-connections-v2.ts 90.32% <0.00%> (ø)
...delgen-plugin/src/visitors/appsync-dart-visitor.ts 96.91% <0.00%> (ø)
...ugin/src/languages/typescript-declaration-block.ts 72.38% <0.00%> (ø)
...tor/src/compiler/visitors/collectAndMergeFields.ts 92.45% <0.00%> (+0.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4fd9d4...6d62ba9. Read the comment docs.

@AaronZyLee AaronZyLee merged commit 186f8cd into aws-amplify:master Oct 19, 2021
@AaronZyLee AaronZyLee deleted the fixJavaNonModel branch October 19, 2021 17:21
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.

Java code generated by amplify codegen is not compilable
3 participants