[TASK] Always setSaveRewritesHistory to 1 for API calls so there will…

[lewisvoncken]

… be created a SEO rewrite for the old url

Description

Permanent Redirect for old URL missing via API

Fixed Issues (if relevant)

1. Permanent Redirect for old URL missing via API or no documentation #16789: Permanent Redirect for old URL missing via API or no documentation

Manual testing scenarios

1. Update a product by a PUT request through the REST API: V1/products/{sku}

{
  "product": {
  "id": 1,
  "sku": "{sku}",
  "name": "Test Product Default",
  "attribute_set_id": 4,
  "price": 10,
  "status": 1,
  "visibility": 4,
  "type_id": "simple",
  "created_at": "2018-10-05 19:07:16",
  "updated_at": "2018-10-05 19:07:45",
  "extension_attributes": {
    "website_ids": [
      1
    ],
    "stock_item": {
      "item_id": 1,
      "product_id": 1,
      "stock_id": 1,
      "qty": 0,
      "is_in_stock": false,
      "is_qty_decimal": false,
      "show_default_notification_message": false,
      "use_config_min_qty": true,
      "min_qty": 0,
      "use_config_min_sale_qty": 1,
      "min_sale_qty": 1,
      "use_config_max_sale_qty": true,
      "max_sale_qty": 10000,
      "use_config_backorders": true,
      "backorders": 0,
      "use_config_notify_stock_qty": true,
      "notify_stock_qty": 1,
      "use_config_qty_increments": true,
      "qty_increments": 0,
      "use_config_enable_qty_inc": true,
      "enable_qty_increments": false,
      "use_config_manage_stock": true,
      "manage_stock": true,
      "low_stock_date": "2018-10-05 19:07:45",
      "is_decimal_divided": false,
      "stock_status_changed_auto": 1
    }
  },
  "product_links": [],
  "options": [],
  "media_gallery_entries": [],
  "tier_prices": [],
  "custom_attributes": [
    {
      "attribute_code": "options_container",
      "value": "container2"
    },
    {
      "attribute_code": "url_key",
      "value": "test-product-default-new"
    },
    {
      "attribute_code": "gift_message_available",
      "value": "2"
    },
    {
      "attribute_code": "required_options",
      "value": "0"
    },
    {
      "attribute_code": "has_options",
      "value": "0"
    },
    {
      "attribute_code": "meta_title",
      "value": "Test Product Default"
    },
    {
      "attribute_code": "meta_keyword",
      "value": "Test Product Default"
    },
    {
      "attribute_code": "meta_description",
      "value": "Test Product Default "
    },
    {
      "attribute_code": "tax_class_id",
      "value": "2"
    },
    {
      "attribute_code": "category_ids",
      "value": []
    }
  ]
},
  "saveOptions": true
}

2. Expected Result Product URL changes to domain/test-product-default-new
3. Now when visiting the old url it should redirect to the new url but it shows a 404

Apply Changes and the URL Rewrite History will successfully be updated. See screenshots for more information:

Before:

Incorrect Result After Update Call:

After Fix:

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)

[orlangur]

Why do we need to create a new observer here? How it works in admin?

[davidverholen]

@orlangur in adminhtml a plugin is defined https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/CatalogUrlRewrite/etc/adminhtml/di.xml#L23

however, the key 'url_key_create_redirect' has do be set on the product in order to trigger history saving, which might not be possible without changes to the product api?

https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/CatalogUrlRewrite/Plugin/Catalog/Controller/Adminhtml/Product/Initialization/Helper.php#L38

EDIT1: also, the method, the plugin is on, is not executed in an api request. So it can't be handled with the same plugin

EDIT2: The correct change would probably be, to have this logic in 1 plugin on the productreposítory. But, I'm not sure, that the product in adminhtml is saved via repository. Also, a question would be, if this key in the product should be set to trigger the history saving, or the configuration under catalog -> catalog -> seo should handle it for the api

[hostep]

In my opinion, this should work the same as with the product importer or the adminhtml.
A user should be able to decide if he/she wants to create a redirect by setting the flag save_rewrites_history or not, and based on the presence of that flag it should either create a redirect or not create one.
This PR seems to suggest it should always be created, which isn't in line with how the adminhtml or the product importer works currently.

Has it already been tested if it works by simply providing that save_rewrites_history flag in the data which is send to the API endpoint?

There was recently a PR (#20344) submitted which fixed this in the product importer, maybe this can help as inspiration for how this can be implemented on the API side if it turns out this doesn't already work?

[orlangur]

Agree with @hostep, we should take into account all scenarios.

Closing due to inactivity. Feel free to reach me out anytime later if you wish to continue work on this pull request and it will be reopened.
Thanks for collaboration @lewisvoncken!

[ghost]

Hi @lewisvoncken, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.