FIX: Allow .ics for polymorphic bookmarks (#16694)

We have a .ics endpoint for user bookmarks, this
commit makes it so polymorphic bookmarks work on
that endpoint, using the serializer associated with
the RegisteredBookmarkable.

app/controllers/users_controller.rb

@@ -1704,6 +1704,7 @@ class UsersController < ApplicationController
   def bookmarks
     user = fetch_user_from_params
     guardian.ensure_can_edit!(user)
+    user_guardian = Guardian.new(user)
 
     respond_to do |format|
       format.json do
@@ -1720,12 +1721,22 @@ class UsersController < ApplicationController
           render_serialized(bookmark_list, UserBookmarkListSerializer)
         end
       end
-      # TODO (martin) Make a separate PR for .ics reminders for polymorphic bookmarks
       format.ics do
-        @bookmark_reminders = Bookmark.with_reminders
+        if SiteSetting.use_polymorphic_bookmarks
-          .where(user_id: user.id)
+          @bookmark_reminders = Bookmark.with_reminders
-          .includes(:topic)
+            .where(user_id: user.id)
-          .order(:reminder_at)
+            .order(:reminder_at)
+            .map do |bookmark|
+            bookmark.registered_bookmarkable.serializer.new(
+              bookmark, scope: user_guardian, root: false
+            )
+          end
+        else
+          @bookmark_reminders = Bookmark.with_reminders
+            .where(user_id: user.id)
+            .includes(:topic)
+            .order(:reminder_at)
+        end
       end
     end
   end

app/serializers/user_bookmark_base_serializer.rb

@@ -6,6 +6,8 @@ class UserBookmarkBaseSerializer < ApplicationSerializer
              :updated_at,
              :name,
              :reminder_at,
+             :reminder_at_ics_start,
+             :reminder_at_ics_end,
              :pinned,
              :title,
              :fancy_title,
@@ -34,6 +36,22 @@ class UserBookmarkBaseSerializer < ApplicationSerializer
     raise NotImplementedError
   end
 
+  def include_reminder_at_ics_start?
+    reminder_at.present?
+  end
+
+  def include_reminder_at_ics_end?
+    reminder_at.present?
+  end
+
+  def reminder_at_ics_start
+    object.reminder_at_ics
+  end
+
+  def reminder_at_ics_end
+    object.reminder_at_ics(offset: 1.hour)
+  end
+
   # Note: This assumes that the bookmarkable has a user attached to it,
   # we may need to revisit this assumption at some point.
   has_one :user, serializer: BasicUserSerializer, embed: :objects
@@ -41,4 +59,10 @@ class UserBookmarkBaseSerializer < ApplicationSerializer
   def user
     bookmarkable_user
   end
+
+  private
+
+  def bookmarkable
+    object.bookmarkable
+  end
 end

app/serializers/user_bookmark_list_serializer.rb

@@ -6,7 +6,7 @@ class UserBookmarkListSerializer < ApplicationSerializer
   def bookmarks
     if SiteSetting.use_polymorphic_bookmarks
       object.bookmarks.map do |bm|
-        serialize_registered_type(bm)
+        bm.registered_bookmarkable.serializer.new(bm, scope: scope, root: false)
       end
     else
       object.bookmarks.map { |bm| UserBookmarkSerializer.new(bm, scope: scope, root: false) }
@@ -16,12 +16,4 @@ class UserBookmarkListSerializer < ApplicationSerializer
   def include_more_bookmarks_url?
     @include_more_bookmarks_url ||= object.bookmarks.size == object.per_page
   end
-
-  private
-
-  def serialize_registered_type(bookmark)
-    Bookmark.registered_bookmarkable_from_type(
-      bookmark.bookmarkable_type
-    ).serializer.new(bookmark, scope: scope, root: false)
-  end
 end

app/serializers/user_post_bookmark_serializer.rb

@@ -31,6 +31,12 @@ class UserPostBookmarkSerializer < UserPostTopicBookmarkBaseSerializer
     @bookmarkable_user ||= post.user
   end
 
+  # NOTE: In the UI there are special topic-status and topic-link components to
+  # display the topic URL, this is only used for certain routes like the .ics bookmarks.
+  def bookmarkable_url
+    post.full_url
+  end
+
   private
 
   def topic

app/serializers/user_post_topic_bookmark_base_serializer.rb

@@ -63,12 +63,6 @@ class UserPostTopicBookmarkBaseSerializer < UserBookmarkBaseSerializer
     topic.slug
   end
 
-  # Note: This is nil because in the UI there are special topic-status and
-  # topic-link components to display the topic URL, and this is not used.
-  def bookmarkable_url
-    nil
-  end
-
   private
 
   def topic_user

app/serializers/user_topic_bookmark_serializer.rb

@@ -53,6 +53,12 @@ class UserTopicBookmarkSerializer < UserPostTopicBookmarkBaseSerializer
     @bookmarkable_user ||= first_post.user
   end
 
+  # NOTE: In the UI there are special topic-status and topic-link components to
+  # display the topic URL, this is only used for certain routes like the .ics bookmarks.
+  def bookmarkable_url
+    topic.url
+  end
+
   private
 
   def topic

app/views/users/bookmarks.ics.erb

@@ -1,3 +1,21 @@
+<% if SiteSetting.use_polymorphic_bookmarks %>
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//Discourse//<%= Discourse.current_hostname %>//<%= Discourse.full_version %>//EN
+<% @bookmark_reminders.each do |bookmark| %>
+BEGIN:VEVENT
+UID:bookmark_reminder_#<%= bookmark.id %>@<%= Discourse.current_hostname %>
+DTSTAMP:<%= bookmark.updated_at.strftime(I18n.t("datetime_formats.formats.calendar_ics")) %>
+DTSTART:<%= bookmark.reminder_at_ics_start %>
+DTEND:<%= bookmark.reminder_at_ics_end %>
+SUMMARY:<%= bookmark.name.presence || bookmark.title %>
+DESCRIPTION:<%= bookmark.bookmarkable_url %>
+URL:<%= bookmark.bookmarkable_url %>
+END:VEVENT
+<% end %>
+END:VCALENDAR
+
+<% else %>
 BEGIN:VCALENDAR
 VERSION:2.0
 PRODID:-//Discourse//<%= Discourse.current_hostname %>//<%= Discourse.full_version %>//EN
@@ -13,3 +31,4 @@ URL:<%= Discourse.base_url %>/t/-/<%= bookmark.topic_id %>
 END:VEVENT
 <% end %>
 END:VCALENDAR
+<% end %>

spec/requests/users_controller_spec.rb

@@ -5254,7 +5254,9 @@ describe UsersController do
         sign_in(user1)
         get "/u/#{user1.username}/bookmarks.json"
         expect(response.status).to eq(200)
-        expect(response.parsed_body['user_bookmark_list']['bookmarks'].map { |b| b['id'] }).to match_array([bookmark1.id, bookmark2.id])
+        expect(response.parsed_body['user_bookmark_list']['bookmarks'].map { |b| b['id'] }).to match_array(
+          [bookmark1.id, bookmark2.id]
+        )
       end
 
       it "returns an .ics file of bookmark reminders for the user in date order" do
@@ -5317,39 +5319,6 @@ describe UsersController do
     end
 
     context "for polymorphic bookmarks" do
-      class UserTestBookmarkSerializer < UserBookmarkBaseSerializer
-        def title
-          fancy_title
-        end
-
-        def fancy_title
-          @fancy_title ||= user.username
-        end
-
-        def cooked
-          "<p>Something cooked</p>"
-        end
-
-        def bookmarkable_user
-          @bookmarkable_user ||= user
-        end
-
-        def bookmarkable_url
-          "#{Discourse.base_url}/u/#{user.username}"
-        end
-
-        def excerpt
-          return nil unless cooked
-          @excerpt ||= PrettyText.excerpt(cooked, 300, keep_emoji_images: true)
-        end
-
-        private
-
-        def user
-          object.bookmarkable
-        end
-      end
-
       before do
         SiteSetting.use_polymorphic_bookmarks = true
         register_test_bookmarkable
@@ -5370,6 +5339,8 @@ describe UsersController do
 
       it "returns a list of serialized bookmarks for the user including custom registered bookmarkables" do
         sign_in(user1)
+        bookmark3.bookmarkable.user_profile.update!(bio_raw: "<p>Something cooked</p>")
+        bookmark3.bookmarkable.user_profile.rebake!
         get "/u/#{user1.username}/bookmarks.json"
         expect(response.status).to eq(200)
         response_bookmarks = response.parsed_body['user_bookmark_list']['bookmarks']
@@ -5378,6 +5349,49 @@ describe UsersController do
         )
         expect(response_bookmarks.find { |b| b['id'] == bookmark3.id }['excerpt']).to eq('Something cooked')
       end
+
+      it "returns an .ics file of bookmark reminders for the user in date order" do
+        bookmark1.update!(name: nil, reminder_at: 1.day.from_now)
+        bookmark2.update!(name: "Some bookmark note", reminder_at: 1.week.from_now)
+        bookmark3.update!(name: nil, reminder_at: 2.weeks.from_now)
+
+        sign_in(user1)
+        get "/u/#{user1.username}/bookmarks.ics"
+        expect(response.status).to eq(200)
+        expect(response.body.chomp).to eq(<<~ICS)
+        BEGIN:VCALENDAR
+        VERSION:2.0
+        PRODID:-//Discourse//#{Discourse.current_hostname}//#{Discourse.full_version}//EN
+        BEGIN:VEVENT
+        UID:bookmark_reminder_##{bookmark1.id}@#{Discourse.current_hostname}
+        DTSTAMP:#{bookmark1.updated_at.strftime(I18n.t("datetime_formats.formats.calendar_ics"))}
+        DTSTART:#{bookmark1.reminder_at_ics}
+        DTEND:#{bookmark1.reminder_at_ics(offset: 1.hour)}
+        SUMMARY:#{bookmark1.bookmarkable.topic.title}
+        DESCRIPTION:#{bookmark1.bookmarkable.full_url}
+        URL:#{bookmark1.bookmarkable.full_url}
+        END:VEVENT
+        BEGIN:VEVENT
+        UID:bookmark_reminder_##{bookmark2.id}@#{Discourse.current_hostname}
+        DTSTAMP:#{bookmark2.updated_at.strftime(I18n.t("datetime_formats.formats.calendar_ics"))}
+        DTSTART:#{bookmark2.reminder_at_ics}
+        DTEND:#{bookmark2.reminder_at_ics(offset: 1.hour)}
+        SUMMARY:Some bookmark note
+        DESCRIPTION:#{bookmark2.bookmarkable.url}
+        URL:#{bookmark2.bookmarkable.url}
+        END:VEVENT
+        BEGIN:VEVENT
+        UID:bookmark_reminder_##{bookmark3.id}@#{Discourse.current_hostname}
+        DTSTAMP:#{bookmark3.updated_at.strftime(I18n.t("datetime_formats.formats.calendar_ics"))}
+        DTSTART:#{bookmark3.reminder_at_ics}
+        DTEND:#{bookmark3.reminder_at_ics(offset: 1.hour)}
+        SUMMARY:#{bookmark3.bookmarkable.username}
+        DESCRIPTION:#{Discourse.base_url}/u/#{bookmark3.bookmarkable.username}
+        URL:#{Discourse.base_url}/u/#{bookmark3.bookmarkable.username}
+        END:VEVENT
+        END:VCALENDAR
+        ICS
+      end
     end
   end
 

spec/support/bookmarkable_helper.rb

@@ -1,6 +1,38 @@
 # frozen_string_literal: true
 
-class UserTestBookmarkSerializer < UserBookmarkBaseSerializer; end
+class UserTestBookmarkSerializer < UserBookmarkBaseSerializer
+  def title
+    fancy_title
+  end
+
+  def fancy_title
+    @fancy_title ||= user.username
+  end
+
+  def cooked
+    user.user_profile&.bio_cooked
+  end
+
+  def bookmarkable_user
+    @bookmarkable_user ||= user
+  end
+
+  def bookmarkable_url
+    "#{Discourse.base_url}/u/#{user.username}"
+  end
+
+  def excerpt
+    return nil unless cooked
+    @excerpt ||= PrettyText.excerpt(cooked, 300, keep_emoji_images: true)
+  end
+
+  private
+
+  def user
+    object.bookmarkable
+  end
+end
+
 class UserTestBookmarkable < BaseBookmarkable
   def self.model
     User