-
Notifications
You must be signed in to change notification settings - Fork 5.1k
feat(source-shopify): Add id and position fields to product_variants.options schema (AI-Triage PR) #72810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(source-shopify): Add id and position fields to product_variants.options schema (AI-Triage PR) #72810
Conversation
…options schema Adds id and position fields to the options array in the product_variants stream schema. These fields come from Shopify's ProductOption object and are needed by users to: - Reference specific product options for API operations (using id) - Display options in the correct order on product pages (using position) Changes: 1. Extended GraphQL query to fetch product.options with id, name, position 2. Added _enrich_options_with_product_options() method to match options by name and copy id/position values from ProductOption to variant options 3. Updated JSON schema to include id (integer) and position (integer) fields Resolves #72277 Related to airbytehq/oncall#11147 Co-Authored-By: unknown <>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
|
|
/ai-prove-fix
|
Fix Validation EvidenceOutcome: Fix/Feature Proven Successfully Evidence SummaryTested on 1 internal Airbyte connection with the Next Steps
Connector & PR DetailsConnector: Evidence PlanProving CriteriaA sync that successfully returns the new Disproving Criteria
Cases Attempted
Pre-flight Checks
Detailed Evidence LogTest Timeline (UTC):
Results:
Note: Connection IDs and detailed logs are recorded in the linked private issue. Session: https://app.devin.ai/sessions/3922f69d67ba4fdd9b6b58787c86601a |
|
|
/bump-version
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| for product_option in product_options: | ||
| if product_option: | ||
| name = product_option.get("name") | ||
| if name: | ||
| product_options_map[name] = { | ||
| "id": self.tools.resolve_str_id(product_option.get("id")), | ||
| "position": product_option.get("position"), | ||
| } | ||
|
|
||
| # Enrich each option with id and position from the matching ProductOption | ||
| for option in options: | ||
| if option: | ||
| option_name = option.get("name") | ||
| if option_name and option_name in product_options_map: | ||
| option["id"] = product_options_map[option_name]["id"] | ||
| option["position"] = product_options_map[option_name]["position"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm somewhat worried about performance implications of enriching each record via additional calls x number of records.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! To clarify - this enrichment does not make any additional API calls. The product.options data is fetched in the same Bulk GraphQL query alongside the variant data (see the query_nodes change at line 2794-2800).
The enrichment is purely in-memory: we're just matching the variant's selectedOptions (which only have name and value) with the product's options (which have id, name, position) by name, then copying over the id and position values.
The performance impact is minimal - it's O(n*m) where n = number of options per variant (typically 1-3) and m = number of product options (also typically 1-3). In practice, this is a handful of dictionary lookups per record with no network overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a big help and that resolves my concern. Thanks for the clarification.
|
/ai-review
Reviewing PR for connector safety and quality.
|
|
AI PR Review in Progress Reviewing PR #72810 for connector safety and quality gates. Session: https://app.devin.ai/sessions/dad9aef3c161441582564138b3835761 |
|
Deploy preview for airbyte-docs ready! ✅ Preview Built with commit 77a0f43. |
AI PR Review ReportReview Action: NO ACTION (NOT ELIGIBLE)
📋 PR Details & EligibilityConnector & PR InfoConnector(s): Auto-Approve EligibilityEligible: No Review Action DetailsNO ACTION (NOT ELIGIBLE) - All gates pass but PR is not eligible for auto-approval. No PR review submitted. Human review required.
🔍 Gate Evaluation DetailsGate-by-Gate Analysis
Files Changed
📚 Evidence ConsultedEvidence
|
| name="product", | ||
| fields=[ | ||
| Field(name="id", alias="product_id"), | ||
| Field(name="options", alias="product_options", fields=["id", "name", "position"]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question! The value field is already present - it comes from the variant's selectedOptions (which we alias as options).
Here's how the data flows:
From variant's selectedOptions (already fetched):
name- e.g., "Color"value- e.g., "Red" (the specific value selected for this variant)
From product.options (ProductOption) (added in this PR):
id- the ProductOption IDposition- display order
The key distinction is that ProductOption.values (plural) contains ALL possible values for an option (e.g., ["Red", "Blue", "Green"]), while SelectedOption.value (singular) is the specific value chosen for that variant.
We match by name and enrich with id and position - the value is already there from selectedOptions.
aldogonzalez8
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APPROVED
aaronsteers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved!

What
Adds
idandpositionfields to theoptionsarray in theproduct_variantsstream schema. These fields come from Shopify's ProductOption API and are needed by users to:id)position)Resolves #72277
Related to airbytehq/oncall#11147
Requested by @pkozikow.
Link to Devin run: https://app.devin.ai/sessions/1d654c1925bc4ab789e40bc43a026e56
How
product.options(withid,name,position) alongside the existingproduct.id_enrich_options_with_product_options()method that matches eachselectedOptionto its parentProductOptionby name and copies over theidandpositionvaluesid(integer) andposition(integer) fields in the options array itemsThe matching is done by option name because Shopify's
SelectedOption(what variants return) doesn't directly expose the parentProductOption.idorProductOption.position- these fields only exist on theProductOptionobject at the product level.Review guide
source_shopify/shopify_graphql/bulk/query.py- GraphQL query extension and new enrichment methodsource_shopify/schemas/product_variants.json- Schema additions foridandpositionKey areas to verify:
selectedOptionsandProductOptionis correct for Shopify's API (assumes option names are unique per product)product.optionsfield is a simple array, not a connection)resolve_str_idconversion is appropriate for ProductOption IDsproductfield is removed from the recordUser Impact
Users syncing the
product_variantsstream will now have access toidandpositionfields in each option object. This is an additive, non-breaking change - existing syncs will continue to work and will simply include the new fields.Can this PR be safely reverted and rolled back?