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

Validation change for Text type EAV attributes #62

Closed
wants to merge 4 commits into from

Conversation

tim-reynolds
Copy link
Contributor

The call on Mage_Eav_Model_Attribute_Data_Text:67 to empty returns true of a value of "0" is entered, as specified in the PHP spec. This prevents the entry of zero on the backend admin for fields. Zero should be a valid value, as it was in our case.

No unit tests were created, as no unit tests exist currently for the Eav module.

I originally changed the code to the more complex:
if ($attribute->getIsRequired() &&
(
(is_array($value) && empty($value)) ||
(is_string($value) && strlen(trim($value))==0)
)

However, I wasn't sure that would catch all cases. I was also unsure if an array or numeric value would ever pass into the validator. I felt the cast-less comparison against the primary issue input was the best solution.

The call on Mage_Eav_Model_Attribute_Data_Text:67 to empty returns true of a value of "0" is entered, as specified in the PHP spec. This prevents the entry of zero on the backend admin for fields. Zero should be a valid value, as it was in our case.

No unit tests were created, as no unit tests exist currently for the Eav module.
@tim-reynolds
Copy link
Contributor Author

Ignore my comment about Unit tests. I hadn't synced my fork in a while. I am so absent minded. Working on a unit test right now.

@Forestsoft-de
Copy link

why do you close your pull request tim?

@tim-reynolds
Copy link
Contributor Author

I opened a new one. Github didn't give me the option to amend my pull request.

Sent from my iPhone

On Aug 17, 2012, at 6:47 PM, "Sebastian" <notifications@github.commailto:notifications@github.com> wrote:

why do you close your pull request tim?


Reply to this email directly or view it on GitHubhttps://github.com//pull/62#issuecomment-7837264.

magento-team pushed a commit that referenced this pull request Jan 30, 2015
@stevieyu stevieyu mentioned this pull request Apr 3, 2015
@chrom chrom mentioned this pull request Oct 7, 2015
okorshenko pushed a commit that referenced this pull request Oct 28, 2015
MAGETWO-44707: Remove @api sign from CatalogInventory module
magento-team pushed a commit that referenced this pull request Mar 23, 2016
JS-338: [Configurable.JS] Do not reload full image set when "First by First" is enabled
okorshenko pushed a commit that referenced this pull request Jun 12, 2016
…e-cm-cache-backend-file

[Extensibility] Bug fixes
@ghost ghost mentioned this pull request Oct 21, 2017
magento-engcom-team pushed a commit that referenced this pull request Aug 27, 2018
Merge 2.3-develop to EPAM-PR-2
@FabXav FabXav mentioned this pull request Oct 11, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants