Skip to content

Fixed query issues and added ability to pre-filter purchase patterns#3

Merged
Tam merged 1 commit intoethercreative:v1-devfrom
jmauzyk:v1
Dec 17, 2019
Merged

Fixed query issues and added ability to pre-filter purchase patterns#3
Tam merged 1 commit intoethercreative:v1-devfrom
jmauzyk:v1

Conversation

@jmauzyk
Copy link
Copy Markdown

@jmauzyk jmauzyk commented Dec 12, 2019

Changes proposed in this pull request:

Change ProductQueryExtended join

Currently, using innerJoin() only returns products that have a record in the purchase_counts table. As a result, it could render the paddingQuery useless, especially in cases such as new shops where a majority of products may not have any purchase records. Using leftJoin() resolves this issue.

Exclude purchase pattern IDs from paddingQuery

Currently product IDs queried from the purchase_patterns table are not excluded from the paddingQuery. This can result in returning a query with fewer products that the designated limit when a duplicate ID is returned by the paddingQuery. The _mergeIds() method examines any existing id property of the paddingQuery and excludes any of the product IDs found in purchase_patterns before executing the query.

Add ability to filter IDs queried from purchase_patterns

This was something I tried to resolve in a previous pull request but was not merged for the reasons outlined in this comment. While I initially thought I could resolve the issue using method suggested in the comment, I found that there are scenarios (especially when using a limit) where it does not work as desired.

{% set customersAlsoBought = craft.purchasePatterns.related(
    product,
    3,
    craft.products.relatedTo(product.myCategory).availableForPurchase(1)
).availableForPurchase(1).all() %}

Example 1:
The related() above returns three products, but they all have availableForPurchase set to false. They are subsequently filtered by .availableForPurchase(1), returning an empty array.

Example 2:
The related() above retrieves 1 product from purchase_patterns, filling in the remaining two products with the paddingQuery. The two products have availableForPurchase set to true, but the product from purchase_patterns does not. The query is subsequently filtered by .availableForPurchase(1), returning an array of two products instead of three.

I added a filters parameter and deferred limiting until a second query that applies the filters to product Ids from purchase_patterns. Doing so allows these relationships to be queried with criteria while still ensuring results that consistently match the limit. The query above would be revised to:

{% set customersAlsoBought = craft.purchasePatterns.related(
    product,
    3,
    craft.products.relatedTo(product.myCategory).availableForPurchase(1),
    { availableForPurchase: 1 }
).all() %}

This could also be useful in other situations, such as only retrieving purchase patterns for certain product types (e.g. excluding replacement parts so only retail products are returned).

Thanks for your consideration!

@Tam Tam changed the base branch from v1 to v1-dev December 17, 2019 16:17
@Tam Tam merged commit cbf4862 into ethercreative:v1-dev Dec 17, 2019
@Tam
Copy link
Copy Markdown
Member

Tam commented Dec 17, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants