Test more Ruby gaps; fix CMS export and conflict policy bugs

app/services/cms_content_loaders/conflict_policies/update.rb

@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 class CmsContentLoaders::ConflictPolicies::Update < CmsContentLoaders::ConflictPolicy
   class SkippedItem
     attr_reader :existing_item, :new_item, :attrs, :reason
@@ -18,12 +19,14 @@ def message
   attr_reader :previous_content_by_identifier, :skipped_items
 
   def initialize(previous_content_by_identifier)
+    super()
     @previous_content_by_identifier = previous_content_by_identifier
     @skipped_items = []
   end
 
   def all_previous_content_keys
-    @all_previous_content_keys ||= Set.new(previous_content_by_identifier.flat_map(&:keys))
+    @all_previous_content_keys ||=
+      Set.new(previous_content_by_identifier.values.flat_map { |content| content.keys.map(&:to_s) })
   end
 
   def previous_content_for(identifier)
@@ -35,14 +38,17 @@ def previous_content_for(identifier)
   end
 
   def action_for(existing_item, new_item, attrs)
+    # nothing in the database to conflict with
+    return :overwrite unless existing_item.model
+
     previous_content = previous_content_for(existing_item.identifier)
 
     modified_keys = []
     previous_content.stringify_keys.each do |key, previous_value|
-      existing_value = existing_item.model&.public_send(key)&.strip
-      new_value = attrs.stringify_keys[key]&.strip
+      existing_value = strip_if_string(existing_item.model.public_send(key))
+      new_value = strip_if_string(attrs.stringify_keys[key])
 
-      modified_keys << key if existing_value != previous_value&.strip && existing_value != new_value
+      modified_keys << key if existing_value != strip_if_string(previous_value) && existing_value != new_value
     end
 
     if modified_keys.any?
@@ -57,4 +63,11 @@ def action_for(existing_item, new_item, attrs)
       :overwrite
     end
   end
+
+  private
+
+  # (attributes aren't all text; booleans and so on have no whitespace to ignore)
+  def strip_if_string(value)
+    value.is_a?(String) ? value.strip : value
+  end
 end

app/services/export_cms_content_set_service.rb

@@ -45,8 +45,8 @@ def export_content_from_adapter(storage_adapter) # rubocop:disable Metrics/Metho
       inherited_item = inherited_items[item.identifier]
 
       if inherited_item
