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.

Sign up for GitHub

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

Jump to bottom

Check Data should validate gallery information #271

Closed
barbazul opened this issue Apr 1, 2013 · 7 comments
Closed

Check Data should validate gallery information #271

barbazul opened this issue Apr 1, 2013 · 7 comments

Comments

@barbazul
Copy link
Contributor

barbazul commented Apr 1, 2013

Current implementation ignores media gallery information during data check and then, if data is invalid (i.e. a row is missing the _media_attribute value) errors might occur.

I confirmed this bug in 1.7.0.2 and could not test in magento2 due to not having a PHP < 5.4 available at the moment. I did, however check magento2 code and it looks as if it is still there since there is no validation of media gallery data furing the first step and in app/code/core/Mage/ImportExport/Model/Import/Entity/Product.php line 1275 it blindly assigns $insertValue['attribute_id'] to $valueArr['attribute_id'] which gets passed to the database "as is" in line 1281 and raises an SQL exception if its null
@piotrekkaminski
Copy link
Contributor

Hello. Would you be able to contribute a pull request with a fix?

@barbazul
Copy link
Contributor Author

barbazul commented Apr 2, 2013

I could code a fix for 1.7.0.2 (I already have) then port it to magento2
and hope for the best. I wouldn't be able to test it because of the pending
issue with the admin panel in 5.4
El abr 2, 2013 7:14 AM, "piotrekkaminski" notifications@github.com
escribió:

Hello. Would you be able to contribute a pull request with a fix?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/271#issuecomment-15767073
.

@piotrekkaminski
Copy link
Contributor

Thanks. That would help as well.

@barbazul
Copy link
Contributor Author

barbazul commented Apr 2, 2013

