Drop forum subscriptions.

Few people used forum subscriptions (only around 100), and even fewer
people were subscribed to active threads. Most subscriptions were for
old threads that will never be bumped again. The implementation also had
a few problems:

* Unsubscribe links in emails didn't work (they unset the user's
  receive_email_notifications flag, but forum subscriptions didn't
  respect this flag).
* Some users had invalid email addresses, which caused notifications to
  bounce. There was no mechanism for preventing bounces.
* The implementation wasn't scalable. It involved a daily linear scan
  over _all_ forum subscriptions looking for any topics that had been updated.
This commit is contained in:
evazion
2020-01-19 13:44:09 -06:00
parent cae9a5d7e3
commit 13528ac2d3
12 changed files with 11 additions and 149 deletions

View File

@@ -4,8 +4,8 @@ class ForumTopicsController < ApplicationController
before_action :member_only, :except => [:index, :show] before_action :member_only, :except => [:index, :show]
before_action :moderator_only, :only => [:new_merge, :create_merge] before_action :moderator_only, :only => [:new_merge, :create_merge]
before_action :normalize_search, :only => :index before_action :normalize_search, :only => :index
before_action :load_topic, :only => [:edit, :show, :update, :destroy, :undelete, :new_merge, :create_merge, :subscribe, :unsubscribe] before_action :load_topic, :only => [:edit, :show, :update, :destroy, :undelete, :new_merge, :create_merge]
before_action :check_min_level, :only => [:show, :edit, :update, :new_merge, :create_merge, :destroy, :undelete, :subscribe, :unsubscribe] before_action :check_min_level, :only => [:show, :edit, :update, :new_merge, :create_merge, :destroy, :undelete]
skip_before_action :api_check skip_before_action :api_check
def new def new
@@ -88,20 +88,6 @@ class ForumTopicsController < ApplicationController
redirect_to forum_topic_path(@merged_topic) redirect_to forum_topic_path(@merged_topic)
end end
def subscribe
subscription = ForumSubscription.where(:forum_topic_id => @forum_topic.id, :user_id => CurrentUser.user.id).first
unless subscription
ForumSubscription.create(:forum_topic_id => @forum_topic.id, :user_id => CurrentUser.user.id, :last_read_at => @forum_topic.updated_at)
end
respond_with(@forum_topic)
end
def unsubscribe
subscription = ForumSubscription.where(:forum_topic_id => @forum_topic.id, :user_id => CurrentUser.user.id).first
subscription&.destroy
respond_with(@forum_topic)
end
private private
def normalize_search def normalize_search

View File

@@ -9,7 +9,6 @@ module DanbooruMaintenance
safely { Upload.prune! } safely { Upload.prune! }
safely { Delayed::Job.where('created_at < ?', 45.days.ago).delete_all } safely { Delayed::Job.where('created_at < ?', 45.days.ago).delete_all }
safely { PostDisapproval.prune! } safely { PostDisapproval.prune! }
safely { ForumSubscription.process_all! }
safely { PostDisapproval.dmail_messages! } safely { PostDisapproval.dmail_messages! }
safely { regenerate_post_counts! } safely { regenerate_post_counts! }
safely { SuperVoter.init! } safely { SuperVoter.init! }

View File

@@ -7,11 +7,4 @@ class UserMailer < ActionMailer::Base
@dmail = dmail @dmail = dmail
mail(:to => "#{dmail.to.name} <#{dmail.to.email}>", :subject => "#{Danbooru.config.app_name} - Message received from #{dmail.from.name}") mail(:to => "#{dmail.to.name} <#{dmail.to.email}>", :subject => "#{Danbooru.config.app_name} - Message received from #{dmail.from.name}")
end end
def forum_notice(user, forum_topic, forum_posts)
@user = user
@forum_topic = forum_topic
@forum_posts = forum_posts
mail(:to => "#{user.name} <#{user.email}>", :subject => "#{Danbooru.config.app_name} forum topic #{forum_topic.title} updated")
end
end end

View File

@@ -1,24 +0,0 @@
class ForumSubscription < ApplicationRecord
belongs_to :user
belongs_to :forum_topic
def self.prune!
where("last_read_at < ?", 3.months.ago).delete_all
end
def self.process_all!
ForumSubscription.find_each do |subscription|
forum_topic = subscription.forum_topic
if forum_topic.updated_at > subscription.last_read_at
CurrentUser.scoped(subscription.user, "127.0.0.1") do
forum_posts = forum_topic.posts.where("created_at > ?", subscription.last_read_at).order("id desc")
begin
UserMailer.forum_notice(subscription.user, forum_topic, forum_posts).deliver_now
rescue Net::SMTPSyntaxError
end
subscription.update_attribute(:last_read_at, forum_topic.updated_at)
end
end
end
end
end

View File

@@ -16,7 +16,6 @@ class ForumTopic < ApplicationRecord
has_many :posts, -> {order("forum_posts.id asc")}, :class_name => "ForumPost", :foreign_key => "topic_id", :dependent => :destroy has_many :posts, -> {order("forum_posts.id asc")}, :class_name => "ForumPost", :foreign_key => "topic_id", :dependent => :destroy
has_many :moderation_reports, through: :posts has_many :moderation_reports, through: :posts
has_one :original_post, -> {order("forum_posts.id asc")}, class_name: "ForumPost", foreign_key: "topic_id", inverse_of: :topic has_one :original_post, -> {order("forum_posts.id asc")}, class_name: "ForumPost", foreign_key: "topic_id", inverse_of: :topic
has_many :subscriptions, :class_name => "ForumSubscription"
before_validation :initialize_is_deleted, :on => :create before_validation :initialize_is_deleted, :on => :create
validates_presence_of :title validates_presence_of :title
validates_associated :original_post validates_associated :original_post
@@ -126,16 +125,9 @@ class ForumTopic < ApplicationRecord
end end
end end
module SubscriptionMethods
def user_subscription(user)
subscriptions.where(:user_id => user.id).first
end
end
extend SearchMethods extend SearchMethods
include CategoryMethods include CategoryMethods
include VisitMethods include VisitMethods
include SubscriptionMethods
def editable_by?(user) def editable_by?(user)
(creator_id == user.id || user.is_moderator?) && visible?(user) (creator_id == user.id || user.is_moderator?) && visible?(user)

View File

@@ -18,11 +18,6 @@
<% if CurrentUser.is_member? && @forum_topic && !@forum_topic.new_record? %> <% if CurrentUser.is_member? && @forum_topic && !@forum_topic.new_record? %>
<li>|</li> <li>|</li>
<%= subnav_link_to "Reply", new_forum_post_path(:topic_id => @forum_topic.id) %> <%= subnav_link_to "Reply", new_forum_post_path(:topic_id => @forum_topic.id) %>
<% if @forum_topic.user_subscription(CurrentUser.user) %>
<%= subnav_link_to "Unsubscribe", unsubscribe_forum_topic_path(@forum_topic), :method => :post %>
<% else %>
<%= subnav_link_to "Subscribe", subscribe_forum_topic_path(@forum_topic), :method => :post, :data => {:confirm => "Are you sure you want to receive email notifications for this forum topic?"} %>
<% end %>
<% if !@forum_topic.new_record? && @forum_topic.editable_by?(CurrentUser.user) %> <% if !@forum_topic.new_record? && @forum_topic.editable_by?(CurrentUser.user) %>
<%= subnav_link_to "Edit", edit_forum_topic_path(@forum_topic), "data-shortcut": "e" %> <%= subnav_link_to "Edit", edit_forum_topic_path(@forum_topic), "data-shortcut": "e" %>
<% if CurrentUser.is_moderator? %> <% if CurrentUser.is_moderator? %>

View File

@@ -1,13 +0,0 @@
<p>"<%= @forum_topic.title %>" was updated:</p>
<% @forum_posts.each do |forum_post| %>
<p>
<strong><%= forum_post.creator.name %> said:</strong>
</p>
<%= format_text(forum_post.body, base_url: root_url) %>
<br>
<hr>
<br>
<% end %>
<p><%= link_to "View topic", forum_topic_url(@forum_topic, :page => @forum_topic.last_page, :host => Danbooru.config.hostname, :only_path => false) %> | <%= link_to "Unsubscribe", maintenance_user_email_notification_url(:user_id => @user.id, :sig => email_sig(@user), :host => Danbooru.config.hostname, :only_path => false) %></p>

View File

@@ -142,8 +142,6 @@ Rails.application.routes.draw do
post :undelete post :undelete
get :new_merge get :new_merge
post :create_merge post :create_merge
post :subscribe
post :unsubscribe
end end
collection do collection do
post :mark_all_as_read post :mark_all_as_read

View File

@@ -0,0 +1,7 @@
require_relative "20140725003232_create_forum_subscriptions"
class DropForumSubscriptions < ActiveRecord::Migration[6.0]
def change
revert CreateForumSubscriptions
end
end

View File