-        inherited_attrs = storage_adapter.read_item_attrs(inherited_item).stringify_keys.transform_values(&:strip)
-        own_attrs = item.model.attributes.slice(*inherited_attrs.keys).stringify_keys.transform_values(&:strip)
+        inherited_attrs = strip_string_values(storage_adapter.read_item_attrs(inherited_item).stringify_keys)
+        own_attrs = strip_string_values(item.model.attributes.slice(*inherited_attrs.keys).stringify_keys)
 
         if own_attrs == inherited_attrs
           Rails.logger.info(
@@ -60,6 +60,11 @@ def export_content_from_adapter(storage_adapter) # rubocop:disable Metrics/Metho
     end
   end
 
+  # (attributes include booleans and so on as well as text, and only the text has whitespace to ignore)
+  def strip_string_values(attrs)
+    attrs.transform_values { |value| value.is_a?(String) ? value.strip : value }
+  end
+
   def export_metadata
     File.open(File.expand_path("metadata.yml", content_set.root_path), "w") do |f|
       metadata = {
@@ -95,11 +100,11 @@ def serialize_navigation_items(items)
   end
 
   def serialize_variables
-    cms_parent.cms_variables.order(:key).each_with_object({}) { |variable, hash| hash[variable.key] = variable.value }
+    cms_parent.cms_variables.order(:key).to_h { |variable| [variable.key, variable.value] }
   end
 
   def inherited_content_sets
-    @inherit_content_sets ||= inherit.map { |content_set_name| CmsContentSet.new(name: content_set_name) }
+    @inherited_content_sets ||= inherit.map { |content_set_name| CmsContentSet.new(name: content_set_name) }
   end
 
   def ensure_no_conflicting_folder

test/controllers/calendars_controller_test.rb

@@ -0,0 +1,150 @@
+# frozen_string_literal: true
+require "test_helper"
+
+describe CalendarsController do
+  let(:convention) { create(:convention, name: "Test Con", timezone_name: "America/New_York") }
+  let(:user_con_profile) do
+    create(:user_con_profile, convention:, user: create(:user, first_name: "Alice", last_name: "Attendee"))
+  end
+
+  setup { set_convention convention }
+
+  def get_schedule(profile = user_con_profile, secret: profile.ical_secret)
+    get :user_schedule, params: { id: secret }
+    response
+  end
+
+  def signup_to(
+    event_title,
+    state: "confirmed",
+    title_suffix: nil,
+    rooms: [],
+    starts_at: convention.starts_at + 2.hours
+  )
+    event = create(:event, convention:, title: event_title, short_blurb: "About #{event_title}")
+    run = create(:run, event:, starts_at:, title_suffix:)
+    rooms.each { |name| run.rooms << create(:room, convention:, name:) }
+    create(:signup, run:, user_con_profile:, state:, counted: state == "confirmed")
+  end
+
+  def calendar
+    Icalendar::Calendar.parse(response.body).first
+  end
+
+  describe "GET user_schedule" do
+    it "serves a calendar" do
+      get_schedule
+
+      assert_response :success
+      assert_equal "text/calendar", response.media_type
+      assert_not_nil calendar
+    end
+
+    it "names the calendar for the convention and the attendee" do
+      get_schedule
+
+      assert_equal "Test Con Schedule for Alice Attendee", calendar.x_wr_calname.first.to_s
+    end
+
+    it "includes the convention's timezone" do
+      get_schedule
+
+      assert_equal(["America/New_York"], calendar.timezones.map { |timezone| timezone.tzid.to_s })
+    end
+
+    it "has an event for each signup" do
+      signup_to "First Game"
+      signup_to "Second Game"
+
+      get_schedule
+
+      assert_equal ["First Game", "Second Game"], calendar.events.map { |event| event.summary.to_s }.sort
+    end
+
+    it "describes each event with its time, description and link to the event page" do
+      signup = signup_to("First Game", starts_at: Time.utc(2016, 10, 28, 20, 0, 0))
+
+      get_schedule
+
+      event = calendar.events.first
+      assert_equal signup.run.starts_at, event.dtstart.utc
+      assert_equal signup.run.ends_at, event.dtend.utc
+      assert_equal "America/New_York", event.dtstart.ical_params["tzid"].first
+      assert_equal "About First Game", event.description.to_s
+      assert_includes event.url.to_s, "/events/#{signup.event.to_param}"
+      assert_includes event.url.to_s, convention.domain
+    end
+
+    it "marks waitlisted signups" do
+      signup_to "Full Game", state: "waitlisted"
+
+      get_schedule
+
+      assert_equal "[WAITLISTED] Full Game", calendar.events.first.summary.to_s
+    end
+
+    it "adds a run's title suffix to the summary" do
+      signup_to "Repeat Game", title_suffix: "Late Night"
+
+      get_schedule
+
+      assert_equal "Repeat Game (Late Night)", calendar.events.first.summary.to_s
+    end
+
+    it "lists the run's rooms" do
+      signup_to "Roomy Game", rooms: %w[Salon Ballroom]
+
+      get_schedule
+
+      assert_equal %w[Ballroom Salon],
+                   Array(calendar.events.first.location)
+                     .flatten
+                     .map(&:to_s)
+                     .flat_map { |l| l.split(",").map(&:strip) }
+                     .sort
+    end
+
+    it "leaves out signups that were withdrawn" do
+      signup_to "Dropped Game", state: "withdrawn"
+      signup_to "Kept Game"
+
+      get_schedule
+
+      assert_equal(["Kept Game"], calendar.events.map { |event| event.summary.to_s })
+    end
+
+    it "does not include other attendees' signups" do
+      other = create(:user_con_profile, convention:)
+      event = create(:event, convention:, title: "Somebody Else's Game")
+      create(:signup, run: create(:run, event:), user_con_profile: other)
+
+      get_schedule
+
+      assert_empty calendar.events
+    end
+
+    it "is an empty calendar for someone with no signups" do
+      get_schedule
+
+      assert_response :success
+      assert_empty calendar.events
+    end
+
+    it "does not need a login, just the secret in the address" do
+      get_schedule
+
+      assert_response :success
+    end
+
+    it "is not found with the wrong secret" do
+      assert_raises(ActiveRecord::RecordNotFound) { get_schedule(secret: "not-the-secret") }
+    end
+
+    it "is not found at another convention with this attendee's secret" do
+      other_convention = create(:convention)
+      set_convention other_convention
+
+      assert_raises(ActiveRecord::RecordNotFound) { get_schedule }
+    end
+  end
+end