Skip to content

Commit 385d1e6

Browse files
lizkenyonlizkenyon
committed
Update error types
1 parent a7bb69c commit 385d1e6

File tree

4 files changed

+91
-133
lines changed

4 files changed

+91
-133
lines changed

‎lib/shopify_app/controller_concerns/login_protection.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ def load_current_session(shopify_id_token: nil, cookies: nil, is_online: false)
269269
return ShopifyAPI::Context.load_private_session if ShopifyAPI::Context.private?
270270

271271
session_id = ShopifyAPI::Utils::SessionUtils.current_session_id(shopify_id_token, cookies, is_online)
272-
return nil unless session_id
272+
return unless session_id
273273

274274
ShopifyApp::SessionRepository.load_session(session_id)
275275
end

‎lib/shopify_app/managers/script_tags_manager.rb

+8-131
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,15 @@ def create_script_tags(session:)
3838
@session = session
3939
return unless required_script_tags.present?
4040

41-
# Check if any scripttag has template_types defined
4241
template_types_to_check = required_script_tags.flat_map { |tag| tag[:template_types] }.compact.uniq
4342

44-
# If template types are specified, check if the theme supports app blocks for those templates
4543
if template_types_to_check.any?
46-
# First fetch the active theme
4744
active_theme = fetch_active_theme
48-
49-
# If we failed to fetch the active theme, don't proceed with script tag creation
5045
unless active_theme
5146
ShopifyApp::Logger.debug("Failed to fetch active theme. Skipping script tag creation.")
5247
return
5348
end
5449

