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(amplify-codegen-appsync-model-plugin): Support Embeddable Types for iOS #4545

Merged
merged 11 commits into from
Jun 16, 2020
Merged

fix(amplify-codegen-appsync-model-plugin): Support Embeddable Types for iOS #4545

merged 11 commits into from
Jun 16, 2020

Conversation

lawmicha
Copy link
Contributor

Issue #, if available:

Description of changes:

  • Renamed customType to embedded(type) and embeddedCollection(of)
  • Adds support in amplify codegen models to generate Extensions for embedded types

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

@lawmicha lawmicha marked this pull request as ready for review June 12, 2020 18:16
@codecov
Copy link

codecov bot commented Jun 12, 2020

Codecov Report

Merging #4545 into master will decrease coverage by 0.40%.
The diff coverage is 81.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4545      +/-   ##
==========================================
- Coverage   61.57%   61.17%   -0.41%     
==========================================
  Files         333      333              
  Lines       14550    14652     +102     
  Branches     2764     2935     +171     
==========================================
+ Hits         8959     8963       +4     
- Misses       5164     5245      +81     
- Partials      427      444      +17     
Impacted Files Coverage Δ
...amplify-codegen-appsync-model-plugin/src/preset.ts 0.00% <0.00%> (ø)
...ejs-function-runtime-provider/src/utils/execute.ts 75.47% <50.00%> (-0.46%) ⬇️
...aphql-auth-transformer/src/ModelAuthTransformer.ts 87.96% <50.00%> (+0.10%) ⬆️
...-model-plugin/src/visitors/appsync-java-visitor.ts 90.75% <100.00%> (+0.16%) ⬆️
...model-plugin/src/visitors/appsync-swift-visitor.ts 97.51% <100.00%> (+0.03%) ⬆️
packages/graphql-transformer-core/src/index.ts 96.77% <100.00%> (-0.20%) ⬇️
...ackages/graphql-transformer-core/src/validation.ts 97.67% <100.00%> (ø)
packages/graphql-mapping-template/src/print.ts 35.71% <0.00%> (ø)
packages/amplify-util-mock/src/storage/storage.ts 0.00% <0.00%> (ø)
... and 18 more

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 2c04c28...33fcfe2. Read the comment docs.

@SwaySway SwaySway requested a review from yuth June 12, 2020 18:32
@lgtm-com
Copy link

lgtm-com bot commented Jun 12, 2020

This pull request introduces 1 alert when merging b9be43b into 2c04c28 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@@ -269,19 +279,17 @@ export class AppSyncSwiftVisitor extends AppSyncModelVisitor {

private getSwiftModelTypeName(field: CodeGenField) {
if (this.isEnumType(field)) {
const name = this.getEnumName(field.type);
return field.isList ? `[${name}].self` : `${name}.self`;
return `${this.getEnumName(field.type)}.self`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

confirm enum collections

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you

Copy link
Contributor Author

@lawmicha lawmicha Jun 16, 2020

Choose a reason for hiding this comment

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

ran codegen + provisioned API with schema using array of an enum. API calls look good, as long as it is an enum, it will be embedded inside .embedded(type) or .embeddedCollection(of:) when it is an array, without needing square brackets (as removed here)

@lgtm-com
Copy link

lgtm-com bot commented Jun 12, 2020

This pull request introduces 1 alert when merging da1150f into 2c04c28 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lawmicha
Copy link
Contributor Author

lawmicha commented Jun 15, 2020

in circleCI workflow

Test suite failed to run

    [BABEL] /home/circleci/repo/packages/amplify-appsync-simulator/src/velocity/value-mapper/map.ts: ENOMEM: not enough memory, read

      at Object.<anonymous> (../../node_modules/lodash/values.js:1:18)

FAIL src/__tests__/velocity/value-mapper/string.test.ts
  ● Test suite failed to run

    ENOMEM: not enough memory, read

      at Object.<anonymous> (../../node_modules/@babel/core/lib/config/files/configuration.js:54:41)

@@ -269,19 +279,17 @@ export class AppSyncSwiftVisitor extends AppSyncModelVisitor {

private getSwiftModelTypeName(field: CodeGenField) {
if (this.isEnumType(field)) {
const name = this.getEnumName(field.type);
return field.isList ? `[${name}].self` : `${name}.self`;
return `${this.getEnumName(field.type)}.self`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you

lawmicha and others added 2 commits June 15, 2020 21:33
…sync-swift-visitor.ts

Co-authored-by: Yathi <511386+yuth@users.noreply.github.com>
…sync-swift-visitor.ts

Co-authored-by: Yathi <511386+yuth@users.noreply.github.com>
@lawmicha lawmicha requested a review from yuth June 16, 2020 04:48
@lawmicha
Copy link
Contributor Author

thanks for reviewing @yuth !

@lawmicha lawmicha changed the title [Codegen][iOS] Support Embedded Types fix():Codegen for iOS to support Embedded Types Jun 16, 2020
@lawmicha lawmicha changed the title fix():Codegen for iOS to support Embedded Types fix(amplify-codegen-appsync-model-plugin): Support Embeddable Types for iOS Jun 16, 2020
@yuth yuth merged commit 7d4652b into aws-amplify:master Jun 16, 2020
nikhname pushed a commit to nikhname/amplify-cli that referenced this pull request Jul 27, 2020
…or iOS (aws-amplify#4545)

- embedded types no longer return encased in square bracket
- generate schemas for non models
- fix Location Schema test
- updated swift presets to generate for all models, including the embedded non-models
- Update packages/amplify-codegen-appsync-model-plugin/src/preset.ts
- remove unsured hasDirective
- rename to Embeddable
- Update packages/amplify-codegen-appsync-model-plugin/src/visitors/appsync-swift-visitor.ts

Co-authored-by: Yathi <511386+yuth@users.noreply.github.com>
@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants