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

Fix for duplicate images that would cause multiple realtions... #483

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix for duplicate images that would cause multiple realtions... #483

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 15 additions & 6 deletions magmi/plugins/extra/itemprocessors/imageprocessor/imageitattributeemprocessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,29 +224,38 @@ public function handleVarcharAttribute($pid, &$item, $storeid, $attrcode, $attrd
}

/**
* imageInGallery
* Creates or updates image and returns its id.
*
* @param int $pid
* : product id to test image existence in gallery
* @param string $imgname
* : image file name (relative to /products/media in magento dir)
* @param int $refid
* : A reference to an attribute type, typically of image,
* small_image or thumbnail
* @return bool : if image is already present in gallery for a given product id
*/
public function getImageId($pid, $attid, $imgname, $refid = null, $store_id = 0)
{
$t = $this->tablename('catalog_product_entity_media_gallery');

$sql = "SELECT $t.value_id FROM $t ";

// Try finding matching refid
if ($refid != null) {
$vc = $this->tablename('catalog_product_entity_varchar');
$sql .= " JOIN $vc ON $t.entity_id=$vc.entity_id AND $t.value=$vc.value AND $vc.attribute_id=?
$full_sql = $sql . " JOIN $vc ON $t.entity_id=$vc.entity_id AND $t.value=$vc.value AND $vc.attribute_id=?
WHERE $t.entity_id=? AND $vc.store_id=?";
$imgid = $this->selectone($sql, array($refid, $pid, $store_id), 'value_id');
} else {
$sql .= " WHERE value=? AND entity_id=? AND attribute_id=?";
$imgid = $this->selectone($sql, array($imgname, $pid, $attid), 'value_id');
$imgid = $this->selectone($full_sql, array($refid, $pid, $store_id), 'value_id');
}

// If not found just match on type and value
if ($imgid == null) {
$full_sql = $sql . " WHERE value=? AND entity_id=? AND attribute_id=?";
$imgid = $this->selectone($full_sql, array($imgname, $pid, $attid), 'value_id');
Copy link
Owner

Choose a reason for hiding this comment

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

In case of "multistore" setup, then this select may return all images with same name than the one you try to import.

Say : Store A , item has image2.jpg, Store B has image.jpg and you want only to import "image.jpg" for image of an item in storeA.

What will happen is that the imageid will be the same than in store B.

This is not a real bug since magento image storage allows only "unique" named images,
So "image.jpg" is guaranteed to be unique in the whole magento system and as you said, magento has trouble with "duplicate" images.

Then just add a comment above to explain this "side effect" is indeed wanted and exploits magento 1 name = 1 image reference limitation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are not intended to have multiple set of media gallery images for each store.

Although you can create that using Magmi it is kind of on borrowed time since when you go and save a product in the product view for which you didn't upload the image Magento will simply insert a new row to catalog_product_entity_media_gallery_value.

You don't even have to be on the Images tab. It will happen no matter what. The same if you remove an image from a store view it will be removed from all store views. A product has a media gallery with same images

What you CAN do is set 'exclude' differently for each store view so that an image is excluded from some and not for others. (not now of course since the code pre this PR would create duplicates, that's later removed by Magento). See this article

So coming to think of it you might not even need to do the join with catalog_product_entity_varchar first. (I'll try that out).

ps: when I was talking about duplicates and uniqueness it was not with regards to system wide images but only on the same product, which is where the problem is.

Copy link
Contributor Author

@mblarsen mblarsen Sep 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I can confirm removing the catalog_product_entity_varchar check in getImageId() works just as well = to the same effect.

Updates: I guess if you do an update a new row will be created if the image on the product doesn't exists. If it does an existing image will be used and returned as the id. Still if it is an update you won't find an image doing the join since updates to catalog_product_entity_varchar happens after getImage() when the plugin returns the ovalue to the underlying engine, right?

I forgot to mention before that I am using a multistore multiwebsite setup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I tried swapping the photo of a product to another photo that already existed on the product in media gallery. This works someone fine.

The default values on the products are wrong:

  • image
  • small_image
  • thumbnail

Will all remain as they were before, on the old image. But on the store views these will be correctly swapped to the new image.

(As I see this is a different problem and has nothing to do with what changes I'm proposing here.)

Also magmi doesn't remove the the previous excludes essentially hiding the old small_image and thumbnail from the gallery 👍 .

But the default values should probably be addressed as well in another issue.

In the end if you update an image you end up with a bunch of small_image and thumbnails that are hidden from all stores. Doesn't do any harm but it would be nice to have them go away.

I've tried with the media_gallery_reset to values 0 and 1 and the column completely left out. No difference.

}

// If still not found, create it
if ($imgid == null) {
// insert image in media_gallery
$sql = "INSERT INTO $t
Expand Down