FIX: Don't load all groups when rendering <GroupChooser /> (#31271)

In a few places throughout the app, when we render the `<GroupChooser
/>` component, we fetch the full groups list of the site from the
`/groups/search` endpoint. This is wasteful because the full groups list
is already included in the preloaded data that's sent to the client app
on the initial page load, so we can just use this preloaded list for
`<GroupChooser />` and we can avoid making an HTTP request.

Internal topic: t/147297.
This commit is contained in:
Osama Sayegh
2025-02-11 21:32:02 +03:00
committed by GitHub
parent 3e056b5127
commit 4db3389f3d
7 changed files with 57 additions and 54 deletions

View File

@@ -1,7 +1,9 @@
import Group from "discourse/models/group"; import { service } from "@ember/service";
import DiscourseRoute from "discourse/routes/discourse"; import DiscourseRoute from "discourse/routes/discourse";
export default class AdminUserIndexRoute extends DiscourseRoute { export default class AdminUserIndexRoute extends DiscourseRoute {
@service site;
model() { model() {
return this.modelFor("adminUser"); return this.modelFor("adminUser");
} }
@@ -10,19 +12,10 @@ export default class AdminUserIndexRoute extends DiscourseRoute {
return this.currentModel.username; return this.currentModel.username;
} }
afterModel(model) {
if (this.currentUser.admin) {
return Group.findAll().then((groups) => {
this._availableGroups = groups.filterBy("automatic", false);
return model;
});
}
}
setupController(controller, model) { setupController(controller, model) {
controller.setProperties({ controller.setProperties({
originalPrimaryGroupId: model.primary_group_id, originalPrimaryGroupId: model.primary_group_id,
availableGroups: this._availableGroups, availableGroups: this.site.groups.filter((g) => !g.automatic),
customGroupIdsBuffer: model.customGroups.mapBy("id"), customGroupIdsBuffer: model.customGroups.mapBy("id"),
ssoExternalEmail: null, ssoExternalEmail: null,
ssoLastPayload: null, ssoLastPayload: null,

View File

@@ -1,15 +1,16 @@
import Component from "@ember/component"; import Component from "@ember/component";
import EmberObject, { action } from "@ember/object"; import EmberObject, { action } from "@ember/object";
import { alias, and, equal, readOnly } from "@ember/object/computed"; import { alias, and, equal, readOnly } from "@ember/object/computed";
import { service } from "@ember/service";
import { isEmpty } from "@ember/utils"; import { isEmpty } from "@ember/utils";
import { computedI18n } from "discourse/lib/computed"; import { computedI18n } from "discourse/lib/computed";
import discourseComputed from "discourse/lib/decorators"; import discourseComputed from "discourse/lib/decorators";
import { getNativeContact } from "discourse/lib/pwa-utils"; import { getNativeContact } from "discourse/lib/pwa-utils";
import { emailValid } from "discourse/lib/utilities"; import { emailValid } from "discourse/lib/utilities";
import Group from "discourse/models/group";
import { i18n } from "discourse-i18n"; import { i18n } from "discourse-i18n";
export default class InvitePanel extends Component { export default class InvitePanel extends Component {
@service site;
@readOnly("currentUser.staff") isStaff; @readOnly("currentUser.staff") isStaff;
@readOnly("currentUser.admin") isAdmin; @readOnly("currentUser.admin") isAdmin;
@alias("inviteModel.id") topicId; @alias("inviteModel.id") topicId;
@@ -307,9 +308,10 @@ export default class InvitePanel extends Component {
} }
setGroupOptions() { setGroupOptions() {
Group.findAll().then((groups) => { this.set(
this.set("allGroups", groups.filterBy("automatic", false)); "allGroups",
}); this.site.groups.filter((g) => !g.automatic)
);
} }
@action @action

View File

@@ -16,7 +16,6 @@ import { canNativeShare, nativeShare } from "discourse/lib/pwa-utils";
import { sanitize } from "discourse/lib/text"; import { sanitize } from "discourse/lib/text";
import { applyValueTransformer } from "discourse/lib/transformer"; import { applyValueTransformer } from "discourse/lib/transformer";
import { emailValid, hostnameValid } from "discourse/lib/utilities"; import { emailValid, hostnameValid } from "discourse/lib/utilities";
import Group from "discourse/models/group";
import Invite from "discourse/models/invite"; import Invite from "discourse/models/invite";
import I18n, { i18n } from "discourse-i18n"; import I18n, { i18n } from "discourse-i18n";
import { FORMAT as DATE_INPUT_FORMAT } from "select-kit/components/future-date-input-selector"; import { FORMAT as DATE_INPUT_FORMAT } from "select-kit/components/future-date-input-selector";
@@ -37,21 +36,13 @@ export default class CreateInvite extends Component {
@tracked flashClass = "info"; @tracked flashClass = "info";
@tracked topics = this.invite.topics ?? this.model.topics ?? []; @tracked topics = this.invite.topics ?? this.model.topics ?? [];
@tracked allGroups; allGroups = this.site.groups.filter((g) => !g.automatic);
model = this.args.model; model = this.args.model;
invite = this.model.invite ?? Invite.create(); invite = this.model.invite ?? Invite.create();
sendEmail = false; sendEmail = false;
formApi; formApi;
constructor() {
super(...arguments);
Group.findAll().then((groups) => {
this.allGroups = groups.filter((group) => !group.automatic);
});
}
get linkValidityMessageFormat() { get linkValidityMessageFormat() {
return I18n.messageFormat("user.invited.invite.link_validity_MF", { return I18n.messageFormat("user.invited.invite.link_validity_MF", {
user_count: this.defaultRedemptionsAllowed, user_count: this.defaultRedemptionsAllowed,

View File

@@ -10,34 +10,21 @@ let deleteAndBlock;
acceptance("Admin - User Index", function (needs) { acceptance("Admin - User Index", function (needs) {
needs.user(); needs.user();
needs.pretender((server, helper) => {
server.get("/groups/search.json", () => { needs.site({
return helper.response([ groups: [
{ {
id: 42, id: 42,
automatic: false, automatic: false,
name: "Macdonald", name: "Macdonald",
user_count: 0,
alias_level: 99,
visible: true,
automatic_membership_email_domains: "",
primary_group: false,
title: null,
grant_trust_level: null,
has_messages: false,
flair_url: null, flair_url: null,
flair_bg_color: null, flair_bg_color: null,
flair_color: null, flair_color: null,
bio_raw: null,
bio_cooked: null,
public_admission: false,
allow_membership_requests: true,
membership_request_template: "Please add me",
full_name: null,
}, },
]); ],
}); });
needs.pretender((server, helper) => {
server.put("/users/sam/preferences/username", () => { server.put("/users/sam/preferences/username", () => {
return helper.response({ id: 2, username: "new-sam" }); return helper.response({ id: 2, username: "new-sam" });
}); });

View File

@@ -97,7 +97,15 @@ class SiteSerializer < ApplicationSerializer
object object
.groups .groups
.order(:name) .order(:name)
.select(:id, :name, :flair_icon, :flair_upload_id, :flair_bg_color, :flair_color) .select(
:id,
:name,
:flair_icon,
:flair_upload_id,
:flair_bg_color,
:flair_color,
:automatic,
)
.map do |g| .map do |g|
{ {
id: g.id, id: g.id,
@@ -105,6 +113,7 @@ class SiteSerializer < ApplicationSerializer
flair_url: g.flair_url, flair_url: g.flair_url,
flair_bg_color: g.flair_bg_color, flair_bg_color: g.flair_bg_color,
flair_color: g.flair_color, flair_color: g.flair_color,
automatic: g.automatic,
} }
end end
.as_json .as_json

View File

@@ -288,6 +288,11 @@
"string", "string",
"null" "null"
] ]
},
"automatic": {
"type": [
"boolean"
]
} }
}, },
"required": [ "required": [
@@ -295,7 +300,8 @@
"name", "name",
"flair_url", "flair_url",
"flair_bg_color", "flair_bg_color",
"flair_color" "flair_color",
"automatic"
] ]
} }
}, },

View File

@@ -497,4 +497,19 @@ RSpec.describe SiteSerializer do
expect(site_json[:full_name_visible_in_signup]).to eq(false) expect(site_json[:full_name_visible_in_signup]).to eq(false)
end end
end end
describe "#groups" do
fab!(:group)
fab!(:admin)
it "serializes the automatic field of each group" do
serialized_groups =
described_class.new(Site.new(admin.guardian), scope: admin.guardian, root: false).as_json[
:groups
]
expect(serialized_groups.find { |g| g["name"] == "everyone" }["automatic"]).to eq(true)
expect(serialized_groups.find { |g| g["name"] == group.name }["automatic"]).to eq(false)
end
end
end end