Initial Commit of MSI module #1
Conversation
| { | ||
| public function apply(Builder $builder, Model $model) | ||
| { | ||
| $stockId = config()->has('rapidez.stock_id') ? config('rapidez.stock_id') : $this->getInventoryStockId(); |
There was a problem hiding this comment.
maybe $stockId = config('rapidez.stock_id') ?? $this->getInventoryStockId(); is enough? 🤔
There was a problem hiding this comment.
Actually the config() helper should have a fallback attribute available so
$stockId = config('rapidez.stock_id', $this->getInventoryStockId()); should suffice 🙂
https://laravel.com/docs/8.x/helpers#method-config
There was a problem hiding this comment.
@indykoning Changed to use fallback from config function itself.
| { | ||
| public function apply(Builder $builder, Model $model) | ||
| { | ||
| $stockId = config()->has('rapidez.stock_id') ? config('rapidez.stock_id') : $this->getInventoryStockId(); |
There was a problem hiding this comment.
Changed to use fallback from config function itself.
README.md
Outdated
| ##Installation | ||
| ```composer require rapidez/msi``` | ||
|
|
||
| To expose the `stock_qty` to the product detail and overview page you can add the following configuration to the ```config/rapidez.php``` file: |
There was a problem hiding this comment.
My preference here would go out to this module having it's own config file. e.g. msi.php
You can see an example here https://github.com/rapidez/strapi/blob/master/src/StrapiServiceProvider.php
where it loads the config from /config/strapi.php and if you publish the config you can override those values
There was a problem hiding this comment.
You would still need to do what @rbnmulder says here to have this var available in the JS. look here: https://github.com/rapidez/core/blob/master/config/rapidez.php#L23
There was a problem hiding this comment.
@indykoning I made an msi config with a preset of default for exposing stock.
| public function handle($request, Closure $next) | ||
| { | ||
| $stockId = DB::table('inventory_stock_sales_channel') | ||
| ->where('inventory_stock_sales_channel.type', 'website') |
There was a problem hiding this comment.
Do we know this will always be website? If so this will be fine, if not perhaps we could use the mage_run_type
https://devdocs.magento.com/guides/v2.4/config-guide/multi-site/ms_over.html#:~:text=variables%20as%20follows%3A-,MAGE_RUN_TYPE,-can%20be%20either
If i remember correctly this is store by default
There was a problem hiding this comment.
@indykoning according to Magento it should be possible to match a stocks with websit, customer, groups, etc. to form a sales channel. But I can't seem to find the proper documentation for that. I currently can set website and it's also the default type. https://github.com/magento/inventory/blob/develop/InventorySalesApi/Api/Data/SalesChannelInterface.php#L29
| * @param Closure $next | ||
| * @return mixed | ||
| */ | ||
| public function handle($request, Closure $next) |
There was a problem hiding this comment.
Perhaps we could cache the result in the user's session or something similar somehow.
I don't expect this to change much while someone is using the website and it saves on a query running every request
There was a problem hiding this comment.
I updated this part of the PR with a cache layer.
There was a problem hiding this comment.
@indykoning I added the website_id to the cache key. You raised a good point.
| */ | ||
| public function handle($request, Closure $next) | ||
| { | ||
| $stockId = Cache::rememberForever('stock_id', function () { |
There was a problem hiding this comment.
It might be best to add the website id to the cache key in order to prevent the incorrect stock id returning for the different websites
No description provided.