API - Comment Model with Post association GraphQL requests #172

lawmicha · 2019-11-27T03:30:15Z

Major changes

ensure Association is added to input variables, ie. "commentPostId"
Add Comment Model tests for createComment and OnCreateComment subscription

Minor changes

the schema.graphql in AmplifyTestCommon for Post contains rating with type Double. This should be Appsync scalar type Float

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

…ables, ie. "commentPostId"

drochetti · 2019-11-27T09:28:22Z

AmplifyPlugins/API/AWSAPICategoryPlugin/Model/Model+GraphQL.swift

-                input[name] = (value as? Model)?.id
+                // For Models, append the model name in front, ie. "comment" + "PostId"
+                let fieldName = modelName.lowerCaseFirstLetter() + name
+                input[fieldName] = (value as? Model)?.id


The new version of the codegen will pass that commentPostId to us. That said:

I'm not opposed to have a fallback and generate our own like you're doing now. This could enable a cleaner Schema Definition for users who decide to write it by hand in the future (convention over configuration)

I believe you can merge this for the time being. Once the codegen changes are validated we change this (later today)

thanks for letting me know, will clean up ->merge -> test with new code gen

drochetti · 2019-11-27T09:41:50Z

AmplifyPlugins/API/AWSAPICategoryPlugin/Support/Internal/String+Casing.swift

