DEV: Add tests

app/models/house_ad.rb

@@ -88,14 +88,16 @@ def self.all_for_logged_in_users(scope)
         self.all.select(&:visible_to_logged_in_users)
       else
         # otherwise, filter by group and visible categories
-        self.all.select do |ad|
+           self.all.select do |ad|
           (
             ad.group_ids.any? { |group_id| scope.user.groups.pluck(:id).include?(group_id) } ||
               ad.group_ids.include?(Group::AUTO_GROUPS[:everyone]) || ad.group_ids.empty?
           ) && ad.visible_to_logged_in_users &&
-            ad.category_ids.any? do |category_id|
-              Category.secured(scope).pluck(:id).include?(category_id)
-            end
+            (
+              ad.category_ids.any? do |category_id|
+                Category.secured(scope).pluck(:id).include?(category_id)
+              end || true
+            )
         end
       end
     end

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

@@ -78,8 +78,12 @@ export default AdComponent.extend({
       }
       let ad = houseAds.creatives[adNames[adIndex[placement]]] || "";
       adIndex[placement] = (adIndex[placement] + 1) % adNames.length;
-      // check if the ad includes the current category, otherwise don't show it
-      if (ad.category_ids?.includes(this.currentCategoryId)) {
+      // check if the ad includes the current category, or if no category restrictions are set for the ad
+      // otherwise don't show it
+      if (
+        !ad.category_ids?.length ||
+        ad.category_ids.includes(this.currentCategoryId)
+      ) {
         return ad.html;
       }
     } else {

spec/requests/site_controller_spec.rb

@@ -1,7 +1,12 @@
 # frozen_string_literal: true
 
 RSpec.describe SiteController do
+  fab!(:group)
+  fab!(:private_category) { Fabricate(:private_category, group: group) }
+
   fab!(:user)
+  fab!(:group_2) { Fabricate(:group) }
+  fab!(:user_with_group) { Fabricate(:user, group_ids: [group.id]) }
 
   let!(:anon_ad) do
     AdPlugin::HouseAd.create(
@@ -25,6 +30,28 @@
     )
   end
 
+  let!(:logged_in_ad_with_category) do
+    AdPlugin::HouseAd.create(
+      name: "logged-in-ad-with-category",
+      html: "<div>LOGGED IN WITH CATEGORY</div>",
+      visible_to_logged_in_users: true,
+      visible_to_anons: false,
+      group_ids: [group.id],
+      category_ids: [private_category.id],
+    )
+  end
+
+  let!(:logged_in_ad_with_group_2) do
+    AdPlugin::HouseAd.create(
+      name: "logged-in-ad-with-group",
+      html: "<div>LOGGED IN WITH GROUP</div>",
+      visible_to_logged_in_users: true,
+      visible_to_anons: false,
+      group_ids: [group_2.id],
+      category_ids: [],
+    )
+  end
+
   let!(:everyone_ad) do
     AdPlugin::HouseAd.create(
       name: "everyone-ad",
@@ -36,16 +63,44 @@
     )
   end
 
-  before { AdPlugin::HouseAdSetting.update("topic_list_top", "logged-in-ad|anon-ad|everyone-ad") }
+  let!(:everyone_group_ad) do
+    AdPlugin::HouseAd.create(
+      name: "everyone-group-ad",
+      html: "<div>EVERYONE</div>",
+      visible_to_logged_in_users: true,
+      visible_to_anons: false,
+      group_ids: [Group::AUTO_GROUPS[:everyone]],
+      category_ids: [],
+    )
+  end
+
+  before do
+    AdPlugin::HouseAdSetting.update(
+      "topic_list_top",
+      "logged-in-ad|anon-ad|everyone-ad|logged-in-ad-with-category|logged-in-ad-with-group|everyone-group-ad",
+    )
+  end
 
   describe "#site" do
     context "when logged in" do
-      before { sign_in(user) }
-
       it "only includes ads that are visible to logged in users" do
+        sign_in(user)
+        get "/site.json"
+        # excluded logged_in_ad_with_group_2 and logged_in_ad_with_category
+        expect(response.parsed_body["house_creatives"]["creatives"].keys).to contain_exactly(
+          "logged-in-ad",
+          "everyone-group-ad",
+          "everyone-ad",
+        )
+      end
+
+      it "includes ads that are within the logged in user's category permissions" do
+        sign_in(user_with_group)
         get "/site.json"
         expect(response.parsed_body["house_creatives"]["creatives"].keys).to contain_exactly(
           "logged-in-ad",
+          "everyone-group-ad",
+          "logged-in-ad-with-category",
           "everyone-ad",
         )
       end
@@ -54,6 +109,7 @@
     context "when anonymous" do
       it "only includes ads that are visible to anonymous users" do
         get "/site.json"
+        # excludes everyone_group_ad
         expect(response.parsed_body["house_creatives"]["creatives"].keys).to contain_exactly(
           "anon-ad",
           "everyone-ad",

test/javascripts/acceptance/house-ad-test.js

@@ -25,14 +25,26 @@ acceptance("House Ads", function (needs) {
         after_nth_topic: 6,
       },
       creatives: {
-        "Topic List Top": "<div class='h-topic-list'>TOPIC LIST TOP</div>",
-        "Above Post Stream":
-          "<div class='h-above-post-stream'>ABOVE POST STREAM</div>",
-        "Above Suggested":
-          "<div class='h-above-suggested'>ABOVE SUGGESTED</div>",
-        Post: "<div class='h-post'>BELOW POST</div>",
-        "Between Topic List":
-          "<div class='h-between-topic-list'>BETWEEN TOPIC LIST</div>",
+        "Topic List Top": {
+          html: "<div class='h-topic-list'>TOPIC LIST TOP</div>",
+          category_ids: [],
+        },
+        "Above Post Stream": {
+          html: "<div class='h-above-post-stream'>ABOVE POST STREAM</div>",
+          category_ids: [],
+        },
+        "Above Suggested": {
+          html: "<div class='h-above-suggested'>ABOVE SUGGESTED</div>",
+          category_ids: [],
+        },
+        Post: {
+          html: "<div class='h-post'>BELOW POST</div>",
+          category_ids: [],
+        },
+        "Between Topic List": {
+          html: "<div class='h-between-topic-list'>BETWEEN TOPIC LIST</div>",
+          category_ids: [],
+        },
       },
     },
   });
@@ -114,3 +126,79 @@ acceptance("House Ads", function (needs) {
       );
   });
 });
+
+acceptance(
+  "House Ads | Category and Group Permissions | Display Ad",
+  function (needs) {
+    needs.user();
+    needs.settings({
+      no_ads_for_categories: "",
+    });
+    needs.site({
+      house_creatives: {
+        settings: {
+          topic_list_top: "Topic List Top",
+        },
+        creatives: {
+          "Topic List Top": {
+            html: "<div class='h-topic-list'>TOPIC LIST TOP</div>",
+            // match /c/bug/1
+            category_ids: [1],
+          },
+        },
+      },
+    });
+
+    test("displays ad to users when current category id is included in ad category_ids", async (assert) => {
+      updateCurrentUser({
+        staff: false,
+        trust_level: 1,
+        show_to_groups: true,
+      });
+      await visit("/c/bug/1");
+      assert
+        .dom(".h-topic-list")
+        .exists(
+          "ad is displayed above the topic list because the current category id is included in the ad category_ids"
+        );
+    });
+  }
+);
+
+acceptance(
+  "House Ads | Category and Group Permissions | Hide Ad",
+  function (needs) {
+    needs.user();
+    needs.settings({
+      no_ads_for_categories: "",
+    });
+    needs.site({
+      house_creatives: {
+        settings: {
+          topic_list_top: "Topic List Top",
+        },
+        creatives: {
+          "Topic List Top": {
+            html: "<div class='h-topic-list'>TOPIC LIST TOP</div>",
+            // restrict ad to a different category than /c/bug/1
+            category_ids: [2],
+          },
+        },
+      },
+    });
+
+    test("hides ad to users when current category id is not included in ad category_ids", async (assert) => {
+      updateCurrentUser({
+        staff: false,
+        trust_level: 1,
+        show_to_groups: true,
+      });
+      await visit("/c/bug/1");
+      assert
+        .dom(".h-topic-list")
+        .doesNotExist(
+          "ad is not displayed because the current category id is included in the ad category_ids"
+        );
+    });
+  }
+);