magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.56k stars 9.32k forks source link

Saving product with non-default store scope causes untouched attributes to become store scoped if loaded using ProductRepository #8897

Closed phirunson closed 11 months ago

phirunson commented 7 years ago

This is related to #7720 and #7879 and #9486

Preconditions

  1. Magento 2.4-develop

Steps to reproduce

  1. Create a simple product that belongs to multiple websites.
  2. In a script, change the current store to non-default store
  3. Load the product using the ProductRepository class
  4. Change an attribute value for the loaded product
  5. Save the product either using the ProductRepository class or calling $product->save() (deprecated)
    $this->storeManager->setCurrentStore(2);
    $product = $this->productRepository->getById(1, true, 2);
    $product->setShortDescription('Setting short desc');
    $this->productRepository->save($product); // or $product->save();

Expected result

  1. The value for short_description should be overridden, all other attributes should remain as Use default value

Actual result

  1. All store scoped eav attributes are overridden. Values other than short_description are copied over from the default store scope values.

You can see the results of this by checking the catalog_product_entity_text table and seeing that new values for not just short_description, but description and meta_keyword as well. Checking the catalog_product_entity_{varchar,int,decimal,etc} tables will also show new entries for store scope 2 even though none of that was modified.

Checking the how the admin backend handles this, it seems the Save controller requires passing in values for whether to use default values in a param use_default. (https://github.com/magento/magento2/blob/develop/app/code/Magento/Catalog/Controller/Adminhtml/Product/Initialization/Helper.php#L171) This makes it tightly coupled to the admin ui, and so it is hard to replicate proper save behaviour in code.

The use case is that products are being pushed externally via the Magento api, but this makes it so that doing store scoped save operations means anytime something is changed in default scope, it needs to also update in all store scopes independently rather than inheriting from default scope like it should.

magento-engcom-team commented 7 years ago

@phirunson, thank you for your report. We've created internal ticket(s) MAGETWO-81741 to track progress on the issue.

mustdobetter commented 6 years ago

This is also an issue when updating a product via the REST API in 2.1.19. When updating a product under a non-default store scope, all store scoped eav attributes are overridden, not just those that are being updated.

This seems to be a major bug, and so should be addressed in 2.1.x and 2.2.x, no?

versdivers commented 6 years ago

This is still an issue in CE 2.2.1. If you save a product programmaticly without touching the attributes that have a default value in that store it will save those attributes with the value from the default value and thus unsetting the 'use default value' check

koenner01 commented 6 years ago

We are also having this problem in 2.2.2

versdivers commented 6 years ago

For the ones who have this issue. You will need to use the resourcemodel to post your own mysql queries to the database. No other solution that i have found

koenner01 commented 6 years ago

We are using the api to update to product data so what you are saying is that we should trash the api Magento provides and create our own calls...

hws47a commented 6 years ago

I believe I have a related issue. When creating a product with selected website id (non-single store mode) it saves attributes (at least some of them) on store view level even if it's a new product and there are no default values.

For varchar attributes you set, there will be values for selected store_id (related to a website) and admin one, for varchar attributes with default values like media_images there will only be "no_selection" on a store level.

So if you later upload an image on admin side on default scope, an image will not appear. One more problem is that you'll have to upload the image again on store view level because there is no "use default scope" checkbox in the images section.

koenner01 commented 6 years ago

snippet of the wall of text from our cloud support ticket:

1) Setup a clean 2.2.2 installation

2) Create a new (extra) storeview: => Store='Main Website Store' => Name='Some Random Storeview' => Code='random' => Status='Enabled' => Sort Order='0'

3) Make sure the shop is using flat tables => Stores > Configuration > Catalog > Catalog > Storefront > Use Flat Catalog Category = Yes => Stores > Configuration > Catalog > Catalog > Storefront > Use Flat Catalog Product = Yes

4) Create a simple product and save it => Fill in all required attributes with random stuff => Fill in all fields with storeview/website scope in all the closed tabs => Make sure to add 2 random images and set the 1th as base/small_image/thumbnail/swatch

5) Take note of the values in the database:

Break stuff through the backend:

6) Switch the storeview to our newly created 'random' storeview => Don't change any data AND DON'T OPEN ANY TABS

8) Save the product on storeview scope => If you look at the closed tabs after the product save reloaded the page, you will see that the 'use default value' checkboxes have all been unchecked

