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

Unable to save subscription checkbox on Admin customer save #6811

Merged
merged 3 commits into from
Mar 23, 2017

Conversation

rich1990
Copy link

@rich1990 rich1990 commented Sep 29, 2016

Preconditions

Default Magento code base with sample data.

Steps to reproduce

  1. Log in to Admin
  2. Select a customer/ create a customer
  3. Subscription tab
  4. Save

the "Subscribed to Newsletter" will be always 'checked' because the condition of the if is clearly wrong

Actual and Expected result

To make sure that everybody involved in the fix are on the same page, precisely describe the result you expected to get and the result you actually observed after performing the steps.

Expected result:
It would be expect to see the "Subscribed to Newsletter" checkbox being saved correctly

Additional information

The change is fairy simple, in the if statement before was comparing with false as a string, so when getting the the value from the post, i make sure i cast it with (bool).

Then just check is !== false

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Sep 29, 2016

CLA assistant check
All committers have signed the CLA.

@rich1990
Copy link
Author

After PHPunit check i changed the if statement, by removing the casting of the Post parameter and change the if statement accordingly

@vrann vrann self-assigned this Mar 21, 2017
@vrann vrann added this to the March 2017 milestone Mar 21, 2017
@vrann
Copy link
Contributor

vrann commented Mar 21, 2017

@rich1990 Thank you for the contribution.
In this code https://github.com/magento/magento2/blob/develop/app/code/Magento/Customer/Block/Adminhtml/Edit/Tab/Newsletter.php#L167 I see that it sets the value to the string "false", while you changed it to check for the string "0". Will it really work?

@rich1990
Copy link
Author

rich1990 commented Mar 22, 2017

@vrann Yes with my change works! if you asking why is a string, ask to the guy who wrote the form :)

@vrann
Copy link
Contributor

vrann commented Mar 23, 2017

@rich1990 ok, I see, I debugged it and it works good. Will be merging.

@magento-team magento-team merged commit eb8f462 into magento:develop Mar 23, 2017
magento-team pushed a commit that referenced this pull request Mar 23, 2017
…save #6811

- update test according to the way UI works
magento-team pushed a commit that referenced this pull request Mar 23, 2017
…save #6811

- update test according to the way UI works
@ishakhsuvarov
Copy link
Contributor

@rich1990 Thank you for contribution!

@rich1990 rich1990 deleted the subscription_customer_admin branch March 25, 2017 19:25
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.

None yet

6 participants