Skip to content
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

Add dot notation accessing for graphql responses #1246

Merged
merged 6 commits into from
Dec 4, 2023

Conversation

lizkenyon
Copy link
Contributor

@lizkenyon lizkenyon commented Nov 23, 2023

Description

dot-notation.mp4

Fixes #1210

Please, include a summary of what the PR is for:
Starting in the version 10 upgrade graphql response were no longer available to be access with dot notation.

This PR adds a context option(response_as_struct) that will return the graphql API responses as a struct, so that they will be accessible via either dot or hash notation.

 created_product = response.body["data"]["productCreate"]["product"]
 created_product2 = response.body.data.productCreate.product

How has this been tested?

  • This gem was used locally to test GraphQL, REST, rest resources and install flow

Checklist:

  • My commit message follow the pattern described in here
  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the project documentation.
  • I have added a changelog line.

Previously responses could only be accessed through hash

Add an optional context param to transform to an openstruct object
lib/shopify_api/context.rb Outdated Show resolved Hide resolved
lib/shopify_api/clients/http_response.rb Outdated Show resolved Hide resolved
lib/shopify_api/context.rb Outdated Show resolved Hide resolved
test/clients/http_response_test.rb Outdated Show resolved Hide resolved
sig { params(request: HttpRequest).returns(HttpResponse) }
def request(request)
sig { params(request: HttpRequest, graphql_response_object: T::Boolean).returns(HttpResponse) }
def request(request, graphql_response_object: false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an even safer approach, I like it!

Maybe we rename this param as response_as_struct: false to be a little more explicit as what the option is doing.

@lizkenyon lizkenyon marked this pull request as ready for review November 30, 2023 17:47
@lizkenyon lizkenyon requested a review from a team as a code owner November 30, 2023 17:47
@lizkenyon lizkenyon changed the title [Draft] Add dot notation accessing for graphql responses Add dot notation accessing for graphql responses Nov 30, 2023
@lizkenyon lizkenyon merged commit 342547f into main Dec 4, 2023
7 checks passed
@lizkenyon lizkenyon deleted the liz/graphql-response-object branch December 4, 2023 18:50
@oliwoodsuk
Copy link

Hey guys,
Adding this option throws a type error from this file when the option is setup in ShopifyAPI::Context like so:

ShopifyAPI::Context.setup(
            ...
            response_as_struct: true
        )

Thanks for the change though. We're currently migrating from an older version, but are just going to re-instantiate the response using ActiveSupport::OrderedOptions for now to get dot notation.

@lizkenyon
Copy link
Contributor Author

Hi @oliwoodsuk thanks for the ping!

Could you provide a bit more details about your setup so I can try to reproduce and resolve it. Are you using the ruby template?

@oliwoodsuk
Copy link

oliwoodsuk commented Dec 20, 2023

Hey @lizkenyon ,

Yep sure, we're not using the ruby template. Instead we're using the shopify_app gem @ 21.8.0 and the shopify-api-ruby gem effectively on main (through our own fork), only changes are the addition of a lib/shopify_api/rest/resources/2024_01 folder and "2024-01" to ShopifyAPI::AdminVersions::SUPPORTED_ADMIN_VERSIONS

We're running a SPA style app via Turbo/Hotwire and below is the initializers/shopify_app.rb file:

API_DOMAIN     = (Rails.env.production?) ? 'https://api.xxx.io'                   : 'https://xxx.ngrok.io'
WEBHOOK_DOMAIN = (Rails.env.production?) ? 'https://xxx-new-webhook.onrender.com' : 'https://xxx.ngrok.io'
APP_DOMAIN     = (Rails.env.production?) ? 'https://xxx-app.onrender.com'         : 'https://xxx.ngrok.io'

api_scopes =  "read_products,write_products,read_script_tags,write_script_tags,read_themes,write_themes,read_orders,write_orders,read_inventory,write_inventory,read_price_rules,write_price_rules,read_discounts,write_discounts,read_customers,write_customers,read_order_edits,write_order_edits,read_merchant_managed_fulfillment_orders,write_merchant_managed_fulfillment_orders,read_assigned_fulfillment_orders,write_assigned_fulfillment_orders,read_third_party_fulfillment_orders,write_third_party_fulfillment_orders,read_purchase_options,write_purchase_options,read_payment_mandate,write_payment_mandate"
api_scopes += ",read_all_orders" if Rails.env.production? #requires permission from shopify to request this scope, which we don't have for dev app


ShopifyApp.configure do |config|
    config.application_name = "xxx"
    config.old_secret = ""
    config.embedded_app = true
    config.after_authenticate_job = false
    config.api_version = '2024-01'
    config.shop_session_repository = 'Shop'
    config.scope =  api_scopes
    config.log_level = :info

    config.api_key = ENV['SHOPIFY_API_KEY'],
    config.secret  = ENV['SHOPIFY_API_SECRET']

    config.scripttags = [
        {event:'onload', src:"#{ API_DOMAIN }/xxx-embed.js"}
    ]
    config.webhooks = [
        {topic: 'orders/create',   address:  "#{ WEBHOOK_DOMAIN }/webhooks/orders_create",   path: 'webhooks/orders_create',   format: 'json'},
        {topic: 'orders/updated',  address:  "#{ WEBHOOK_DOMAIN }/webhooks/orders_updated",  path: 'webhooks/orders_updated',  format: 'json'},
        {topic: 'products/update', address:  "#{ WEBHOOK_DOMAIN }/webhooks/products_update", path: 'webhooks/products_update', format: 'json'},
        {topic: 'products/delete', address:  "#{ WEBHOOK_DOMAIN }/webhooks/products_delete", path: 'webhooks/products_delete', format: 'json'},
        {topic: 'app/uninstalled', address:  "#{ WEBHOOK_DOMAIN }/webhooks/app_uninstalled", path: 'webhooks/app_uninstalled', format: 'json'}
    ]

    if defined? Rails::Server
        raise('Missing SHOPIFY_API_KEY. See https://github.com/Shopify/shopify_app#requirements') unless config.api_key
        raise('Missing SHOPIFY_API_SECRET. See https://github.com/Shopify/shopify_app#requirements') unless config.secret
    end
end

Rails.application.config.after_initialize do
    if ShopifyApp.configuration.api_key.present? && ShopifyApp.configuration.secret.present?
        ShopifyAPI::Context.setup(
            api_key:            ENV['SHOPIFY_API_KEY'],
            api_secret_key:     ENV['SHOPIFY_API_SECRET'],
            host:               APP_DOMAIN,
            scope:              api_scopes,
            is_embedded:        true,
            is_private:         false,
            api_version:        ShopifyApp.configuration.api_version,
            user_agent_prefix:  "xxx",
            log_level:          Rails.env.test? ? :error : :info,
            response_as_struct: true #currently running this as false to avoid mentioned error
        )
        
        ShopifyApp::WebhooksManager.add_registrations
    end
end

EDIT: I has a single pesky , at the end of the config.api_key line, which caused some weirdness throughout the app. Not sure if it caused this, but noting anyway.

@lizkenyon
Copy link
Contributor Author

Hi @oliwoodsuk

I wasn't able to reproduce the issue on my end.

If you are still experiencing this issue, after removing your trailing comma please let me know.

@oliwoodsuk
Copy link

Hey @lizkenyon , maybe it was just an odd side effect from that trailing comma turning the .api_key method response into an array. Thanks for looking into it!

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.

GraphQL Client inadequate since v10 upgrade
3 participants