9) Take note of the values in:

10) Switch back to the 'All store views' scope

11) Change all product attributes that have scope storeview or website with new values and save the product => Make sure to select the 2th image as base/small_image/thumbnail/swatch! => Make sure to change the meta_title, meta_keywords and meta_description

12) Take note of the values in:

13) Look at the frontend where the product is using the incorrect data on our 'random' storeview => the image used as base/small_image/thumbail/swatch is wrong => the meta data in the header is wrong

The problem can be split into multiple origins:

Break stuff through the api:

6) Update the product's name through the api: => PUT '../rest/{OurRandomStoreviewCode}/V1/products/{createdProductSku} => request body: {"product":{"name":"Test API"}}

7) Take note of the values in:

I can't even wrap my head around this problem. We are updating 1 attribute but the database is being filled with a lot of other (unwanted) values. If we now check the product through the adminhtml the same behaviour as the closed tabs was reproduced. All attributes in the closed tabs have had their 'use default value' checkbox unchecked...

jacobsenj commented 6 years ago

This is still an issue in 2.2.4

versdivers commented 6 years ago

The product attributes are solved by using the emulation to set your script to admin store id 0

use Magento\Store\Model\App\Emulation;
use Magento\Framework\App\Area;

    public function __construct(
        FilterFactory $filter,
        Emulation $emulation
)
    {
        $this->filter = $filter;
        $this->emulation = $emulation;
    }

.....
  $this->emulation->startEnvironmentEmulation(0,Area::AREA_ADMINHTML);
                $productrep->save($product);
                $this->emulation->stopEnvironmentEmulation();
....
magento2dev commented 6 years ago

Still an issue on last Magento 2 version.

ladle3000 commented 6 years ago

Same in 2.2.5.

we are running in single store mode and creating products from the configurable products and several seemingly random attribute values are getting filled.

apedicdev commented 6 years ago

Hi, is there any news about this? seems quite concerning has been reproduced on 2.1, 2.2, 2.3 but still no update.

irehmankhan commented 5 years ago

I am also having same problem. This is happening when product is updated via api on all store view in product admin grid it is showing old image though I have uploaded new image when I delete image from cache it start showing me placeholder image instead of image I have set base,small, thumbnail on all store view but it is working fine for store image.

ladle3000 commented 5 years ago

I randomly get data from other products of other store views getting copied into wrong products. Does anyone else see this weird behavior?

On Wed, Jan 30, 2019 at 7:40 AM irehmankhan notifications@github.com wrote:

I am also having same problem. This is happening when product is updated via api on all store view in product admin grid it is showing old image though I have uploaded new image when I delete image from cache it start showing me placeholder image instead of image I have set base,small, thumbnail on all store view but it is working fine for store image.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/magento/magento2/issues/8897#issuecomment-458929511, or mute the thread https://github.com/notifications/unsubscribe-auth/ACQbJYNsla6LxQJsMgWvm6YF2tQbOKRkks5vIZLIgaJpZM4Me0lk .

magento-seis10 commented 5 years ago

Still happening on EE2.2.6

abrittis commented 5 years ago

Still seeing this issue in 2.1.16. Any updates?

Also - there is sample code above by @versdivers using EnvironmentEmulation. Does this work and is there documentation anywhere that describes how EnvironmentEmulation works?

CompactCodeEU commented 5 years ago

@abrittis I can confirm that it works since we use it for a very big synchronization module with Exact Online for our customers. But except from some forums i did not find any documentation on it no (This is the new account of @versdivers )

Beagon commented 5 years ago

We're encountering the same issue here on 2.2.8 Commerce.

We have a process running that should update an attribute if some criteria are met on a store scope basis, once we save the product in this process (CRON) all attributes get overwritten with the attributes of the default scope.

The hotfix @CompactCodeEU proposed didn't work for this and the problem still persists.

zolthan commented 5 years ago

Also reproduced in Magento 2.3.0 and 2.3.1. If a product is edited in the backend in a specific store view, the images are saved store specific. This store specific setting for the images cannot be removed, as there are no "Use Default Value" checkboxes at the images.

ghost commented 5 years ago

Experience in 2.3.1, load product using product repository & on save it added store_view_code as default. on export it duplicate row with just value dded store_view_code = default

m2-assistant[bot] commented 4 years ago

Hi @engcom-Alfa. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:


magento-engcom-team commented 4 years ago

@engcom-Alfa Thank you for verifying the issue.

Unfortunately, not enough information was provided to acknowledge ticket. Please consider adding the following:

Once all required information is added, please add label "Issue: Confirmed" again. Thanks!

magento-engcom-team commented 4 years ago

:white_check_mark: Confirmed by @engcom-Alfa Thank you for verifying the issue. Based on the provided information internal tickets MC-30077 were created

Issue Available: @engcom-Alfa, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

lbajsarowicz commented 4 years ago

Please assign that to me, I am going to fix the issue.

Other entities

I tried to reproduce the issue on other Entities to make sure if it's global issue of EAV mechanism or just related to products.

Categories

ebanolopes commented 4 years ago

Hey, guys, I was having similar issue using Magento API to create products on M2 2.3.4. Calling /V1/products with website_ids: [1] element or leaving it empty and adding websites using /V1/products/{sku}/websites was resulting on having specific non default values to stores inside the associated website.

I've work around this by creating a plugin on Magento\Catalog\Model\ProductWebsiteLinkRepository::save method like below. Hope it helps someone until we have an official resolution from Magento. If you're having similar issue on another part of the code, consider using ProductWebsiteLinkRepository instead of set website_ids directly to product.

class ProductWebsiteLinkRepository
{
    /**
     * @var ProductWebsiteResourceModel
     */
    protected $productWebsiteResourceModel;

    /**
     * @var ProductRepositoryInterface
     */
    protected $productRepository;

    /**
     * ProductWebsiteLinkRepository constructor.
     * @param ProductWebsiteResourceModel $productWebsiteResourceModel
     * @param ProductRepositoryInterface $productRepository
     */
    public function __construct(
        ProductWebsiteResourceModel $productWebsiteResourceModel,
        ProductRepositoryInterface $productRepository
    ) {
        $this->productWebsiteResourceModel = $productWebsiteResourceModel;
        $this->productRepository = $productRepository;
    }

    /**
     * @param MagentoProductWebsiteLinkRepository $subject
     * @param callable $proceed
     * @param ProductWebsiteLinkInterface $productWebsiteLink
     * @return bool
     * @throws CouldNotSaveException
     * @throws InputException
     */
    public function aroundSave(
        MagentoProductWebsiteLinkRepository $subject,
        callable $proceed,
        ProductWebsiteLinkInterface $productWebsiteLink
    ) {
        if (!$productWebsiteLink->getWebsiteId()) {
            throw new InputException(__('There are not websites for assign to product'));
        }

        try {
            $product = $this->productRepository->get($productWebsiteLink->getSku());
            $websitesIds = array_merge($product->getWebsiteIds(), [$productWebsiteLink->getWebsiteId()]);
            $this->productWebsiteResourceModel->addProducts($websitesIds, [$product->getId()]);
        } catch (\Exception $e) {
            throw new CouldNotSaveException(
                __(
                    'Could not assign product "%1" to websites "%2"',
                    $product->getId(),
                    $productWebsiteLink->getWebsiteId()
                ),
                $e
            );
        }
        return true;
    }
}
LiamKarlMitchell commented 4 years ago

So in the meantime should I just send two API requests to save product data using the Rest API? One to all and one to default? (Admin and Default values get out of sync)

Can someone please explain why are there two store views/copies of same data all over the show in the EAV/DB? As I do not understand why there is an Admin and a Frontend store id when website has the single store/site enabled.

LiamKarlMitchell commented 4 years ago

So saving has been bugged since at least 2016 or earlier? How can we get an update on the status of MAGETWO-81741 fix?

LiamKarlMitchell commented 4 years ago

This also impacts the bulk api... even when using all/ it adds store id 1 when my site is single store. How frustrating, so I can't use the bulk api.

stkrelax commented 3 years ago

This is the same in the Category Repository. Having store admin is_active set to 1 and other stores is_active not set (use default)

running foreach store

                    $this->getContext()->getStoreManager()->setCurrentStore($store);

                    /** @var Category $category */
                    $category = $this->_categoryRepository->get($entity->getId(), $store->getId());
                    // some changes (but not is_active flag)

                    try {
                        $this->_categoryRepository->save($category);
                    } catch (\Exception $e) {
                    }

