FIX: Add exclude groups for each ad platforms (#197)

With the new group system for displaying ads we no longer can check if a
user belongs to a trust level group lower than specified. The other
problem is that ALL users including staff and higher trust levels all
belong to trust level 0. So without this fix if we say that an ad should
be visible to trust level 0 users then it will be shown to all users.

This fix adds a new default setting for each ad platform for excluding
trust level 3, 4, and staff users from being shown ads.

- Make display_groups hidden (they will be removed in a later commit)
- Switch to using only exclude_groups instead of display groups and
  exclude groups
- rename showToDisplayGroups to showXAds for each provider
This commit is contained in:
Blake Erickson 2024-02-08 19:42:40 -07:00 committed by GitHub
parent 6b5412826f
commit 9581367239
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 338 additions and 33 deletions

View File

@ -112,7 +112,7 @@ export default AdComponent.extend({
},
@discourseComputed
showToDisplayGroups() {
showAdbutlerAds() {
if (!this.currentUser) {
return true;
}
@ -122,21 +122,21 @@ export default AdComponent.extend({
@discourseComputed(
"publisherId",
"showToDisplayGroups",
"showAdbutlerAds",
"showToGroups",
"showAfterPost",
"showOnCurrentPage"
)
showAd(
publisherId,
showToDisplayGroups,
showAdbutlerAds,
showToGroups,
showAfterPost,
showOnCurrentPage
) {
return (
publisherId &&
showToDisplayGroups &&
showAdbutlerAds &&
showToGroups &&
showAfterPost &&
showOnCurrentPage

View File

@ -7,7 +7,7 @@ export default AdComponent.extend({
classNames: ["amazon-product-links"],
showAd: and(
"showToDisplayGroups",
"showAmazonAds",
"showToGroups",
"showAfterPost",
"showOnCurrentPage"
@ -174,7 +174,7 @@ export default AdComponent.extend({
},
@discourseComputed
showToDisplayGroups() {
showAmazonAds() {
if (!this.currentUser) {
return true;
}

View File

@ -20,7 +20,7 @@ export default AdComponent.extend({
},
@discourseComputed
showToDisplayGroups() {
showCarbonAds() {
if (!this.currentUser) {
return true;
}
@ -31,23 +31,13 @@ export default AdComponent.extend({
@discourseComputed(
"placement",
"serve_id",
"showToDisplayGroups",
"showCarbonAds",
"showToGroups",
"showOnCurrentPage"
)
showAd(
placement,
serveId,
showToDisplayGroups,
showToGroups,
showOnCurrentPage
) {
showAd(placement, serveId, showCarbonAds, showToGroups, showOnCurrentPage) {
return (
placement &&
serveId &&
showToDisplayGroups &&
showToGroups &&
showOnCurrentPage
placement && serveId && showCarbonAds && showToGroups && showOnCurrentPage
);
},
});

View File

@ -207,7 +207,7 @@ export default AdComponent.extend({
},
@discourseComputed
showToDisplayGroups() {
showAdsenseAds() {
if (!this.currentUser) {
return true;
}
@ -217,21 +217,21 @@ export default AdComponent.extend({
@discourseComputed(
"publisher_id",
"showToDisplayGroups",
"showAdsenseAds",
"showToGroups",
"showAfterPost",
"showOnCurrentPage"
)
showAd(
publisherId,
showToDisplayGroups,
showAdsenseAds,
showToGroups,
showAfterPost,
showOnCurrentPage
) {
return (
publisherId &&
showToDisplayGroups &&
showAdsenseAds &&
showToGroups &&
showAfterPost &&
showOnCurrentPage

View File

@ -299,7 +299,7 @@ export default AdComponent.extend({
@discourseComputed(
"publisherId",
"showToDisplayGroups",
"showDfpAds",
"showToGroups",
"showAfterPost",
"showOnCurrentPage",
@ -307,7 +307,7 @@ export default AdComponent.extend({
)
showAd(
publisherId,
showToDisplayGroups,
showDfpAds,
showToGroups,
showAfterPost,
showOnCurrentPage,
@ -315,7 +315,7 @@ export default AdComponent.extend({
) {
return (
publisherId &&
showToDisplayGroups &&
showDfpAds &&
showToGroups &&
showAfterPost &&
showOnCurrentPage &&
@ -324,7 +324,7 @@ export default AdComponent.extend({
},
@discourseComputed
showToDisplayGroups() {
showDfpAds() {
if (!this.currentUser) {
return true;
}

View File

@ -15,6 +15,7 @@ en:
dfp_publisher_id: "Input your Google Ad Manager (formerly called DFP) network code, which is found in your network settings. NOTE: You will need to update the 'content security policy script src' setting. See <a href='https://meta.discourse.org/t/33734' target='_blank'>this topic</a> for the latest instructions."
dfp_publisher_id_mobile: "(Optional) If you want to use a different Ad Manager publisher id for the mobile version of the site, enter it here. Leave blank to use dfp_publisher_id on mobile."
dfp_through_trust_level: "Show your ads to users based on trust levels. Users with trust level higher than this value will not see ads."
dfp_exclude_groups: "Ads will not be shown to members of these groups"
dfp_topic_list_top_code: "Enter the unique Code of the ad unit to display above topic lists. This is the short code (max 100 chars) given to the ad unit when it was created, not the JavaScript code."
dfp_topic_list_top_ad_sizes: "Choose your ad size for the ad unit above topic lists."
@ -47,6 +48,7 @@ en:
adsense_publisher_code: "Your publisher ID. Enter only the number, excluding 'pub-'. NOTE: You will need to update the 'content security policy script src' setting. See <a href='https://meta.discourse.org/t/33734' target='_blank'>this topic</a> for the latest instructions."
adsense_through_trust_level: "Show your ads to users based on trust levels. Users with trust level higher than this value will not see ads."
adsense_exclude_groups: "Ads will not be shown to members of these groups"
adsense_topic_list_top_code: "Enter code of the ad unit to display at topic list top location. This is the number assigned to the ad unit, not the JavaScript code."
adsense_mobile_topic_list_top_code: "Enter code of the ad unit to display mobile ads at topic list top location. This is the number assigned to the ad unit, not the JavaScript code."
adsense_topic_list_top_ad_sizes: "Choose your ad size"
@ -66,6 +68,7 @@ en:
adsense_nth_post_code: "Show an ad after every N posts, where N is this value."
amazon_through_trust_level: "Show your ads to users based on trust levels. Users with trust level higher than this value will not see ads."
amazon_exclude_groups: "Ads will not be shown to members of these groups"
amazon_topic_list_top_src_code: "Enter src code to display at topic list top location"
amazon_topic_list_top_ad_width_code: "Input your ad width"
amazon_topic_list_top_ad_height_code: "Input your ad height"
@ -95,6 +98,7 @@ en:
carbonads_serve_id: "Your Carbon Ads Serve ID"
carbonads_placement: "Your Carbon Ads Placement"
carbonads_through_trust_level: "Show your ads to users based on trust levels. Users with trust level higher than this value will not see ads."
carbonads_exclude_groups: "Ads will not be shown to members of these groups"
carbonads_topic_list_top_enabled: "Show an ad above the topic list"
carbonads_above_post_stream_enabled: "Show an ad above the post stream"
@ -109,4 +113,5 @@ en:
adbutler_topic_above_suggested_zone_id: "Zone ID for topic above suggested location"
adbutler_nth_post: "Show an ad after every N posts, where N is this value"
adbutler_through_trust_level: "Show your ads to users based on trust levels. Users with trust level higher than this value will not see ads"
adbutler_exclude_groups: "Ads will not be shown to members of these groups"
adbutler_adserver_hostname: "The hostname that AdButler is serving your ads from"

View File

@ -56,6 +56,13 @@ adsense_plugin:
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
hidden: true
adsense_exclude_groups:
default: "3|13|14"
type: group_list
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
adsense_topic_list_top_code:
client: true
default: ""
@ -177,6 +184,13 @@ dfp_plugin:
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
hidden: true
dfp_exclude_groups:
default: "3|13|14"
type: group_list
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
dfp_topic_list_top_code:
client: true
default: ""
@ -334,6 +348,13 @@ amazon_plugin:
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
hidden: true
amazon_exclude_groups:
default: "3|13|14"
type: group_list
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
amazon_topic_list_top_src_code:
client: true
default: ""
@ -430,6 +451,13 @@ carbonads_plugin:
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
hidden: true
carbonads_exclude_groups:
default: "3|13|14"
type: group_list
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
carbonads_topic_list_top_enabled:
client: true
default: false
@ -453,6 +481,13 @@ adbutler_plugin:
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
hidden: true
adbutler_exclude_groups:
default: "3|13|14"
type: group_list
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
adbutler_topic_list_top_zone_id:
client: true
default: ""

View File

@ -0,0 +1,33 @@
# frozen_string_literal: true
class MigrateAdsenseToExcludeGroups < ActiveRecord::Migration[7.0]
def up
adsense_display_groups_raw =
DB.query_single("SELECT value FROM site_settings WHERE name = 'adsense_display_groups'").first
if adsense_display_groups_raw.present?
adsense_exclude_groups =
case adsense_display_groups_raw
when "10"
"3|11|12|13|14"
when "10|11"
"3|12|13|14"
when "10|11|12"
"3|13|14"
when "10|11|12|13"
"3|14"
when "10|11|12|14"
"3"
end
DB.exec(<<~SQL, setting: adsense_exclude_groups)
INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
VALUES('adsense_exclude_groups', :setting, '20', NOW(), NOW())
SQL
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -0,0 +1,33 @@
# frozen_string_literal: true
class MigrateAmazonToExcludeGroups < ActiveRecord::Migration[7.0]
def up
amazon_display_groups_raw =
DB.query_single("SELECT value FROM site_settings WHERE name = 'amazon_display_groups'").first
if amazon_display_groups_raw.present?
amazon_exclude_groups =
case amazon_display_groups_raw
when "10"
"3|11|12|13|14"
when "10|11"
"3|12|13|14"
when "10|11|12"
"3|13|14"
when "10|11|12|13"
"3|14"
when "10|11|12|14"
"3"
end
DB.exec(<<~SQL, setting: amazon_exclude_groups)
INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
VALUES('amazon_exclude_groups', :setting, '20', NOW(), NOW())
SQL
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -0,0 +1,33 @@
# frozen_string_literal: true
class MigrateDfpToExcludeGroups < ActiveRecord::Migration[7.0]
def up
dfp_display_groups_raw =
DB.query_single("SELECT value FROM site_settings WHERE name = 'dfp_display_groups'").first
if dfp_display_groups_raw.present?
dfp_exclude_groups =
case dfp_display_groups_raw
when "10"
"3|11|12|13|14"
when "10|11"
"3|12|13|14"
when "10|11|12"
"3|13|14"
when "10|11|12|13"
"3|14"
when "10|11|12|14"
"3"
end
DB.exec(<<~SQL, setting: dfp_exclude_groups)
INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
VALUES('dfp_exclude_groups', :setting, '20', NOW(), NOW())
SQL
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -0,0 +1,33 @@
# frozen_string_literal: true
class MigrateCarbonToExcludeGroups < ActiveRecord::Migration[7.0]
def up
carbon_display_groups_raw =
DB.query_single("SELECT value FROM site_settings WHERE name = 'carbon_display_groups'").first
if carbon_display_groups_raw.present?
carbon_exclude_groups =
case carbon_display_groups_raw
when "10"
"3|11|12|13|14"
when "10|11"
"3|12|13|14"
when "10|11|12"
"3|13|14"
when "10|11|12|13"
"3|14"
when "10|11|12|14"
"3"
end
DB.exec(<<~SQL, setting: carbon_exclude_groups)
INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
VALUES('carbon_exclude_groups', :setting, '20', NOW(), NOW())
SQL
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -0,0 +1,35 @@
# frozen_string_literal: true
class MigrateAdbutlerToExcludeGroups < ActiveRecord::Migration[7.0]
def up
adbutler_display_groups_raw =
DB.query_single(
"SELECT value FROM site_settings WHERE name = 'adbutler_display_groups'",
).first
if adbutler_display_groups_raw.present?
adbutler_exclude_groups =
case adbutler_display_groups_raw
when "10"
"3|11|12|13|14"
when "10|11"
"3|12|13|14"
when "10|11|12"
"3|13|14"
when "10|11|12|13"
"3|14"
when "10|11|12|14"
"3"
end
DB.exec(<<~SQL, setting: adbutler_exclude_groups)
INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
VALUES('adbutler_exclude_groups', :setting, '20', NOW(), NOW())
SQL
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -3,23 +3,23 @@
module ::AdPlugin
module GuardianExtensions
def show_dfp_ads?
self.user.in_any_groups?(SiteSetting.dfp_display_groups_map)
!self.user.in_any_groups?(SiteSetting.dfp_exclude_groups_map)
end
def show_adsense_ads?
self.user.in_any_groups?(SiteSetting.adsense_display_groups_map)
!self.user.in_any_groups?(SiteSetting.adsense_exclude_groups_map)
end
def show_carbon_ads?
self.user.in_any_groups?(SiteSetting.carbonads_display_groups_map)
!self.user.in_any_groups?(SiteSetting.carbonads_exclude_groups_map)
end
def show_amazon_ads?
self.user.in_any_groups?(SiteSetting.amazon_display_groups_map)
!self.user.in_any_groups?(SiteSetting.amazon_exclude_groups_map)
end
def show_adbutler_ads?
self.user.in_any_groups?(SiteSetting.adbutler_display_groups_map)
!self.user.in_any_groups?(SiteSetting.adbutler_exclude_groups_map)
end
end
end

View File

@ -0,0 +1,108 @@
# frozen_string_literal: true
RSpec.describe CurrentUserSerializer do
fab!(:tl0_user) { Fabricate(:user, trust_level: 0, refresh_auto_groups: true) }
fab!(:tl2_user) { Fabricate(:user, trust_level: 2, refresh_auto_groups: true) }
fab!(:tl3_user) { Fabricate(:user, trust_level: 3, refresh_auto_groups: true) }
fab!(:admin) { Fabricate(:admin) }
let(:tl0_serializer) { described_class.new(tl0_user, scope: Guardian.new(tl0_user), root: false) }
let(:tl2_serializer) { described_class.new(tl2_user, scope: Guardian.new(tl2_user), root: false) }
let(:tl3_serializer) { described_class.new(tl3_user, scope: Guardian.new(tl3_user), root: false) }
let(:admin_serializer) { described_class.new(admin, scope: Guardian.new(admin), root: false) }
before { SiteSetting.discourse_adplugin_enabled = true }
describe "#adsense" do
it "is displayed for TL0 by default" do
expect(tl0_serializer.show_adsense_ads).to eq(true)
end
it "is displayed for TL2 by default" do
expect(tl2_serializer.show_adsense_ads).to eq(true)
end
it "is off for TL3 by default" do
expect(tl3_serializer.show_adsense_ads).to eq(false)
end
it "is off for admin by default" do
expect(admin_serializer.show_adsense_ads).to eq(false)
end
end
describe "#amazon" do
it "is displayed for TL0 by default" do
expect(tl0_serializer.show_amazon_ads).to eq(true)
end
it "is displayed for TL2 by default" do
expect(tl2_serializer.show_amazon_ads).to eq(true)
end
it "is off for TL3 by default" do
expect(tl3_serializer.show_amazon_ads).to eq(false)
end
it "is off for admin by default" do
expect(admin_serializer.show_amazon_ads).to eq(false)
end
end
describe "#dfp" do
it "is displayed for TL0 by default" do
expect(tl0_serializer.show_dfp_ads).to eq(true)
end
it "is displayed for TL2 by default" do
expect(tl2_serializer.show_dfp_ads).to eq(true)
end
it "is off for TL3 by default" do
expect(tl3_serializer.show_dfp_ads).to eq(false)
end
it "is off for admin by default" do
expect(admin_serializer.show_dfp_ads).to eq(false)
end
end
describe "#carbon" do
it "is displayed for TL0 by default" do
expect(tl0_serializer.show_carbon_ads).to eq(true)
end
it "is displayed for TL2 by default" do
expect(tl2_serializer.show_carbon_ads).to eq(true)
end
it "is off for TL3 by default" do
expect(tl3_serializer.show_carbon_ads).to eq(false)
end
it "is off for admin by default" do
expect(admin_serializer.show_carbon_ads).to eq(false)
end
end
describe "#adbutler" do
it "is displayed for TL0 by default" do
expect(tl0_serializer.show_adbutler_ads).to eq(true)
end
it "is displayed for TL2 by default" do
expect(tl2_serializer.show_adbutler_ads).to eq(true)
end
it "is off for TL3 by default" do
expect(tl3_serializer.show_adbutler_ads).to eq(false)
end
it "is off for admin by default" do
expect(admin_serializer.show_adbutler_ads).to eq(false)
end
end
end