From 2038c9c03f4fe16a13b3378ea3478142bc479a95 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 24 Apr 2025 15:47:22 +0800 Subject: [PATCH] DEV: Simplify "Admin Flags Page" system test to reduce runtime duration (#32431) This particular system test is taking a long time (~20 seconds) on CI because it is doing many full page loads. This commit refactors the test to be more efficient about the number of full page loads triggered by the tests thus reducing the runtime by half. Co-authored-by: Krzysztof Kotlarek --- .../components/admin-config-areas/flags.gjs | 9 - .../addon/components/admin-flags-form.gjs | 2 +- .../admin/addon/templates/config-flags.gjs | 24 ++- .../app/components/d-page-action-button.gjs | 2 + .../components/d-page-header-test.gjs | 24 +++ spec/fabricators/flag_fabricator.rb | 2 +- spec/services/flags/destroy_flag_spec.rb | 3 +- spec/system/admin_flags_spec.rb | 190 ++++++------------ spec/system/page_objects/modals/flag.rb | 2 +- .../page_objects/pages/admin_flag_form.rb | 7 +- spec/system/page_objects/pages/admin_flags.rb | 10 +- 11 files changed, 119 insertions(+), 156 deletions(-) diff --git a/app/assets/javascripts/admin/addon/components/admin-config-areas/flags.gjs b/app/assets/javascripts/admin/addon/components/admin-config-areas/flags.gjs index 926378f859d..d1e48d28bcc 100644 --- a/app/assets/javascripts/admin/addon/components/admin-config-areas/flags.gjs +++ b/app/assets/javascripts/admin/addon/components/admin-config-areas/flags.gjs @@ -7,7 +7,6 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; import { bind } from "discourse/lib/decorators"; import { i18n } from "discourse-i18n"; import AdminFlagItem from "admin/components/admin-flag-item"; -import { SYSTEM_FLAG_IDS } from "admin/lib/constants"; export default class AdminConfigAreasFlags extends Component { @service site; @@ -15,14 +14,6 @@ export default class AdminConfigAreasFlags extends Component { @tracked flags = this.site.flagTypes; - get addFlagButtonDisabled() { - return ( - this.flags.filter( - (flag) => !Object.values(SYSTEM_FLAG_IDS).includes(flag.id) - ).length >= this.siteSettings.custom_flags_limit - ); - } - @bind isFirstFlag(flag) { return this.flags.indexOf(flag) === 1; diff --git a/app/assets/javascripts/admin/addon/components/admin-flags-form.gjs b/app/assets/javascripts/admin/addon/components/admin-flags-form.gjs index e445c30781c..300a1db7e93 100644 --- a/app/assets/javascripts/admin/addon/components/admin-flags-form.gjs +++ b/app/assets/javascripts/admin/addon/components/admin-flags-form.gjs @@ -96,7 +96,7 @@ export default class AdminFlagsForm extends Component { type: "POST", data, }); - this.site.flagTypes.push(response.flag); + this.site.flagTypes.pushObject(response.flag); this.router.transitionTo("adminConfig.flags"); } catch (error) { popupAjaxError(error); diff --git a/app/assets/javascripts/admin/addon/templates/config-flags.gjs b/app/assets/javascripts/admin/addon/templates/config-flags.gjs index e7daacdaf0a..2fc8c758afc 100644 --- a/app/assets/javascripts/admin/addon/templates/config-flags.gjs +++ b/app/assets/javascripts/admin/addon/templates/config-flags.gjs @@ -3,8 +3,24 @@ import DBreadcrumbsItem from "discourse/components/d-breadcrumbs-item"; import DPageHeader from "discourse/components/d-page-header"; import NavItem from "discourse/components/nav-item"; import { i18n } from "discourse-i18n"; +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { service } from "@ember/service"; +import { SYSTEM_FLAG_IDS } from "admin/lib/constants"; -export default RouteTemplate( +class FlagsTemplate extends Component { + @service site; + @service siteSettings; + + @tracked flags = this.site.flagTypes; + + get addFlagButtonDisabled() { + return ( + this.flags.filter( + (flag) => !Object.values(SYSTEM_FLAG_IDS).includes(flag.id) + ).length >= this.siteSettings.custom_flags_limit + ); + } -); +} + +export default RouteTemplate(FlagsTemplate); diff --git a/app/assets/javascripts/discourse/app/components/d-page-action-button.gjs b/app/assets/javascripts/discourse/app/components/d-page-action-button.gjs index fbc2e7eb404..154952e22c5 100644 --- a/app/assets/javascripts/discourse/app/components/d-page-action-button.gjs +++ b/app/assets/javascripts/discourse/app/components/d-page-action-button.gjs @@ -12,6 +12,7 @@ export const DPageActionButton = ; @@ -36,6 +37,7 @@ export const PrimaryButton = ; diff --git a/app/assets/javascripts/discourse/tests/integration/components/d-page-header-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/d-page-header-test.gjs index e898199f63b..2818174a319 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/d-page-header-test.gjs +++ b/app/assets/javascripts/discourse/tests/integration/components/d-page-header-test.gjs @@ -180,6 +180,30 @@ module("Integration | Component | DPageHeader", function (hooks) { await click(".edit-groupings-btn"); assert.true(actionCalled); }); + test("renders disabled action buttons in yielded <:actions>", async function (assert) { + await render( + + ); + + assert + .dom( + ".d-page-header__actions .d-page-action-button.new-badge.btn.btn-small.btn-primary" + ) + .hasAttribute("disabled"); + }); test("@headerActionComponent is rendered with actions arg", async function (assert) { await render( diff --git a/spec/fabricators/flag_fabricator.rb b/spec/fabricators/flag_fabricator.rb index 792da20f55f..6e2eece65fc 100644 --- a/spec/fabricators/flag_fabricator.rb +++ b/spec/fabricators/flag_fabricator.rb @@ -1,3 +1,3 @@ # frozen_string_literal: true -Fabricator(:flag) { name "offtopic", applies_to { %w[Post Chat::Message] } } +Fabricator(:flag) { name "offtopic", applies_to { %w[Post] } } diff --git a/spec/services/flags/destroy_flag_spec.rb b/spec/services/flags/destroy_flag_spec.rb index 2936eadfd00..bedfea69100 100644 --- a/spec/services/flags/destroy_flag_spec.rb +++ b/spec/services/flags/destroy_flag_spec.rb @@ -60,8 +60,7 @@ RSpec.describe(Flags::DestroyFlag) do expect { result }.to change { UserHistory.count }.by(1) expect(UserHistory.last).to have_attributes( custom_type: "delete_flag", - details: - "name: offtopic\ndescription: \napplies_to: [\"Post\", \"Chat::Message\"]\nenabled: true", + details: "name: offtopic\ndescription: \napplies_to: [\"Post\"]\nenabled: true", ) end end diff --git a/spec/system/admin_flags_spec.rb b/spec/system/admin_flags_spec.rb index 8d7f480b232..f9f402569fe 100644 --- a/spec/system/admin_flags_spec.rb +++ b/spec/system/admin_flags_spec.rb @@ -10,76 +10,54 @@ describe "Admin Flags Page", type: :system do let(:flag_modal) { PageObjects::Modals::Flag.new } let(:d_page_header) { PageObjects::Components::DPageHeader.new } - before do - sign_in(admin) - SiteSetting.custom_flags_limit = 1 - end + before { sign_in(admin) } it "allows admin to disable, change order, create, update and delete flags" do - # disable - topic_page.visit_topic(post.topic).open_flag_topic_modal + SiteSetting.custom_flags_limit = 4 - expect(flag_modal).to have_choices( - "It's Inappropriate", - "It's Spam", - "It's Illegal", - "Something Else", - ) + custom_flag = + Fabricate(:flag, name: "flag1", description: "custom flag 1", applies_to: %w[Post Topic]) + + custom_flag_2 = + Fabricate(:flag, name: "flag2", description: "custom flag 2", applies_to: %w[Post Topic]) + + custom_flag_3 = + Fabricate(:flag, name: "flag3", description: "custom flag 3", applies_to: %w[Post Topic]) admin_flags_page.visit expect(d_page_header).to be_visible + expect(admin_flags_page).to have_flags( + "Send @%{username} a message", + "Off-Topic", + "Inappropriate", + "Spam", + "Illegal", + "Something Else", + "flag1", + "flag2", + "flag3", + ) + + # disable flag admin_flags_page.toggle("spam") - topic_page.visit_topic(post.topic).open_flag_topic_modal - - expect(flag_modal).to have_choices("It's Inappropriate", "It's Illegal", "Something Else") - - Flag.system.where(name: "spam").update!(enabled: true) # change order - topic_page.visit_topic(post.topic).open_flag_topic_modal + admin_flags_page.move_down("illegal").move_up("custom_flag1") - expect(flag_modal).to have_choices( - "It's Inappropriate", - "It's Spam", - "It's Illegal", + expect(admin_flags_page).to have_flags( + "Send @%{username} a message", + "Off-Topic", + "Inappropriate", + "Spam", "Something Else", + "flag1", + "Illegal", + "flag2", + "flag3", ) - admin_flags_page.visit.move_down("spam") - topic_page.visit_topic(post.topic).open_flag_topic_modal - - expect(flag_modal).to have_choices( - "It's Inappropriate", - "It's Illegal", - "It's Spam", - "Something Else", - ) - - admin_flags_page.visit.move_up("spam") - topic_page.visit_topic(post.topic).open_flag_topic_modal - - expect(flag_modal).to have_choices( - "It's Inappropriate", - "It's Spam", - "It's Illegal", - "Something Else", - ) - - # create - topic_page.visit_topic(post.topic).open_flag_topic_modal - - expect(flag_modal).to have_choices( - "It's Inappropriate", - "It's Spam", - "It's Illegal", - "Something Else", - ) - - admin_flags_page.visit - - expect(admin_flags_page).to have_add_flag_button_enabled - + # create flag admin_flags_page.click_add_flag expect(d_page_header).to be_hidden @@ -88,76 +66,52 @@ describe "Admin Flags Page", type: :system do ) admin_flag_form_page - .fill_in_name("Vulgar") - .fill_in_description("New flag description") + .fill_in_name("flag4") + .fill_in_description("custom flag 4") .select_applies_to("Topic") .select_applies_to("Post") .click_save + expect(admin_flags_page).to have_add_flag_button_disabled + expect(admin_flags_page).to have_flags( "Send @%{username} a message", "Off-Topic", "Inappropriate", "Spam", + "Something Else", + "flag1", "Illegal", - "Something Else", - "Vulgar", + "flag2", + "flag3", + "flag4", ) - expect(admin_flags_page).to have_add_flag_button_disabled + # update flag + admin_flags_page.visit.click_edit_flag("custom_flag1") - topic_page.visit_topic(post.topic).open_flag_topic_modal - - expect(flag_modal).to have_choices( - "It's Inappropriate", - "It's Spam", - "It's Illegal", - "Something Else", - "Vulgar", - ) - - # update - admin_flags_page.visit.click_edit_flag("custom_vulgar") expect(d_page_header).to be_hidden + expect(admin_flag_form_page).to have_text( I18n.t("admin_js.admin.config_areas.flags.form.edit_warning"), ) - admin_flag_form_page.fill_in_name("Tasteless").click_save - expect(admin_flags_page).to have_flags( - "Send @%{username} a message", - "Off-Topic", - "Inappropriate", - "Spam", - "Illegal", - "Something Else", - "Tasteless", - ) + admin_flag_form_page.fill_in_name("flag edited").click_save + + # delete flag + admin_flags_page.visit.click_delete_flag("custom_flag3").confirm_delete + + expect(admin_flags_page).to have_no_flag("custom_flag3") topic_page.visit_topic(post.topic).open_flag_topic_modal expect(flag_modal).to have_choices( "It's Inappropriate", - "It's Spam", - "It's Illegal", "Something Else", - "Tasteless", - ) - - # delete - admin_flags_page.visit.click_delete_flag("custom_tasteless").confirm_delete - - expect(admin_flags_page).to have_no_flag("custom_tasteless") - - expect(admin_flags_page).to have_add_flag_button_enabled - - topic_page.visit_topic(post.topic).open_flag_topic_modal - - expect(flag_modal).to have_choices( - "It's Inappropriate", - "It's Spam", + "flag edited", "It's Illegal", - "Something Else", + "flag2", + "flag4", ) end @@ -206,42 +160,20 @@ describe "Admin Flags Page", type: :system do "Something Else", "Inappropriate", ) - - topic_page.visit_topic(post.topic).open_flag_topic_modal - - expect(flag_modal).to have_choices( - "It's Inappropriate", - "It's Spam", - "It's Illegal", - "Something Else", - "Inappropriate", - ) - - admin_flags_page.visit.click_delete_flag("custom_inappropriate").confirm_delete end - it "does not allow to move notify user flag" do + it "restricts actions on certain flags" do admin_flags_page.visit + expect(admin_flags_page).to have_no_action_for_flag("notify_user") - end - - it "does not allow bottom flag to move down" do - admin_flags_page.visit.open_flag_menu("notify_moderators") - expect(admin_flags_page).to have_no_item_action("move-down") - end - - it "does not allow to system flag to be edited" do - admin_flags_page.visit expect(admin_flags_page).to have_disabled_edit_for_flag("off_topic") - end - it "does not allow to system flag to be deleted" do - admin_flags_page.visit.open_flag_menu("notify_moderators") + admin_flags_page.toggle_flag_menu("notify_moderators") + expect(admin_flags_page).to have_no_item_action("move-down") expect(admin_flags_page).to have_disabled_item_action("delete") - end + admin_flags_page.toggle_flag_menu("notify_moderators") - it "does not allow top flag to move up" do - admin_flags_page.visit.open_flag_menu("off_topic") + admin_flags_page.visit.toggle_flag_menu("off_topic") expect(admin_flags_page).to have_no_item_action("move-up") end end diff --git a/spec/system/page_objects/modals/flag.rb b/spec/system/page_objects/modals/flag.rb index da901894a1f..983b240fcde 100644 --- a/spec/system/page_objects/modals/flag.rb +++ b/spec/system/page_objects/modals/flag.rb @@ -30,7 +30,7 @@ module PageObjects end def has_choices?(*choices) - body.all(".flag-action-type-details strong").map(&:text) == choices + expect(body.all(".flag-action-type-details strong").map(&:text)).to eq(choices) end end end diff --git a/spec/system/page_objects/pages/admin_flag_form.rb b/spec/system/page_objects/pages/admin_flag_form.rb index bd24cd2ef80..f7b5c59f620 100644 --- a/spec/system/page_objects/pages/admin_flag_form.rb +++ b/spec/system/page_objects/pages/admin_flag_form.rb @@ -23,11 +23,8 @@ module PageObjects def click_save form.submit - expect(page).to have_no_css( - ".admin-config.flags.new", - wait: Capybara.default_max_wait_time * 3, - ) - expect(page).to have_css(".admin-flag-item__name", wait: Capybara.default_max_wait_time * 3) + expect(page).to have_no_css(".admin-config.flags.new") + expect(page).to have_css(".admin-flag-item__name") end def form diff --git a/spec/system/page_objects/pages/admin_flags.rb b/spec/system/page_objects/pages/admin_flags.rb index fc4a6787c0f..b89b499daa1 100644 --- a/spec/system/page_objects/pages/admin_flags.rb +++ b/spec/system/page_objects/pages/admin_flags.rb @@ -14,7 +14,7 @@ module PageObjects self end - def open_flag_menu(key) + def toggle_flag_menu(key) find(".#{key} .flag-menu-trigger").click self end @@ -44,7 +44,7 @@ module PageObjects end def has_flags?(*flags) - all(".admin-flag-item__name").map(&:text) == flags + expect(all(".admin-flag-item__name").map(&:text)).to eq(flags) end def has_add_flag_button_enabled? @@ -52,7 +52,7 @@ module PageObjects end def has_add_flag_button_disabled? - has_no_css?(".admin-flags__header-add-flag[disabled]") + has_css?(".admin-flags__header-add-flag[disabled]") end def has_flag?(flag) @@ -72,14 +72,14 @@ module PageObjects end def move_down(key) - open_flag_menu(key) + toggle_flag_menu(key) find(".admin-flag-item__move-down").click has_closed_flag_menu? self end def move_up(key) - open_flag_menu(key) + toggle_flag_menu(key) find(".admin-flag-item__move-up").click has_closed_flag_menu? self