From f5d1aff8dd0ef2df7f99d9e8619bbd34155ee83d Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 17 Oct 2019 16:56:40 +1100 Subject: [PATCH] FEATURE: experimental hidden setting for draft backups Under exceptional situations the automatic draft feature can fail. This new **hidden, default off** site setting `backup_drafts_to_pm_length` will automatically backup any draft that is saved by the system to a dedicated PM (originating from self) The body of that PM will contain the text of the reply. We can enable this feature strategically on sites exhibiting issues to diagnose issues with the draft system and offer a recourse to users who appear to lose drafts. We automatically checkpoint these drafts every 5 minutes forcing a new revision each 5 minutes so you can revert to old content. Longer term we are considering automatically enabling this kind of feature for extremely long drafts where the risk is really high one could lose days of writing. --- app/models/backup_draft_post.rb | 23 +++++ app/models/backup_draft_topic.rb | 22 +++++ app/models/draft.rb | 90 +++++++++++++++++++ config/locales/server.en.yml | 5 +- config/site_settings.yml | 4 + .../20191017044811_add_draft_backup_tables.rb | 25 ++++++ lib/post_creator.rb | 1 + spec/models/draft_spec.rb | 77 +++++++++++----- 8 files changed, 224 insertions(+), 23 deletions(-) create mode 100644 app/models/backup_draft_post.rb create mode 100644 app/models/backup_draft_topic.rb create mode 100644 db/migrate/20191017044811_add_draft_backup_tables.rb diff --git a/app/models/backup_draft_post.rb b/app/models/backup_draft_post.rb new file mode 100644 index 0000000000..9dd552452b --- /dev/null +++ b/app/models/backup_draft_post.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class BackupDraftPost < ActiveRecord::Base + belongs_to :user + belongs_to :post +end + +# == Schema Information +# +# Table name: backup_draft_posts +# +# id :bigint not null, primary key +# user_id :integer not null +# post_id :integer not null +# key :string not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_backup_draft_posts_on_post_id (post_id) UNIQUE +# index_backup_draft_posts_on_user_id_and_key (user_id,key) UNIQUE +# diff --git a/app/models/backup_draft_topic.rb b/app/models/backup_draft_topic.rb new file mode 100644 index 0000000000..c3fdfe1204 --- /dev/null +++ b/app/models/backup_draft_topic.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class BackupDraftTopic < ActiveRecord::Base + belongs_to :user + belongs_to :topic +end + +# == Schema Information +# +# Table name: backup_draft_topics +# +# id :bigint not null, primary key +# user_id :integer not null +# topic_id :integer not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_backup_draft_topics_on_topic_id (topic_id) UNIQUE +# index_backup_draft_topics_on_user_id (user_id) UNIQUE +# diff --git a/app/models/draft.rb b/app/models/draft.rb index 9905cbe1c5..287d25c263 100644 --- a/app/models/draft.rb +++ b/app/models/draft.rb @@ -6,6 +6,10 @@ class Draft < ActiveRecord::Base EXISTING_TOPIC ||= 'topic_' def self.set(user, key, sequence, data) + if SiteSetting.backup_drafts_to_pm_length > 0 && SiteSetting.backup_drafts_to_pm_length < data.length + backup_draft(user, key, sequence, data) + end + if d = find_draft(user, key) return if d.sequence > sequence @@ -96,6 +100,92 @@ class Draft < ActiveRecord::Base Draft.where("updated_at < ?", delete_drafts_older_than_n_days).destroy_all end + def self.backup_draft(user, key, sequence, data) + reply = JSON.parse(data)["reply"] || "" + return if reply.length < SiteSetting.backup_drafts_to_pm_length + + post_id = BackupDraftPost.where(user_id: user.id, key: key).pluck(:post_id).first + post = Post.where(id: post_id).first if post_id + + if post_id && !post + BackupDraftPost.where(user_id: user.id, key: key).delete_all + end + + indented_reply = reply.split("\n").map! do |l| + " #{l}" + end + draft_body = <<~MD + #{indented_reply.join("\n")} + + ```text + seq: #{sequence} + key: #{key} + ``` + MD + + return if post && post.raw == draft_body + + if !post + topic = ensure_draft_topic!(user) + Post.transaction do + post = PostCreator.new( + user, + raw: draft_body, + skip_jobs: true, + skip_validations: true, + topic_id: topic.id, + ).create + BackupDraftPost.create!(user_id: user.id, key: key, post_id: post.id) + end + elsif post.updated_at > 5.minutes.ago + # bypass all validations here to maximize speed + post.update_columns( + raw: draft_body, + cooked: PrettyText.cook(draft_body), + updated_at: Time.zone.now + ) + else + revisor = PostRevisor.new(post, post.topic) + revisor.revise!(user, { raw: draft_body }, + bypass_bump: true, + skip_validations: true, + skip_staff_log: true, + bypass_rate_limiter: true + ) + end + + rescue => e + Discourse.warn_exception(e, message: "Failed to backup draft") + end + + def self.ensure_draft_topic!(user) + topic_id = BackupDraftTopic.where(user_id: user.id).pluck(:topic_id).first + topic = Topic.find_by(id: topic_id) if topic_id + + if topic_id && !topic + BackupDraftTopic.where(user_id: user.id).delete_all + end + + if !topic + Topic.transaction do + creator = PostCreator.new( + user, + title: I18n.t("draft_backup.pm_title"), + archetype: Archetype.private_message, + raw: I18n.t("draft_backup.pm_body"), + skip_jobs: true, + skip_validations: true, + target_usernames: user.username + ) + topic = creator.create.topic + BackupDraftTopic.create!(topic_id: topic.id, user_id: user.id) + end + end + + topic + + end + end # == Schema Information diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1e72fbf09b..d88a01ccc1 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -871,6 +871,9 @@ en: short_description: "Like this post" long_form: "liked this" + draft_backup: + pm_title: "Backup Drafts from ongoing topics" + pm_body: "Topic containing backup drafts" user_activity: no_default: self: "You have no activity yet." @@ -1993,7 +1996,7 @@ en: notify_about_flags_after: "If there are flags that haven't been handled after this many hours, send a personal message to moderators. Set to 0 to disable." show_create_topics_notice: "If the site has fewer than 5 public topics, show a notice asking admins to create some topics." - delete_drafts_older_than_n_days: Delete drafts older than (n) days. + delete_drafts_older_than_n_days: "Delete drafts older than (n) days." bootstrap_mode_min_users: "Minimum number of users required to disable bootstrap mode (set to 0 to disable)" diff --git a/config/site_settings.yml b/config/site_settings.yml index 603ac3cc89..c452320eac 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1858,6 +1858,10 @@ uncategorized: delete_drafts_older_than_n_days: default: 180 + backup_drafts_to_pm_length: + default: 0 + hidden: true + tos_topic_id: default: -1 hidden: true diff --git a/db/migrate/20191017044811_add_draft_backup_tables.rb b/db/migrate/20191017044811_add_draft_backup_tables.rb new file mode 100644 index 0000000000..6177426c50 --- /dev/null +++ b/db/migrate/20191017044811_add_draft_backup_tables.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddDraftBackupTables < ActiveRecord::Migration[6.0] + def change + + create_table :backup_draft_topics do |t| + t.integer :user_id, null: false + t.integer :topic_id, null: false + t.timestamps + end + + create_table :backup_draft_posts do |t| + t.integer :user_id, null: false + t.integer :post_id, null: false + t.string :key, null: false + t.timestamps + end + + add_index :backup_draft_posts, [:user_id, :key], unique: true + add_index :backup_draft_posts, [:post_id], unique: true + + add_index :backup_draft_topics, [:user_id], unique: true + add_index :backup_draft_topics, [:topic_id], unique: true + end +end diff --git a/lib/post_creator.rb b/lib/post_creator.rb index bf1fe848d7..561720dfb0 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -34,6 +34,7 @@ class PostCreator # dequeue before the commit finishes. If you do this, be sure to # call `enqueue_jobs` after the transaction is comitted. # hidden_reason_id - Reason for hiding the post (optional) + # skip_validations - Do not validate any of the content in the post # # When replying to a topic: # topic_id - topic we're replying to diff --git a/spec/models/draft_spec.rb b/spec/models/draft_spec.rb index 2cdf5fa5d4..b65c20c3b7 100644 --- a/spec/models/draft_spec.rb +++ b/spec/models/draft_spec.rb @@ -3,37 +3,70 @@ require 'rails_helper' describe Draft do - before do - @user = Fabricate(:user) + + fab!(:user) do + Fabricate(:user) end + + context 'backup_drafts_to_pm_length' do + it "correctly backs up drafts to a personal message" do + SiteSetting.backup_drafts_to_pm_length = 1 + + draft = { + reply: "this is a reply", + random_key: "random" + } + + Draft.set(user, "new_private_message", 0, draft.to_json) + draft["reply"] = "test" * 100 + Draft.set(user, "new_private_message", 77, draft.to_json) + + draft_post = BackupDraftPost.find_by(user_id: user.id, key: "new_private_message").post + + expect(draft_post.revisions.count).to eq(0) + + freeze_time 10.minutes.from_now + + # this should trigger a post revision as 10 minutes have passed + draft["reply"] = "hello" + Draft.set(user, "new_private_message", 77, draft.to_json) + + draft_topic = BackupDraftTopic.find_by(user_id: user.id) + expect(draft_topic.topic.posts_count).to eq(2) + + draft_post.reload + expect(draft_post.revisions.count).to eq(1) + end + end + it "can get a draft by user" do - Draft.set(@user, "test", 0, "data") - expect(Draft.get(@user, "test", 0)).to eq "data" + Draft.set(user, "test", 0, "data") + expect(Draft.get(user, "test", 0)).to eq "data" end it "uses the user id and key correctly" do - Draft.set(@user, "test", 0, "data") + Draft.set(user, "test", 0, "data") expect(Draft.get(Fabricate.build(:coding_horror), "test", 0)).to eq nil end it "should overwrite draft data correctly" do - Draft.set(@user, "test", 0, "data") - Draft.set(@user, "test", 0, "new data") - expect(Draft.get(@user, "test", 0)).to eq "new data" + Draft.set(user, "test", 0, "data") + Draft.set(user, "test", 0, "new data") + expect(Draft.get(user, "test", 0)).to eq "new data" end it "should clear drafts on request" do - Draft.set(@user, "test", 0, "data") - Draft.clear(@user, "test", 0) - expect(Draft.get(@user, "test", 0)).to eq nil + Draft.set(user, "test", 0, "data") + Draft.clear(user, "test", 0) + expect(Draft.get(user, "test", 0)).to eq nil end it "should disregard old draft if sequence decreases" do - Draft.set(@user, "test", 0, "data") - Draft.set(@user, "test", 1, "hello") - Draft.set(@user, "test", 0, "foo") - expect(Draft.get(@user, "test", 0)).to eq nil - expect(Draft.get(@user, "test", 1)).to eq "hello" + Draft.set(user, "test", 0, "data") + Draft.set(user, "test", 1, "hello") + Draft.set(user, "test", 0, "foo") + expect(Draft.get(user, "test", 0)).to eq nil + expect(Draft.get(user, "test", 1)).to eq "hello" end it 'can cleanup old drafts' do @@ -71,25 +104,25 @@ describe Draft do let(:public_topic) { public_post.topic } let(:stream) do - Draft.stream(user: @user) + Draft.stream(user: user) end it "should include the correct number of drafts in the stream" do - Draft.set(@user, "test", 0, '{"reply":"hey.","action":"createTopic","title":"Hey"}') - Draft.set(@user, "test2", 0, '{"reply":"howdy"}') + Draft.set(user, "test", 0, '{"reply":"hey.","action":"createTopic","title":"Hey"}') + Draft.set(user, "test2", 0, '{"reply":"howdy"}') expect(stream.count).to eq(2) end it "should include the right topic id in a draft reply in the stream" do - Draft.set(@user, "topic_#{public_topic.id}", 0, '{"reply":"hi"}') + Draft.set(user, "topic_#{public_topic.id}", 0, '{"reply":"hi"}') draft_row = stream.first expect(draft_row.topic_id).to eq(public_topic.id) end it "should include the right draft username in the stream" do - Draft.set(@user, "topic_#{public_topic.id}", 0, '{"reply":"hey"}') + Draft.set(user, "topic_#{public_topic.id}", 0, '{"reply":"hey"}') draft_row = stream.first - expect(draft_row.draft_username).to eq(@user.username) + expect(draft_row.draft_username).to eq(user.username) end end