diff --git a/lib/imap/sync.rb b/lib/imap/sync.rb index 207d65c9493..ecad53a0d50 100644 --- a/lib/imap/sync.rb +++ b/lib/imap/sync.rb @@ -264,6 +264,7 @@ module Imap to_sync.each do |incoming_email| Logger.log("[IMAP] (#{@group.name}) Updating email and incoming email ID = #{incoming_email.id}") update_email(incoming_email) + incoming_email.update(imap_sync: false) end end end @@ -305,6 +306,8 @@ module Imap tags.subtract([nil, '']) + return if !tagging_enabled? + # TODO: Optimize tagging. # `DiscourseTagging.tag_topic_by_names` does a lot of lookups in the # database and some of them could be cached in this context. @@ -312,7 +315,6 @@ module Imap end def update_email(incoming_email) - return if !SiteSetting.tagging_enabled || !SiteSetting.allow_staff_to_tag_pms return if !incoming_email || !incoming_email.imap_sync post = incoming_email.post @@ -334,10 +336,10 @@ module Imap email = @provider.emails(incoming_email.imap_uid, ['FLAGS', 'LABELS']).first return if !email.present? - incoming_email.update(imap_sync: false) - labels = email['LABELS'] flags = email['FLAGS'] + new_labels = [] + new_flags = [] # Topic has been deleted if it is not present from the post, so we need # to trash the IMAP server email @@ -347,11 +349,6 @@ module Imap return @provider.trash(incoming_email.imap_uid) end - # Sync topic status and labels with email flags and labels. - tags = topic.tags.pluck(:name) - new_flags = tags.map { |tag| @provider.tag_to_flag(tag) }.reject(&:blank?) - new_labels = tags.map { |tag| @provider.tag_to_label(tag) }.reject(&:blank?) - # the topic is archived, and the archive should be reflected in the IMAP # server topic_archived = topic.group_archived_messages.any? @@ -374,10 +371,21 @@ module Imap @provider.archive(incoming_email.imap_uid) end + # Sync topic status and labels with email flags and labels. + if tagging_enabled? + tags = topic.tags.pluck(:name) + new_flags = tags.map { |tag| @provider.tag_to_flag(tag) }.reject(&:blank?) + new_labels = new_labels.concat(tags.map { |tag| @provider.tag_to_label(tag) }.reject(&:blank?)) + end + # regardless of whether the topic needs to be archived we still update # the flags and the labels @provider.store(incoming_email.imap_uid, 'FLAGS', flags, new_flags) @provider.store(incoming_email.imap_uid, 'LABELS', labels, new_labels) end + + def tagging_enabled? + SiteSetting.tagging_enabled && SiteSetting.allow_staff_to_tag_pms + end end end diff --git a/spec/components/imap/sync_spec.rb b/spec/components/imap/sync_spec.rb index f1388f41bf5..f78cbd2eacd 100644 --- a/spec/components/imap/sync_spec.rb +++ b/spec/components/imap/sync_spec.rb @@ -100,6 +100,28 @@ describe Imap::Sync do expect(incoming_email.imap_group_id).to eq(group.id) end + context "when tagging not enabled" do + before do + SiteSetting.tagging_enabled = false + SiteSetting.allow_staff_to_tag_pms = false + end + + it "creates a topic from an incoming email but with no tags added" do + expect { sync_handler.process } + .to change { Topic.count }.by(1) + .and change { Post.where(post_type: Post.types[:regular]).count }.by(1) + .and change { IncomingEmail.count }.by(1) + + expect(group.imap_uid_validity).to eq(1) + expect(group.imap_last_uid).to eq(100) + + topic = Topic.last + expect(topic.title).to eq(subject) + expect(topic.user.email).to eq(from) + expect(topic.tags).to eq([]) + end + end + it 'does not duplicate topics' do expect { sync_handler.process } .to change { Topic.count }.by(1) @@ -451,7 +473,7 @@ describe Imap::Sync do it "does not archive email if not archived in discourse, it unarchives it instead" do @incoming_email.update(imap_sync: true) provider.stubs(:store).with(100, 'FLAGS', anything, anything) - provider.stubs(:store).with(100, 'LABELS', ["\\Inbox"], ["seen", "\\Inbox"]) + provider.stubs(:store).with(100, 'LABELS', ["\\Inbox"], ["\\Inbox", "seen"]) provider.expects(:archive).with(100).never provider.expects(:unarchive).with(100)