Skip to content

Commit

Permalink
Merge pull request #1313 from eshopguide/fix/registry-process-with-re…
Browse files Browse the repository at this point in the history
…sponse-as-struct

Webhook Registry: Ensuring that the response body is always a Hash, even if ShopifyAPI.Context.response_as_struct is true.
  • Loading branch information
lizkenyon committed May 13, 2024
2 parents 7c4ecb3 + d8a2e8c commit 661a8de
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Note: For changes to the API, see https://shopify.dev/changelog?filter=api

## 14.3.0
- [#1312](https://github.com/Shopify/shopify-api-ruby/pull/1312) Use same leeway for `exp` and `nbf` when parsing JWT
- [#1313](https://github.com/Shopify/shopify-api-ruby/pull/1313) Fix: Webhook Registry now working with response_as_struct enabled
- [#1314](https://github.com/Shopify/shopify-api-ruby/pull/1314)
- Add new session util method `SessionUtils::session_id_from_shopify_id_token`
- `SessionUtils::current_session_id` now accepts shopify Id token in the format of `Bearer this_token` or just `this_token`
Expand Down
5 changes: 3 additions & 2 deletions lib/shopify_api/clients/graphql/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ def initialize(session:, base_path:, api_version: nil)
variables: T.nilable(T::Hash[T.any(Symbol, String), T.untyped]),
headers: T.nilable(T::Hash[T.any(Symbol, String), T.untyped]),
tries: Integer,
response_as_struct: T.nilable(T::Boolean),
).returns(HttpResponse)
end
def query(query:, variables: nil, headers: nil, tries: 1)
def query(query:, variables: nil, headers: nil, tries: 1, response_as_struct: Context.response_as_struct)
body = { query: query, variables: variables }
@http_client.request(
HttpRequest.new(
Expand All @@ -42,7 +43,7 @@ def query(query:, variables: nil, headers: nil, tries: 1)
body_type: "application/json",
tries: tries,
),
response_as_struct: Context.response_as_struct || false,
response_as_struct: response_as_struct || false,
)
end
end
Expand Down
6 changes: 4 additions & 2 deletions lib/shopify_api/clients/graphql/storefront.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,13 @@ def initialize(shop, storefront_access_token = nil, private_token: nil, public_t
variables: T.nilable(T::Hash[T.any(Symbol, String), T.untyped]),
headers: T.nilable(T::Hash[T.any(Symbol, String), T.untyped]),
tries: Integer,
response_as_struct: T.nilable(T::Boolean),
).returns(HttpResponse)
end
def query(query:, variables: nil, headers: {}, tries: 1)
def query(query:, variables: nil, headers: {}, tries: 1, response_as_struct: Context.response_as_struct)
T.must(headers).merge!({ @storefront_auth_header => @storefront_access_token })
super(query: query, variables: variables, headers: headers, tries: tries)
super(query: query, variables: variables, headers: headers, tries: tries,
response_as_struct: response_as_struct)
end
end
end
Expand Down
9 changes: 5 additions & 4 deletions lib/shopify_api/webhooks/registry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def unregister(topic:, session:)
}
MUTATION

delete_response = client.query(query: delete_mutation)
delete_response = client.query(query: delete_mutation, response_as_struct: false)
raise Errors::WebhookRegistrationError,
"Failed to delete webhook from Shopify" unless delete_response.ok?
result = T.cast(delete_response.body, T::Hash[String, T.untyped])
Expand Down Expand Up @@ -170,7 +170,7 @@ def get_webhook_id(topic:, client:)
}
QUERY

fetch_id_response = client.query(query: fetch_id_query)
fetch_id_response = client.query(query: fetch_id_query, response_as_struct: false)
raise Errors::WebhookRegistrationError,
"Failed to fetch webhook from Shopify" unless fetch_id_response.ok?
body = T.cast(fetch_id_response.body, T::Hash[String, T.untyped])
Expand Down Expand Up @@ -216,7 +216,7 @@ def process(request)
).returns(T::Hash[Symbol, T.untyped])
end
def webhook_registration_needed?(client, registration)
check_response = client.query(query: registration.build_check_query)
check_response = client.query(query: registration.build_check_query, response_as_struct: false)
raise Errors::WebhookRegistrationError,
"Failed to check if webhook was already registered" unless check_response.ok?
parsed_check_result = registration.parse_check_result(T.cast(check_response.body, T::Hash[String, T.untyped]))
Expand All @@ -233,7 +233,8 @@ def webhook_registration_needed?(client, registration)
).returns(T::Hash[String, T.untyped])
end
def send_register_request(client, registration, webhook_id)
register_response = client.query(query: registration.build_register_query(webhook_id: webhook_id))
register_response = client.query(query: registration.build_register_query(webhook_id: webhook_id),
response_as_struct: false)

raise Errors::WebhookRegistrationError, "Failed to register webhook with Shopify" unless register_response.ok?

Expand Down
23 changes: 23 additions & 0 deletions test/webhooks/registry_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,29 @@ def test_process
assert(handler_called)
end

def test_process_with_response_as_struct
modify_context(response_as_struct: true)

handler_called = false

handler = TestHelpers::FakeWebhookHandler.new(
lambda do |topic, shop, body,|
assert_equal(@topic, topic)
assert_equal(@shop, shop)
assert_equal({}, body)
handler_called = true
end,
)

ShopifyAPI::Webhooks::Registry.add_registration(
topic: @topic, path: "path", delivery_method: :http, handler: handler,
)

ShopifyAPI::Webhooks::Registry.process(@webhook_request)

assert(handler_called)
end

def test_process_new_handler
handler_called = false

Expand Down

0 comments on commit 661a8de

Please sign in to comment.