From 37afe1f1a30cca4a0b59fd2f99554291f9663bb0 Mon Sep 17 00:00:00 2001 From: Duncan Stuart Date: Mon, 15 Apr 2024 11:21:51 +0200 Subject: [PATCH 1/3] Fix: Map: "All" works correctly with date in URL The issue seems to be that if there was a date in params, then that gets passed to the url helper by default, which I guess in other situations would be what we want, but here we specifically want the url to not include a date so that we get all dates. The solution is to split the url helpers so that we explicitly only get dates when we ask for them. --- app/presenters/social_listing.rb | 2 +- app/views/maps/classes.html.erb | 6 +++--- app/views/maps/socials.html.erb | 6 +++--- config/routes.rb | 6 ++++-- spec/presenters/social_listing_spec.rb | 5 +++-- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/presenters/social_listing.rb b/app/presenters/social_listing.rb index 83fcf2cc..753731be 100644 --- a/app/presenters/social_listing.rb +++ b/app/presenters/social_listing.rb @@ -32,7 +32,7 @@ def location def map_url(date) return unless show_on_map? - url_helpers.map_socials_path(date: date.to_fs(:db), venue_id: event.venue_id) + url_helpers.map_socials_date_path(date: date.to_fs(:db), venue_id: event.venue_id) end private diff --git a/app/views/maps/classes.html.erb b/app/views/maps/classes.html.erb index 03a08338..454143fa 100644 --- a/app/views/maps/classes.html.erb +++ b/app/views/maps/classes.html.erb @@ -21,9 +21,9 @@
  • <%= map_update_control_link( day.to_s, - { day: day.to_param}, + map_classes_day_path(day.to_param), selected: day.selected?, - url: map_classes_path(day.to_param, format: :json) + url: map_classes_day_path(day.to_param, format: :json) ) %>
    • <% else %> @@ -35,7 +35,7 @@
  • <%= map_update_control_link( "All", - "#", + map_classes_path, selected: @days.selected_day.blank?, url: map_classes_path(format: :json) ) %> diff --git a/app/views/maps/socials.html.erb b/app/views/maps/socials.html.erb index d27a5228..d1526446 100644 --- a/app/views/maps/socials.html.erb +++ b/app/views/maps/socials.html.erb @@ -25,9 +25,9 @@ <%= tag.li class: ("end-of-week" if date.sunday?) do %> <%= map_update_control_link( date.to_s, - { date: date.to_param }, + map_socials_date_path(date.to_param), selected: date.selected?, - url: map_socials_path(date.to_param, format: :json) + url: map_socials_date_path(date.to_param, format: :json) ) %> <% end %> <% else %> @@ -39,7 +39,7 @@
  • <%= map_update_control_link( "All", - "#", + map_socials_path, selected: @map_dates.selected_date.blank?, url: map_socials_path(format: :json) ) %> diff --git a/config/routes.rb b/config/routes.rb index ba027262..27255431 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,8 +18,10 @@ resources :external_events, only: %i[edit update] - get "map/classes/(:day)" => "maps#classes", as: :map_classes - get "map/socials/(:date)" => "maps#socials", as: :map_socials + get "map/classes" => "maps#classes", as: :map_classes + get "map/classes/:day" => "maps#classes", as: :map_classes_day + get "map/socials" => "maps#socials", as: :map_socials + get "map/socials/:date" => "maps#socials", as: :map_socials_date get "map" => "maps#socials" get "venue_map_info/:id" => "maps#venue_map_info", :as => :venue_map_info get "occasional" => "occasional_events#index" diff --git a/spec/presenters/social_listing_spec.rb b/spec/presenters/social_listing_spec.rb index 52c32394..29f380fb 100644 --- a/spec/presenters/social_listing_spec.rb +++ b/spec/presenters/social_listing_spec.rb @@ -4,6 +4,7 @@ require "active_support" require "active_support/core_ext/module/delegation" require "active_support/core_ext/string/conversions" +require "active_support/core_ext/object/blank" # needed for String#to_date require "app/presenters/social_listing" require "spec/support/time_formats_helper" @@ -76,13 +77,13 @@ context "when the venue has coordinates" do it "is a link to the venue on the map" do event = instance_double("Event", venue_id: 5, venue_coordinates: double) - url_helpers = double(map_socials_path: "a-map-url") # rubocop:disable RSpec/VerifiedDoubles + url_helpers = double(map_socials_date_path: "a-map-url") # rubocop:disable RSpec/VerifiedDoubles social_listing = described_class.new(event, url_helpers:) aggregate_failures do expect(social_listing.map_url("11 July 1936".to_date)).to eq("a-map-url") - expect(url_helpers).to have_received(:map_socials_path) + expect(url_helpers).to have_received(:map_socials_date_path) .with(date: "1936-07-11", venue_id: 5) end end From 40d357d722b05601b3de7816c9b902aa1f3c8519 Mon Sep 17 00:00:00 2001 From: Duncan Stuart Date: Mon, 15 Apr 2024 12:04:14 +0200 Subject: [PATCH 2/3] Refactor: DRY up map update control link code These are more similar than they initially appeared. --- app/helpers/map_helper.rb | 24 +++++++++++++++++++++--- app/views/maps/classes.html.erb | 14 ++------------ app/views/maps/socials.html.erb | 14 ++------------ 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/app/helpers/map_helper.rb b/app/helpers/map_helper.rb index 1980ba06..8211fb96 100644 --- a/app/helpers/map_helper.rb +++ b/app/helpers/map_helper.rb @@ -1,13 +1,31 @@ # frozen_string_literal: true module MapHelper - def map_update_control_link(text, query, selected:, url:) - link_to text, query, { + def map_update_control_link_item(url_helper, item) + map_update_control_link( + text: item.to_s, + html_url: public_send(url_helper, item.to_param), + json_url: public_send(url_helper, item.to_param, format: :json), + selected: item.selected? + ) + end + + def map_update_control_link_all(text, url_helper, selected:) + map_update_control_link( + text:, + html_url: public_send(url_helper), + json_url: public_send(url_helper, format: :json), + selected: + ) + end + + def map_update_control_link(text:, html_url:, json_url:, selected:) + link_to text, html_url, { class: (:selected if selected), data: { action: "map#update:prevent map-menu#choose:prevent", map_menu_target: "updateControl", - url: + url: json_url } } end diff --git a/app/views/maps/classes.html.erb b/app/views/maps/classes.html.erb index 454143fa..b77e5204 100644 --- a/app/views/maps/classes.html.erb +++ b/app/views/maps/classes.html.erb @@ -19,12 +19,7 @@ <% @days.menu_days.each do |day| %> <% if day.events? %>
  • - <%= map_update_control_link( - day.to_s, - map_classes_day_path(day.to_param), - selected: day.selected?, - url: map_classes_day_path(day.to_param, format: :json) - ) %> + <%= map_update_control_link_item(:map_classes_day_path, day) %>
    • <% else %>
  • @@ -33,12 +28,7 @@ <% end %> <% end %>
  • - <%= map_update_control_link( - "All", - map_classes_path, - selected: @days.selected_day.blank?, - url: map_classes_path(format: :json) - ) %> + <%= map_update_control_link_all("All", :map_classes_path, selected: @days.selected_day.blank?) %>

    • diff --git a/app/views/maps/socials.html.erb b/app/views/maps/socials.html.erb index d1526446..f0227bd2 100644 --- a/app/views/maps/socials.html.erb +++ b/app/views/maps/socials.html.erb @@ -23,12 +23,7 @@ <% @map_dates.menu_dates.each do |date| %> <% if date.events? %> <%= tag.li class: ("end-of-week" if date.sunday?) do %> - <%= map_update_control_link( - date.to_s, - map_socials_date_path(date.to_param), - selected: date.selected?, - url: map_socials_date_path(date.to_param, format: :json) - ) %> + <%= map_update_control_link_item(:map_socials_date_path, date) %> <% end %> <% else %> <%= tag.li class: ["no-events", ("end-of-week" if date.sunday?)] do %> @@ -37,12 +32,7 @@ <% end %> <% end %>

  • - <%= map_update_control_link( - "All", - map_socials_path, - selected: @map_dates.selected_date.blank?, - url: map_socials_path(format: :json) - ) %> + <%= map_update_control_link_all("All", :map_socials_path, selected: @map_dates.selected_date.blank?) %>
    • <% end %> From 113a560139b788adae5852503a6575fa7862d7bc Mon Sep 17 00:00:00 2001 From: Duncan Stuart Date: Mon, 15 Apr 2024 14:54:16 +0200 Subject: [PATCH 3/3] fixup! Fix: Map: "All" works correctly with date in URL --- app/helpers/listings_helper.rb | 2 +- app/views/sitemaps/index.xml.builder | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/listings_helper.rb b/app/helpers/listings_helper.rb index 9403c3cc..c190184c 100644 --- a/app/helpers/listings_helper.rb +++ b/app/helpers/listings_helper.rb @@ -57,7 +57,7 @@ def swingclass_listing(swingclass, day) end def class_map_url(day, venue) - map_classes_path(day:, venue_id: venue.id) unless venue.coordinates.nil? + map_classes_day_path(day:, venue_id: venue.id) unless venue.coordinates.nil? end def swingclass_link(event) diff --git a/app/views/sitemaps/index.xml.builder b/app/views/sitemaps/index.xml.builder index b425165b..ff0f6f06 100644 --- a/app/views/sitemaps/index.xml.builder +++ b/app/views/sitemaps/index.xml.builder @@ -23,7 +23,7 @@ xml.tag!( render("url", builder: xml, link_url: map_classes_url, change_frequency: "weekly", priority: 0.9) DAYNAMES.each do |day| - render("url", builder: xml, link_url: map_classes_url(day:), change_frequency: "weekly", priority: 0.7) + render("url", builder: xml, link_url: map_classes_day_url(day:), change_frequency: "weekly", priority: 0.7) end render("url", builder: xml, link_url: privacy_url, change_frequency: "yearly", priority: 0.4)