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/magento2#8590: M2.1.4 : ArrayBackend cannot save and Added country regions for Croatia #8912

Merged
merged 2 commits into from
Mar 17, 2017

Conversation

nkajic
Copy link
Member

@nkajic nkajic commented Mar 16, 2017

Description

  1. Changed Magento\Eav\Model\Entity\Attribute\Backend\ArrayBackend.php to pass '0' values inside beforeSave() function
  2. Added country regions for Croatia

Fixed Issues (if relevant)

  1. magento/magetno2#8590: M2.1.4 : ArrayBackend cannot save
  2. (Update) Added country regions for Croatia - data from https://en.wikipedia.org/wiki/ISO_3166-2:HR

Manual testing scenarios

  1. Create a mutliselect attribute
  2. Set it's source model (in the database to a custom source model.
  3. Have this source model return options that have a value of '0'.

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)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Mar 16, 2017

CLA assistant check
All committers have signed the CLA.

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

This is a PR for issue #8590

@magento-team magento-team merged commit 7bb1fb6 into magento:develop Mar 17, 2017
@okorshenko
Copy link
Contributor

@nkajic thank you for your contribution!

@ishakhsuvarov
Copy link
Contributor

Hi @nkajic

  • I've noticed you have not included any tests for the fix you provided.
  • You have updated the InstallData script, however did not update the Upgrade script.

Please let us know if you wish to continue with those items.
Thanks!

@nkajic
Copy link
Member Author

nkajic commented Mar 20, 2017

Hi @ishakhsuvarov

I would like to help.

  • M2.1.4 : ArrayBackend cannot save value '0' #8590: M2.1.4 : ArrayBackend cannot save:
    Sorry to ask, but what kind of test? For you guys to test functionalities easily or unit test?

  • 7bb1fb6 Added country regions for Croatia
    I can create UpgradeData.php - does it mean I should increase patch version on Magento/Directory/etc/module.xml from 2.0.0 to 2.0.1 ? Should remove data added to InstallData or leave data in both InstallData and UpgradeData

@nkajic nkajic changed the title magento/magento2#8590: M2.1.4 : ArrayBackend cannot save magento/magento2#8590: M2.1.4 : ArrayBackend cannot save and Added country regions for Croatia Mar 21, 2017
@skovalenk
Copy link
Contributor

Hi @ishakhsuvarov, @nkajic
We couldn`t add new installation data to InstallData script -> we need to create UpgradeData for this purposes

@nkajic
Copy link
Member Author

nkajic commented Apr 18, 2017

@sereban This nkajic@93e44a7 should work? If you think so I can reopen issue and create new PR to myself.

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.

6 participants