Refactor \Order\Shipment\Save Controller to use ResultInterface

[JeroenVanLeusden]

Description (*)

Refactor \Order\Shipment\Save Controller to use ResultInterface

Fixed Issues (if relevant)

1. Found references to internal ticket MAGETWO-28356: Refactor controller actions to new ResultInterface.

Manual testing scenarios (*)

1. Create shipment and see if redirects are working.

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)

[magento-engcom-team]

Hi @JeroenVanLeusden. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[ihor-sviziev]

@magento-engcom-team give me test instance

[magento-engcom-team]

Hi @ihor-sviziev. Thank you for your request. I'm working on Magento instance for you

[magento-engcom-team]

Hi @ihor-sviziev, here is your new Magento instance.
Admin access: https://pr-20057.instances.magento-community.engineering/admin
Login: admin Password: 123123q

[ihor-sviziev]

Hi @paliarush,
I'd like to mark

magento2/app/code/Magento/Backend/App/AbstractAction.php

Lines 323 to 337 in b475068

	/**
	* Forward to action
	*
	* @TODO MAGETWO-28356: Refactor controller actions to new ResultInterface
	* @param string $action
	* @param string|null $controller
	* @param string|null $module
	* @param array|null $params
	* @return void
	*/
	protected function _forward($action, $controller = null, $module = null, array $params = null)
	{
	$this->_getSession()->setIsUrlNotice($this->_actionFlag->get('', self::FLAG_IS_URLS_CHECKED));
	return parent::_forward($action, $controller, $module, $params);
	}

and

magento2/app/code/Magento/Backend/App/AbstractAction.php

Lines 308 to 321 in b475068

	/**
	* Set redirect into response
	*
	* @TODO MAGETWO-28356: Refactor controller actions to new ResultInterface
	* @param string $path
	* @param array $arguments
	* @return \Magento\Framework\App\ResponseInterface
	*/
	protected function _redirect($path, $arguments = [])
	{
	$this->_getSession()->setIsUrlNotice($this->_actionFlag->get('', self::FLAG_IS_URLS_CHECKED));
	$this->getResponse()->setRedirect($this->getUrl($path, $arguments));
	return $this->getResponse();
	}

as deprecated with description that we need to use result object instead. What do you think? Can we add deprecation for them?

[ihor-sviziev]

Looks like here still few issues:

1. added not needed dependency: Refactor \Order\Shipment\Save Controller to use ResultInterface #20057 (diff)
2. here is regression, previously there were if-else, but now it will execute both parts.
   Refactor \Order\Shipment\Save Controller to use ResultInterface #20057 (diff)
3. In one place there still NOT return result object, but setting response body via $this->getResponse()->representJson($responseAjax->toJson()).. Would be better to fix all places to return ResultInterface
   Refactor \Order\Shipment\Save Controller to use ResultInterface #20057 (diff)
4. There are some unit tests failures. Please fix them.

There also integration tests failures, but I think they should NOT be related to this change, so it's actually ok.

[JeroenVanLeusden]

@ihor-sviziev Should be all solved now.

[ihor-sviziev]

Now everything is fine, will fix one really minor issue little bit later and approve your pr

[magento-engcom-team]

Hi @sivaschenko, thank you for the review.
ENGCOM-3831 has been created to process this Pull Request

[ghost]

Hi @JeroenVanLeusden, 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.

[magento-engcom-team]

Hi @JeroenVanLeusden. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.