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

magento/architecture#: GraphQl. Add a mutation for subscribe feature #367

Merged
merged 1 commit into from
May 28, 2020

Conversation

atwixfirster
Copy link
Contributor

@atwixfirster atwixfirster commented Apr 4, 2020

Problem

There is a subscribe field on a standard Magento storefront:

27337

Currently, GraphQl does not have a mutation which allow to subscribe guest/customer into a newsletter subscription.

Solution

type Mutation {
    subscribeEmailToNewsletter(email: String!): SubscribeOutput @doc(description:"Adds an email into a newsletter subscription")
}

type SubscribeOutput {
    status: Int! @doc(description: "Returns a status of subscription")
}

SubscribeOutput.status may return the next statuses:

Depends on status the possible messages will be displayed, for example on PWA side:

PR: magento/magento2#27586
Issue in CORE repo: magento/magento2#27337

Requested Reviewers

Thank you!

@okorshenko
Copy link

СС: @nrkapoor as a PO of GraphQL scope

@okorshenko okorshenko requested a review from paliarush April 4, 2020 11:09
@atwixfirster atwixfirster force-pushed the subscribeEmailToNewsletter branch from da80fb4 to a00c6ec Compare April 4, 2020 13:58
}

type SubscribeOutput {
status: Int! @doc(description: "Returns a status of subscription")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please introduce enum instead of int here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,7 @@
type Mutation {
subscribeEmailToNewsletter(email: String!): SubscribeOutput @doc(description:"Adds an email into a newsletter subscription")
Copy link
Contributor

Choose a reason for hiding this comment

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

To be consistent with other mutations, please change output to SubscribeEmailToNewsletterOutput (mutation name + suffix)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@paliarush , did you have a chance to review the applied changes?

Thank you!

@atwixfirster atwixfirster force-pushed the subscribeEmailToNewsletter branch from a00c6ec to e388de7 Compare April 7, 2020 19:42
@atwixfirster atwixfirster force-pushed the subscribeEmailToNewsletter branch from e388de7 to 22c3327 Compare April 8, 2020 22:55
Copy link
Contributor

@DrewML DrewML left a comment

Choose a reason for hiding this comment

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

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants