From 4037cdb6dbc49504676b90cd8d16e9f9dcc6ce53 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 11 May 2022 09:29:24 +1000 Subject: [PATCH] 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 | 21 +++-- .../user_bookmark_base_serializer.rb | 24 ++++++ .../user_bookmark_list_serializer.rb | 10 +-- .../user_post_bookmark_serializer.rb | 6 ++ ...ser_post_topic_bookmark_base_serializer.rb | 6 -- .../user_topic_bookmark_serializer.rb | 6 ++ app/views/users/bookmarks.ics.erb | 19 +++++ spec/requests/users_controller_spec.rb | 82 +++++++++++-------- spec/support/bookmarkable_helper.rb | 34 +++++++- 9 files changed, 153 insertions(+), 55 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 124fbd77c9c..53ecf2f9baf 100644 --- a/app/controllers/users_controller.rb +++ b/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 - .where(user_id: user.id) - .includes(:topic) - .order(:reminder_at) + if SiteSetting.use_polymorphic_bookmarks + @bookmark_reminders = Bookmark.with_reminders + .where(user_id: user.id) + .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 diff --git a/app/serializers/user_bookmark_base_serializer.rb b/app/serializers/user_bookmark_base_serializer.rb index 94ac277c79d..41c5e69eb2a 100644 --- a/app/serializers/user_bookmark_base_serializer.rb +++ b/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 diff --git a/app/serializers/user_bookmark_list_serializer.rb b/app/serializers/user_bookmark_list_serializer.rb index 25c840f9c64..b8e472d15b7 100644 --- a/app/serializers/user_bookmark_list_serializer.rb +++ b/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 diff --git a/app/serializers/user_post_bookmark_serializer.rb b/app/serializers/user_post_bookmark_serializer.rb index d1cafcf6ff5..a9b32aad7e4 100644 --- a/app/serializers/user_post_bookmark_serializer.rb +++ b/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 diff --git a/app/serializers/user_post_topic_bookmark_base_serializer.rb b/app/serializers/user_post_topic_bookmark_base_serializer.rb index b33898d2381..e62d82e670f 100644 --- a/app/serializers/user_post_topic_bookmark_base_serializer.rb +++ b/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 diff --git a/app/serializers/user_topic_bookmark_serializer.rb b/app/serializers/user_topic_bookmark_serializer.rb index c6a02b8772a..2b07f4b5cc1 100644 --- a/app/serializers/user_topic_bookmark_serializer.rb +++ b/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 diff --git a/app/views/users/bookmarks.ics.erb b/app/views/users/bookmarks.ics.erb index a0c93c13881..8b0b6be6c96 100644 --- a/app/views/users/bookmarks.ics.erb +++ b/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 %> diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 244f5fe5494..9e90df0a79a 100644 --- a/spec/requests/users_controller_spec.rb +++ b/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 - "

Something 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 - 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: "

Something cooked

") + 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 diff --git a/spec/support/bookmarkable_helper.rb b/spec/support/bookmarkable_helper.rb index 9f111b36d56..6e4bdce1718 100644 --- a/spec/support/bookmarkable_helper.rb +++ b/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