Add endpoint for fetching/deleting vault tokens #307

pmclain · 2018-12-27T13:41:19Z

Description (*)

Add query for fetching visible payment tokens:

customerPaymentTokens {
     items {
        public_hash
        details
        payment_method_code
        type
    }
}

The details property is the string value of vault_payment_token.details. This field contains serialized details that vary by the payment implementation. I'm not sure if we want to expose an interface for formatting this information similar to what is used in Magento_InstantPurchase or leave as-is.

Add mutation for deleting payment tokens:

deletePaymentToken (
    public_hash: {{someHash}}
) {
    result
}

Fixed Issues (if relevant)

[Query] My Account > Stored Payment Methods #46: [Query] My Account > Stored Payment Methods

Manual testing scenarios (*)

Configure payment method supporting vault and enable
Checkout via storefront or admin using payment method and select option to save for later
Use query above to retrieve stored payment tokens via graphql requires authentication
Use mutation above to delete stored payment token via graphql requires authentication
Use query above to validate deleted payment token is no longer returned via graphql requires authentication

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

paliarush · 2019-01-08T17:10:40Z

app/code/Magento/VaultGraphQl/Model/PaymentTokenManagement.php

+/**
+ * Vault payment token repository
+ */
+class PaymentTokenManagement


How about VisibleTokenRetriever or something similar?
*Management should be avoided in class names as too generic.

paliarush · 2019-01-08T17:13:12Z

app/code/Magento/VaultGraphQl/Model/Resolver/DeletePaymentToken.php

+use Magento\CustomerGraphQl\Model\Customer\CheckCustomerAccount;
+
+/**
+ * Payment Token resolver, used for GraphQL request processing.


Please update description

paliarush · 2019-01-08T17:18:27Z

app/code/Magento/VaultGraphQl/README.md

+
+**VaultGraphQl** provides type and resolver information for the GraphQl module
+to generate Vault (stored payment information) information endpoints. Also
+provides endpoints for modifying a payment token.


Suggested change

provides endpoints for modifying a payment token.

provides mutations for modifying a payment token.

paliarush · 2019-01-08T17:19:19Z

app/code/Magento/VaultGraphQl/composer.json

+        "magento/module-customer-graph-ql": "*"
+    },
+    "suggest": {
+        "magento/module-graph-ql": "*"


It is interesting, will it even work without GraphQL module?

No. You caught me copying the composer.json from another module :)

paliarush · 2019-01-08T17:23:56Z

app/code/Magento/VaultGraphQl/etc/schema.graphqls

+}
+
+type DeletePaymentTokenOutput {
+    result: Boolean!


I believe it makes sense to allow retrieval of updated customerPaymentTokens value as a result of mutation to prevent subsequent query when rendering the list again.

How about having two fields here:

result: Bolean! customerPaymentTokens: CustomerPaymentTokens

paliarush · 2019-01-08T17:26:31Z

app/code/Magento/VaultGraphQl/etc/schema.graphqls

+}
+
+type CustomerPaymentTokens {
+    items: [PaymentToken] @doc(description: "An array of payment tokens")


Suggested change

items: [PaymentToken] @doc(description: "An array of payment tokens")

items: [PaymentToken]! @doc(description: "An array of payment tokens")

I assume at least an empty array will always be returned.

paliarush · 2019-01-08T17:27:58Z

app/code/Magento/VaultGraphQl/etc/schema.graphqls

+type PaymentToken @doc(description: "Stored payment method available to customer") {
+    public_hash: String! @doc(description: "Token public hash")
+    payment_method_code: String! @doc(description: "Payment method code associated with token")
+    type: String! @doc(description: "Token type [card|account]")


It can be declared as enum if just two values are possible.

paliarush · 2019-01-08T17:28:45Z

dev/tests/api-functional/testsuite/Magento/GraphQl/Vault/CustomerPaymentTokensTest.php

+QUERY;
+        $response = $this->graphQlQuery($query, [], '', $this->getCustomerAuthHeaders($currentEmail, $currentPassword));
+
+        $this->assertEquals(1, count($response['customerPaymentTokens']['items']));


Ideally, we should validate each field

pmclain · 2019-01-08T18:46:51Z

@paliarush Thanks for the feedback. The requested changes have been made.

keharper

My comments are for descriptions and exception messages.

keharper · 2019-01-10T20:22:31Z

app/code/Magento/VaultGraphQl/Model/Resolver/DeletePaymentToken.php

+        $token = $this->paymentTokenManagement->getByPublicHash($args['public_hash'], $currentUserId);
+        if (!$token) {
+            throw new GraphQlNoSuchEntityException(
+                __('Token could not be found by public hash: %1', $args['public_hash'])


Suggested change

__('Token could not be found by public hash: %1', $args['public_hash'])

__('Could not find a token using public hash: %1', $args['public_hash'])

keharper · 2019-01-10T20:25:09Z

app/code/Magento/VaultGraphQl/README.md

+# VaultGraphQl
+
+**VaultGraphQl** provides type and resolver information for the GraphQl module
+to generate Vault (stored payment information) information endpoints. Also


Suggested change

to generate Vault (stored payment information) information endpoints. Also

to generate Vault (stored payment information) information endpoints. This module also

keharper · 2019-01-10T20:28:35Z

app/code/Magento/VaultGraphQl/etc/schema.graphqls

+# See COPYING.txt for license details.
+
+type Mutation {
+    deletePaymentToken(public_hash: String!): DeletePaymentTokenOutput @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\DeletePaymentToken") @doc(description:"Delete customer Payment Token")


Suggested change

deletePaymentToken(public_hash: String!): DeletePaymentTokenOutput @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\DeletePaymentToken") @doc(description:"Delete customer Payment Token")

deletePaymentToken(public_hash: String!): DeletePaymentTokenOutput @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\DeletePaymentToken") @doc(description:"Delete a customer payment token")

keharper · 2019-01-22T16:47:32Z

app/code/Magento/VaultGraphQl/etc/schema.graphqls

+}
+
+type Query {
+    customerPaymentTokens: CustomerPaymentTokens @doc(description: "List of customer payment tokens") @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\PaymentTokens")


Suggested change

customerPaymentTokens: CustomerPaymentTokens @doc(description: "List of customer payment tokens") @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\PaymentTokens")

customerPaymentTokens: CustomerPaymentTokens @doc(description: "Return a list of customer payment tokens") @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\PaymentTokens")

keharper · 2019-01-22T16:49:52Z

app/code/Magento/VaultGraphQl/etc/schema.graphqls

+}
+
+type PaymentToken @doc(description: "Stored payment method available to customer") {
+    public_hash: String! @doc(description: "Token public hash")


Suggested change

public_hash: String! @doc(description: "Token public hash")

public_hash: String! @doc(description: "The public hash of the token")

keharper · 2019-01-22T16:50:58Z

app/code/Magento/VaultGraphQl/etc/schema.graphqls

+    items: [PaymentToken]! @doc(description: "An array of payment tokens")
+}
+
+type PaymentToken @doc(description: "Stored payment method available to customer") {


Suggested change

type PaymentToken @doc(description: "Stored payment method available to customer") {

type PaymentToken @doc(description: "The stored payment method available to the customer") {

keharper · 2019-01-22T16:51:51Z

app/code/Magento/VaultGraphQl/etc/schema.graphqls

+
+type PaymentToken @doc(description: "Stored payment method available to customer") {
+    public_hash: String! @doc(description: "Token public hash")
+    payment_method_code: String! @doc(description: "Payment method code associated with token")


Suggested change

payment_method_code: String! @doc(description: "Payment method code associated with token")

payment_method_code: String! @doc(description: "The payment method code associated with the token")

pmclain · 2019-01-22T18:17:15Z

@keharper Thanks for review! The suggested changes have been applied.

There was 1 failure: 1) Magento\Test\Integrity\DependencyTest::testRedundant Redundant dependencies found! Module Magento\VaultGraphQl: hard [Magento\GraphQl]

# Conflicts: # composer.lock

-- fix static tests

naydav · 2019-02-06T21:41:35Z

@pmclain
Could you fix static tests errors?

FILE: ...al/testsuite/Magento/GraphQl/Vault/CustomerPaymentTokensTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 155 | ERROR | [x] Function closing brace must go on the next line
     |       |     following the body; found 1 blank lines before
     |       |     brace

Magento\Test\Php\LiveCodeTest::testCopyPaste
PHP Copy/Paste Detector has found error(s):
<?xml version="1.0" encoding="UTF-8"?>
<pmd-cpd>
  <duplication lines="35" tokens="110">
    <file path="/var/www/html/app/code/Magento/Vault/Model/PaymentTokenManagement.php" line="129"/>
    <file path="/var/www/html/app/code/Magento/VaultGraphQl/Model/VisibleTokenRetriever.php" line="74"/>
    <codefragment>    public function getVisibleAvailableTokens($customerId)
    {
        $customerFilter = [
            $this-&gt;filterBuilder-&gt;setField(PaymentTokenInterface::CUSTOMER_ID)
                -&gt;setValue($customerId)
                -&gt;create()
            ];
        $visibleFilter = [
            $this-&gt;filterBuilder-&gt;setField(PaymentTokenInterface::IS_VISIBLE)
                -&gt;setValue(1)
                -&gt;create()
            ];
        $isActiveFilter = [
            $this-&gt;filterBuilder-&gt;setField(PaymentTokenInterface::IS_ACTIVE)
                -&gt;setValue(1)
                -&gt;create()
            ];
        $expiresAtFilter = [
            $this-&gt;filterBuilder-&gt;setField(PaymentTokenInterface::EXPIRES_AT)
                -&gt;setConditionType('gt')
                -&gt;setValue(
                    $this-&gt;dateTimeFactory-&gt;create(
                        'now',
                        new \DateTimeZone('UTC')
                    )-&gt;format('Y-m-d 00:00:00')
                )
                -&gt;create()
            ];
        $this-&gt;searchCriteriaBuilder-&gt;addFilters($customerFilter);
        $this-&gt;searchCriteriaBuilder-&gt;addFilters($visibleFilter);
        $this-&gt;searchCriteriaBuilder-&gt;addFilters($isActiveFilter);
        // add filters to different filter groups in order to filter by AND expression
        $searchCriteria = $this-&gt;searchCriteriaBuilder-&gt;addFilters($expiresAtFilter)-&gt;create();

        return $this-&gt;paymentTokenRepository-&gt;getList($searchCriteria)-&gt;getItems();
</codefragment>
  </duplication>
</pmd-cpd>

Thanks

pmclain · 2019-02-06T22:38:08Z

@naydav cc0b1cd should resolve the static tests.

# Conflicts: # composer.lock

-- Update composer lock

ghost · 2019-02-14T05:37:44Z

Hi @pmclain, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento-engcom-team added Partner: Something Digital partners-contribution labels Dec 27, 2018

pmclain force-pushed the issue/46 branch from a0889d3 to 205fa1a Compare December 27, 2018 14:13

paliarush suggested changes Jan 8, 2019

View reviewed changes

Add endpoint for fetching/deleting vault tokens

e35aad7

pmclain force-pushed the issue/46 branch from 205fa1a to e35aad7 Compare January 8, 2019 18:44

paliarush approved these changes Jan 8, 2019

View reviewed changes

keharper reviewed Jan 22, 2019

View reviewed changes

Update strings

53e465c

pmclain force-pushed the issue/46 branch from 196fea4 to 53e465c Compare January 22, 2019 20:13

pmclain and others added 3 commits January 25, 2019 20:09

Resolve Redundant dependencies found in static test

56064a2

There was 1 failure: 1) Magento\Test\Integrity\DependencyTest::testRedundant Redundant dependencies found! Module Magento\VaultGraphQl: hard [Magento\GraphQl]

Merge remote-tracking branch 'origin/2.3-develop' into issue/46

2fad20d

# Conflicts: # composer.lock

GraphQL-256: [Query] My Account > Stored Payment Methods

89808ed

-- fix static tests

magento-engcom-team assigned naydav Feb 6, 2019

magento-engcom-team added the Release Line: 2.3 label Feb 6, 2019

Fix static tests

cc0b1cd

naydav added 2 commits February 13, 2019 14:09

Merge remote-tracking branch 'origin/2.3-develop' into issue/46

fba5e6d

# Conflicts: # composer.lock

GraphQL-46: [Query] My Account > Stored Payment Methods

867c134

-- Update composer lock

magento-engcom-team merged commit 867c134 into magento:2.3-develop Feb 14, 2019

magento-engcom-team pushed a commit that referenced this pull request Feb 14, 2019

ENGCOM-4158: Add endpoint for fetching/deleting vault tokens #307

7023a5e

magento-engcom-team added Award: advanced Award: test coverage labels Feb 14, 2019

naydav mentioned this pull request Feb 14, 2019

[Query] My Account > Stored Payment Methods #46

Closed

naydav mentioned this pull request Feb 27, 2019

GraphQL-60: Manage Stored Payment Methods #228

Closed

4 tasks

keharper mentioned this pull request Apr 10, 2019

Docs: Add reference information for updates to QueryGraphQl module #588

Closed

pmclain mentioned this pull request May 3, 2019

[Mutations] My Account > Manage Stored Payment Methods #60

Closed

keharper mentioned this pull request Jun 14, 2019

Add GraphQL vault documentation magento/devdocs#4760

Merged

	provides endpoints for modifying a payment token.
	provides mutations for modifying a payment token.

	items: [PaymentToken] @doc(description: "An array of payment tokens")
	items: [PaymentToken]! @doc(description: "An array of payment tokens")

	__('Token could not be found by public hash: %1', $args['public_hash'])
	__('Could not find a token using public hash: %1', $args['public_hash'])

	to generate Vault (stored payment information) information endpoints. Also
	to generate Vault (stored payment information) information endpoints. This module also

	deletePaymentToken(public_hash: String!): DeletePaymentTokenOutput @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\DeletePaymentToken") @doc(description:"Delete customer Payment Token")
	deletePaymentToken(public_hash: String!): DeletePaymentTokenOutput @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\DeletePaymentToken") @doc(description:"Delete a customer payment token")

	customerPaymentTokens: CustomerPaymentTokens @doc(description: "List of customer payment tokens") @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\PaymentTokens")
	customerPaymentTokens: CustomerPaymentTokens @doc(description: "Return a list of customer payment tokens") @resolver(class: "\\Magento\\VaultGraphQl\\Model\\Resolver\\PaymentTokens")

	public_hash: String! @doc(description: "Token public hash")
	public_hash: String! @doc(description: "The public hash of the token")

	type PaymentToken @doc(description: "Stored payment method available to customer") {
	type PaymentToken @doc(description: "The stored payment method available to the customer") {

	payment_method_code: String! @doc(description: "Payment method code associated with token")
	payment_method_code: String! @doc(description: "The payment method code associated with the token")

Add endpoint for fetching/deleting vault tokens #307

Add endpoint for fetching/deleting vault tokens #307

Uh oh!

Conversation

pmclain commented Dec 27, 2018 • edited by naydav Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmclain commented Jan 8, 2019

Uh oh!

keharper left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmclain commented Jan 22, 2019

Uh oh!

naydav commented Feb 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pmclain commented Feb 6, 2019

Uh oh!

ghost commented Feb 14, 2019

Uh oh!

Uh oh!

pmclain commented Dec 27, 2018 •

edited by naydav

Loading

naydav commented Feb 6, 2019 •

edited

Loading