From a157dfd41813e11d5dbe91c9eef37a459c18daf9 Mon Sep 17 00:00:00 2001 From: OsamaSayegh Date: Thu, 2 Aug 2018 22:43:53 +0300 Subject: [PATCH] UX: better rejection message when reply via email is too short --- config/locales/server.en.yml | 8 +++++ lib/email/processor.rb | 5 +++ lib/email/receiver.rb | 10 +++++- spec/components/email/processor_spec.rb | 46 +++++++++++++++++++++++++ spec/components/email/receiver_spec.rb | 2 +- 5 files changed, 69 insertions(+), 2 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2780d6cf9cc..da3a72871da 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2502,6 +2502,14 @@ en: If you can correct the problem, please try again. + email_reject_post_too_short: + title: "Email Reject Post Too Short" + subject_template: "[%{email_prefix}] Email issue -- Post too short" + text_body_template: | + We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work. + + To promote more in depth conversations, very short replies are not allowed. Can you please reply with at least %{count} characters? Alternatively, you can like a post via email by replying with "+1". + email_reject_invalid_post_action: title: "Email Reject Invalid Post Action" subject_template: "[%{email_prefix}] Email issue -- Invalid Post Action" diff --git a/lib/email/processor.rb b/lib/email/processor.rb index 6bac32fe3a9..da1c39e04fe 100644 --- a/lib/email/processor.rb +++ b/lib/email/processor.rb @@ -52,6 +52,7 @@ module Email when Email::Receiver::TopicNotFoundError then :email_reject_topic_not_found when Email::Receiver::TopicClosedError then :email_reject_topic_closed when Email::Receiver::InvalidPost then :email_reject_invalid_post + when Email::Receiver::TooShortPost then :email_reject_post_too_short when Email::Receiver::UnsubscribeNotAllowed then :email_reject_invalid_post when ActiveRecord::Rollback then :email_reject_invalid_post when Email::Receiver::InvalidPostAction then :email_reject_invalid_post_action @@ -69,6 +70,10 @@ module Email template_args[:post_error] = e.message end + if message_template == :email_reject_post_too_short + template_args[:count] = SiteSetting.min_post_length + end + if message_template == :email_reject_unrecognized_error msg = "Unrecognized error type (#{e.class}: #{e.message}) when processing incoming email" msg += "\n\nBacktrace:\n#{e.backtrace.map { |l| " #{l}" }.join("\n")}" diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index f8ab257d2f9..b819ef253f6 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -31,6 +31,7 @@ module Email class TopicNotFoundError < ProcessingError; end class TopicClosedError < ProcessingError; end class InvalidPost < ProcessingError; end + class TooShortPost < ProcessingError; end class InvalidPostAction < ProcessingError; end class UnsubscribeNotAllowed < ProcessingError; end class EmailNotAllowed < ProcessingError; end @@ -932,7 +933,14 @@ module Email user = options.delete(:user) result = NewPostManager.new(user, options).perform - raise InvalidPost, result.errors.full_messages.join("\n") if result.errors.any? + errors = result.errors.full_messages + if errors.any? do |message| + message.include?(I18n.t("activerecord.attributes.post.raw").strip) && + message.include?(I18n.t("errors.messages.too_short", count: SiteSetting.min_post_length).strip) + end + raise TooShortPost + end + raise InvalidPost, errors.join("\n") if result.errors.any? if result.post @incoming_email.update_columns(topic_id: result.post.topic_id, post_id: result.post.id) diff --git a/spec/components/email/processor_spec.rb b/spec/components/email/processor_spec.rb index 727ea23bb58..1cec58aca00 100644 --- a/spec/components/email/processor_spec.rb +++ b/spec/components/email/processor_spec.rb @@ -5,6 +5,52 @@ describe Email::Processor do let(:from) { "foo@bar.com" } + context "when reply via email is too short" do + let(:mail) { file_from_fixtures("email_reply_3.eml", "emails").read } + let(:post) { Fabricate(:post) } + let!(:user) { Fabricate(:user, email: "three@foo.com") } + let!(:email_log) do + Fabricate(:email_log, + message_id: "35@foo.bar.mail", # don't change, based on fixture file "email_reply_3.eml" + email_type: "user_posted", + post_id: post.id, + to_address: "asdas@dasdfd.com" + ) + end + + before do + SiteSetting.min_post_length = 1000 + SiteSetting.find_related_post_with_key = false + end + + it "rejects reply and sends an email with custom error message" do + begin + receiver = Email::Receiver.new(mail) + receiver.process! + rescue Email::Receiver::TooShortPost => e + error = e + end + + expect(error.class).to eq(Email::Receiver::TooShortPost) + processor = Email::Processor.new(mail) + processor.process! + + rejection_raw = processor.send(:handle_failure, mail, error).body.raw_source + + count = SiteSetting.min_post_length + destination = receiver.mail.to + former_title = receiver.mail.subject + + expect(rejection_raw.gsub(/\r/, "")).to eq( + I18n.t("system_messages.email_reject_post_too_short.text_body_template", + count: count, + destination: destination, + former_title: former_title + ).gsub(/\r/, "") + ) + end + end + describe "rate limits" do let(:mail) { "From: #{from}\nTo: bar@foo.com\nSubject: FOO BAR\n\nFoo foo bar bar?" } diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 60f1fa40e2a..3ee03ae17f9 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -207,7 +207,7 @@ describe Email::Receiver do end it "raises an InvalidPost when there was an error while creating the post" do - expect { process(:too_small) }.to raise_error(Email::Receiver::InvalidPost) + expect { process(:too_small) }.to raise_error(Email::Receiver::TooShortPost) end it "raises an InvalidPost when there are too may mentions" do