Skip to content

REST API: Cannot delete custom attribute from product #10807

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.

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

Closed
siliconalchemy opened this issue Sep 7, 2017 · 29 comments
Closed

REST API: Cannot delete custom attribute from product #10807

siliconalchemy opened this issue Sep 7, 2017 · 29 comments
Assignees
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed

Comments

@siliconalchemy
Copy link

siliconalchemy commented Sep 7, 2017

Cannot delete an existing product custom attribute through the rest API.

Preconditions

  1. Magento CE 2.1.8
  2. PHP 5.6.31

Steps to reproduce

  1. Have a custom attribute with an assigned value in a product
  2. PUT product update (/rest/all/V1/products/100030774) with custom attributes altered by either by assigning custom_attribute value None or removing the key/value

Expected result

  1. Expect for the custom attribute to be removed/deleted from the product custom_attribute list

Actual result

  1. Product doesn't remove/delete. If value is assigned None/Null, I get this error:
Updating product: 100030774: Put Error: Error occurred during "custom_attributes" processing. Value is not set for attribute code "fridge_capacity" Error updating product:  HTTPError('400 Client Error: Bad Request',)

If key/value is removed from the custom_attributes, then the Put call goes through without error, but the key/value is still present within custom_attributes, unaltered.

All other values update using exactly the same mechanism/code without issue, can add/change custom attributes correctly. Just cannot delete the custom attributes.

@orlangur
Copy link
Contributor

orlangur commented Sep 7, 2017

According to https://magento.stackexchange.com/a/179153 null should do the trick. Please provide full request body.

@siliconalchemy
Copy link
Author

It's passing this:
{"attribute_code": "fridge_capacity", "value": null}

Here's the complete json:

Updating product: 100030774: put - url: http://xxx.com/rest/all/V1/products/100030774 , data: {"product": {"sku": "100030774", "status": 1, "media_gallery_entries": [], "name": "Bosch WAN28201GB Washing Machine", "type_id": "simple", "price": 389.99, "attribute_set_id": 4, "updated_at": "2017-09-07 11:29:21", "visibility": 4, "options": [], "tier_prices": [], "custom_attributes": [{"attribute_code": "short_description", "value": "Bosch WAN28201GB 1400Rpm 8Kg A+++ With Ecosilence Drive - White"}, {"attribute_code": "manufacturer", "value": "10"}, {"attribute_code": "category_ids", "value": ["10126", "10127"]}, {"attribute_code": "options_container", "value": "container2"}, {"attribute_code": "required_options", "value": "0"}, {"attribute_code": "has_options", "value": "0"}, {"attribute_code": "url_key", "value": "bosch-wan28201gb"}, {"attribute_code": "tax_class_id", "value": "2"}, {"attribute_code": "gift_message_available", "value": "0"}, {"attribute_code": "height", "value": "848"}, {"attribute_code": "width", "value": "598"}, {"attribute_code": "depth", "value": "550"}, {"attribute_code": "model", "value": "WAN28201GB"}, {"attribute_code": "colour", "value": "145"}, {"attribute_code": "sm_featured", "value": "0"}, {"attribute_code": "group", "value": "Washing Machine"}, {"attribute_code": "build", "value": "2926"}, {"attribute_code": "warranty", "value": "2022"}, {"attribute_code": "energy_rating", "value": "2785"}, {"attribute_code": "size", "value": "2794"}, {"attribute_code": "fridge_capacity", "value": null}, {"attribute_code": "spinspeed", "value": "1400rpm Max spin speed"}, {"attribute_code": "spin_speed_rpm", "value": "1400"}], "created_at": "2017-09-07 11:29:21", "id": 82351, "product_links": [], "extension_attributes": {"stock_item": {"stock_id": 1, "qty": 27, "low_stock_date": null, "is_qty_decimal": true, "use_config_notify_stock_qty": true, "use_config_min_sale_qty": 1, "use_config_manage_stock": true, "backorders": 2, "min_sale_qty": 1, "show_default_notification_message": false, "max_sale_qty": 10000, "is_in_stock": true, "is_decimal_divided": false, "manage_stock": false, "item_id": 79318, "use_config_enable_qty_inc": true, "min_qty": 0, "qty_increments": 0, "notify_stock_qty": 1, "enable_qty_increments": false, "product_id": 82351, "stock_status_changed_auto": 0, "use_config_min_qty": true, "use_config_max_sale_qty": true, "use_config_qty_increments": true, "use_config_backorders": true}}}, "saveOptions": true}

@siliconalchemy
Copy link
Author

That stackexchange link btw is the only one I came across with any answer, and looking at the code I think they might have been confused, the code clearly rejects null values to a key/value pair. There are lots and lots of other similar questions with no answers, however, so I suspect this is a real problem.

@davidmcnight
Copy link

davidmcnight commented Sep 7, 2017

I can confirm null is not working for me either. I am trying to remove an attribute. I have passed null, a blank array, an array with a value 0, an array with Null. None of them worked. When I pass null I get this: error. message
:
"Error occurred during "custom_attributes" processing. Value is not set for attribute code "conflict""

Conflict is the attribute name

The other ones get a success message but nothing is changed.

Edit: Like siliconalchemy that stackexchange is the only answer but that does not seem to be the correct answer.

@davidmcnight
Copy link

So I did something that seems to work for now -- I set the value for of 1 which is a value of another attribute and it set the attribute to null. This fixed the issue for now and I do not see any errors on the product or and the catalog/search page. I would not recommend this solution but if it is pressing this is at least a hotfix.

@magento-engcom-team magento-engcom-team added Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 11, 2017
@okorshenko okorshenko added the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Sep 14, 2017
@cieslix
Copy link
Contributor

cieslix commented Sep 17, 2017

I will take care of it

@maksek
Copy link
Contributor

maksek commented Sep 17, 2017

Hi @cieslix, can you accept invitation from github, so we can assign it on you?

@cieslix
Copy link
Contributor

cieslix commented Sep 17, 2017

@maksek yes sure, sorry for delay.

@cieslix
Copy link
Contributor

cieslix commented Sep 17, 2017

already fixed in 9e3ca7b
this feature is working as intended (however it is quite fuzzy)

@siliconalchemy
Copy link
Author

@cieslix Hi, When you say 'this feature is working as intended', do you mean before your fix? Can you explain how we can delete attributes from products in the current release?

@orlangur
Copy link
Contributor

@cieslix thanks for your finding! However, although this issue was fixed in #9314 for develop, it is still not backported to 2.1.x against which this issue was reported.

Also, the test is missing for this fix. It should be at least unit test, ideally - unit + WebAPI test.

@siliconalchemy fix not released in 2.1.x yet, for now you can apply fix from mentioned commit to your instance to get it fixed.

@siliconalchemy
Copy link
Author

Yet another bug fixed half a year ago that hasn't made it into the stable tree. This project really needs to address the braindead policy of only ever making fixes into the dev branch until enough people complain about an issue. This is an enterprise product, companies commit to a branch for a long time for lots of good reasons. Fixing bugs in point releases is a good thing, people.

@orlangur
Copy link
Contributor

@siliconalchemy this is already changed: no 2.1.x ticket closed with remark "fixed in develop" nowadays. Obviously there is no other way to apply all relevant fixes from develop to 2.1.x but discovering bugs and backporting fixes. Hope after 2.2 release bugfixes will be tracked better and delivered as 2.2.x in a timely manner.

@siliconalchemy
Copy link
Author

Confirmed this fix works when backported into 2.1.9. The path is different - vendor/magento/framework/Webapi/ServiceInputProcessor.php.

@orlangur Encouraging that process is improving. I still don't understand why 2.1.x issues are fixed in dev and closed. If it's a 2.1.x. issue and the fix isn't breaking, why isn't it applied to 2.1.x branch and released in the next point release?

@orlangur
Copy link
Contributor

The path is different - vendor/magento/framework/Webapi/ServiceInputProcessor.php.

Well, it's because you have Composer-based installation. It can be rewritten with next composer update. A proper way it to rewrite this class in your custom module.

If it's a 2.1.x. issue and the fix isn't breaking, why isn't it applied to 2.1.x branch and released in the next point release?

Have no idea, was always wondering this as well. As such process is already obsoleted we probably will never know the truth. Maybe 2.1.x was planned to have short EOL initially while 2.2.x was planned as LTS version.

@siliconalchemy
Copy link
Author

OK, so how do we get this into the next 2.1.x release?

@orlangur
Copy link
Contributor

According to my understanding @cieslix is going to work on backport. See https://community.magento.com/t5/Magento-DevBlog/Pull-Requests-for-2-1-x-Patch-Releases/ba-p/65630 for more details.

@magento-engcom-team magento-engcom-team added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Sep 19, 2017
@magento-engcom-team
Copy link
Contributor

@siliconalchemy , thank you for your report.
The fix for this issue is already available on Magento 2.2.0 branch.

@siliconalchemy
Copy link
Author

Why is this closed? It's not fixed in the current product codebase. It might be months or years before large sites migrate everything to (currently still in development ) 2.2.

@orlangur
Copy link
Contributor

@siliconalchemy it's because Magento 2.2.0 is pretty close to release.

This is not an impediment for @cieslix or anyone else to work on backport to 2.1-develop if needed.

@siliconalchemy
Copy link
Author

But 2.2 isn't released. And even when it is, the sheer magnitude of change involved means companies will be running 2.1 for a long time, until 2.2 is stable, bugfixed and all 3rd party extensions supported.
This is insane.

@orlangur
Copy link
Contributor

@siliconalchemy there is no difference if the issue is closed or not as it is managed using dashboard https://github.com/magento/magento2/projects/13#card-4672681.

@korostii
Copy link
Contributor

korostii commented Oct 1, 2017

there is no difference if the issue is closed or not

That's not completely true.
People outside of Magento (the company) still look at the issue state first, when trying to understand whether the bug is fixed in the latest release yet. The curret flow is severly misleading.

Maybe 2.1.x was planned to have short EOL initially while 2.2.x was planned as LTS version.

So that's why it felt like a "beta" version.
As in: we call it beta 'cause it's betta than nothin' :D

@orlangur
Copy link
Contributor

orlangur commented Oct 2, 2017

@korostii people will get used to Fixed in 2.x.x labels soon. Now fixVersion is much more visible than before.

@korostii
Copy link
Contributor

korostii commented Oct 2, 2017

Now fixVersion is much more visible than before.

Yes, I agree. So?

The open\closed state is even more visible and it's always present.
Having some extra labels shouln't be an excuse to ignore or mishandle issue state.

@orlangur
Copy link
Contributor

orlangur commented Oct 3, 2017

Let repository owners decide how issue states and labels shall be used 😉

Improvement suggestions are always welcomed in Community Forums, according to my understanding it includes processes improvement suggestions as well.

@korostii
Copy link
Contributor

Veeeery funny.
Let it rot in forums since the "repository owners" don't care about outsiders visiting issues here anyway and these labels should only by used by them, and the conveniences of community members can be disregarded.

I see your point but I have not attempted any actions to force any policy on others apart from underlining the deficiencies of current issue flow. My points will remain valid as long as the flow is imperfect.
And it doesn't matter who decides to impose this flow, and what are his or her or their preferred channels for receiving the incoming constructive criticism.

@siliconalchemy
Copy link
Author

siliconalchemy commented Oct 13, 2017

@korostii people will get used to Fixed in 2.x.x labels soon. Now fixVersion is much more visible than before.

When 'people' are running a proven stable 2.2, we'll be overjoyed to see a 'Fixed in 2.2.x' label. As we're not running 2.2, and a lot of these important issues were fixed in months and months before 2.2 was released, forgive us if we're slightly bemused by this workflow which to a lot of people is completely braindead. I've literally lost count of the number of issues that I've come across in 2.1 (the stable branch used by 99% of users) that I've eventually found long fixed in 2.2.
This is an opensource public github project. Don't be surprised if 'people' voice their opinions.

@orlangur
Copy link
Contributor

My points will remain valid as long as the flow is imperfect.

This is nothing more than your subjective opinion.

I've literally lost count of the number of issues that I've come across in 2.1 (the stable branch used by 99% of users) that I've eventually found long fixed in 2.2

That's exactly why default branch is now 2.2-develop instead of develop (which will be removed soon). Every single reported issue will be now fixed in 2.2.x first if it is possible considering backward compatibility constraints.

This is an opensource public github project. Don't be surprised if 'people' voice their opinions.

Surely, no objections. Let's just use proper communication channels for each separate activity, like Slack or something. Useful thoughts rotting in random GitHub issue is no better than raising them in Community Forums.

Thanks all for your valuable inputs!

@magento magento locked and limited conversation to collaborators Oct 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed
Projects
None yet
Development

No branches or pull requests

10 participants