From b0c95114eabc409f8afebe53afbf4a38c7da7ca3 Mon Sep 17 00:00:00 2001
From: Blake Erickson <o.blakeerickson@gmail.com>
Date: Thu, 15 Feb 2024 14:52:15 -0700
Subject: [PATCH] FIX: Calculate no ads for groups server side (#200)

If the selected group to not display ads to had its visibility set to
not be visible then this setting wouldn't work correctly because that
group wouldn't be available client side. The change moves that group
check to be server side so that we can correctly see all the groups that
should not see ads.
---
 .../discourse/components/ad-component.js      | 20 ++++---------------
 config/settings.yml                           |  1 -
 lib/adplugin/guardian_extensions.rb           |  4 ++++
 plugin.rb                                     |  4 ++++
 test/javascripts/acceptance/adsense-test.js   |  1 +
 test/javascripts/acceptance/dfp-test.js       |  1 +
 test/javascripts/acceptance/house-ad-test.js  |  2 +-
 test/javascripts/acceptance/mixed-ads-test.js |  1 +
 8 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/assets/javascripts/discourse/components/ad-component.js b/assets/javascripts/discourse/components/ad-component.js
index 6c1cae5..ea0f8f9 100644
--- a/assets/javascripts/discourse/components/ad-component.js
+++ b/assets/javascripts/discourse/components/ad-component.js
@@ -45,25 +45,13 @@ export default Component.extend({
     return topicType === "private_message";
   },
 
-  @discourseComputed("currentUser.groups")
-  showToGroups(groups) {
-    const currentUser = this.currentUser;
-
-    if (
-      !currentUser ||
-      !groups ||
-      !this.siteSettings.no_ads_for_groups ||
-      this.siteSettings.no_ads_for_groups.length === 0
-    ) {
+  @discourseComputed
+  showToGroups() {
+    if (!this.currentUser) {
       return true;
     }
 
-    let noAdsGroups = this.siteSettings.no_ads_for_groups
-      .split("|")
-      .filter(Boolean);
-    let currentGroups = groups.map((g) => g.id.toString());
-
-    return !currentGroups.any((g) => noAdsGroups.includes(g));
+    return this.currentUser.show_to_groups;
   },
 
   @discourseComputed(
diff --git a/config/settings.yml b/config/settings.yml
index 148c5af..6077dcd 100755
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -9,7 +9,6 @@ ad_plugin:
     client: true
     default: false
   no_ads_for_groups:
-    client: true
     default: ""
     type: group_list
   no_ads_for_categories:
diff --git a/lib/adplugin/guardian_extensions.rb b/lib/adplugin/guardian_extensions.rb
index db786ad..427c46c 100644
--- a/lib/adplugin/guardian_extensions.rb
+++ b/lib/adplugin/guardian_extensions.rb
@@ -21,5 +21,9 @@ module ::AdPlugin
     def show_adbutler_ads?
       !self.user.in_any_groups?(SiteSetting.adbutler_exclude_groups_map)
     end
+
+    def show_to_groups?
+      !self.user.in_any_groups?(SiteSetting.no_ads_for_groups_map)
+    end
   end
 end
diff --git a/plugin.rb b/plugin.rb
index 9357730..08c2204 100755
--- a/plugin.rb
+++ b/plugin.rb
@@ -71,6 +71,10 @@ after_initialize do
     scope.show_adbutler_ads?
   end
 
+  add_to_serializer :current_user, :show_to_groups do
+    scope.show_to_groups?
+  end
+
   class ::AdstxtController < ::ApplicationController
     skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required
 
diff --git a/test/javascripts/acceptance/adsense-test.js b/test/javascripts/acceptance/adsense-test.js
index 381b333..f959e9e 100644
--- a/test/javascripts/acceptance/adsense-test.js
+++ b/test/javascripts/acceptance/adsense-test.js
@@ -48,6 +48,7 @@ acceptance("AdSense", function (needs) {
       trust_level: 1,
       groups: [AUTO_GROUPS.trust_level_1],
       show_adsense_ads: true,
+      show_to_groups: true,
     });
     await visit("/t/280"); // 20 posts
 
diff --git a/test/javascripts/acceptance/dfp-test.js b/test/javascripts/acceptance/dfp-test.js
index 94ba8f4..7e0e610 100644
--- a/test/javascripts/acceptance/dfp-test.js
+++ b/test/javascripts/acceptance/dfp-test.js
@@ -45,6 +45,7 @@ acceptance("DFP Ads", function (needs) {
       staff: false,
       trust_level: 1,
       show_dfp_ads: true,
+      show_to_groups: true,
     });
     await visit("/t/280"); // 20 posts
 
diff --git a/test/javascripts/acceptance/house-ad-test.js b/test/javascripts/acceptance/house-ad-test.js
index faef2b4..e6ab6d0 100644
--- a/test/javascripts/acceptance/house-ad-test.js
+++ b/test/javascripts/acceptance/house-ad-test.js
@@ -38,7 +38,7 @@ acceptance("House Ads", function (needs) {
   });
 
   test("correct ads show", async (assert) => {
-    updateCurrentUser({ staff: false, trust_level: 1 });
+    updateCurrentUser({ staff: false, trust_level: 1, show_to_groups: true });
     await visit("/t/280"); // 20 posts
 
     assert
diff --git a/test/javascripts/acceptance/mixed-ads-test.js b/test/javascripts/acceptance/mixed-ads-test.js
index d5caa1e..566325c 100644
--- a/test/javascripts/acceptance/mixed-ads-test.js
+++ b/test/javascripts/acceptance/mixed-ads-test.js
@@ -49,6 +49,7 @@ acceptance("Mixed Ads", function (needs) {
       staff: false,
       trust_level: 1,
       show_dfp_ads: true,
+      show_to_groups: true,
     });
     await visit("/t/280"); // 20 posts