Skip to content
This repository has been archived by the owner on Apr 29, 2019. It is now read-only.

product images with same name overwrite previous image fix #99

Conversation

umarch06
Copy link
Contributor

@umarch06 umarch06 commented Jan 22, 2018

Description

Images were not being renamed while uploading bulk products with images. For example, while importing images from paths a/b.jpg, b/b.jpg, c/b.jpg for three different products were overwriting the same file b.jpg in magento 2. I allowed to rename file if was already existed in Magento 2 catalog product folder. So there is no overwriting while uploading same image name with different folders.

Manual testing scenarios

  1. Create A CSV File for import by same name image within multi directory
  2. Upload file in pub/media/import directory with directory
  3. Login Backend
  4. Got to setting >> import
  5. Select "Entity Type" Product
  6. Select "Import Behavior" Add/Update
  7. Upload CSV File
  8. Click Button "Check Data "
  9. Click "Import"
  10. Import will be successful and it will not override the existing images file with same names.

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)

@umarch06 umarch06 self-assigned this Jan 22, 2018
@@ -180,6 +180,7 @@ public function move($fileName, $renameFileOff = false)
$filePath = $this->_directory->getRelativePath($this->getTmpDir() . '/' . $fileName);
$this->_setUploadFile($filePath);
$destDir = $this->_directory->getAbsolutePath($this->getDestDir());
$this->setAllowRenameFiles(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it would make more sense to see why the variable $renameFileOff is false when passed into the method in this case. It looks like during the lines 152 we are setting the naming options as false.

         if ($renameFileOff) {
             $this->setAllowRenameFiles(false);
         }

@dmanners
Copy link
Contributor

Please also take a look at the integration tests for this section of the system.

@umarch06
Copy link
Contributor Author

@dmanners, probably I missed seeing the if check, that is fixed in new commit.

Umar and others added 2 commits February 16, 2018 15:11
 - this is done as the image paths are now being rewritten and are already covered in the import/export tests
@dmanners dmanners force-pushed the 50-product-images-import-path-fix branch from 2f4aa9e to 0c6120e Compare February 16, 2018 15:16
…ages contain the same name

 - when importing files with the same name will now be renamed for this reason we check names are similar now
@magento-engcom-team
Copy link
Contributor

@umarch06 thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@magento-engcom-team magento-engcom-team merged commit 768d1d7 into magento-engcom:2.3-develop Feb 28, 2018
magento-engcom-team added a commit that referenced this pull request Jul 24, 2018
 - Merge Pull Request magento/graphql-ce#99 from magento/graphql-ce:96-compatibility-with-content-staging
 - Merged commits:
   1. e79f381
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants