DEV: Scope guardian methods to the plugin and some cleanup (#138)

Scope guardian methods to the plugin
Move old method to add to serializer to the proper API method

lib/discourse_translator/guardian_extension.rb

@@ -1,25 +1,13 @@
 # frozen_string_literal: true
 module DiscourseTranslator::GuardianExtension
-  def user_group_allowed?
-    authorized_groups = SiteSetting.restrict_translation_by_group.split("|").map(&:to_i)
-
-    authorized? current_user, authorized_groups
+  def user_group_allow_translate?
+    return false if !current_user
+    current_user.in_any_groups?(SiteSetting.restrict_translation_by_group_map)
   end
 
-  def post_group_allowed?(post)
-    authorized_poster_groups =
-      SiteSetting.restrict_translation_by_poster_group.split("|").map(&:to_i)
-    return true if authorized_poster_groups.empty?
-
-    poster = User.find post.user_id
-    authorized? poster, authorized_poster_groups
-  end
-
-  def authorized?(user, authorized_groups)
-    return false if !user
-
-    user_groups = user.groups.pluck :id
-    authorized = authorized_groups.intersection user_groups
-    !authorized.empty?
+  def poster_group_allow_translate?(post)
+    return false if !current_user
+    return true if SiteSetting.restrict_translation_by_poster_group_map.empty?
+    post.user.in_any_groups?(SiteSetting.restrict_translation_by_poster_group_map)
   end
 end

plugin.rb

@@ -55,7 +55,7 @@ def translate
       raise Discourse::InvalidParameters.new(:post_id) if post.blank?
       guardian.ensure_can_see!(post)
 
-      if !guardian.user_group_allowed?
+      if !guardian.user_group_allow_translate?
         raise Discourse::InvalidAccess.new(
                 "not_in_group",
                 SiteSetting.restrict_translation_by_group,
@@ -64,7 +64,7 @@ def translate
               )
       end
 
-      if !guardian.post_group_allowed?(post)
+      if !guardian.poster_group_allow_translate?(post)
         raise Discourse::InvalidAccess.new(
                 "not_in_group",
                 SiteSetting.restrict_translation_by_poster_group,
@@ -167,28 +167,24 @@ def post_process(post)
     Guardian.class_eval { prepend DiscourseTranslator::GuardianExtension }
   end
 
-  class ::PostSerializer
-    attributes :can_translate
-
-    def can_translate
-      if !(
-           SiteSetting.translator_enabled && scope.user_group_allowed? &&
-             scope.post_group_allowed?(object)
-         )
-        return false
-      end
+  add_to_serializer :post, :can_translate do
+    if !(
+         SiteSetting.translator_enabled && scope.user_group_allow_translate? &&
+           scope.poster_group_allow_translate?(object)
+       )
+      return false
+    end
 
-      detected_lang = post_custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]
+    detected_lang = post_custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]
 
-      if !detected_lang
-        Jobs.enqueue(:detect_translation, post_id: object.id)
-        false
-      else
-        detected_lang !=
-          "DiscourseTranslator::#{SiteSetting.translator}::SUPPORTED_LANG_MAPPING".constantize[
-            I18n.locale
-          ]
-      end
+    if !detected_lang
+      Jobs.enqueue(:detect_translation, post_id: object.id)
+      false
+    else
+      detected_lang !=
+        "DiscourseTranslator::#{SiteSetting.translator}::SUPPORTED_LANG_MAPPING".constantize[
+          I18n.locale
+        ]
     end
   end
 

spec/controllers/translator_controller_spec.rb

@@ -114,19 +114,21 @@
         end
 
         describe "user is not in a allowlisted group" do
-          before { SiteSetting.restrict_translation_by_group = "not_in_the_list" }
+          before do
+            SiteSetting.restrict_translation_by_group = "#{Group::AUTO_GROUPS[:moderators]}"
+          end
 
           include_examples "deny_request_to_translate"
         end
 
         describe "restrict_translation_by_poster_group" do
-          fab!(:admin)
+          fab!(:group)
+          fab!(:user) { Fabricate(:user, groups: [group]) }
 
           before do
-            SiteSetting.restrict_translation_by_group =
-              "#{Group.find_by(name: "admins").id}|not_in_the_list"
+            SiteSetting.restrict_translation_by_group = "#{group.id}|"
 
-            log_in_user(admin)
+            log_in_user(user)
           end
           describe "post made by an user in a allowlisted group" do
             before do
@@ -136,7 +138,10 @@
           end
 
           describe "post made by an user not in a allowlisted group" do
-            before { SiteSetting.restrict_translation_by_poster_group = "not_in_the_list" }
+            before do
+              SiteSetting.restrict_translation_by_poster_group =
+                "#{Group::AUTO_GROUPS[:moderators]}"
+            end
             include_examples "deny_request_to_translate"
           end
         end

spec/lib/guardian_extension_spec.rb

@@ -3,86 +3,59 @@
 require "rails_helper"
 
 describe DiscourseTranslator::GuardianExtension do
-  shared_examples "post_group_allowed" do
-    it "returns true when the post was made by an user in a allowlisted group" do
-      SiteSetting.restrict_translation_by_poster_group = "#{Group[:trust_level_1].id}"
-
-      expect(guardian.post_group_allowed?(post)).to eq(true)
-    end
-
-    it "returns true when no group has selected in settings" do
-      SiteSetting.restrict_translation_by_poster_group = ""
-
-      expect(guardian.post_group_allowed?(post)).to eq(true)
-    end
-  end
-
-  shared_examples "post_group_not_allowed" do
-    it "returns true when the post was made by an user not in a allowlisted group" do
-      SiteSetting.restrict_translation_by_poster_group = "#{Group[:trust_level_4].id}"
-
-      expect(guardian.post_group_allowed?(post)).to eq(false)
-    end
-  end
-
   describe "anon user" do
     let!(:guardian) { Guardian.new }
-    let(:post) { Fabricate(:post) }
-    describe "#user_group_allowed?" do
-      it "returns false when the user is not logged in" do
-        SiteSetting.restrict_translation_by_group = "not_in_the_list"
+    fab!(:post) { Fabricate(:post) }
 
-        expect(guardian.user_group_allowed?).to eq(false)
-      end
+    before do
+      SiteSetting.restrict_translation_by_group = "#{Group::AUTO_GROUPS[:everyone]}"
+      SiteSetting.restrict_translation_by_poster_group = "#{Group::AUTO_GROUPS[:everyone]}"
     end
 
-    describe "#post_group_allowed?" do
-      include_examples "post_group_not_allowed"
+    describe "#user_group_allow_translate?" do
+      it "returns false" do
+        expect(guardian.user_group_allow_translate?).to eq(false)
+      end
     end
 
-    describe "#authorized?" do
-      it "returns false with authorized groups" do
-        expect(guardian.authorized?(nil, ["authorized_group"])).to eq(false)
+    describe "#poster_group_allow_translate?" do
+      it "returns false" do
+        expect(guardian.poster_group_allow_translate?(post)).to eq(false)
       end
     end
   end
 
   describe "logged in user" do
-    let(:user) do
-      user = Fabricate(:user)
-      user.group_users << Fabricate(:group_user, user: user, group: Group[:trust_level_1])
-      user
-    end
+    fab!(:group) { Fabricate(:group) }
+    fab!(:user) { Fabricate(:user, groups: [group]) }
+    fab!(:post) { Fabricate(:post, user: user) }
     let(:guardian) { Guardian.new(user) }
-    let(:post) { Fabricate(:post, user: user) }
 
-    describe "#user_group_allowed?" do
-      it "returns true when the user is in a allowlisted group" do
-        SiteSetting.restrict_translation_by_group =
-          "#{Group.find_by(name: user.groups.first.name).id}|not_in_the_list"
+    describe "#user_group_allow_translate?" do
+      it "returns true when the user is in restrict_translation_by_group" do
+        SiteSetting.restrict_translation_by_group = "#{group.id}"
 
-        expect(guardian.user_group_allowed?).to eq(true)
+        expect(guardian.user_group_allow_translate?).to eq(true)
       end
 
-      it "returns false when the user is not in a allowlisted group" do
-        SiteSetting.restrict_translation_by_group = "not_in_the_list"
+      it "returns false when the user is not in restrict_translation_by_group" do
+        SiteSetting.restrict_translation_by_group = "#{Group::AUTO_GROUPS[:moderators]}"
 
-        expect(guardian.user_group_allowed?).to eq(false)
+        expect(guardian.user_group_allow_translate?).to eq(false)
       end
     end
 
-    describe "#post_group_allowed?" do
-      include_examples "post_group_allowed"
-      include_examples "post_group_not_allowed"
-    end
+    describe "#poster_group_allow_translate??" do
+      it "returns true when the post user is in restrict_translation_by_poster_group" do
+        SiteSetting.restrict_translation_by_poster_group = "#{group.id}"
 
-    describe "#authorized?" do
-      it "returns true with user in autorized groups" do
-        expect(guardian.authorized? user, [Group[:trust_level_1].id]).to eq(true)
+        expect(guardian.poster_group_allow_translate?(post)).to eq(true)
       end
 
-      it "returns false with user not in autoried groups" do
-        expect(guardian.authorized? user, ["not_in_the_list"]).to eq(false)
+      it "returns false when the post user is not in restrict_translation_by_poster_group" do
+        SiteSetting.restrict_translation_by_poster_group = "#{Group::AUTO_GROUPS[:moderators]}"
+
+        expect(guardian.poster_group_allow_translate?(post)).to eq(false)
       end
     end
   end