MSI-1486: Create new infrastructure for StockItem Configuration #1553
Conversation
| ->where('config_option = ?', $configOption) | ||
| ->limit(1); | ||
|
|
||
| if (isset($stockId)) { |
There was a problem hiding this comment.
Hi @seruymt, I don't understand why, when $stockId, $sourceCode, and $sku are set you add the IS NULL clause.
I may be wrong but shouldn't it be the opposite?
Or am I missing something?
Thank you
There was a problem hiding this comment.
Hi @aleron75, thanks for your comment. This is a way how we divide config option on scopes (Stock, StockItem, Source, SourceItem and Global). You can read more about it here - https://github.com/magento-engcom/msi/wiki/Inventory-configuration-Design-and-DB-structure
There was a problem hiding this comment.
Hi @seruymt I've read the document you pointed out. There, for example, I read:
To retrieve Source level configuration options we need to execute next query:
select * from inventory_configuration where source_code = %source_code% and sku is NULL and stock_id is NULL
as far as I understand by the example above, to retrieve a value at source level we only use the following match: source_code = %source_code%
so I still miss the point in committed code when, for example at line 61 of GetConfigurationValue class, if $sourceCode is set, we generate a $select->where('source_code IS NULL'); statement instead of a $select->where('source_code = ?', $sourceCode);
Thanks
There was a problem hiding this comment.
@aleron75 oh, it's my fault.. you're totally right.. (in previous version I've used "empty" instead of "isset"). I'll fix it, thanks:)
| --> | ||
| <config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd"> | ||
| <type name="Magento\InventoryAdminUi\Ui\DataProvider\StockDataProvider"> | ||
| <plugin name="inventory_stock_configuration" type="Magento\InventoryConfigurationAdminUi\Plugin\InventoryAdminUi\Ui\StockDataProvider\StockConfigurationPlugin" /> |
There was a problem hiding this comment.
We should choose plugin names that convey the idea of what they do; in this case what about using name="add_stock_configuration_data"?
| <plugin name="inventory_stock_configuration" type="Magento\InventoryConfigurationAdminUi\Plugin\InventoryAdminUi\Ui\StockDataProvider\StockConfigurationPlugin" /> | ||
| </type> | ||
| <type name="Magento\InventoryAdminUi\Ui\DataProvider\SourceDataProvider"> | ||
| <plugin name="inventory_source_configuration" type="Magento\InventoryConfigurationAdminUi\Plugin\InventoryAdminUi\Ui\SourceDataProvider\SourceConfigurationPlugin" /> |
There was a problem hiding this comment.
We should choose plugin names that convey the idea of what they do; in this case what about using name="add_source_configuration_data"?
| */ | ||
| --> | ||
| <config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd"> | ||
| <module name="Magento_InventoryConfigurationAdminUi" setup_version="1.0.0"/> |
There was a problem hiding this comment.
We don't need to use the setup_version since we are using declarative schema definition
|
@aleron75 Also, this is just a prototype, we still work on it:) If you have any ideas how to improve interfaces or other parts of InventoryConfiguration feel free to share with us |
|
I am working on documentation drafts for the new options, and have a question. The custom Source and Stock pages and Product Advanced Options have checkboxes for Use Config Settings. Is this the default config options set on Source and Stock? Or the default System Configs set in Stores > Configuration > Catalog > Inventory? Or was this just a naming issue for the checkbox, and it should be Use system value?
|
# Conflicts: # app/code/Magento/InventoryLowQuantityNotificationAdminUi/view/adminhtml/ui_component/product_form.xml # composer.lock
|
@lorikrell, There are several layers of configuration. So, the longest chain currently may be: |
|
Thank you for the info, @nmalevanec! That's great info! I understand the fall back relationship of Product setting > Source settings / Stock settings > System settings. The naming of the option was different than the setting everywhere else. Just wanted to make sure it was intended. I'll add detailed info based on your feedback, and give a diagram to users for understanding it. And will update the docs with the checkbox name. |
Wrong database isolation between tests or missing cleanups
…error with fixture generation test)
MSI-1486: Create new infrastructure for StockItem Configuration (fix …
The applied settings were equivalent to default settings, so they are useless.
Isolation is not needed anymore in the modified tests and it is causing issues with DDL operations
Msi 1486 tests isolation
|
latest tests run on Jenkins - https://m2build-ur.devops.magento.com/job/All-User-Requested-Tests/6177/ |
|
link to the latest builds run https://m2build-ur.devops.magento.com/job/All-User-Requested-Tests/6217/ |
|
tests on 2.3-develop for comparison https://m2build-ur.devops.magento.com/job/All-User-Requested-Tests/6340/ |
|
@magento-engcom-team give me test instance |
|
Unfortunately we have to close this PR now as it seems very outdated and in a state beyond repair. Another effort to implement the same feature is now happening in #2297 Thanks |
#1486