@@ -2014,38 +2014,6 @@ CREATE SEQUENCE public.forum_posts_id_seq
ALTER SEQUENCE public.forum_posts_id_seq OWNED BY public.forum_posts.id; ALTER SEQUENCE public.forum_posts_id_seq OWNED BY public.forum_posts.id;
--
-- Name: forum_subscriptions; Type: TABLE; Schema: public; Owner: -
--
CREATE TABLE public.forum_subscriptions (
id integer NOT NULL,
user_id integer,
forum_topic_id integer,
last_read_at timestamp without time zone,
delete_key character varying
);
--
-- Name: forum_subscriptions_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--
CREATE SEQUENCE public.forum_subscriptions_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
--
-- Name: forum_subscriptions_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
--
ALTER SEQUENCE public.forum_subscriptions_id_seq OWNED BY public.forum_subscriptions.id;
-- --
-- Name: forum_topic_visits; Type: TABLE; Schema: public; Owner: - -- Name: forum_topic_visits; Type: TABLE; Schema: public; Owner: -
-- --
@@ -4040,13 +4008,6 @@ ALTER TABLE ONLY public.forum_post_votes ALTER COLUMN id SET DEFAULT nextval('pu
ALTER TABLE ONLY public.forum_posts ALTER COLUMN id SET DEFAULT nextval('public.forum_posts_id_seq'::regclass); ALTER TABLE ONLY public.forum_posts ALTER COLUMN id SET DEFAULT nextval('public.forum_posts_id_seq'::regclass);
--
-- Name: forum_subscriptions id; Type: DEFAULT; Schema: public; Owner: -
--
ALTER TABLE ONLY public.forum_subscriptions ALTER COLUMN id SET DEFAULT nextval('public.forum_subscriptions_id_seq'::regclass);
-- --
-- Name: forum_topic_visits id; Type: DEFAULT; Schema: public; Owner: - -- Name: forum_topic_visits id; Type: DEFAULT; Schema: public; Owner: -
-- --
@@ -4394,14 +4355,6 @@ ALTER TABLE ONLY public.forum_posts
ADD CONSTRAINT forum_posts_pkey PRIMARY KEY (id); ADD CONSTRAINT forum_posts_pkey PRIMARY KEY (id);
--
-- Name: forum_subscriptions forum_subscriptions_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--
ALTER TABLE ONLY public.forum_subscriptions
ADD CONSTRAINT forum_subscriptions_pkey PRIMARY KEY (id);
-- --
-- Name: forum_topic_visits forum_topic_visits_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- Name: forum_topic_visits forum_topic_visits_pkey; Type: CONSTRAINT; Schema: public; Owner: -
-- --
@@ -6425,20 +6378,6 @@ CREATE INDEX index_forum_posts_on_topic_id ON public.forum_posts USING btree (to
CREATE INDEX index_forum_posts_on_updated_at ON public.forum_posts USING btree (updated_at); CREATE INDEX index_forum_posts_on_updated_at ON public.forum_posts USING btree (updated_at);
--
-- Name: index_forum_subscriptions_on_forum_topic_id; Type: INDEX; Schema: public; Owner: -
--
CREATE INDEX index_forum_subscriptions_on_forum_topic_id ON public.forum_subscriptions USING btree (forum_topic_id);
--
-- Name: index_forum_subscriptions_on_user_id; Type: INDEX; Schema: public; Owner: -
--
CREATE INDEX index_forum_subscriptions_on_user_id ON public.forum_subscriptions USING btree (user_id);
-- --
-- Name: index_forum_topic_visits_on_forum_topic_id; Type: INDEX; Schema: public; Owner: - -- Name: index_forum_topic_visits_on_forum_topic_id; Type: INDEX; Schema: public; Owner: -
-- --
@@ -7426,6 +7365,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20200115010442'), ('20200115010442'),
('20200117220602'), ('20200117220602'),
('20200118015014'), ('20200118015014'),
('20200119184442'); ('20200119184442'),
('20200119193110');

View File

@@ -1,3 +0,0 @@
FactoryBot.define do
factory(:forum_subscription)
end

View File

@@ -3,12 +3,4 @@ class UserMailerPreview < ActionMailer::Preview
dmail = User.admins.first.dmails.first dmail = User.admins.first.dmails.first
UserMailer.dmail_notice(dmail) UserMailer.dmail_notice(dmail)
end end
def forum_notice
topic = ForumTopic.first
posts = topic.posts
user = topic.creator
UserMailer.forum_notice(user, topic, posts)
end
end end