Refactored Downloadable Files Import#393
Refactored Downloadable Files Import#393sprankhub wants to merge 4 commits intoavstudnitz:developfrom
Conversation
nhp
left a comment
nhp
left a comment
There was a problem hiding this comment.
Adding a bit of test data, especially for new functionality would be great. Makes it a whole lot easier to reproduce and check.
Nevertheless great effort and work. Thanks Simon
|
|
||
| $tmpDir = Mage::getConfig()->getOptions()->getMediaDir() . '/import'; | ||
| $destDir = Mage::getModel('downloadable/link')->getBasePath(); | ||
| if ( ! is_writable($destDir)) { |
There was a problem hiding this comment.
Is this really what you want to do? Check if not writeable and then create it? I really have strong opinions against the usage of @ for error suppression like in this case. Either a correct check or a correct handling of the result of the check you already do would be the cleaner alternative.
| $this->_downloadableUploader = Mage::getModel('fastsimpleimport/import_uploader_downloadable', null); | ||
| $this->_downloadableUploader->init(); | ||
|
|
||
| $tmpDir = Mage::getConfig()->getOptions()->getMediaDir() . '/import'; |
There was a problem hiding this comment.
Usage of DS instead of / for compatibility reasons.
| if ( ! file_exists($tmpDir)) { | ||
| @mkdir($tmpDir, 0777, true); | ||
| } | ||
| if ( ! $this->_downloadableUploader->setTmpDir($tmpDir)) { |
There was a problem hiding this comment.
If you change the check for existence and writability you could supply better Exception messages.
|
Thanks for your review! Please see my last commits. I agree that the usage of Test data can be taken form here. I will try to write test scripts and/or update the wiki and/or the website as soon as my work on downloadable products is finished. |
|
@nhp Would you approve the changes after my updates? Thanks! |
|
@nhp would you have a short look again? Thanks! |
2 participants
Mage_ImportExport_Model_Import_Uploader/media/downloadable/files/links/f/i/file.pdf