From ca88571355913c996f2929367989a7c4a80abd6e Mon Sep 17 00:00:00 2001 From: Jade Ornelas Date: Fri, 28 Apr 2023 11:59:32 -0700 Subject: [PATCH 1/5] only replace font urls when content type is not present or it is a text --- .../lib/shopify_cli/theme/dev_server/cdn_fonts.rb | 10 ++++++++-- .../shopify-cli/theme/dev_server/cdn_fonts_test.rb | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/cdn_fonts.rb b/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/cdn_fonts.rb index 9472e024d57..74e384970bc 100644 --- a/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/cdn_fonts.rb +++ b/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/dev_server/cdn_fonts.rb @@ -21,8 +21,14 @@ def call(env) # Proxy the request, and replace the URLs in the response status, headers, body = @app.call(env) - body = replace_font_urls(body) - [status, headers, body] + # Use Rack::Response to get the content type header regardless of case + response = Rack::Response.new(nil, status, headers) + content_type = response.get_header("Content-Type") + if content_type.nil? || content_type == "" || content_type&.start_with?("text/") + Rack::Response.new(replace_font_urls(body), status, headers).finish + else + Rack::Response.new(body, status, headers).finish + end end private diff --git a/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb b/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb index 74515463d35..f7a00539981 100644 --- a/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb +++ b/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb @@ -102,11 +102,21 @@ def test_404_on_missing_cdn_fonts assert_equal("Not found", response.body) end + def test_do_not_replace_when_content_type_does_not_match_text + CdnFonts.any_instance.expects(:replace_font_urls).never + response = serve(path: "/cdn/shop/products/tan-colored-hat-on-monochrome-background.jpg", content_type: "image/jpeg") + end + + def test_replace_when_content_type_does_match_text + CdnFonts.any_instance.expects(:replace_font_urls).once + response = serve(path: "/cdn/shop/products/style.css", content_type: "text/css") + end + private - def serve(response_body = "", path: "/") + def serve(response_body = "", path: "/", content_type: "text/html") app = lambda do |_env| - [200, {}, [response_body]] + [200, {"Content-Type" => content_type}, [response_body]] end stack = CdnFonts.new(app, theme: theme) request = Rack::MockRequest.new(stack) From 9fdbdf86d1c05db081b4a17b90df7521d2cc6ce9 Mon Sep 17 00:00:00 2001 From: Jade Ornelas Date: Fri, 28 Apr 2023 12:08:05 -0700 Subject: [PATCH 2/5] add changeset explaining changes --- .changeset/fast-terms-report.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fast-terms-report.md diff --git a/.changeset/fast-terms-report.md b/.changeset/fast-terms-report.md new file mode 100644 index 00000000000..ae52ad63cd0 --- /dev/null +++ b/.changeset/fast-terms-report.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +fixes theme preview for new shopify cdn url format From d4fab541704b2415434f75dcf08c4508cc065281 Mon Sep 17 00:00:00 2001 From: Jade Ornelas Date: Fri, 28 Apr 2023 14:14:32 -0700 Subject: [PATCH 3/5] lint cdn_fonts_test.rb --- .../test/shopify-cli/theme/dev_server/cdn_fonts_test.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb b/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb index f7a00539981..1c80699a2fe 100644 --- a/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb +++ b/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb @@ -104,19 +104,20 @@ def test_404_on_missing_cdn_fonts def test_do_not_replace_when_content_type_does_not_match_text CdnFonts.any_instance.expects(:replace_font_urls).never - response = serve(path: "/cdn/shop/products/tan-colored-hat-on-monochrome-background.jpg", content_type: "image/jpeg") + serve(path: "/cdn/shop/products/tan-colored-hat-on-monochrome-background.jpg", + content_type: "image/jpeg") end def test_replace_when_content_type_does_match_text CdnFonts.any_instance.expects(:replace_font_urls).once - response = serve(path: "/cdn/shop/products/style.css", content_type: "text/css") + serve(path: "/cdn/shop/products/style.css", content_type: "text/css") end private def serve(response_body = "", path: "/", content_type: "text/html") app = lambda do |_env| - [200, {"Content-Type" => content_type}, [response_body]] + [200, { "Content-Type" => content_type }, [response_body]] end stack = CdnFonts.new(app, theme: theme) request = Rack::MockRequest.new(stack) From 6aa98569542975df2925a9c4013fdfa170b1af54 Mon Sep 17 00:00:00 2001 From: Jade Ornelas Date: Mon, 1 May 2023 10:08:24 -0700 Subject: [PATCH 4/5] add test that doesn't rely on mocks --- .../shopify-cli/theme/dev_server/cdn_fonts_test.rb | 12 +++++++++--- packages/cli-kit/assets/cli-ruby/test/test_helper.rb | 4 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb b/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb index 1c80699a2fe..4323fc997a4 100644 --- a/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb +++ b/packages/cli-kit/assets/cli-ruby/test/shopify-cli/theme/dev_server/cdn_fonts_test.rb @@ -104,13 +104,19 @@ def test_404_on_missing_cdn_fonts def test_do_not_replace_when_content_type_does_not_match_text CdnFonts.any_instance.expects(:replace_font_urls).never - serve(path: "/cdn/shop/products/tan-colored-hat-on-monochrome-background.jpg", + original_input = "https://fonts.shopifycdn.com/font.woff2" + response = serve(original_input, path: "/cdn/shop/products/tan-colored-hat-on-monochrome-background.jpg", content_type: "image/jpeg") + + assert_equal(original_input, response.body) end def test_replace_when_content_type_does_match_text - CdnFonts.any_instance.expects(:replace_font_urls).once - serve(path: "/cdn/shop/products/style.css", content_type: "text/css") + original_input = "https://fonts.shopifycdn.com/font.woff2" + response = serve(original_input, path: "/cdn/shop/products/style.css", content_type: "text/css") + response_body = response.body + refute_equal(original_input, response_body) + assert_equal("/fonts/font.woff2", response_body) end private diff --git a/packages/cli-kit/assets/cli-ruby/test/test_helper.rb b/packages/cli-kit/assets/cli-ruby/test/test_helper.rb index ef707b4e84e..1d83d8b6091 100644 --- a/packages/cli-kit/assets/cli-ruby/test/test_helper.rb +++ b/packages/cli-kit/assets/cli-ruby/test/test_helper.rb @@ -3,8 +3,8 @@ require "rubygems" require "bundler/setup" require "shopify_cli" -require "byebug" -require "pry" +# require "byebug" +# require "pry" require "minitest/autorun" require "minitest/reporters" From 4be9dd70a69f08fc9498f50a829e812f2d89ad0a Mon Sep 17 00:00:00 2001 From: Jade Ornelas Date: Mon, 1 May 2023 10:13:04 -0700 Subject: [PATCH 5/5] undo change that is not required --- packages/cli-kit/assets/cli-ruby/test/test_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli-kit/assets/cli-ruby/test/test_helper.rb b/packages/cli-kit/assets/cli-ruby/test/test_helper.rb index 1d83d8b6091..ef707b4e84e 100644 --- a/packages/cli-kit/assets/cli-ruby/test/test_helper.rb +++ b/packages/cli-kit/assets/cli-ruby/test/test_helper.rb @@ -3,8 +3,8 @@ require "rubygems" require "bundler/setup" require "shopify_cli" -# require "byebug" -# require "pry" +require "byebug" +require "pry" require "minitest/autorun" require "minitest/reporters"