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

usesSource can be unsafe criteria for deciding to use getAttributeText #32

Open
mpchadwick opened this issue Aug 4, 2016 · 1 comment

Comments

@mpchadwick
Copy link
Contributor

I spent a bunch of time debugging why this extension wasn't working for me (none of the attributes were actually pulling values).

It turns out that usesSource() will incorrectly return true on an attribute if getSource() was previously called on that attribute.

getSource() sets source model to default source model...

https://github.com/OpenMage/magento-mirror/blob/magento-1.9/app/code/core/Mage/Eav/Model/Entity/Attribute/Abstract.php#L381

_getDefaultSourceModel() returns eav/entity_attribute_source_table

https://github.com/OpenMage/magento-mirror/blob/d409dff20e992e97546568974399c456958299f9/app/code/core/Mage/Catalog/Model/Resource/Eav/Attribute.php#L336-L339

usesSource thinks that there's a source model...

https://github.com/OpenMage/magento-mirror/blob/d409dff20e992e97546568974399c456958299f9/app/code/core/Mage/Eav/Model/Entity/Attribute/Abstract.php#L397

This is probably a bug in Magento, but unfortunately it's quite common for 3rd party modules to incorrectly call getSource on attributes that don't have source models.

I first ran into an issue here, which was calling getSource() on everything it needed to send to GA (sku, name).

Next there was an SEO module that was calling getSource() on the name attribute. So it was being done by 2 different modules in the same project!

Not sure just yet what the best solution is, but might be nice to add some defense to detect for this situation. I'm willing to submit a PR.

mpchadwick added a commit to mpchadwick/nicer-image-names that referenced this issue Aug 4, 2016
Per Vinai#32 usesSource may incorrectly return true if getSource was previously called on the attribute.

If we didn't get the value the first time around (perhaps due to the getSource issue) we try to fetch again using the mechanics that would be used for attributes without source models.
@philwinkle
Copy link

Dang NomadMage # 2 speaker issuing a PR to NomadMage # 1 speaker. Doesn't get better than this.

mpchadwick added a commit to mpchadwick/nicer-image-names that referenced this issue Aug 5, 2016
Per Vinai#32 usesSource may incorrectly return true if getSource was previously called on the attribute.
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

No branches or pull requests

2 participants