Ok, I have a fix for the specific _media_attribute_id error. At this point I am wondering:

  1. Should the file be validated? Right now if file is missing its ignored during the import step (can't tell you how many headaches this has brought me) but I worry that checking it with file_exists would make the validation process too slow.
  2. Should the _media_attribute be validated as being the actual media_gallery attribute? Right now, if you provide a wrong attribute_id, it is saved into catalog_product_entity_media_gallery as it is provided, which creates rows that will never be accessed.
  3. Why is there even such a column? Looking at Mage_Catalog_Model_Product::addImageToMediaGallery, it specifically looks for the media_gallery attribute code (which obviously maps to an attribute_id later on). Why is there the need to specify the _media_attribute_id column? why not load the attribute, look for its id and use that? I am guessing the intention at some point was to have multiple media galleries for one product but, since the attribute code is hardwired into the models involved I guess that idea was abandoned

barbazul added a commit to barbazul/magento2 that referenced this issue Apr 2, 2013
@barbazul
Fix magento#271.
4a49e29 
Added _media_attribute_id validation during the first step of the import procoess
@piotrekkaminski
Copy link
Contributor

Thank you. We will review this thread and try to answer the questions.

@barbazul
Copy link
Contributor Author

barbazul commented May 2, 2013

@piotrekkaminski Did you get a chance to look into this?

magento-team added a commit that referenced this issue Nov 29, 2013
@magento-team
2.0.0.0-dev54
6e24c0b 
* Modularity improvements:
  * Breakdown of the Adminhtml module:
     * Moved Newsletter, Report logic to the respective modules
     * Moved blocks, config, view, layout files of other components from Adminhtml folder to respective modules
  * Removed application dependencies from the library
* Move Magento\Core common blocks in the library
* Application areas rework:
  * Areas are independent from Store
  * Removed deprecated annotation from the getArea methods
* GitHub requests:
  * [#245](#245) -- Resolve design flaws in core URL helper
  * [#247](#247) -- Bug in Mage_Page_Block_Html_Header->getIsHomePage
  * [#259](#259) -- Turkish Lira (TRY) is supported for Turkish members.
  * [#262](#262) -- Update Rule.php
  * [#373](#373) -- [Magento/Sales] Fixed typos
  * [#382](#382) -- [Magento/Core] Fixed typos
  * [#304](#304) -- Removed Erroneous closing "
  * [#323](#323) -- InstanceController.php - made setBody protected
  * [#349](#349) -- Move Mage_Catalog menu declaration into Mage_Catalog module.
  * [#265](#265) -- Update Merge.php
  * [#271](#271) -- Check Data should validate gallery information
  * [#305](#305) -- Extra ", tidied up nested quotes
  * [#352](#352) -- Add Croatia Country as part of European Union since 1st July 2013 for default european local countries in configuration
  * [#224](#224) -- Tax formatting is locale aware and should not
  * [#338](#338) -- Correcting SQL for required_options column
  * [#327](#327) -- cart api bug fix & partial invoice credit memo divide by zero warning
* Themes update:
  * Old frontend (magento_demo) and backend (magento_basic) themes are removed
  * Updated templates and layout updates in the Bundle, Catalog, CatalogInventory, CatalogSearch, Downloadable, ProductAlert, Reports, Sendfriend modules
* Fixed bugs:
  * Fixed the error when  Magento cannot be reinstalled to the same database with table prefix
  * Fixed report Products in Cart
  * Fixed error on attempt to insert image to CMS pages under version control
  * Fixed order status grid so that you can assign state, edit, and view custom order status
  * Fixed Related Products Rule page so that category can be selected on conditions tab
  * Fixed Magento_Paypal_Controller_ExpressTest integration test so it is re-enabled
  * Fixed the bug with international DHL quotes
@verklov
Copy link
Contributor

verklov commented Nov 29, 2013

Hello barbazul,
We have processed your pull request. The code is released in version dev54.

@verklov verklov closed this as completed Nov 29, 2013
magento-team pushed a commit that referenced this issue Dec 28, 2015
@magicbunneh
Merge pull request #271 from magento-tango/MAGETWO-47054
8ebcb63 
[Tango] Bug: BaseURL in Static View Files
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Refactoring customer attributes validation
9f1ba1a
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Attribute validator fix
40e2363
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Adding required attributes from customer da…
b4db736 
…ta model to validation
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Code style fixes
4b7acdc
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] API functional tests fix
3641627
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Minor API test fix
1c65121
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Updating customer mutations validation: all…
cc69055 
…owing partial customer data update
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Minor fix
f358267
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Minor fixes for validation and exceptions h…
eb48f44 
…andling
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Attributes validation: parameter missing in…
2eb7c15 
… request fix
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Code style fixes
1c30336
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@furseyev
magento#271: [My Account] Refactoring the addition of required attrib…
da8d6ac 
…utes to validation of customer creation
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@magento-engcom-team
ENGCOM-5606: magento#271 [My Account] Add support of Customer attribu…
2c39ba3 
…tes magento#489

 - Merge Pull Request magento/graphql-ce#489 from magento/graphql-ce:feature/271-Customer-Attributes-Validation
 - Merged commits:
   1. dba9800
   2. 7f6718d
   3. 3735e91
   4. 342675a
   5. e177801
   6. 9b5b0a1
   7. 9f1ba1a
   8. 670116c
   9. 40e2363
   10. d071027
   11. b4db736
   12. 4b7acdc
   13. 48f1891
   14. 3641627
   15. ef078b6
   16. 1c65121
   17. cc69055
   18. 7ee6a1a
   19. f358267
   20. eb48f44
   21. 2eb7c15
   22. 7ca7acc
   23. 1c30336
   24. 4382c3a
   25. da8d6ac
   26. 0b4c496
   27. 5ae0633
taskula pushed a commit to Hypernova-Oy/magento2 that referenced this issue Aug 16, 2019
@lenaorobei
ENGCOM-5606: magento#271 [My Account] Add support of Customer attribu…
2d48fbc 
…tes magento#489
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@barbazul @piotrekkaminski @verklov