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 <kotlarek.krzysztof@gmail.com>

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;

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);

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
+    );
+  }
   <template>
     <DPageHeader
       @titleLabel={{i18n "admin.config.flags.title"}}
@@ -23,7 +39,7 @@ export default RouteTemplate(
           @route="adminConfig.flags.new"
           @title="admin.config_areas.flags.add"
           @label="admin.config_areas.flags.add"
-          @disabled={{@controller.addFlagButtonDisabled}}
+          @disabled={{this.addFlagButtonDisabled}}
           class="admin-flags__header-add-flag"
         />
       </:actions>
@@ -45,4 +61,6 @@ export default RouteTemplate(
       {{outlet}}
     </div>
   </template>
-);
+}
+
+export default RouteTemplate(FlagsTemplate);

app/assets/javascripts/discourse/app/components/d-page-action-button.gjs

@@ -12,6 +12,7 @@ export const DPageActionButton = <template>
     @title={{@title}}
     @icon={{@icon}}
     @isLoading={{@isLoading}}
+    @disabled={{@disabled}}
   />
 </template>;
 
@@ -36,6 +37,7 @@ export const PrimaryButton = <template>
     @label={{@label}}
     @title={{@title}}
     @isLoading={{@isLoading}}
+    @disabled={{@disabled}}
   />
 </template>;
 

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(
+      <template>
+        <DPageHeader>
+          <:actions as |actions|>
+            <actions.Primary
+              @route="adminBadges.show"
+              @routeModels="new"
+              @icon="plus"
+              @label="admin.badges.new"
+              @disabled={{true}}
+              class="new-badge"
+            />
+          </:actions>
+        </DPageHeader>
+      </template>
+    );
+
+    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(

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] } }

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

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

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

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

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