-
Notifications
You must be signed in to change notification settings - Fork 248
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
Msi/source management ui #28
Conversation
</item> | ||
</argument> | ||
<field name="source_id"> | ||
<argument name="data" xsi:type="array"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use also settings, instead of data -> config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Term
Source
should be used instead ofWarehouse
- UI Component declarations should stick to the up-to-date format (please see integration test failures)
/** | ||
* Class AbstractButton | ||
*/ | ||
abstract class AbstractButton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to avoid usage of abstract classes in such cases.
We do not encourage usage and introduction of the new inheritance based APIs
* | ||
* @return bool | ||
*/ | ||
protected function isEditMode() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid using protected methods
* | ||
* @return null | int | ||
*/ | ||
protected function getSourceId() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid using protected methods
try { | ||
$sourceId = $this->context->getRequest()->getParam('id'); | ||
return $this->sourceRepository->getById($sourceId)->getId(); | ||
} catch (NoSuchEntityException $exception) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exception must be processed
/** | ||
* @return string | ||
*/ | ||
private function getDeleteUrl() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is not really needed, as it is used only once. You may inline this code in this case.
/** | ||
* @var \Magento\Backend\Model\View\Result\ForwardFactory | ||
*/ | ||
protected $resultForwardFactory; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magento does not encourage inheritance-based API. This method should be private.
/** | ||
* @var \Magento\Framework\View\Result\PageFactory | ||
*/ | ||
protected $resultPageFactory; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magento does not encourage inheritance-based API. This property should be private.
public function getButtonData() | ||
{ | ||
$data = []; | ||
// This part should be refactored. Implemented for test purposes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- There is no reason to get Source by Id from SourceRepository, since delete controller will validate it anyways.
- We should research building delete url on frontend, to avoid getting Request in block, which is very bad.
…d for shipping origin
# Conflicts: # dev/tests/functional/tests/app/Magento/CatalogInventoryConfigurableProduct/Test/TestCase/CreateConfigurableProductEntityTest.xml
-- fix static tests
…ement-ui # Conflicts: # composer.lock
Fixed issues: - MAGETWO-64249 Issue with integration test Magento/Setup/Console/Command/I18nCollectPhrasesCommandTest.php on Travis CI - MAGETWO-62271 Inconsistent saving of Stock Data
…ribute to a product
-- index action
…AGETWO-69690-PR-9690
…d for shipping origin
…cts with fixed price
…d for shipping origin
…ess on Travis #9872 - Merge Pull Request magento/magento2#9872 from davidalger/magento2:feature/fix-travis-tests
Fixed an issue: - MAGETWO-62227 Unable to sort attributes table when trying to Add Attribute to a product
…-upload-Favicon-Icon' into Okapis-develop-pr
…s' into Okapis-develop-pr
Public Pull Requests magento/magento2#9943 magento/magento2#9941
Fixed issue: - MAGETWO-69515: Static files are deployed too slow for multiple locales
MAGETWO-67431 Functional test \Magento\ConfigurableProduct\Test\TestCase\CreateConfigurableProductEntityTest fails randomly
… into Okapis-develop-pr
…Stock' into Okapis-develop-pr
…heck' into Okapis-develop-pr
…Okapis-develop-pr
[Okapis] P1 P2
-- add carriers links assignment
-- add carriers links assignment
-- add carriers links assignment
-- update composer.lock
Internal build |
Description
Fixed Issues (if relevant)
Manual testing scenarios
Contribution checklist