55-
# Check if all templates support app blocks
5650
if all_templates_support_app_blocks?(active_theme["id"], template_types_to_check)
5751
ShopifyApp::Logger.info(
5852
"Theme supports app blocks for templates: #{template_types_to_check.join(", ")}. " \
@@ -156,44 +150,32 @@ def fetch_active_theme
156150

157151
response = client.query(query: ACTIVE_THEME_QUERY)
158152

159-
# Check for errors in the response
160153
if response.body["errors"].present?
161154
error_message = response.body["errors"].map { |e| e["message"] }.join(", ")
162-
raise "GraphQL error: #{error_message}"
163-
end
164-
165-
# Check if the response has the expected structure
166-
unless response.body["data"] && response.body["data"]["themes"] && response.body["data"]["themes"]["nodes"]
167-
raise "Invalid response structure"
155+
raise ShopifyAPI::Errors::InvalidGraphqlRequestError, error_message
168156
end
169157

170158
themes = response.body["data"]["themes"]["nodes"]
171-
return nil if themes.empty?
159+
return if themes.empty?
172160

173161
themes.first
174162
rescue => e
175163
ShopifyApp::Logger.warn("Failed to fetch active theme: #{e.message}")
176164
nil
177165
end
178166

179-
# New method to check all templates at once
180167
def all_templates_support_app_blocks?(theme_id, template_types)
181168
client = graphql_client
182169

183-
# First, fetch all JSON templates at once
184170
template_filenames = template_types.map { |type| "templates/#{type}.json" }
185171
json_templates = fetch_json_templates(client, theme_id, template_filenames)
186172

187-
# If any template is missing, return false
188173
return false if json_templates.length != template_types.length
189174

190-
# Extract main sections from all templates
191175
main_sections = extract_main_sections(json_templates)
192176

193-
# If any template doesn't have a main section, return false
194177
return false if main_sections.length != template_types.length
195178

196-
# Check if all main sections support app blocks
197179
all_sections_support_app_blocks?(client, theme_id, main_sections)
198180
rescue => e
199181
ShopifyApp::Logger.error("Error checking template support: #{e.message}")
@@ -208,10 +190,9 @@ def fetch_json_templates(client, theme_id, template_filenames)
208190

209191
files_response = client.query(query: FILES_QUERY, variables: files_variables)
210192

211-
# Check for errors in the response
212193
if files_response.body["errors"].present?
213194
error_message = files_response.body["errors"].map { |e| e["message"] }.join(", ")
214-
raise "GraphQL error: #{error_message}"
195+
raise ShopifyAPI::Errors::InvalidGraphqlRequestError, error_message
215196
end
216197

217198
files_response.body["data"]["theme"]["files"]["nodes"]
@@ -250,15 +231,13 @@ def all_sections_support_app_blocks?(client, theme_id, section_filenames)
250231

251232
section_response = client.query(query: FILES_QUERY, variables: section_variables)
252233

253-
# Check for errors in the section response
254234
if section_response.body["errors"].present?
255235
error_message = section_response.body["errors"].map { |e| e["message"] }.join(", ")
256-
raise "GraphQL error: #{error_message}"
236+
raise ShopifyAPI::Errors::InvalidGraphqlRequestError, error_message
257237
end
258238

259239
section_files = section_response.body["data"]["theme"]["files"]["nodes"]
260240

261-
# If we didn't get all the sections we asked for, return false
262241
return false if section_files.length != section_filenames.length
263242

264243
# Check if all sections support app blocks
@@ -275,89 +254,6 @@ def all_sections_support_app_blocks?(client, theme_id, section_filenames)
275254
false
276255
end
277256

278-
# Keep these methods for backward compatibility
279-
def template_supports_app_blocks?(theme_id, template_type)
280-
client = graphql_client
281-
282-
# First, check if the JSON template exists and find the main section
283-
main_section = find_main_section(client, theme_id, template_type)
284-
return false unless main_section
285-
286-
# Now check if the main section supports app blocks
287-
schema_supports_app_blocks?(client, theme_id, main_section)
288-
end
289-
290-
def find_main_section(client, theme_id, template_type)
291-
filename = "templates/#{template_type}.json"
292-
files_variables = {
293-
themeId: theme_id,
294-
filenames: [filename],
295-
}
296-
297-
files_response = client.query(query: FILES_QUERY, variables: files_variables)
298-
299-
# Check for errors in the response
300-
if files_response.body["errors"].present?
301-
error_message = files_response.body["errors"].map { |e| e["message"] }.join(", ")
302-
raise "GraphQL error: #{error_message}"
303-
end
304-
305-
template_files = files_response.body["data"]["theme"]["files"]["nodes"]
306-
307-
# If the JSON template doesn't exist, return nil
308-
return nil if template_files.empty?
309-
310-
# Parse the JSON template to find the main section
311-
template_content = template_files.first["body"]["content"]
312-
template_data = JSON.parse(template_content)
313-
314-
main_section = nil
315-
template_data["sections"].each do |id, section|
316-
if id == "main" || section["type"].to_s.start_with?("main-")
317-
main_section = "sections/#{section["type"]}.liquid"
318-
break
319-
end
320-
end
321-
322-
main_section
323-
rescue => e
324-
ShopifyApp::Logger.error("Error finding main section: #{e.message}")
325-
nil
326-
end
327-
328-
def schema_supports_app_blocks?(client, theme_id, section_filename)
329-
section_variables = {
330-
themeId: theme_id,
331-
filenames: [section_filename],
332-
}
333-
334-
section_response = client.query(query: FILES_QUERY, variables: section_variables)
335-
336-
# Check for errors in the section response
337-
if section_response.body["errors"].present?
338-
error_message = section_response.body["errors"].map { |e| e["message"] }.join(", ")
339-
raise "GraphQL error: #{error_message}"
340-
end
341-
342-
section_files = section_response.body["data"]["theme"]["files"]["nodes"]
343-
344-
return false if section_files.empty?
345-
346-
section_content = section_files.first["body"]["content"]
347-
348-
# Extract schema from the section content
349-
schema_match = section_content.match(/\{\%\s+schema\s+\%\}([\s\S]*?)\{\%\s+endschema\s+\%\}/m)
350-
return false unless schema_match
351-
352-
schema = JSON.parse(schema_match[1])
353-
354-
# Check if the schema has blocks that support app blocks
355-
schema["blocks"]&.any? { |block| block["type"] == "@app" } || false
356-
rescue => e
357-
ShopifyApp::Logger.error("Error checking schema support: #{e.message}")
358-
false
359-
end
360-
361257
def expanded_script_tags
362258
self.class.build_src(required_script_tags, shop_domain)
363259
end
@@ -379,25 +275,13 @@ def create_script_tag(attributes)
379275

380276
response = client.query(query: SCRIPT_TAG_CREATE_MUTATION, variables: variables)
381277

382-
# Add proper nil checks for the response structure
383-
if response.body["data"] &&
384-
response.body["data"]["scriptTagCreate"] &&
385-
response.body["data"]["scriptTagCreate"]["userErrors"] &&
386-
response.body["data"]["scriptTagCreate"]["userErrors"].any?
278+
if response.body["data"]["scriptTagCreate"]["userErrors"].any?
387279
errors = response.body["data"]["scriptTagCreate"]["userErrors"]
388280
error_messages = errors.map { |e| "#{e["field"]}: #{e["message"]}" }.join(", ")
389281
raise ::ShopifyApp::CreationFailed, "ScriptTag creation failed: #{error_messages}"
390282
end
391283

392-
# Safely access the script tag data
393-
if response.body["data"] &&
394-
response.body["data"]["scriptTagCreate"] &&
395-
response.body["data"]["scriptTagCreate"]["scriptTag"]
396-
response.body["data"]["scriptTagCreate"]["scriptTag"]
397-
else
398-
ShopifyApp::Logger.warn("Script tag creation response missing expected data structure")
399-
nil
400-
end
284+
response.body["data"]["scriptTagCreate"]["scriptTag"]
401285
rescue ShopifyAPI::Errors::HttpResponseError => e
402286
raise ::ShopifyApp::CreationFailed, e.message
403287
end
@@ -425,19 +309,12 @@ def fetch_all_script_tags
425309

426310
response = client.query(query: SCRIPT_TAGS_QUERY)
427311

428-
# Check for errors in the response
429312
if response.body["errors"].present?
430-
ShopifyApp::Logger.warn("GraphQL error fetching script tags: #{response.body["errors"].map do |e|
431-
e["message"]
432-
end.join(", ")}")
313+
error_messages = response.body["errors"].map { |e| e["message"] }.join(", ")
314+
ShopifyApp::Logger.warn("GraphQL error fetching script tags: #{error_messages}")
433315
return []
434316
end
435317

436-
# Handle nil data or missing structure
437-
return [] unless response.body["data"] &&
438-
response.body["data"]["scriptTags"] &&
439-
response.body["data"]["scriptTags"]["edges"]
440-
441318
response.body["data"]["scriptTags"]["edges"].map { |edge| edge["node"] }
442319
rescue => e
443320
ShopifyApp::Logger.warn("Error fetching script tags: #{e.message}")

‎lib/shopify_app/utils.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class << self
1212

1313
def sanitize_shop_domain(shop_domain)
1414
uri = uri_from_shop_domain(shop_domain)
15-
return nil if uri.nil? || uri.host.nil?
15+
return if uri.nil? || uri.host.nil?
1616

1717
trusted_domains.each do |trusted_domain|
1818
no_shop_name_in_subdomain = uri.host == trusted_domain

‎test/shopify_app/managers/script_tags_manager_test.rb

+81
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,87 @@ class ShopifyApp::ScriptTagsManagerTest < ActiveSupport::TestCase
488488
manager.create_script_tags(session: @session)
489489
end
490490

491+
test "#fetch_active_theme raises InvalidGraphqlRequestError when GraphQL errors are present" do
492+
manager = ShopifyApp::ScriptTagsManager.new(@script_tags, "example-app.com")
493+
manager.stubs(:graphql_client).returns(@mock_client)
494+
495+
# Mock GraphQL error response
496+
error_response = mock
497+
error_response.stubs(:body).returns({
498+
"errors" => [
499+
{
500+
"message" => "GraphQL error message",
501+
"locations" => [{ "line" => 2, "column" => 11 }],
502+
"path" => ["themes"],
503+
},
504+
],
505+
"data" => nil,
506+
})
507+
508+
# Set up the response for the GraphQL client
509+
@mock_client.expects(:query).returns(error_response).once
510+
511+
# Stub the rescue block to verify the error is raised before it's caught
512+
ShopifyApp::Logger.expects(:warn).with("Failed to fetch active theme: GraphQL error message")
513+
514+
# The method will return nil due to the rescue block
515+
assert_nil manager.send(:fetch_active_theme)
516+
end
517+
518+
test "#fetch_json_templates raises InvalidGraphqlRequestError when GraphQL errors are present" do
519+
manager = ShopifyApp::ScriptTagsManager.new(@script_tags, "example-app.com")
520+
manager.stubs(:graphql_client).returns(@mock_client)
521+
522+
# Mock GraphQL error response
523+
error_response = mock
524+
error_response.stubs(:body).returns({
525+
"errors" => [
526+
{
527+
"message" => "GraphQL error message",
528+
"locations" => [{ "line" => 2, "column" => 11 }],
529+
"path" => ["theme", "files"],
530+
},
531+
],
532+
"data" => nil,
533+
})
534+
535+
# Set up the response for the GraphQL client
536+
@mock_client.expects(:query).returns(error_response).once
537+
538+
# This method doesn't have a rescue block, so we can test for the exception directly
539+
assert_raises(ShopifyAPI::Errors::InvalidGraphqlRequestError) do
540+
manager.send(:fetch_json_templates, @mock_client, "theme_id", ["templates/product.json"])
541+
end
542+
end
543+
544+
test "#all_sections_support_app_blocks? raises InvalidGraphqlRequestError when GraphQL errors are present" do
545+
manager = ShopifyApp::ScriptTagsManager.new(@script_tags, "example-app.com")
546+
manager.stubs(:graphql_client).returns(@mock_client)
547+
548+
# Mock GraphQL error response
549+
error_response = mock
550+
error_response.stubs(:body).returns({
551+
"errors" => [
552+
{
553+
"message" => "GraphQL error message",
554+
"locations" => [{ "line" => 2, "column" => 11 }],
555+
"path" => ["theme", "files"],
556+
},
557+
],
558+
"data" => nil,
559+
})
560+
561+
# Set up the response for the GraphQL client
562+
@mock_client.expects(:query).returns(error_response).once
563+
564+
# Stub the rescue block to verify the error is raised before it's caught
565+
ShopifyApp::Logger.expects(:error).with("Error checking section support: GraphQL error message")
566+
567+
# The method will return false due to the rescue block
568+
assert_equal false,
569+
manager.send(:all_sections_support_app_blocks?, @mock_client, "theme_id", ["sections/main-product.liquid"])
570+
end
571+
491572
private
492573

493574
def mock_empty_script_tags_response

0 commit comments

Comments
 (0)