The current data model has a number of problems which must be fixed. This will break backwards compatibility:

1) Entity types and their tables need to be prefixed with "commerce_". Pricelist needs to be one word to match the module name.
So, "commerce_pricelist" and "commerce_pricelist_item", not "price_list" and "price_list_item".

2) The "stores" pricelist field should be a base field, not a configurable field. It is configurable on some Commerce entity types (such as Products) because of core limitations, resolved around 8.5.0. Price lists should no longer reference items, since there can potentially be thousands of them. The "purchased_entity" price list litem field should be "purchasable_entity". Open question on whether price lists should have an owner, promotions don't.

3) Variations reference pricelist items (via $variation->field_price_list_item). This means that each price list item ID must be loaded when a variation is displayed on the add to cart form or elsewhere, even though there are potentially thousands, and they're not really needed.

We must remove this field. Instead, the PriceResolver should use an EntityQuery to load matching price list items.

4) An EntityQuery for price list items needs to know the purchasable entity type. We currently have no such field on the price list item entity, which is the same problem that order items have. Matt had the idea to kill two birds with one stone: we remove the commerce_price_list_item_type entity type, and generate an automatic bundle per purchasable entity type (commerce_product_variation, commerce_product_bundle). This gives us something to query on and simplifies the code.

5) There is an open question on what to do with the parent commerce_price_list_type entity type (do the same per-purchasable-entity-type bundle generation?). Price lists are never rendered, so "Manage display" is useless. And I'd envision most custom fields being used to alter the price list resolving query in same way, which requires code anyway.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz created an issue. See original summary.

bojanz’s picture

bojanz’s picture

Open question on whether price lists should have an owner, promotions don't.

And neither do shipping methods. Let's skip it for now.

5) There is an open question on what to do with the parent commerce_price_list_type entity type (do the same per-purchasable-entity-type bundle generation?). Price lists are never rendered, so "Manage display" is useless. And I'd envision most custom fields being used to alter the price list resolving query in same way, which requires code anyway.

Decided to proceed with auto-generated bundles, because it simplifies (create) access checking significantly, to be able to ask "what is the price list type for this price list item type". It also allows filtering price lists without looking at the bundle entities.

bojanz credited mglaman.

bojanz’s picture

Updating credits.

bojanz’s picture

Current progress. The data model is complete, I want to expand access, the UI, tests.

I first renamed price_list and price_list_item entity types to commerce_price_list and commerce_price_list_item, but then realized that it has an underscore between price and list, which doesn't match the module name. So I caught myself constantly typing the wrong version. Now unified (commerce_pricelist and commerce_pricelist_item).

Note that commerce_pricelist no longer stores item IDs. There can potentially be thousands of them, so it's not feasible.

Status: Needs review » Needs work

The last submitted patch, 6: 2986269-6-price-list.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

lawxen’s picture

Fix the failing test(coding standards).

But how to add commerce_pricelist_item? I don't find the add link on the pricelist page.

bojanz’s picture

You can't, the UI is incomplete :)

I'll push the final version in a few hours.

bojanz’s picture

Status: Needs work » Needs review
FileSize
165.32 KB

Final patch. Complete UI, expanded UI tests, rewritten PriceListPriceResolver.

Status: Needs review » Needs work

The last submitted patch, 10: 2986269-10-price-list.patch, failed testing. View results

  • bojanz committed b0941ec on 8.x-2.x
    Issue #2986269 by bojanz: Rebuild the data model for better performance...
bojanz’s picture

Status: Needs work » Fixed

Applied the #8 fixes on commit, and pushed. The rewrite has landed!

bojanz’s picture

Issue summary: View changes

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.