From 725e146dca16a49f218b66efb385a376c6217ec2 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Tue, 7 Jan 2025 11:27:24 +1000
Subject: [PATCH] FIX: Calculate experiment_enabled on server for "What's new?"
 (#30599)

Experimental "What's new?" feature feed items previously calculated
a boolean for experimentEnabled on the client based on the siteSettings
service, and this would control the initial state of the experiment
toggle.

However this requires the person who creates the site setting for the
experiment to remember to set it to `client: true`. This commit removes
that manual step by calculating whether the experiment is enabled
server-side, where we have access to all the site settings.
---
 .../addon/components/dashboard-new-feature-item.gjs  | 12 ++----------
 lib/discourse_updates.rb                             | 10 ++++++++--
 spec/lib/discourse_updates_spec.rb                   |  3 +++
 spec/system/admin_dashboard_new_features_spec.rb     |  6 ++++--
 spec/system/page_objects/pages/admin_new_features.rb |  7 +++++--
 5 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/components/dashboard-new-feature-item.gjs b/app/assets/javascripts/admin/addon/components/dashboard-new-feature-item.gjs
index bc934f03383..a086209a3e1 100644
--- a/app/assets/javascripts/admin/addon/components/dashboard-new-feature-item.gjs
+++ b/app/assets/javascripts/admin/addon/components/dashboard-new-feature-item.gjs
@@ -2,7 +2,6 @@ import Component from "@glimmer/component";
 import { tracked } from "@glimmer/tracking";
 import { on } from "@ember/modifier";
 import { action } from "@ember/object";
-import didInsert from "@ember/render-modifiers/modifiers/did-insert";
 import { service } from "@ember/service";
 import { htmlSafe } from "@ember/template";
 import { and, not } from "truth-helpers";
@@ -11,22 +10,15 @@ import DToggleSwitch from "discourse/components/d-toggle-switch";
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import dIcon from "discourse-common/helpers/d-icon";
-import { bind } from "discourse-common/utils/decorators";
 import { i18n } from "discourse-i18n";
 import DTooltip from "float-kit/components/d-tooltip";
 
 export default class DiscourseNewFeatureItem extends Component {
   @service siteSettings;
   @service toasts;
-  @tracked experimentEnabled;
+  @tracked experimentEnabled = this.args.item.experiment_enabled;
   @tracked toggleExperimentDisabled = false;
 
-  @bind
-  initEnabled() {
-    this.experimentEnabled =
-      this.siteSettings[this.args.item.experiment_setting];
-  }
-
   @action
   async toggleExperiment() {
     if (this.toggleExperimentDisabled) {
@@ -68,7 +60,7 @@ export default class DiscourseNewFeatureItem extends Component {
   }
 
   <template>
-    <div class="admin-new-feature-item" {{didInsert this.initEnabled}}>
+    <div class="admin-new-feature-item">
       <div class="admin-new-feature-item__content">
         <div class="admin-new-feature-item__header">
           {{#if (and @item.emoji (not @item.screenshot_url))}}
diff --git a/lib/discourse_updates.rb b/lib/discourse_updates.rb
index b280728740a..07ce3900e82 100644
--- a/lib/discourse_updates.rb
+++ b/lib/discourse_updates.rb
@@ -156,8 +156,14 @@ module DiscourseUpdates
       entries.map! do |item|
         next item if !item["experiment_setting"]
 
-        item["experiment_setting"] = nil if !SiteSetting.respond_to?(item["experiment_setting"]) ||
-          SiteSetting.type_supervisor.get_type(item["experiment_setting"].to_sym) != :bool
+        if !SiteSetting.respond_to?(item["experiment_setting"]) ||
+             SiteSetting.type_supervisor.get_type(item["experiment_setting"].to_sym) != :bool
+          item["experiment_setting"] = nil
+          item["experiment_enabled"] = false
+        else
+          item["experiment_enabled"] = SiteSetting.send(item["experiment_setting"].to_sym) if item
+        end
+
         item
       end
 
diff --git a/spec/lib/discourse_updates_spec.rb b/spec/lib/discourse_updates_spec.rb
index 3b5dd23adba..b53c05e5b47 100644
--- a/spec/lib/discourse_updates_spec.rb
+++ b/spec/lib/discourse_updates_spec.rb
@@ -277,8 +277,11 @@ RSpec.describe DiscourseUpdates do
 
       expect(result.length).to eq(3)
       expect(result[0]["experiment_setting"]).to eq("enable_mobile_theme")
+      expect(result[0]["experiment_enabled"]).to eq(true)
       expect(result[1]["experiment_setting"]).to be_nil
+      expect(result[1]["experiment_enabled"]).to eq(false)
       expect(result[2]["experiment_setting"]).to be_nil
+      expect(result[2]["experiment_enabled"]).to eq(false)
     end
 
     it "correctly shows features when related plugins are installed" do
diff --git a/spec/system/admin_dashboard_new_features_spec.rb b/spec/system/admin_dashboard_new_features_spec.rb
index 553c60e6cef..fc053d8bcb0 100644
--- a/spec/system/admin_dashboard_new_features_spec.rb
+++ b/spec/system/admin_dashboard_new_features_spec.rb
@@ -88,7 +88,7 @@ describe "Admin New Features Page", type: :system do
     expect(new_features_page).to have_no_screenshot
   end
 
-  it "displays experimental feature toggle" do
+  it "displays experimental feature toggle and has the correct state" do
     DiscourseUpdates.stubs(:new_features).returns(
       [
         {
@@ -103,11 +103,12 @@ describe "Admin New Features Page", type: :system do
           "created_at" => "2023-11-10T02:52:41.462Z",
           "updated_at" => "2023-11-10T04:28:47.020Z",
           "experiment_setting" => "experimental_form_templates",
+          "experiment_enabled" => true,
         },
       ],
     )
     new_features_page.visit
-    expect(new_features_page).to have_toggle_experiment_button
+    expect(new_features_page).to have_toggle_experiment_button(true)
   end
 
   it "displays experimental text next to feature title when feature is experimental" do
@@ -125,6 +126,7 @@ describe "Admin New Features Page", type: :system do
           "created_at" => "2023-11-10T02:52:41.462Z",
           "updated_at" => "2023-11-10T04:28:47.020Z",
           "experiment_setting" => "experimental_form_templates",
+          "experiment_enabled" => true,
         },
       ],
     )
diff --git a/spec/system/page_objects/pages/admin_new_features.rb b/spec/system/page_objects/pages/admin_new_features.rb
index 1aefd4ee428..26eece6c335 100644
--- a/spec/system/page_objects/pages/admin_new_features.rb
+++ b/spec/system/page_objects/pages/admin_new_features.rb
@@ -16,8 +16,11 @@ module PageObjects
         page.has_no_css?(".admin-new-feature-item__screenshot")
       end
 
-      def has_toggle_experiment_button?
-        page.has_css?(".admin-new-feature-item__feature-toggle")
+      def has_toggle_experiment_button?(enabled)
+        page.has_css?(
+          ".admin-new-feature-item__feature-toggle .d-toggle-switch__checkbox[aria-checked='#{enabled}']",
+          visible: false,
+        )
       end
 
       def has_learn_more_link?