From 6f8ff34ffa10170b9aaa036563e9f31edf28e0d0 Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Wed, 31 Jan 2024 11:04:24 -0700 Subject: [PATCH] DEV: Convert TL settings to groups (#195) * DEV: Convert TL settings to groups This change converts the TL site settings in this plugin to use groups instead. See: https://meta.discourse.org/t/283408 Co-authored-by: Martin Brennan --- .../discourse/components/adbutler-ad.js | 18 +++++---- .../components/amazon-product-links.js | 14 ++++--- .../discourse/components/carbonads-ad.js | 18 +++++---- .../discourse/components/google-adsense.js | 18 +++++---- .../discourse/components/google-dfp-ad.js | 18 +++++---- config/settings.yml | 38 +++++++++++++++++++ ...tl_to_group_settings_adsense_through_tl.rb | 35 +++++++++++++++++ ...ate_tl_to_group_settings_dfp_through_tl.rb | 35 +++++++++++++++++ ..._tl_to_group_settings_amazon_through_tl.rb | 35 +++++++++++++++++ ..._to_group_settings_carbonads_through_tl.rb | 35 +++++++++++++++++ ...l_to_group_settings_adbutler_through_tl.rb | 35 +++++++++++++++++ lib/adplugin/guardian_extensions.rb | 25 ++++++++++++ plugin.rb | 23 +++++++++++ test/javascripts/acceptance/adsense-test.js | 19 ++++++++-- test/javascripts/acceptance/dfp-test.js | 18 ++++++++- test/javascripts/acceptance/mixed-ads-test.js | 9 ++++- 16 files changed, 348 insertions(+), 45 deletions(-) create mode 100644 db/migrate/20240118195155_migrate_tl_to_group_settings_adsense_through_tl.rb create mode 100644 db/migrate/20240118195156_migrate_tl_to_group_settings_dfp_through_tl.rb create mode 100644 db/migrate/20240118195157_migrate_tl_to_group_settings_amazon_through_tl.rb create mode 100644 db/migrate/20240118195158_migrate_tl_to_group_settings_carbonads_through_tl.rb create mode 100644 db/migrate/20240118195159_migrate_tl_to_group_settings_adbutler_through_tl.rb create mode 100644 lib/adplugin/guardian_extensions.rb diff --git a/assets/javascripts/discourse/components/adbutler-ad.js b/assets/javascripts/discourse/components/adbutler-ad.js index ab78698..3fba41b 100644 --- a/assets/javascripts/discourse/components/adbutler-ad.js +++ b/assets/javascripts/discourse/components/adbutler-ad.js @@ -111,30 +111,32 @@ export default AdComponent.extend({ scheduleOnce("afterRender", this, this._triggerAds); }, - @discourseComputed("currentUser.trust_level") - showToTrustLevel(trustLevel) { - return !( - trustLevel && trustLevel > this.siteSettings.adbutler_through_trust_level - ); + @discourseComputed + showToDisplayGroups() { + if (!this.currentUser) { + return true; + } + + return this.currentUser.show_adbutler_ads; }, @discourseComputed( "publisherId", - "showToTrustLevel", + "showToDisplayGroups", "showToGroups", "showAfterPost", "showOnCurrentPage" ) showAd( publisherId, - showToTrustLevel, + showToDisplayGroups, showToGroups, showAfterPost, showOnCurrentPage ) { return ( publisherId && - showToTrustLevel && + showToDisplayGroups && showToGroups && showAfterPost && showOnCurrentPage diff --git a/assets/javascripts/discourse/components/amazon-product-links.js b/assets/javascripts/discourse/components/amazon-product-links.js index 571ed50..4b805ed 100644 --- a/assets/javascripts/discourse/components/amazon-product-links.js +++ b/assets/javascripts/discourse/components/amazon-product-links.js @@ -7,7 +7,7 @@ export default AdComponent.extend({ classNames: ["amazon-product-links"], showAd: and( - "showToTrustLevel", + "showToDisplayGroups", "showToGroups", "showAfterPost", "showOnCurrentPage" @@ -173,11 +173,13 @@ export default AdComponent.extend({ return htmlSafe(`${userInput}`); }, - @discourseComputed("currentUser.trust_level") - showToTrustLevel(trustLevel) { - return !( - trustLevel && trustLevel > this.siteSettings.amazon_through_trust_level - ); + @discourseComputed + showToDisplayGroups() { + if (!this.currentUser) { + return true; + } + + return this.currentUser.show_amazon_ads; }, @discourseComputed("postNumber") diff --git a/assets/javascripts/discourse/components/carbonads-ad.js b/assets/javascripts/discourse/components/carbonads-ad.js index a9e5b95..04955e6 100644 --- a/assets/javascripts/discourse/components/carbonads-ad.js +++ b/assets/javascripts/discourse/components/carbonads-ad.js @@ -19,31 +19,33 @@ export default AdComponent.extend({ ); }, - @discourseComputed("currentUser.trust_level") - showToTrustLevel(trustLevel) { - return !( - trustLevel && trustLevel > this.siteSettings.carbonads_through_trust_level - ); + @discourseComputed + showToDisplayGroups() { + if (!this.currentUser) { + return true; + } + + return this.currentUser.show_carbon_ads; }, @discourseComputed( "placement", "serve_id", - "showToTrustLevel", + "showToDisplayGroups", "showToGroups", "showOnCurrentPage" ) showAd( placement, serveId, - showToTrustLevel, + showToDisplayGroups, showToGroups, showOnCurrentPage ) { return ( placement && serveId && - showToTrustLevel && + showToDisplayGroups && showToGroups && showOnCurrentPage ); diff --git a/assets/javascripts/discourse/components/google-adsense.js b/assets/javascripts/discourse/components/google-adsense.js index d357a37..743b160 100644 --- a/assets/javascripts/discourse/components/google-adsense.js +++ b/assets/javascripts/discourse/components/google-adsense.js @@ -206,30 +206,32 @@ export default AdComponent.extend({ ); }, - @discourseComputed("currentUser.trust_level") - showToTrustLevel(trustLevel) { - return !( - trustLevel && trustLevel > this.siteSettings.adsense_through_trust_level - ); + @discourseComputed + showToDisplayGroups() { + if (!this.currentUser) { + return true; + } + + return this.currentUser.show_adsense_ads; }, @discourseComputed( "publisher_id", - "showToTrustLevel", + "showToDisplayGroups", "showToGroups", "showAfterPost", "showOnCurrentPage" ) showAd( publisherId, - showToTrustLevel, + showToDisplayGroups, showToGroups, showAfterPost, showOnCurrentPage ) { return ( publisherId && - showToTrustLevel && + showToDisplayGroups && showToGroups && showAfterPost && showOnCurrentPage diff --git a/assets/javascripts/discourse/components/google-dfp-ad.js b/assets/javascripts/discourse/components/google-dfp-ad.js index b09f7a7..7989e67 100755 --- a/assets/javascripts/discourse/components/google-dfp-ad.js +++ b/assets/javascripts/discourse/components/google-dfp-ad.js @@ -299,7 +299,7 @@ export default AdComponent.extend({ @discourseComputed( "publisherId", - "showToTrustLevel", + "showToDisplayGroups", "showToGroups", "showAfterPost", "showOnCurrentPage", @@ -307,7 +307,7 @@ export default AdComponent.extend({ ) showAd( publisherId, - showToTrustLevel, + showToDisplayGroups, showToGroups, showAfterPost, showOnCurrentPage, @@ -315,7 +315,7 @@ export default AdComponent.extend({ ) { return ( publisherId && - showToTrustLevel && + showToDisplayGroups && showToGroups && showAfterPost && showOnCurrentPage && @@ -323,11 +323,13 @@ export default AdComponent.extend({ ); }, - @discourseComputed("currentUser.trust_level") - showToTrustLevel(trustLevel) { - return !( - trustLevel && trustLevel > this.siteSettings.dfp_through_trust_level - ); + @discourseComputed + showToDisplayGroups() { + if (!this.currentUser) { + return true; + } + + return this.currentUser.show_dfp_ads; }, @discourseComputed("postNumber") diff --git a/config/settings.yml b/config/settings.yml index 7ba811c..b754f40 100755 --- a/config/settings.yml +++ b/config/settings.yml @@ -49,6 +49,13 @@ adsense_plugin: client: true default: 2 enum: "TrustLevelSetting" + hidden: true + adsense_display_groups: + default: "10|11|12" + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" adsense_topic_list_top_code: client: true default: "" @@ -163,6 +170,13 @@ dfp_plugin: client: true default: 2 enum: "TrustLevelSetting" + hidden: true + dfp_display_groups: + default: "10|11|12" + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" dfp_topic_list_top_code: client: true default: "" @@ -312,6 +326,14 @@ amazon_plugin: client: true default: 2 enum: "TrustLevelSetting" + hidden: true + amazon_display_groups: + client: true + default: "10|11|12" + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" amazon_topic_list_top_src_code: client: true default: "" @@ -400,6 +422,14 @@ carbonads_plugin: client: true default: 2 enum: "TrustLevelSetting" + hidden: true + carbonads_display_groups: + client: true + default: "10|11|12" + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" carbonads_topic_list_top_enabled: client: true default: false @@ -415,6 +445,14 @@ adbutler_plugin: client: true default: 2 enum: "TrustLevelSetting" + hidden: true + adbutler_display_groups: + client: true + default: "10|11|12" + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" adbutler_topic_list_top_zone_id: client: true default: "" diff --git a/db/migrate/20240118195155_migrate_tl_to_group_settings_adsense_through_tl.rb b/db/migrate/20240118195155_migrate_tl_to_group_settings_adsense_through_tl.rb new file mode 100644 index 0000000..46939c2 --- /dev/null +++ b/db/migrate/20240118195155_migrate_tl_to_group_settings_adsense_through_tl.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class MigrateTlToGroupSettingsAdsenseThroughTl < ActiveRecord::Migration[7.0] + def up + adsense_through_trust_level_raw = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'adsense_through_trust_level'", + ).first + + if adsense_through_trust_level_raw.present? + adsense_through_allowed_groups = + case adsense_through_trust_level_raw + when "0" + "10" + when "1" + "10|11" + when "2" + "10|11|12" + when "3" + "10|11|12|13" + when "4" + "10|11|12|13|14" + end + + DB.exec(<<~SQL, setting: adsense_through_allowed_groups) + INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('adsense_through_allowed_groups', :setting, '20', NOW(), NOW()) + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20240118195156_migrate_tl_to_group_settings_dfp_through_tl.rb b/db/migrate/20240118195156_migrate_tl_to_group_settings_dfp_through_tl.rb new file mode 100644 index 0000000..e023ec2 --- /dev/null +++ b/db/migrate/20240118195156_migrate_tl_to_group_settings_dfp_through_tl.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class MigrateTlToGroupSettingsDfpThroughTl < ActiveRecord::Migration[7.0] + def up + dfp_through_trust_level_raw = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'dfp_through_trust_level'", + ).first + + if dfp_through_trust_level_raw.present? + dfp_through_allowed_groups = + case dfp_through_trust_level_raw + when "0" + "10" + when "1" + "10|11" + when "2" + "10|11|12" + when "3" + "10|11|12|13" + when "4" + "10|11|12|13|14" + end + + DB.exec(<<~SQL, setting: dfp_through_allowed_groups) + INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('dfp_through_allowed_groups', :setting, '20', NOW(), NOW()) + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20240118195157_migrate_tl_to_group_settings_amazon_through_tl.rb b/db/migrate/20240118195157_migrate_tl_to_group_settings_amazon_through_tl.rb new file mode 100644 index 0000000..bacc3a5 --- /dev/null +++ b/db/migrate/20240118195157_migrate_tl_to_group_settings_amazon_through_tl.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class MigrateTlToGroupSettingsAmazonThroughTl < ActiveRecord::Migration[7.0] + def up + amazon_through_trust_level_raw = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'amazon_through_trust_level'", + ).first + + if amazon_through_trust_level_raw.present? + amazon_through_allowed_groups = + case amazon_through_trust_level_raw + when "0" + "10" + when "1" + "10|11" + when "2" + "10|11|12" + when "3" + "10|11|12|13" + when "4" + "10|11|12|13|14" + end + + DB.exec(<<~SQL, setting: amazon_through_allowed_groups) + INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('amazon_through_allowed_groups', :setting, '20', NOW(), NOW()) + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20240118195158_migrate_tl_to_group_settings_carbonads_through_tl.rb b/db/migrate/20240118195158_migrate_tl_to_group_settings_carbonads_through_tl.rb new file mode 100644 index 0000000..a5ecf9e --- /dev/null +++ b/db/migrate/20240118195158_migrate_tl_to_group_settings_carbonads_through_tl.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class MigrateTlToGroupSettingsCarbonadsThroughTl < ActiveRecord::Migration[7.0] + def up + carbonads_through_trust_level_raw = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'carbonads_through_trust_level'", + ).first + + if carbonads_through_trust_level_raw.present? + carbonads_through_allowed_groups = + case carbonads_through_trust_level_raw + when "0" + "10" + when "1" + "10|11" + when "2" + "10|11|12" + when "3" + "10|11|12|13" + when "4" + "10|11|12|13|14" + end + + DB.exec(<<~SQL, setting: carbonads_through_allowed_groups) + INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('carbonads_through_allowed_groups', :setting, '20', NOW(), NOW()) + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20240118195159_migrate_tl_to_group_settings_adbutler_through_tl.rb b/db/migrate/20240118195159_migrate_tl_to_group_settings_adbutler_through_tl.rb new file mode 100644 index 0000000..8489612 --- /dev/null +++ b/db/migrate/20240118195159_migrate_tl_to_group_settings_adbutler_through_tl.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class MigrateTlToGroupSettingsAdbutlerThroughTl < ActiveRecord::Migration[7.0] + def up + adbutler_through_trust_level_raw = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'adbutler_through_trust_level'", + ).first + + if adbutler_through_trust_level_raw.present? + adbutler_through_allowed_groups = + case adbutler_through_trust_level_raw + when "0" + "10" + when "1" + "10|11" + when "2" + "10|11|12" + when "3" + "10|11|12|13" + when "4" + "10|11|12|13|14" + end + + DB.exec(<<~SQL, setting: adbutler_through_allowed_groups) + INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('adbutler_through_allowed_groups', :setting, '20', NOW(), NOW()) + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/adplugin/guardian_extensions.rb b/lib/adplugin/guardian_extensions.rb new file mode 100644 index 0000000..440a5eb --- /dev/null +++ b/lib/adplugin/guardian_extensions.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module ::AdPlugin + module GuardianExtensions + def show_dfp_ads? + self.user.in_any_groups?(SiteSetting.dfp_display_groups_map) + end + + def show_adsense_ads? + self.user.in_any_groups?(SiteSetting.adsense_display_groups_map) + end + + def show_carbon_ads? + self.user.in_any_groups?(SiteSetting.carbonads_display_groups_map) + end + + def show_amazon_ads? + self.user.in_any_groups?(SiteSetting.amazon_display_groups_map) + end + + def show_adbutler_ads? + self.user.in_any_groups?(SiteSetting.adbutler_display_groups_map) + end + end +end diff --git a/plugin.rb b/plugin.rb index 35eb4ee..9357730 100755 --- a/plugin.rb +++ b/plugin.rb @@ -36,8 +36,11 @@ after_initialize do require_dependency File.expand_path("../app/models/house_ad_setting", __FILE__) require_dependency File.expand_path("../app/controllers/house_ads_controller", __FILE__) require_dependency File.expand_path("../app/controllers/house_ad_settings_controller", __FILE__) + require_dependency File.expand_path("../lib/adplugin/guardian_extensions", __FILE__) require_dependency "application_controller" + reloadable_patch { Guardian.prepend ::AdPlugin::GuardianExtensions } + add_to_serializer :site, :house_creatives do AdPlugin::HouseAdSetting.settings_and_ads(for_anons: scope.anonymous?) end @@ -48,6 +51,26 @@ after_initialize do !(SiteSetting.no_ads_for_tags.split("|") & object.topic.tags.map(&:name)).empty? end + add_to_serializer :current_user, :show_dfp_ads do + scope.show_dfp_ads? + end + + add_to_serializer :current_user, :show_adsense_ads do + scope.show_adsense_ads? + end + + add_to_serializer :current_user, :show_carbon_ads do + scope.show_carbon_ads? + end + + add_to_serializer :current_user, :show_amazon_ads do + scope.show_amazon_ads? + end + + add_to_serializer :current_user, :show_adbutler_ads do + scope.show_adbutler_ads? + 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 ecdee93..381b333 100644 --- a/test/javascripts/acceptance/adsense-test.js +++ b/test/javascripts/acceptance/adsense-test.js @@ -1,5 +1,6 @@ import { visit } from "@ember/test-helpers"; import { test } from "qunit"; +import { AUTO_GROUPS } from "discourse/lib/constants"; import groupFixtures from "discourse/tests/fixtures/group-fixtures"; import { acceptance, @@ -12,7 +13,10 @@ acceptance("AdSense", function (needs) { no_ads_for_groups: "47", no_ads_for_categories: "1", adsense_publisher_code: "MYADSENSEID", - adsense_through_trust_level: 2, + adsense_display_groups: [ + AUTO_GROUPS.trust_level_1, + AUTO_GROUPS.trust_level_2, + ], adsense_topic_list_top_code: "list_top_ad_unit", adsense_topic_list_top_ad_sizes: "728*90 - leaderboard", adsense_mobile_topic_list_top_code: "mobile_list_top_ad_unit", @@ -39,7 +43,12 @@ acceptance("AdSense", function (needs) { }); test("correct number of ads should show", async (assert) => { - updateCurrentUser({ staff: false, trust_level: 1 }); + updateCurrentUser({ + staff: false, + trust_level: 1, + groups: [AUTO_GROUPS.trust_level_1], + show_adsense_ads: true, + }); await visit("/t/280"); // 20 posts assert @@ -64,7 +73,11 @@ acceptance("AdSense", function (needs) { }); test("no ads for trust level 3", async (assert) => { - updateCurrentUser({ staff: false, trust_level: 3 }); + updateCurrentUser({ + staff: false, + trust_level: 3, + groups: [AUTO_GROUPS.trust_level_3], + }); await visit("/t/280"); assert .dom(".google-adsense.adsense-post-bottom") diff --git a/test/javascripts/acceptance/dfp-test.js b/test/javascripts/acceptance/dfp-test.js index 0c53ecd..94ba8f4 100644 --- a/test/javascripts/acceptance/dfp-test.js +++ b/test/javascripts/acceptance/dfp-test.js @@ -1,5 +1,6 @@ import { visit } from "@ember/test-helpers"; import { test } from "qunit"; +import { AUTO_GROUPS } from "discourse/lib/constants"; import groupFixtures from "discourse/tests/fixtures/group-fixtures"; import { acceptance, @@ -13,6 +14,7 @@ acceptance("DFP Ads", function (needs) { no_ads_for_categories: "1", dfp_publisher_id: "MYdfpID", dfp_through_trust_level: 2, + dfp_display_groups: [AUTO_GROUPS.trust_level_1, AUTO_GROUPS.trust_level_2], dfp_topic_list_top_code: "list_top_ad_unit", dfp_topic_list_top_ad_sizes: "728*90 - leaderboard", dfp_mobile_topic_list_top_code: "mobile_list_top_ad_unit", @@ -39,7 +41,11 @@ acceptance("DFP Ads", function (needs) { }); test("correct number of ads should show", async (assert) => { - updateCurrentUser({ staff: false, trust_level: 1 }); + updateCurrentUser({ + staff: false, + trust_level: 1, + show_dfp_ads: true, + }); await visit("/t/280"); // 20 posts assert @@ -60,7 +66,15 @@ acceptance("DFP Ads", function (needs) { }); test("no ads for trust level 3", async (assert) => { - updateCurrentUser({ staff: false, trust_level: 3 }); + updateCurrentUser({ + staff: false, + trust_level: 3, + groups: [ + AUTO_GROUPS.trust_level_1, + AUTO_GROUPS.trust_level_2, + AUTO_GROUPS.trust_level_3, + ], + }); await visit("/t/280"); assert .dom(".google-dfp-ad.dfp-ad-post-bottom") diff --git a/test/javascripts/acceptance/mixed-ads-test.js b/test/javascripts/acceptance/mixed-ads-test.js index 754c6a1..d5caa1e 100644 --- a/test/javascripts/acceptance/mixed-ads-test.js +++ b/test/javascripts/acceptance/mixed-ads-test.js @@ -1,5 +1,6 @@ import { visit } from "@ember/test-helpers"; import { test } from "qunit"; +import { AUTO_GROUPS } from "discourse/lib/constants"; import { acceptance, count, @@ -12,7 +13,7 @@ acceptance("Mixed Ads", function (needs) { house_ads_after_nth_post: 6, house_ads_frequency: 50, dfp_publisher_id: "MYdfpID", - dfp_through_trust_level: 2, + dfp_display_groups: [AUTO_GROUPS.trust_level_1, AUTO_GROUPS.trust_level_2], dfp_topic_list_top_code: "list_top_ad_unit", dfp_topic_list_top_ad_sizes: "728*90 - leaderboard", dfp_mobile_topic_list_top_code: "mobile_list_top_ad_unit", @@ -44,7 +45,11 @@ acceptance("Mixed Ads", function (needs) { }); test("correct ads show", async (assert) => { - updateCurrentUser({ staff: false, trust_level: 1 }); + updateCurrentUser({ + staff: false, + trust_level: 1, + show_dfp_ads: true, + }); await visit("/t/280"); // 20 posts const houseAdsCount = count(".house-creative");