Reload product before assigning images to variants #3389
Merged
Conversation
This file contains 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
|
@JDutil I think failures will be fixed rebasing against master, thanks! |
Currently Rubocop raises the following warning: Layout/EmptyLineAfterGuardClause: Add empty line after guard clause. Adding empty lines around the return statements fixes them.
These option values are not being used the variant seeds, and therefore we don't need to fetch them from the database.
When fetching products from the database before assigning images we should include the variants, and option values to avoid n+1 database queries.
After adding the variant & option value includes within commit 81ddee979595150df708299e484f8e6ad583ec9c the failure to find option value bug moved to the solidus_girly product. I have no idea why, but simply reloading the product fixes the issue which was my original solution before adding the includes. This doesn't fix whatever the underlying issue is, but is a simple hack that seems to resolve the flakey failures during the sample loading.
kennyadsl
approved these changes
Oct 25, 2019
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.
Description
While this doesn't actually fix the underlying problem as I wasn't able to determine what it was these changes appear to fix the flakiness of the sample loading (in my dev environment anyways).
This change adds a reload to products before assigning their variant images on the sample seeds loading.
It also implements a couple minor rubocop and performance improvements noticed while troubleshooting the issue.
solidus_tshirt&solidus_longproducts.solidus_girlyproduct, but adding areloadbefore the variant lookup which fixed all three products works as a hack to fix the issue.Fixes #2978
Checklist: