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.

assets/javascripts/discourse/components/ad-component.js

@@ -45,25 +45,13 @@ export default Component.extend({
     return topicType === "private_message";
   },
 
-  @discourseComputed("currentUser.groups")
+  @discourseComputed
-  showToGroups(groups) {
+  showToGroups() {
-    const currentUser = this.currentUser;
+    if (!this.currentUser) {
-
-    if (
-      !currentUser ||
-      !groups ||
-      !this.siteSettings.no_ads_for_groups ||
-      this.siteSettings.no_ads_for_groups.length === 0
-    ) {
       return true;
     }
 
-    let noAdsGroups = this.siteSettings.no_ads_for_groups
+    return this.currentUser.show_to_groups;
-      .split("|")
-      .filter(Boolean);
-    let currentGroups = groups.map((g) => g.id.toString());
-
-    return !currentGroups.any((g) => noAdsGroups.includes(g));
   },
 
   @discourseComputed(

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:

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

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
 

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
 

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
 

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

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