From 433fadbd52705b71e18f757294abe2c4ff87e75c Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Tue, 22 Oct 2024 10:56:58 +1100 Subject: [PATCH] FEATURE: allow admins to enable announced experimental features (#29244) Toggle the button to enable the experimental site setting from "What's new" announcement. The toggle button is displayed when: - site setting exists and is boolean; - potentially required plugin is enabled. --- .../components/dashboard-new-feature-item.gjs | 177 +++++++++++++----- .../stylesheets/common/admin/dashboard.scss | 18 ++ app/controllers/admin/dashboard_controller.rb | 12 ++ app/services/experiments/toggle.rb | 34 ++++ config/locales/client.en.yml | 7 + config/routes.rb | 1 + lib/discourse_updates.rb | 18 +- spec/lib/discourse_updates_spec.rb | 58 ++++++ spec/multisite/jobs_spec.rb | 2 +- spec/services/experiments/toggle_spec.rb | 56 ++++++ .../admin_dashboard_new_features_spec.rb | 22 +++ .../page_objects/pages/admin_new_features.rb | 4 + 12 files changed, 363 insertions(+), 46 deletions(-) create mode 100644 app/services/experiments/toggle.rb create mode 100644 spec/services/experiments/toggle_spec.rb 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 793f79ae229..f1cecfe88eb 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 @@ -1,50 +1,141 @@ +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 { and, not } from "truth-helpers"; import CookText from "discourse/components/cook-text"; +import DToggleSwitch from "discourse/components/d-toggle-switch"; +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; import i18n from "discourse-common/helpers/i18n"; +import { bind } from "discourse-common/utils/decorators"; +import I18n from "discourse-i18n"; +import DTooltip from "float-kit/components/d-tooltip"; -const DashboardNewFeatureItem = +} diff --git a/app/assets/stylesheets/common/admin/dashboard.scss b/app/assets/stylesheets/common/admin/dashboard.scss index db364db778c..b80236a39da 100644 --- a/app/assets/stylesheets/common/admin/dashboard.scss +++ b/app/assets/stylesheets/common/admin/dashboard.scss @@ -659,6 +659,24 @@ } } +.admin-new-feature-item__body { + display: flex; + justify-content: space-between; + .d-toggle-switch { + margin-left: 1em; + align-items: flex-start; + } + p { + margin-top: 0; + } +} +.admin-new-feature-item__tooltip-header { + font-weight: bold; +} +.admin-new-feature-item__tooltip-content { + margin-top: 0.5em; +} + .admin-new-feature-item { display: flex; align-items: flex-start; diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 0bbeabd950c..1ce362d2498 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -48,6 +48,18 @@ class Admin::DashboardController < Admin::StaffController render json: data end + def toggle_feature + Experiments::Toggle.call(service_params) do + on_success { render(json: success_json) } + on_failure { render(json: failed_json, status: 422) } + on_failed_policy(:current_user_is_admin) { raise Discourse::InvalidAccess } + on_failed_policy(:setting_is_available) { raise Discourse::InvalidAccess } + on_failed_contract do |contract| + render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400) + end + end + end + private def mark_new_features_as_seen diff --git a/app/services/experiments/toggle.rb b/app/services/experiments/toggle.rb new file mode 100644 index 00000000000..3746b82f53a --- /dev/null +++ b/app/services/experiments/toggle.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class Experiments::Toggle + include Service::Base + + policy :current_user_is_admin + + contract do + attribute :setting_name, :string + validates :setting_name, presence: true + end + + policy :setting_is_available + + transaction { step :toggle } + + private + + def current_user_is_admin(guardian:) + guardian.is_admin? + end + + def setting_is_available(contract:) + SiteSetting.respond_to?(contract.setting_name) + end + + def toggle(contract:, guardian:) + SiteSetting.set_and_log( + contract.setting_name, + !SiteSetting.send(contract.setting_name), + guardian.user, + ) + end +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 28dc31fdea8..aa81ddd6b25 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5104,6 +5104,13 @@ en: subtitle: "We are releasing new features and improvements all the time. This page covers the highlights, but you can click 'Learn more' to see extensive release notes." previous_announcements: "You can see previous new feature announcements on Discourse Meta" learn_more: "Learn more..." + experiment_enabled: "You have enabled the experimental feature." + experiment_disabled: "You have disabled the experimental feature." + experiment_toggled_too_fast: "You have toggled the experimental feature too fast. Please wait a few seconds before trying again." + experiment_tooltip: + title: "Try our experimental feature" + content: "Give our newest feature in development a spin! It's still in the experimental stage, so we might remove it at any time. You can opt-out whenever you like." + last_checked: "Last checked" refresh_problems: "Refresh" no_problems: "No problems were found." diff --git a/config/routes.rb b/config/routes.rb index 48902bd5095..937d9b4e152 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -326,6 +326,7 @@ Discourse::Application.routes.draw do get "dashboard/reports" => "dashboard#reports" get "dashboard/whats-new" => "dashboard#new_features" get "/whats-new" => "dashboard#new_features" + post "/toggle-feature" => "dashboard#toggle_feature" resources :dashboard, only: [:index] do collection { get "problems" } diff --git a/lib/discourse_updates.rb b/lib/discourse_updates.rb index 6fdb2884403..c3e0fbf265e 100644 --- a/lib/discourse_updates.rb +++ b/lib/discourse_updates.rb @@ -150,10 +150,24 @@ module DiscourseUpdates end return nil if entries.nil? + 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 + item + end + entries.select! do |item| begin - item["discourse_version"].nil? || - Discourse.has_needed_version?(current_version, item["discourse_version"]) + valid_version = + item["discourse_version"].nil? || + Discourse.has_needed_version?(current_version, item["discourse_version"]) + + valid_plugin_name = + item["plugin_name"].nil? || Discourse.plugins_by_name[item["plugin_name"]].present? + + valid_version && valid_plugin_name rescue StandardError nil end diff --git a/spec/lib/discourse_updates_spec.rb b/spec/lib/discourse_updates_spec.rb index dbe1e0d2c62..162ab900f3c 100644 --- a/spec/lib/discourse_updates_spec.rb +++ b/spec/lib/discourse_updates_spec.rb @@ -248,6 +248,64 @@ RSpec.describe DiscourseUpdates do expect(result[1]["title"]).to eq("Whistles") expect(result[2]["title"]).to eq("Bells") end + + it "correctly shows features with correct boolean experimental site settings" do + features_with_versions = [ + { + "emoji" => "🤾", + "title" => "Bells", + "created_at" => 2.days.ago, + "experiment_setting" => "enable_mobile_theme", + }, + { + "emoji" => "🙈", + "title" => "Whistles", + "created_at" => 3.days.ago, + "experiment_setting" => "default_theme_id", + }, + { + "emoji" => "🙈", + "title" => "Confetti", + "created_at" => 4.days.ago, + "experiment_setting" => "wrong value", + }, + ] + + Discourse.redis.set("new_features", MultiJson.dump(features_with_versions)) + DiscourseUpdates.last_installed_version = "2.7.0.beta2" + result = DiscourseUpdates.new_features + + expect(result.length).to eq(3) + expect(result[0]["experiment_setting"]).to eq("enable_mobile_theme") + expect(result[1]["experiment_setting"]).to be_nil + expect(result[2]["experiment_setting"]).to be_nil + end + + it "correctly shows features when related plugins are installed" do + Discourse.stubs(:plugins_by_name).returns({ "discourse-ai" => true }) + + features_with_versions = [ + { + "emoji" => "🤾", + "title" => "Bells", + "created_at" => 2.days.ago, + "plugin_name" => "discourse-ai", + }, + { + "emoji" => "🙈", + "title" => "Confetti", + "created_at" => 4.days.ago, + "plugin_name" => "uninstalled-plugin", + }, + ] + + Discourse.redis.set("new_features", MultiJson.dump(features_with_versions)) + DiscourseUpdates.last_installed_version = "2.7.0.beta2" + result = DiscourseUpdates.new_features + + expect(result.length).to eq(1) + expect(result[0]["title"]).to eq("Bells") + end end describe "#get_last_viewed_feature_date" do diff --git a/spec/multisite/jobs_spec.rb b/spec/multisite/jobs_spec.rb index d398f754331..b5afcd09402 100644 --- a/spec/multisite/jobs_spec.rb +++ b/spec/multisite/jobs_spec.rb @@ -9,7 +9,7 @@ RSpec.describe "Running Sidekiq Jobs in Multisite", type: :multisite do it "CheckNewFeatures should only hit the payload once" do # otherwise it will get rate-limited by meta - DiscourseUpdates.expects(:new_features_payload).returns("{}").once + DiscourseUpdates.expects(:new_features_payload).returns([]).once Jobs::CheckNewFeatures.new.perform({}) end end diff --git a/spec/services/experiments/toggle_spec.rb b/spec/services/experiments/toggle_spec.rb new file mode 100644 index 00000000000..d7035383f2e --- /dev/null +++ b/spec/services/experiments/toggle_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +RSpec.describe Experiments::Toggle do + subject(:result) { described_class.call(params) } + + describe described_class::Contract, type: :model do + subject(:contract) { described_class.new } + + it { is_expected.to validate_presence_of :setting_name } + end + + fab!(:admin) + let(:params) { { setting_name:, guardian: } } + let(:setting_name) { :experimental_form_templates } + let(:guardian) { admin.guardian } + + context "when setting_name is blank" do + let(:setting_name) { nil } + + it { is_expected.to fail_a_contract } + end + + context "when setting_name is invalid" do + let(:setting_name) { "wrong_value" } + + it { is_expected.to fail_a_policy(:setting_is_available) } + end + + context "when a non-admin user tries to change a setting" do + let(:guardian) { Guardian.new } + + it { is_expected.to fail_a_policy(:current_user_is_admin) } + end + + context "when the admin toggles the feature" do + it { is_expected.to run_successfully } + + it "enables the specified setting" do + expect { result }.to change { SiteSetting.experimental_form_templates }.to(true) + end + + it "disables the specified setting" do + SiteSetting.experimental_form_templates = true + expect { result }.to change { SiteSetting.experimental_form_templates }.to(false) + end + + it "creates an entry in the staff action logs" do + expect { result }.to change { + UserHistory.where( + action: UserHistory.actions[:change_site_setting], + subject: "experimental_form_templates", + ).count + }.by(1) + end + end +end diff --git a/spec/system/admin_dashboard_new_features_spec.rb b/spec/system/admin_dashboard_new_features_spec.rb index 555fa5c2363..ff0ba2bd1cb 100644 --- a/spec/system/admin_dashboard_new_features_spec.rb +++ b/spec/system/admin_dashboard_new_features_spec.rb @@ -90,6 +90,28 @@ describe "Admin New Features Page", type: :system do expect(new_features_page).to have_no_screenshot end + it "displays experimental feature toggle" do + DiscourseUpdates.stubs(:new_features).returns( + [ + { + "id" => 7, + "user_id" => 1, + "emoji" => "😍", + "title" => "New feature", + "description" => "New feature description", + "link" => "https://meta.discourse.org", + "tier" => [], + "discourse_version" => "", + "created_at" => "2023-11-10T02:52:41.462Z", + "updated_at" => "2023-11-10T04:28:47.020Z", + "experiment_setting" => "experimental_form_templates", + }, + ], + ) + new_features_page.visit + expect(new_features_page).to have_toggle_experiment_button + end + it "displays a new feature indicator on the sidebar and clears it when navigating to what's new" do DiscourseUpdates.stubs(:has_unseen_features?).returns(true) visit "/admin" diff --git a/spec/system/page_objects/pages/admin_new_features.rb b/spec/system/page_objects/pages/admin_new_features.rb index dfa9600e6d3..bad21317a89 100644 --- a/spec/system/page_objects/pages/admin_new_features.rb +++ b/spec/system/page_objects/pages/admin_new_features.rb @@ -16,6 +16,10 @@ 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") + end + def has_learn_more_link? page.has_css?(".admin-new-feature-item__learn-more") end