Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I have used a few deprecated functions that will be removed in D9 those should all be replaced.
Those include: db_insert & db_update
Comment | File | Size | Author |
---|---|---|---|
#10 | stock_2685809_10.patch | 18.68 KB | guy_schneerson |
#5 | 2685809--5.diff | 18.41 KB | steveoliver |
Comments
Comment #2
Nikhilesh Gupta CreditAttribution: Nikhilesh Gupta as a volunteer and at Melity commentedComment #3
Nikhilesh Gupta CreditAttribution: Nikhilesh Gupta as a volunteer and at Melity commentedComment #4
steveoliver CreditAttribution: steveoliver commentedTaking this on.
Comment #5
steveoliver CreditAttribution: steveoliver commentedThis cleans up the API and .install database calls. Also cleaned up the API with tighter functions, re-using one once and making the names of the methods more intuitive.
See PR: https://github.com/steveoliver/commerce_stock/pull/1/files
Patch attached.
Comment #6
kscheirerPatch applies cleanly and looks like a great improvement - the refactoring of method names is much clearer. I had a few comments on the pull request but they're all minor.
Comment #7
guy_schneerson CreditAttribution: guy_schneerson commentedThanks steveoliver and sorry its taken me so long to get around to this.
Patch looks great and will go over it in detail. I agree the renaming is a good idea, some of the function names got a bit out of control, however we should have probably separated to another issue as not all instances got renamed this includes the interface the class is implementing, but no big deal, I should be able to figure it out. Great work and thanks
Comment #8
guy_schneerson CreditAttribution: guy_schneerson commentedMay have jumped the gun, I think I was mixing the getStockLevel in the StockManager and the renamed getTotalStockLevel in the StockStorageAPI.
Comment #9
guy_schneerson CreditAttribution: guy_schneerson commentedHi A couple of issues I have found:
1) Getting stock level stopped working
The renamed getStockLocationLevel into getLocationsStockLevels had code removed, correlates to your github comment "The previous code was calculating the real-time current quantity across all stores by combining the computed levels with the as yet un-processed transactions' quantities. Is this something we really want to be doing?"
A: Yes it is essential to total the "un-processed" transactions as the real stock is the cs_inventory_location_level (processed) + total of unprocessed transactions from cs_inventory_transaction. I can also confirm that the stock widget is no longer showing the correct number.
2) Calculating the location level will not work correctly first time for a product.
Haven't tested this but looks like the logic changed a little, but easy to fix and breaking it down to smaller functions is great.
The original function started of by checking if the cs_inventory_location_level exist and if not insert it with zeroes and later on did an update.
The new function uses the same condition at the end and if found updates otherwise inserts the record with zeroes, at this point it should insert the values used in the update ($qty & $last_txn).
so replace:
with
Comment #10
guy_schneerson CreditAttribution: guy_schneerson commentedI have made the two changes. updated patch attached.
and tested both the calculation of available stock and the cron job that sets cs_inventory_location_level. Both work correctly and the user always gets the corect available stock.
Comment #12
guy_schneerson CreditAttribution: guy_schneerson commentedcommitted the code. Thanks again @steveoliver for your help with this :)