DEV: Move language detection out of the post serializer

Language detection is added to the queue every time the post is serialized,
move this into the post_created and post_edited events instead.

Author: nattsw

lib/discourse_translator/guardian_extension.rb

@@ -11,4 +11,9 @@ def poster_group_allow_translate?(post)
     return false if post.user.nil?
     post.user.in_any_groups?(SiteSetting.restrict_translation_by_poster_group_map)
   end
+
+  def can_detect_language?(post)
+    poster_group_allow_translate?(post) && post.raw.present? &&
+      post.post_type != Post.types[:small_action]
+  end
 end

plugin.rb

@@ -37,23 +37,28 @@ module ::DiscourseTranslator
     Topic.prepend(DiscourseTranslator::TopicExtension)
   end
 
-  add_to_serializer :post, :can_translate do
-    return false if !SiteSetting.translator_enabled
-    if !scope.user_group_allow_translate? || !scope.poster_group_allow_translate?(object)
-      return false
+  on(:post_created) do |post, _, user|
+    if Guardian.new(user).can_detect_language?(post)
+      Discourse.redis.sadd?(DiscourseTranslator::LANG_DETECT_NEEDED, post.id)
     end
-    return false if raw.blank? || post_type == Post.types[:small_action]
+  end
 
+  on(:post_edited) do |post, _, user|
+    if Guardian.new(user).can_detect_language?(post)
+      Discourse.redis.sadd?(DiscourseTranslator::LANG_DETECT_NEEDED, post.id)
+    end
+  end
+
+  add_to_serializer :post, :can_translate do
     detected_lang = post_custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]
 
-    if !detected_lang
-      Discourse.redis.sadd?(DiscourseTranslator::LANG_DETECT_NEEDED, object.id)
-      false
-    else
-      detected_lang.to_sym != I18n.locale &&
-        "DiscourseTranslator::#{SiteSetting.translator}".constantize.language_supported?(
-          detected_lang,
-        )
+    if !scope.user_group_allow_translate? || !detected_lang
+      return false
     end
+
+    detected_lang.to_sym != I18n.locale &&
+      "DiscourseTranslator::#{SiteSetting.translator}".constantize.language_supported?(
+        detected_lang,
+      )
   end
 end

spec/lib/guardian_extension_spec.rb

@@ -77,4 +77,44 @@
       end
     end
   end
+
+  describe "#can_detect_language?" do
+    fab!(:group)
+    fab!(:user) { Fabricate(:user, groups: [group]) }
+    fab!(:post) { Fabricate(:post, user: user, raw: "Hello, world!") }
+    let(:guardian) { Guardian.new(user) }
+
+    it "returns false when anonymous user" do
+      expect(Guardian.new.can_detect_language?(post)).to eq(false)
+    end
+
+    it "returns false when the post user is not in restrict_translation_by_poster_group" do
+      SiteSetting.restrict_translation_by_poster_group = "#{group.id+1}"
+
+      expect(guardian.can_detect_language?(post)).to eq(false)
+    end
+
+    context "when post author is in allowed groups" do
+      before do
+        SiteSetting.restrict_translation_by_group = "#{group.id}"
+        SiteSetting.restrict_translation_by_poster_group = "#{group.id}"
+      end
+
+      it "returns true when the post is not a small action post" do
+        expect(guardian.can_detect_language?(post)).to eq(true)
+      end
+
+      it "returns false when the post is a small action post" do
+        post.update(post_type: Post.types[:small_action])
+
+        expect(guardian.can_detect_language?(post)).to eq(false)
+      end
+
+      it "returns false when the post raw is empty" do
+        post.update(raw: "")
+
+        expect(guardian.can_detect_language?(post)).to eq(false)
+      end
+    end
+  end
 end

spec/models/post_spec.rb

@@ -27,4 +27,40 @@
       expect(post.custom_fields[::DiscourseTranslator::TRANSLATED_CUSTOM_FIELD]).to be_nil
     end
   end
+
+  describe "queueing post for language detection" do
+    fab!(:group)
+    fab!(:topic)
+    fab!(:user) { Fabricate(:user, groups: [group]) }
+
+    context "when user and posts are in the right group" do
+      before do
+        SiteSetting.restrict_translation_by_group = "#{group.id}"
+        SiteSetting.restrict_translation_by_poster_group = "#{group.id}"
+      end
+
+      it "queues the post for language detection" do
+        PostCreator.create!(user, raw: "it's a cat on a boat", topic_id: topic.id)
+        expect(Discourse.redis.sismember(DiscourseTranslator::LANG_DETECT_NEEDED, post.id)).to be_truthy
+      end
+
+      it "queues the post for language detection when edited" do
+        post = PostCreator.create!(user, raw: "it's a cat on a boat", topic_id: topic.id)
+        Discourse.redis.spop(DiscourseTranslator::LANG_DETECT_NEEDED)
+
+        PostRevisor.new(post).revise!(post.user, { raw: "it's a bat on a coat" })
+
+        expect(Discourse.redis.sismember(DiscourseTranslator::LANG_DETECT_NEEDED, post.id)).to be_truthy
+      end
+    end
+
+    context "when user and posts are not in the right group" do
+      it "does not queue the post for language detection" do
+        PostCreator.create!(user, raw: "it's a cat on a boat", topic_id: topic.id)
+
+        expect(Discourse.redis.sismember(DiscourseTranslator::LANG_DETECT_NEEDED, post.id)).to be_falsey
+      end
+    end
+  end
 end
+

spec/serializers/post_serializer_spec.rb

@@ -12,7 +12,7 @@
 
   describe "#can_translate" do
     it "returns false when translator disabled" do
-      SiteSetting.translator_enabled = true
+      SiteSetting.translator_enabled = false
       serializer = PostSerializer.new(post, scope: Guardian.new)
 
       expect(serializer.can_translate).to eq(false)
@@ -21,25 +21,15 @@
     describe "when translator enabled" do
       before do
         SiteSetting.translator_enabled = true
-        Jobs.run_later!
       end
 
-      describe "when small action post" do
-        fab!(:small_action)
-        let(:serializer) { PostSerializer.new(small_action, scope: Guardian.new) }
+      describe "anon user" do
+        let(:serializer) { PostSerializer.new(post, scope: Guardian.new) }
 
-        it "cannot translate" do
-          expect(serializer.can_translate).to eq(false)
-        end
-      end
-
-      describe "when post raw is empty" do
-        fab!(:empty_post) do
-          empty_post = Fabricate.build(:post, raw: "")
-          empty_post.save!(validate: false)
-          empty_post
+        before do
+          SiteSetting.restrict_translation_by_group = "#{Group::AUTO_GROUPS[:everyone]}"
+          SiteSetting.restrict_translation_by_poster_group = ""
         end
-        let(:serializer) { PostSerializer.new(empty_post, scope: Guardian.new) }
 
         it "cannot translate" do
           expect(serializer.can_translate).to eq(false)
@@ -49,61 +39,46 @@
       describe "logged in user" do
         let(:serializer) { PostSerializer.new(post, scope: Guardian.new(user)) }
 
-        describe "when user is not in restrict_translation_by_group" do
-          it "cannot translate" do
-            SiteSetting.restrict_translation_by_group = ""
+        it "cannot translate when user is not in restrict_translation_by_group" do
+          SiteSetting.restrict_translation_by_group = "#{group.id + 1}"
 
-            expect(serializer.can_translate).to eq(false)
-          end
+          expect(serializer.can_translate).to eq(false)
         end
 
-        describe "when post is not in restrict_translation_by_poster_group" do
-          it "cannot translate" do
+        describe "user is in restrict_translation_by_group" do
+          before do
             SiteSetting.restrict_translation_by_group = "#{group.id}"
-            SiteSetting.restrict_translation_by_poster_group = ""
-
-            expect(serializer.can_translate).to eq(false)
           end
-        end
 
-        describe "when user is in restrict_translation_by_group and poster in restrict_translation_by_poster_group" do
-          before do
-            SiteSetting.restrict_translation_by_group = "#{group.id}"
-            SiteSetting.restrict_translation_by_poster_group = "#{post_user_group.id}"
+          it "cannot translate when post author is not in restrict_translation_by_poster_group" do
+            SiteSetting.restrict_translation_by_poster_group = "#{group.id}"
+
+            expect(serializer.can_translate).to eq(false)
           end
 
-          describe "when post does not have DETECTED_LANG_CUSTOM_FIELD" do
-            it "cannot translate" do
-              expect(serializer.can_translate).to eq(false)
+          describe "post author in restrict_translation_by_poster_group and locale is :xx" do
+            before do
+              SiteSetting.restrict_translation_by_poster_group = "#{post_user_group.id}"
+              I18n.stubs(:locale).returns(:pt)
             end
 
-            it "adds post id to redis if detected_language is blank" do
-              post.custom_fields["detected_language"] = nil
-              post.save_custom_fields
-
-              expect { serializer.can_translate }.to change {
-                Discourse.redis.sismember(DiscourseTranslator::LANG_DETECT_NEEDED, post.id)
-              }.from(false).to(true)
+            it "cannot translate when post does not have DETECTED_LANG_CUSTOM_FIELD" do
+              expect(serializer.can_translate).to eq(false)
             end
-          end
 
-          describe "when post has DETECTED_LANG_CUSTOM_FIELD matches user locale" do
-            before do
-              I18n.stubs(:locale).returns(:xx)
-              post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "xx"
+            it "cannot translate when post has DETECTED_LANG_CUSTOM_FIELD matches locale" do
+              post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "pt"
               post.save
-            end
 
-            it { expect(serializer.can_translate).to eq(false) }
-          end
+              expect(serializer.can_translate).to eq(false)
+            end
 
-          describe "when post has DETECTED_LANG_CUSTOM_FIELD does not match user locale" do
-            before do
+            it "can translate when post has DETECTED_LANG_CUSTOM_FIELD does not match locale" do
               post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "jp"
               post.save
-            end
 
-            it { expect(serializer.can_translate).to eq(true) }
+              expect(serializer.can_translate).to eq(true)
+            end
           end
         end
       end