Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

magento/devdocs#7366: GraphQl. subscribeEmailToNewsletter. Mutation for subscribe feature. #7422

Conversation

atwixfirster
Copy link
Contributor

@atwixfirster atwixfirster commented Jun 17, 2020

Purpose of this pull request

This pull request (PR) adds a new topic subscribeEmailToNewsletter mutation requested in #7366.

Affected DevDocs pages

Links to Magento source code

whatsnew
Added the subscribeEmailToNewsletter mutation to the GraphQL Developer Guide.

@devops-devdocs
Copy link
Collaborator

An admin must run tests on this PR before it can be merged.

@atwixfirster atwixfirster force-pushed the issue-7366-subscribeEmailToNewsletter branch 2 times, most recently from 293550f to 2c1525e Compare June 17, 2020 10:56
@rogyar rogyar self-assigned this Jun 17, 2020
@atwixfirster atwixfirster force-pushed the issue-7366-subscribeEmailToNewsletter branch from 2c1525e to b750512 Compare June 17, 2020 13:32
@keharper keharper added 2.4.1 Community Docs impacted by community code contribution New Topic A major update published as an entirely new document labels Jun 17, 2020
@keharper keharper self-assigned this Jun 17, 2020

## Input arguments

Mutation contains a required `email` argument specified an email address which should be added 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.

Suggested change
Mutation contains a required `email` argument specified an email address which should be added into a newsletter subscription.
Mutation contains a required `email` parameter that specifies an email address that should be added into a newsletter subscription.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented


### SubscriptionStatusesEnum object

In general, the `SubscriptionStatusesEnum` object can return one of the following values:
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 precise, the SubscriptionStatusesEnum is not an "object" but a "set of possible states"

I would suggest the following

Suggested change
In general, the `SubscriptionStatusesEnum` object can return one of the following values:
The `SubscriptionStatusesEnum` is a predefined set of possible subscription statuses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented

`UNSUBSCRIBED` | Email address is unsubscribed.
`UNCONFIRMED` | Specified email ties to customer which did not confirm a required customer registration.

For the `subscribeEmailToNewsletter` mutation the `SubscriptionStatusesEnum` object may return only 2 of them:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For the `subscribeEmailToNewsletter` mutation the `SubscriptionStatusesEnum` object may return only 2 of them:
The `subscribeEmailToNewsletter` mutation for the `status` field may return only the following statuses:

Copy link
Contributor

Choose a reason for hiding this comment

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

A minor adjustment just to mention that SubscriptionStatusesEnum cannot physically return anything since it's like an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep.

implemented

@atwixfirster atwixfirster force-pushed the issue-7366-subscribeEmailToNewsletter branch from b750512 to 82dd032 Compare June 18, 2020 22:08
@atwixfirster
Copy link
Contributor Author

@rogyar , all your suggestions have been implemented. Could you please review?

Thank you!

Copy link
Contributor

@keharper keharper left a comment

Choose a reason for hiding this comment

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

Mention that Magento must be configured to allow guests to subscribe.

I think it might be worthwhile to mention the Store request header.

Check whether my rewrite of the UNCONFIRMED enum description is correct.

@keharper
Copy link
Contributor

@atwixfirster I'm pinging you to make sure you're aware that I requested some updates to to #7422

@atwixfirster
Copy link
Contributor Author

atwixfirster commented Jun 25, 2020

@keharper

Mention that Magento must be configured to allow guests to subscribe.

By default, Magento allows a newsletter subscription for quests.

Reference to source: https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Newsletter/etc/config.xml#L15

Check whether my rewrite of the UNCONFIRMED enum description is correct.

approved :)

Thank you so much for your grammatical fixes.

@keharper
Copy link
Contributor

keharper commented Jul 6, 2020

running tests

@keharper keharper merged commit 6208b68 into magento:2.4.1-develop Jul 6, 2020
@ghost
Copy link

ghost commented Jul 6, 2020

Hi @atwixfirster, 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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2.4.1 Community Docs impacted by community code contribution New Topic A major update published as an entirely new document Partner: Atwix partners-contribution PR created by Magento partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants