Products flattened data structure #741
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Conflicts: packages/reaction-core/private/data/Products.json
Conflicts: packages/reaction-core/client/helpers/products.js packages/reaction-core/client/templates/products/productGrid/productGrid.js packages/reaction-core/server/methods/products.js packages/reaction-sample-data/private/data/Products.json
build is broken;
reflect changes in collection2;
Conflicts: .meteor/versions packages/reaction-collections/server/publications/products.js packages/reaction-core/server/methods/cart.js
- schema modified;
- `products/publishProduct` reworked; - `products/createProduct` reworked; - `products/updateProductTags` now works; - `products/removeProductTag` now works; - `products/setHandleTag` now works; - `products/updateVariant` now works;
at least this is important for `inventory/register`. - created `ReactionCore.getVariants` method; - `inventory/adjust` modified, but not completed; - working on `inventory/register`;
Conflicts: packages/reaction-core/server/methods/products.js packages/reaction-inventory/server/methods.js
called after collection.remove; - `inventory/adjust` need to cover situation when product recieved (is it happen?) but else should work fine; - small modifications in `products/createVariant`;
with product.type = "variant"; - fixed bug in `products/cloneVariant` when variants were cloned not in correct order: options first, then parent variant; - now we remove `inventoryQuantity` when cloning product;
Conflicts: .meteor/versions
all products and variants; - now user can't add media if product has no variants; - added `Products.after.insert` hook for `inventory`; - small fixes; I need es6 export/import functionality, so I will try to update meteor to 1.3-beta now;
Conflicts: .meteor/versions
errors; - ReactionCore.Schemas.PriceRange schema added for denormalization of a variant price range to product. The goal here is not fetch products + all their variants within `Products` publications. 10 products + each has 10 variants - it's too much for us; - added more fields for denormalization: * inventoryManagement; * inventoryPolicy; * inventoryQuantity; * lowInventoryWarningThreshold; All these fields needed for notices to `productGrid`; - few product helpers temporary moved to common global scope; - Now we are running denormalization from three methods: * products/updateVariant; * products/deleteVariant; * products/updateProductField; - fixes of media usage;
Conflicts: .meteor/packages .meteor/versions
- fix few typos here and there; - `cartTransform` methods refactored; - added `getSummary` function; - `products/updateVariants` removed. It is were replaced by new `products/updateVariantsPosition` method; - variant sorting reworked; - `schema.productVariant.index` from now will keep the top-level variant list index; - fix all broken tests. Added few new tests, and removed few unnecessary tests; - removed a lot of `this.unblock()`, because I think this is not needed, maybe even bad for our cases; - `minOrderQuantity` field added to productVariant schema, due to reactioncommerce#225; - added `flushQuantity` function to update to zero top-level variant quantity. - added `getVariantQuantity` common function, and the same called template method, which returns the summary of all options if their exists; - added `denormalize` function; - added `isSoldOut` function; - added `isLowQuantity` function; - added `isBackorder` function; All this new function is useful for denormalizing of productVariant SS fields; - `inventory/register`, `adjust` updated. Implemented `initializeUnorderedBulkOp()` instead of `batchInsert`; - A lot of UI updates due to compatability with flattened model; - revert changes from reactioncommerce#632; - products fixture modified; - `shopIdAutoValue` method modified. Now it is ignore update operations;
Add collection2 submodule.
ReactionImport: fixing the workaround for variant document identification
Update collection2 commit.
- fixed `reaction test` flow;
|
|
I'm not sure how I missed this until today, but after a quick overview, it seems like this simplifies a lot of the complexity surrounding products, variants, and inventory management. I'll start watching this issue more closely, and echo the sentiment that great test coverage will be essential before a merge. Is the |
|
@spencern I mentioned to @aldeed, hopefully he'll get time to review the PR that @newsiberian has pending this week. I think that's our primary blocker right now. |
|
I've started adding "acceptance tests" to the reaction-doc repo. If the idea of writing automated tests seems daunting we could start by writing a few simple manual tests that would just mimic how you are testing this as you develop. Then these tests can be part of the acceptance suite that we do before a release. Just a suggestion. |
|
We reviewed this PR in the weekly meeting and it's ready to be merge pending:
I am willing to work on a test plan with somebody if they want some help. |
|
Merge to collections2 repo has been completed now. |
Conflicts: packages/reaction-accounts/tests/jasmine/server/integration/accounts.js packages/reaction-core/package.js packages/reaction-core/tests/jasmine/server/integration/products.js packages/reaction-schemas/common/globals.js packages/reaction-schemas/package.js
- merged with development;
added. |
Conflicts: packages/reaction-collections/server/publications/products.js packages/reaction-core/server/import.js
merged products publication;
aaronjudd
pushed a commit
that referenced
this pull request
Mar 1, 2016
…cture Products flattened data structure
This was referenced Mar 16, 2016
Merged
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello, @tdecaluwe and I are introducing new way to work with products. This work is the result of the following conversation. The very beginning changelog could be found here: reactioncommerce/reaction-catalog#1.
So, the flattened multi schema – a way to keep documents with different schemas inside one collection. Combining with ancestors data structure, we expect a speed boost from this approach. And, in my personal opinion, this is greatly simplified the logic of the products system.
There are a lot of changes that were done, so we need to test this PR carefully and a lot. To make flattened multi-schema works we contributed changes into
collection2packages, but these changes so far hasn't approved and applied by aldeed. So, I hope, you help me to polishcollection2changes to make it more appealing.Perhaps the most daring change relates to Product publication. Currently we have all our products variants inside products docs. By getting products we have all in one. With flattened schema this is unbelievable. Instead of getting 10 product docs + 10 * 3-5-10 variants doc we fetch only 10 products docs. All data we need for
productGridwe get via denormalization. Images we get via manipulations withmedia.metadata.productId. Also I like that we do computation only one time on write operation instead of every time on read operations. I've tested with 300 products in development branch vs this PR, and from the first glance this PR works a little bit better but still Blaze is the weak place.Also, as a result of this conversation, the following logic was implemented for now:
Here is a approximate changelog:
Schemas
ProductVariant
ancestors: Array
new field which contains ancestors of item. Currently it could be one or two
_idsparentId
Removed
cloneId
Removed
index: Number
Previously was a String. Since implementing of flattened model this property is used for keeping array index. This is needed for moving variants through list (i.e. drug'n'drop).
minOrderQuantity: Number
Minimum order quantity. It is used inside cart
quantityProcessingfunction. This change somehow covers #225.shopId: String
We need it.
Product
ancestors: Array
For a product – ancestors array is always clean for now.
cloneId
Removed
variants
Removed
price: String
Keep in mind – this is a string. It is needed for denormalizing variants prices to display it in
productGrid.isLowQuantity: Boolean
Indicates when at least one of variants
inventoryQuantityare lower then theirlowInventoryWarningThreshold.isSoldOut: Boolean
Indicates when all variants'
inventoryQuantityis zeroisBackorder: Boolean
Indicates when the seller has allowed the sale of product which is not in stock
shopIdAutoValue
Method was modified. Now it ignores update operations.
Methods
Removed a lot of lines with
this.unblock()after I read what it does)) Please, read this part of guide, and few articles from Kadira. I think this should be a fine tunning tool with understanding that putting it here really do not have any side effects. It is really dangerous to put it in almost every method, like I did before)Products
Almost all methods were rewritten. Some were removed.
products/cloneVariant
now accept only two arguments:
productId(String), variantId(String)products/createVariant
if we are inserting a child variant, we ran an additional function
flushQuantity()which clean parent quantity and inventory records.products/updateVariant
This method now is used only in one place – top-level variant autoform. It calls
denormalizefunction to keep product denormalized fields up to date.products/updateVariants
Method removed It is not needed anymore. It was used to manage UI sorting stuff. Now
products/updateVariantsPositiondo this. It updates onlyindexfield.products/deleteVariant
Also fires
denormalizefunctionproducts/cloneProduct
Now this method is really working as expected. It supports top-level variants and options media cloning.
products/updateProductField
Now work with all type of items. It also fires
denormalizefunction.products/updateVariantsPosition
New Method It updates top level variant position index (while UI sorting).
Cart
cart/copyCartToOrder
Now fires
orders/inventoryAdjustmethod.Inventory
inventory/register
batchInsertpackage was removed. Method now used nativebulkOpinventory/backorder
Now use
bulkOpCollections
ReactionCore.Helpers.cartTransformwas refactored. All computations were moved togetSummaryfunction.Hooks
ReactionCore.Collections.Products.before.updaterenamed intoReactionCore.Collections.Products.after.removeReactionCore.Collections.Products.after.insertwhich previously wasmodifier.$pushofafter.updateFunctions
Template products helpers got a lot of changes. Besides, some of them we now using on server side, so the idea is to move part of this methods to
/commonfolder to global variable (ReactionCore) and after 1.3 make them export/imported.Currently the common functions are
getProductPriceRange
no need to describe
getVariantPriceRange
the same
getVariantQuantity
New method It calculates a sum of descendants
inventoryQuantity.getVariants
New method Get all parent variants.
getTopVariants
New method Get only product top level variants
Server functions
denormalize
Function denormalized variants fields to product.
isSoldOut
We are stop accepting new orders if product marked as
isSoldOut.isLowQuantity
If at least one of the variants is less than the threshold, then function returns
true.isBackorder
Indicates, is product variants is still available to be ordered after summary variants quantity became zero.
flushQuantity
if variant
inventoryQuantitynot zero, function update it to zero. This needed in case when the option with it's owninventoryQuantityis created inside top-level variant. In that case top-level variant should display sum of his optionsinventoryQuantityfields.How to run
You need four packages to run this.
collection2 branch
mastercollection2-core branch
flattenedreaction-factories branch
flattenedreaction-catalog branch
masterI would like to attract attention of @spencern because he use Reaction in production, and there are a lot of changes here.