Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

/**
* @copyright For copyright and license information, read the COPYING.txt file.
* @link /COPYING.txt
* @license Open Software License (OSL 3.0)
* @package Mage_Adminhtml
*/

/**
* @package Mage_Adminhtml
*/
class Mage_Adminhtml_Model_System_Config_Source_Catalog_ImageFileHandle
{
public function toOptionArray()
{
return [
['value' => Mage_Catalog_Model_Product_Image::ON_REMOVAL_KEEP, 'label' => Mage::helper('adminhtml')->__('Keep image file(s) on the filesystem')],
Copy link
Contributor

Choose a reason for hiding this comment

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

"Keep the images on the filesystem" - is clearer and more straightforward than the initial one. By using "the images", we specify the exact object being referred to, making the options more precise. Removing unnecessary complexity, like the plural indicator "(s)," and using simple, direct language improves readability and ensures better understanding for the user. The word 'file' is omitted because it is understood that images are stored as files on the filesystem, making it unnecessary to repeat 'file' in the label.

['value' => Mage_Catalog_Model_Product_Image::ON_REMOVAL_DELETE, 'label' => Mage::helper('adminhtml')->__('Permanently deletes image file(s)')],
Copy link
Contributor

Choose a reason for hiding this comment

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

"Permanently delete the images" - is clearer and more straightforward than the initial one. By using "the images", we specify the exact object being referred to, making the options more precise. Removing unnecessary complexity, like the plural indicator "(s)," and using simple, direct language improves readability and ensures better understanding for the user. The word 'file' is omitted because it is understood that images are stored as files on the filesystem, making it unnecessary to repeat 'file' in the label.

];
}
}
7 changes: 7 additions & 0 deletions app/code/core/Mage/Catalog/Helper/Image.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class Mage_Catalog_Helper_Image extends Mage_Core_Helper_Abstract

public const XML_NODE_SKIP_IMAGE_ON_DUPLICATE_ACTION = 'catalog/product_image/images_on_duplicate_action';

public const XML_NODE_DELETE_IMAGE_ON_REMOVAL_ACTION = 'catalog/product_image/images_on_removal_action';

protected $_moduleName = 'Mage_Catalog';

/**
Expand Down Expand Up @@ -657,4 +659,9 @@ public function skipProductImageOnDuplicate(): int
{
return Mage::getStoreConfigAsInt(self::XML_NODE_SKIP_IMAGE_ON_DUPLICATE_ACTION);
}

public function deleteImageFileOnRemoval(): int
{
return Mage::getStoreConfigAsInt(self::XML_NODE_DELETE_IMAGE_ON_REMOVAL_ACTION);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ public function afterSave($object)
foreach ($value['images'] as &$image) {
if (!empty($image['removed'])) {
if (isset($image['value_id']) && !isset($picturesInOtherStores[$image['file']])) {
$toDelete[] = $image['value_id'];
$toDelete[] = $image;
}

continue;
Expand Down Expand Up @@ -271,7 +271,49 @@ public function afterSave($object)
$this->_getResource()->insertGalleryValueInStore($data);
}

$this->_getResource()->deleteGallery($toDelete);
$valueIdsToDelete = array_map(fn($img) => $img['value_id'], $toDelete);
$this->_getResource()->deleteGallery($valueIdsToDelete);

/** @var Mage_Catalog_Helper_Image $imgHelper */
$imgHelper = Mage::helper('catalog/image');
if ($imgHelper->deleteImageFileOnRemoval() === Mage_Catalog_Model_Product_Image::ON_REMOVAL_DELETE) {
$filesToDelete = array_map(fn($img) => $img['file'], $toDelete);

$unusedImages = $this->_getResource()->filterUnusedImageFiles($filesToDelete, $object->getId());
$io = new Varien_Io_File();
foreach ($unusedImages as $file) {
$io->rm($this->_getConfig()->getMediaPath($file));
}
}
}

/**
* @param Mage_Catalog_Model_Product $object
* @return Mage_Catalog_Model_Product_Attribute_Backend_Media
*/
public function afterDelete($object)
{
$attrCode = $this->getAttribute()->getAttributeCode();
$value = $object->getData($attrCode);
if (!is_array($value) || !isset($value['images'])) {
return parent::afterDelete($object);
}

/** @var Mage_Catalog_Helper_Image $imgHelper */
$imgHelper = Mage::helper('catalog/image');

if ($imgHelper->deleteImageFileOnRemoval() === Mage_Catalog_Model_Product_Image::ON_REMOVAL_KEEP) {
return parent::afterDelete($object);
}

$images = array_map(fn($image) => $image['file'], $value['images']);
$unusedImages = $this->_getResource()->filterUnusedImageFiles($images, $object->getId());
$io = new Varien_Io_File();
foreach ($unusedImages as $file) {
$io->rm($this->_getConfig()->getMediaPath($file));
}

return parent::afterDelete($object);
}

