From 3ada9e03906293575aaadb8843af92a00a87e885 Mon Sep 17 00:00:00 2001 From: Chris Todorov Date: Tue, 22 Oct 2024 15:06:32 -0700 Subject: [PATCH 1/3] Add additional canonical tag test coverage In a subsequent commit, we want to replace the canonical tag helper. So having a record of the current behaviour will help us safely re-implement the functionality. We changed the spec type to `:feature` because there seems to be a database persistence issue when running `:system` tests headfully. (i.e. `ActiveRecord::RecordNotFound` errors for everything!) Seems like a configuration problem. Co-authored-by: benjamin wil --- .../spec/system/template_rendering_spec.rb | 70 ++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/templates/spec/system/template_rendering_spec.rb b/templates/spec/system/template_rendering_spec.rb index ffee87df..90f9ea21 100644 --- a/templates/spec/system/template_rendering_spec.rb +++ b/templates/spec/system/template_rendering_spec.rb @@ -7,14 +7,80 @@ Capybara.ignore_hidden_elements = true end + let!(:taxon) { + taxonomy = create :taxonomy, name: "Solidus Brand" + create :taxon, name: "Accessories", taxonomy: taxonomy, parent: taxonomy.root + } + before do Capybara.ignore_hidden_elements = false + + Spree::Store.create!( + code: 'spree', + name: 'My Spree Store', + url: 'spreestore.example.com', + mail_from_address: 'test@example.com', + default: true + ) end it 'layout should have canonical tag referencing site url' do - Spree::Store.create!(code: 'spree', name: 'My Spree Store', url: 'spreestore.example.com', mail_from_address: 'test@example.com') - visit root_path + expect(find('link[rel=canonical]')[:href]).to eql('http://spreestore.example.com/') end + + it "renders a canonical tag for products index page with keywords query string" do + create(:product_in_stock, name: 'Solidus Mug', price: 10.00) + + visit products_path(keywords: 'solidus') + + expect(page).to have_content('Solidus Mug') + + expect( + find('link[rel=canonical]')[:href] + ).to eql('http://spreestore.example.com/products/?keywords=solidus') + end + + it "renders a canonical tag for the products index with a taxon query string" do + create(:product_in_stock, name: 'Solidus Mug', taxons: [taxon]) + + visit products_path(keywords: 'solidus', taxon: taxon.id) + + expect(page).to have_content('Solidus Mug') + + expect( + find('link[rel=canonical]')[:href] + ).to eql("http://spreestore.example.com/products/?keywords=solidus&taxon=#{taxon.id}") + end + + it "renders a canonical tag for taxon pages with a search filter query string" do + create(:product_in_stock, name: 'Solidus Mug', taxons: [taxon]) + + visit nested_taxons_path(taxon) + + within "#sidebar_products_search" do + check "Under $10.00" + click_on "Search" + end + expect(page).to have_content "No products found" + + expect( + URI::decode_uri_component find('link[rel=canonical]')[:href] + ).to eql( + "http://spreestore.example.com/t/solidus-brand/accessories?search[price_range_any][]=Under+$10.00" + ) + end + + it "renders a canonical tag for taxon pages with multiple pages of search results" do + create(:product_in_stock, name: 'Solidus Mug 1', taxons: [taxon]) + create(:product_in_stock, name: 'Solidus Mug 2', taxons: [taxon]) + + visit nested_taxons_path(taxon, per_page: 1, page: 2) + + + expect( + find('link[rel=canonical]')[:href] + ).to eql("http://spreestore.example.com/t/solidus-brand/accessories?page=2") + end end From 599bc16510408a7076f791edec26145be827d260 Mon Sep 17 00:00:00 2001 From: Chris Todorov Date: Fri, 15 Nov 2024 15:06:31 -0800 Subject: [PATCH 2/3] Implement a simple canonical tag helper This replaces the `canonical_tag` helper from `canonical-rails`. This adds simple canonical tag generation, without all the options the gem provides, but in a helper that can be customized by application developers. This would allow us to remove the `canonical-rails` gem as a dependency going forward. Co-authored-by: Benjamin Willems --- templates/app/helpers/layout_helper.rb | 36 +++++++++++++++++++ .../app/views/layouts/storefront.html.erb | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 templates/app/helpers/layout_helper.rb diff --git a/templates/app/helpers/layout_helper.rb b/templates/app/helpers/layout_helper.rb new file mode 100644 index 00000000..08c10358 --- /dev/null +++ b/templates/app/helpers/layout_helper.rb @@ -0,0 +1,36 @@ +module LayoutHelper + # Generates a simple canonical tag based on the request path, preserving allowed + # parameters. For collection actions, a trailing slash is added to the href. + # For more advanced use cases, consider using the `canonical-rails` gem. + # + # @see https://github.com/jumph4x/canonical-rails + # @see https://developers.google.com/search/docs/crawling-indexing/consolidate-duplicate-urls + # + # @param host [String] the host to use in the canonical URL + # @param collection_actions [Array] the actions that will include a trailing slash + # @param allowed_parameters [Array] the parameters to preserve in the canonical URL + # @return [String] the generated link rel="canonical" tag + def simple_canonical_tag( + host: current_store&.url, + collection_actions: %w[index], + allowed_parameters: [:keywords, :page, :search, :taxon] + ) + path_without_extension = request.path + .sub(/\.#{params[:format]}$/, "") + .sub(/\/$/, "") + + href = "#{request.protocol}#{host}#{path_without_extension}" + + trailing_slash = request.params.key?('action') && + collection_actions.include?(request.params['action']) + href += '/' if trailing_slash + + query_params = params.select do |key, value| + value.present? && allowed_parameters.include?(key.to_sym) + end.to_unsafe_h + + href += "?#{query_params.to_query}" if query_params.present? + + tag(:link, rel: :canonical, href: href) + end +end diff --git a/templates/app/views/layouts/storefront.html.erb b/templates/app/views/layouts/storefront.html.erb index 12c47a09..7512f031 100644 --- a/templates/app/views/layouts/storefront.html.erb +++ b/templates/app/views/layouts/storefront.html.erb @@ -5,7 +5,7 @@ <%= title %> <%== meta_data_tags %> - <%= canonical_tag(current_store.url) %> + <%= simple_canonical_tag %> <%= csrf_meta_tags %> <%= stylesheet_link_tag "tailwind", "data-turbo-track": "reload" %> From ea158d667a643955cee390b1fa3f2ba1222e7393 Mon Sep 17 00:00:00 2001 From: Chris Todorov Date: Fri, 29 Nov 2024 14:55:53 -0800 Subject: [PATCH 3/3] Remove dependency on `canonical-rails` Now that we have introduced the `simple_canonical_tag` helper in the previous commit, this is no longer used and needed. Co-authored-by: Andrew Stewart Co-authored-by: Benjamin Willems --- template.rb | 2 -- templates/config/initializers/canonical_rails.rb | 16 ---------------- 2 files changed, 18 deletions(-) delete mode 100644 templates/config/initializers/canonical_rails.rb diff --git a/template.rb b/template.rb index 8c90fb58..38260b28 100644 --- a/template.rb +++ b/template.rb @@ -65,7 +65,6 @@ end gem 'responders' - gem 'canonical-rails' gem 'solidus_support' gem 'truncate_html' gem 'view_component', '~> 3.0' @@ -105,7 +104,6 @@ directory 'public', 'public' copy_file 'config/initializers/solidus_auth_devise_unauthorized_redirect.rb' - copy_file 'config/initializers/canonical_rails.rb' copy_file 'config/routes/storefront.rb' copy_file 'config/tailwind.config.js' create_file 'app/assets/builds/tailwind.css' diff --git a/templates/config/initializers/canonical_rails.rb b/templates/config/initializers/canonical_rails.rb deleted file mode 100644 index bd8754bc..00000000 --- a/templates/config/initializers/canonical_rails.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -CanonicalRails.setup do |config| - # http://en.wikipedia.org/wiki/URL_normalization - # Trailing slash represents semantics of a directory, ie a collection view - implying an :index get route; - # otherwise we have to assume semantics of an instance of a resource type, a member view - implying a :show get route - # - # Acts as a whitelist for routes to have trailing slashes - - config.collection_actions # = [:index] - - # Parameter spamming can cause index dilution by creating seemingly different URLs with identical or near-identical content. - # Unless whitelisted, these parameters will be omitted - - config.allowed_parameters = [:keywords, :page, :search, :taxon] -end