you end up having set is_active to all stores explizit

when you have 17 stores like us than you have 18 times is_active = 1 in DB even though one would be enough.

stephansteiner commented 2 years ago

It's 2022, and we are still having this issue with both categories and products especially when updating using the REST APi.

github-jira-sync-bot commented 2 years ago

:white_check_mark: Jira issue https://jira.corp.magento.com/browse/AC-2904 is successfully created for this GitHub issue.

m2-assistant[bot] commented 2 years ago

:white_check_mark: Confirmed by @sidolov. Thank you for verifying the issue.
Issue Available: @sidolov, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

convenient commented 2 years ago

Where is this done 👀 what commit?

CC-RobC commented 2 years ago

I do not see a commit. How is this done?

ihor-sviziev commented 2 years ago

@sidolov @sivaschenko any related commits?

abukatar commented 2 years ago

This issue was marked as "Not a bug", here is why: For REST API or for the case described here, the \Magento\Catalog\Api\ProductRepositoryInterface::save will take the action.

Implementation of \Magento\Catalog\Api\ProductRepositoryInterface::save loads Product Entity by ID according to the provided store, populates product attributes with new values, and saves the whole Product Entity in the scope of the current store.

The whole product is saved in the current scope, not regarding if some attributes had values in this scope before or not.

The solution is to set NULL values for attributes for which you want to preserve inheritance. I've tested that it helps in 2.4-develop.

convenient commented 2 years ago

I'm pretty confident that just describes the bug as it was raised years ago.

Using the API to update a single attribute of a product in a store view, it is expected that you have to list every attribute for the product and mark them as null so they fall back? That's what is being said now?

pstuifzand commented 2 years ago

I guess you need to know which attributes are store view level as well, because if you use attributes that are default, or website, these will get overwritten on these levels as well.

t-heuser commented 2 years ago

We also have the issue, bloated up our whole database 3x its size as we have 3 stores and cronjobs that touched a lot of products and used the save() function. Very neat. As I understand right, a workaround until this is fixed, is to either set all attribute values you dont want to change to null or, if you're not using the REST-Api, to use the addAttributeUpdate() function from the Product Model, which saves a specific attribute value for a specific store.

But the problem I see here is that the save() function triggers SaveHandlers, a cache clean for the saved product and reindexing for the product (just a problem if the indexes are set to update on save and not update on schedule, as update on schedule works with database triggers). Am I missing something if I say the addAttributeUpdate() function, which directly saves the value to the database, doesn't do anything of the above?

And please someone reopen this issue or show us the related commit.

ihor-sviziev commented 2 years ago

@abukatar @sidolov @sdzhepa any comments?

sanganinamrata commented 1 year ago

@abukatar @sidolov @sdzhepa I'm still facing the same issue. Any update?

ihor-sviziev commented 1 year ago

Re-opened for confirmation if this issue was fixed or not because the bot has closed the issue in https://github.com/magento/magento2/issues/8897#event-6518708220 /CC @ishakhsuvarov

MyroslavDobra commented 1 year ago

Hey @ihor-sviziev Please dont forget https://github.com/magento/magento2/issues/14622 This issue also still exists

m2-assistant[bot] commented 1 year ago

Hi @engcom-November. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

m2-assistant[bot] commented 1 year ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Hotel commented 1 year ago

Hello,

Tried to reproduce the issue in the latest 2.4-develop branch by making a custom module. But it seems the issue is not reproducible for us. Please have a look at the below screenshot for reference:

Before Updates: image

After Updates: image

We are also attaching the module for reference:

Magz.zip

Please let us know if we have missed anything.

Thanks

MarianNapi commented 1 year ago

Hey @engcom-Hotel

Please take a look at your second screenshot. As you can see every single "Use Default Value" option is deselected. This means that for example Product Name, Tax Class and so on now use a custom scope value instead of the default scope.

t-heuser commented 1 year ago

Hey @engcom-Hotel

Please take a look at your second screenshot. As you can see every single "Use Default Value" option is deselected. This means that for example Product Name, Tax Class and so on now use a custom scope value instead of the default scope.

Which proofs that the issue is still reproducible in the latest 2.4-develop branch.

engcom-Hotel commented 1 year ago

Hello @MarianNapi @oneserv-heuser,

I agree with you. Hence confirming this issue.

Thanks