/**
Expand Down
16 changes: 16 additions & 0 deletions app/code/core/Mage/Catalog/Model/Product/Image.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,22 @@ class Mage_Catalog_Model_Product_Image extends Mage_Core_Model_Abstract
*/
public const ON_DUPLICATE_SKIP = 1;


/**
* Always ask
*
* @var int
*/
public const ON_REMOVAL_KEEP = 0;


/**
* Always ask
*
* @var int
*/
public const ON_REMOVAL_DELETE = 1;

/**
* Requested width for the scaled image
* @var int
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,4 +274,24 @@ public function loadGallerySet(array $productIds, $storeId)
$this->_removeDuplicates($result);
return $result;
}

/**
* Filter gallery images that are not used by other products
*
* @param array $imageFiles
* @param int $objectId
*/
public function filterUnusedImageFiles($imageFiles, $objectId): array
{
// Duplicate product store values
$adapter = $this->_getReadAdapter();
$select = $adapter->select()
->from($this->getTable(self::GALLERY_TABLE), [
new Zend_Db_Expr('DISTINCT value'),
])
->where('entity_id != ?', $objectId)
->where('value IN (?)', $imageFiles);

return array_diff($imageFiles, $this->_getReadAdapter()->fetchCol($select));
}
}
1 change: 1 addition & 0 deletions app/code/core/Mage/Catalog/etc/config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@
<small_width>210</small_width>
<max_dimension>5000</max_dimension>
<images_on_duplicate_action>-1</images_on_duplicate_action>
<images_on_removal_action>0</images_on_removal_action>
</product_image>
<seo>
<product_url_suffix>.html</product_url_suffix>
Expand Down
10 changes: 10 additions & 0 deletions app/code/core/Mage/Catalog/etc/system.xml
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,16 @@
<show_in_website>1</show_in_website>
<show_in_store>1</show_in_store>
</images_on_duplicate_action>
<images_on_removal_action translate="label comment">
<label>Product image file handling on removal</label>
Copy link
Contributor

Choose a reason for hiding this comment

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

"Delete Product Images Management" is a concise and clear label for managing product images upon deletion. Each word must start with a capital letter as a standard convention in Magento 1 UI/UX design to maintain consistency and readability, especially in settings or configuration sections.

This label will follow the same format as the one above it in the section, 'Duplicate Product Images' – proposed in a PR I opened today.

<comment><![CDATA[Defines how product image files are handled when a product is deleted or when an image is removed from the product media gallery.<br/><br/><strong>Important:</strong> image files referenced by other products will not be deleted. Only image files that are no longer associated with any product are eligible for removal.]]></comment>
Copy link
Contributor

Choose a reason for hiding this comment

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

Modification of the sentences:

1 - "Defines how the product images are managed when a product is deleted or an image is removed from the product media gallery."

2 - "Important! Images used by other products will not be deleted. Only those that are no longer associated with any product can be deleted." - please note the exclamation mark.

3 - Important! must be in red color - here is an example from the existing code

<comment><![CDATA[<strong style="color:red">Important!</strong> Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed.]]></comment>

<frontend_type>select</frontend_type>
<source_model>adminhtml/system_config_source_catalog_imageFileHandle</source_model>
<sort_order>50</sort_order>
<show_in_default>1</show_in_default>
<show_in_website>1</show_in_website>
<show_in_store>1</show_in_store>
</images_on_removal_action>
</fields>
</product_image>
<placeholder translate="label">
Expand Down
4 changes: 4 additions & 0 deletions app/locale/en_US/Mage_Adminhtml.csv
Original file line number Diff line number Diff line change
Expand Up @@ -1321,3 +1321,7 @@
"Duplicate product without images","Duplicate product without images"
"Copy Images on Duplicate","Copy Images on Duplicate"
"'Ask' option only affects Admin interface. Default for programmatical duplication is to persist images.","'Ask' option only affects Admin interface. Default for programmatical duplication is to persist images."
"Product image file handling on removal","Product image file handling on removal"
"Defines how product image files are handled when a product is deleted or when an image is removed from the product media gallery.<br/><br/><strong>Important:</strong> image files referenced by other products will not be deleted. Only image files that are no longer associated with any product are eligible for removal.","Defines how product image files are handled when a product is deleted or when an image is removed from the product media gallery.<br/><br/><strong>Important:</strong> image files referenced by other products will not be deleted. Only image files that are no longer associated with any product are eligible for removal."
"Keep image file(s) on the filesystem","Keep image file(s) on the filesystem"
"Permanently deletes image file(s)","Permanently deletes image file(s)"
Comment on lines +1324 to +1327
Copy link
Contributor

Choose a reason for hiding this comment

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

These sentences must be changed too.

Loading