+// SPDX-License-Identifier: Apache-2.0
+//
+
+extension String {


I like the idea of String+Casing! I've been meaning to add something like this, it was a good call :)
There are more places where this is needed.

Right now this exists: https://github.com/aws-amplify/amplify-ios/blob/bce59bccb62e9052abc5fb1255cdb648e61d426e/AmplifyPlugins/API/AWSAPICategoryPlugin/Model/GraphQLDocument.swift#L27

Also, this is minor, but I struggle with naming for the functions. Swift itself defines uppercased, lowercased. I named it toPascalCase(), which is very Java/JS approach. So my thoughts are:

name them camelcased() (or camelCased()?)

their implementations should do more than just manipulating the first character, but we can go with the naive approach for now (see comments on toPascalCase()

I like camelCased and pascalCase. It's an internal function intended for programmers, not customers

Prefer the computed properties rather than functions, less typing & more swifty let title = string.pascalCased

Noted elsewhere, don't mutate string; return transformed copies of the original

thanks! updating to pascalCased() and camelCased() (and comment about first implementation)

drochetti · 2019-11-27T09:42:52Z

AmplifyPlugins/API/AWSAPICategoryPlugin/Support/Internal/String+Casing.swift

+    mutating func upperCaseFirstLetter() {
+      self = upperCaseFirstLetter()
+    }


is this needed? mutating a string seems a bit weird

Agreed, don't mutate the case

drochetti · 2019-11-27T09:43:37Z

AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLModelBasedTests.swift

     }

     type Comment @model {
         id: ID!
         content: String!
-         createdAt: AWSDateTime!
+         createdAt: AWSDate!


I would assume createdAt should contain the time information too. Why changing to AWSDate?

this was done by accident i believe. will revert this change, and then will look into testing serialization of AWSDate in a separate field that is not createdAt

drochetti · 2019-11-27T09:46:49Z

AmplifyPlugins/API/AWSAPICategoryPluginFunctionalTests/GraphQLModelBasedTests.swift

+        _ = Amplify.API.mutate(of: comment, type: .create, listener: { event in
+            switch event {
+            case .completed(let data):
+                switch data {
+                case .success(let comment):
+                    XCTAssertEqual(comment.content, "commentContent")
+                    XCTAssertNotNil(comment.post)
+                    XCTAssertNotNil(comment.post.id, uuid)
+                    completeInvoked.fulfill()


this is beautiful 😁

thanks! just realized i'm asserting not nil instead of equals, updating 😣

lawmicha · 2019-11-27T18:00:07Z

thanks for looking, captured a few action items

testing on different serialization types (AWSDate, AWSDateTime without fractions)
run latest model codegen to ensure one-to-many works
run model codegen for many to many (ref https://aws-amplify.github.io/docs/cli-toolchain/graphql#connection)

…e is in signing in state (#172) * fix(auth): fix resolvers and tasks for auto sign in when state machine is in signin in state * fix indentation

* feat(auth): adding support for email mfa (#3892) * feat(auth): adding support for email mfa * fix swift lint warning * worked on a review comment * adding integration tests wave 1 * integration tests wave 2 * integration tests wave 3 * Add test setup instructions wave 4 * Add edge case * update readme to include graphQL details * chore: initial commit to add sdk with passwordless models * chore: model update * feat(Auth): Adding WebAuthn APIs (#153) * feat(Auth): Adding List WebAuthn API * feat(Auth): Adding associate and delete WebAuthn credentials APIs * Addding missing transports array in the credentials payload * Adding friendlyName to AuthWebAuthnCredential * Adding excludedCredentials to avoid multiple PassKeys for the same device * Adding pagination support in the list API * Renaming CredentialPayload to CredentialRegistrationPayload * Addressing PR comments * feat(auth): add passwordless sign with otp (#151) * feat(auth): add passwordless OTP implementation * add fallback password and password srp flows * add web auth n states * modifying states * feat(Auth): Adding WebAuthn support to signIn and confirmSignIn APIs (#155) * feat(auth): add passwordless OTP implementation * add fallback password and password srp flows * add web auth n states * modifying states * feat(Auth): Implementing signIn with WebAuthn * Adding support for a presentation anchor in sign in and confirm sign in options * Fixing errors * Addressing PR comments * fix build error --------- Co-authored-by: Harshdeep Singh <6162866+harsh62@users.noreply.github.com> * fix: Fixing build issue when iOS 18/macOS 15 are not installed * feat(WebAuthn): Adding support for retrying a confirmSignIn with WebAuthn request, if the first one fails (#158) * feat(auth): add support for passwordless sign up and auto sign in (#160) * add autoSignIn() category API definitions (#152) * add autoSignIn() category API definitions * add sign up step for auto sign in * add state machine changes for autoSignIn() and signUp() (#154) * add autoSignIn() category API definitions * add sign up step for auto sign in * add state machine changes * add events and update resolvers * update sign up events and resolvers * add updates to resolver for auto sign in * update confirm sign up flow and debug code * Address review comments --------- Co-authored-by: Harsh <6162866+harsh62@users.noreply.github.com> * update auto sign state machine events and resolver (#157) * update auto sign state machine events and resolver * Address review comments * update sign up and auto sign in unit tests (#159) * update sign up and auto sign in unit tests * add auto sign in tests and refactor existing tests * Add more service error tests * Address review changes --------- Co-authored-by: Harsh <6162866+harsh62@users.noreply.github.com> * chore: fix building of unit tests after sign up rebase * feat(auth): adding passwordless sign in preferred flows (#162) * feat(auth): add passwordless preferred flow * adding confirm device and device srp flows to user auth * update message * worked on review comments * update * chore(auth): add more auto sign in and sign up state machine/e2e unit tests (#161) * chore(auth): add more auto sign in and sign up state machine/e2e unit tests * Address review comments * chore: updated SDK and models * chore: update integration test host app * fix: Fixing build errors in watchOS/tvOS due to missing prechecks. * feat(auth): adding an initial passwordless integration test with resources defined (#163) * chore: update no-auth API's in the resolver * chore: Updating to the renamed WebAuthn APIs (#164) * chore: Updating to the renamed WebAuthn APIs * Fixing unit tests * chore: Adding unit tests for the WebAuthn APIs Tasks (#165) * test: Adding AssociateWebAuthn unit tests * test: Adding ListWebAuthnCredentials unit tests * test: Adding DeleteWebAuthnCredential unit tests * chore: simplifying how webauthn errors are handled * adressing PR comments * chore(auth): add integration tests for passwordless signup and auto sign in (#166) * chore(auth): add integration tests for passwordless signup and auto sign in * remove unused code * refactor code * chore: add integration tests for sign in flows (#168) * chore: add integration tests for sign in flows * Update AuthSignInWithPasswordUsingUserAuthTests.swift * Add more integration tests * update * chore: update sdk to use the latest models * test: Adding integration tests for WebAuthn APIs (#169) * test: Adding integration tests for WebAuthn APIs * chore: Adding webauthn integration workflow * Refactoring the code to remove unnecesary waits and make it more easy to read * fix: Fixing service errors being reported as .unknown when sign in fails (#170) * fix: Fixing service errors being reported as .unknown when sign in fails. Also adding proper WebAuthn cases to the AWSCognitoAuthError enum. * addressing PR comment * fix(auth): fix resolvers and tasks for auto sign in when state machine is in signing in state (#172) * fix(auth): fix resolvers and tasks for auto sign in when state machine is in signin in state * fix indentation * feat: Adding visionOS support to the WebAuthn APIs (#171) * chore: using the latest version aws sdk * chore: update changes needed for the sdk updated * chore: fix swiftlint errors (#3921) * chore: running passwordless integration tests on GEN2 backend * chore: update test target for watchOS * chore: fix OTP integration tests * chore: update more integration tests --------- Co-authored-by: Sebastian Villena <97059974+ruisebas@users.noreply.github.com> Co-authored-by: Abhash Kumar Singh <thisisabhash@gmail.com>

Add Comment Model tests and ensure Association is added to input vari…

5e8abc9

…ables, ie. "commentPostId"

lawmicha requested review from drochetti and palpatim November 27, 2019 03:30

lawmicha self-assigned this Nov 27, 2019

lawmicha added the api Issues related to the API category label Nov 27, 2019

drochetti reviewed Nov 27, 2019

View reviewed changes

- consolidated methods in String+Casing.Swift

1d40833

lawmicha merged commit 9f41524 into master Nov 27, 2019

palpatim deleted the lawmicha/api-comment-post branch December 3, 2019 17:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API - Comment Model with Post association GraphQL requests #172

API - Comment Model with Post association GraphQL requests #172

lawmicha commented Nov 27, 2019

drochetti Nov 27, 2019

lawmicha Nov 27, 2019 •

edited

Loading

drochetti Nov 27, 2019

palpatim Nov 27, 2019

lawmicha Nov 27, 2019

drochetti Nov 27, 2019

palpatim Nov 27, 2019

lawmicha Nov 27, 2019

drochetti Nov 27, 2019

lawmicha Nov 27, 2019

drochetti Nov 27, 2019

lawmicha Nov 27, 2019

lawmicha commented Nov 27, 2019 •

edited

Loading

API - Comment Model with Post association GraphQL requests #172

API - Comment Model with Post association GraphQL requests #172

Conversation

lawmicha commented Nov 27, 2019

Major changes

Minor changes

Choose a reason for hiding this comment

lawmicha Nov 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lawmicha commented Nov 27, 2019 • edited Loading

lawmicha Nov 27, 2019 •

edited

Loading

lawmicha commented Nov 27, 2019 •

edited

Loading