From 5b57ed959ee1661a7398295bc1cafc57dc7f7b01 Mon Sep 17 00:00:00 2001 From: Shubham Padia Date: Mon, 8 Jul 2024 16:47:14 +0000 Subject: [PATCH] user_pill: Use group pills to add all users. Fixes #30690. Before this, we were adding all users to subscriber list without adding any pill. But, since we want pills to be the source of truth for adding the subscribers, we will just add the `role:everyone` pill from now on whenever `Add all users` is clicked. While we can just directly call `pill_widget.appendValue` in `stream_create_subscribers`, it's better to expose an API from `add_subscribers_pill` and use that. This lets us control how appending an item would work for subscriber pill in a single place. --- web/src/add_subscribers_pill.ts | 17 +++++++++++++++++ web/src/stream_create_subscribers.ts | 15 ++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/web/src/add_subscribers_pill.ts b/web/src/add_subscribers_pill.ts index ad690b23c8..b409b3688d 100644 --- a/web/src/add_subscribers_pill.ts +++ b/web/src/add_subscribers_pill.ts @@ -1,3 +1,4 @@ +import * as blueslip from "./blueslip"; import * as input_pill from "./input_pill"; import * as keydown_util from "./keydown_util"; import type {User} from "./people"; @@ -5,6 +6,7 @@ import * as pill_typeahead from "./pill_typeahead"; import * as stream_pill from "./stream_pill"; import type {CombinedPill, CombinedPillContainer, CombinedPillItem} from "./typeahead_helper"; import * as user_group_pill from "./user_group_pill"; +import * as user_groups from "./user_groups"; import * as user_pill from "./user_pill"; function create_item_from_text( @@ -132,6 +134,21 @@ export function create_without_add_button({ return pill_widget; } +export function append_user_group_from_name( + user_group_name: string, + pill_widget: CombinedPillContainer, +): void { + const user_group = user_groups.get_user_group_from_name(user_group_name); + if (user_group === undefined) { + // This shouldn't happen, but we'll give a warning for now if it + // does. + blueslip.error("User group with the given name does not exist."); + return; + } + + user_group_pill.append_user_group(user_group, pill_widget); +} + function get_pill_user_ids(pill_widget: CombinedPillContainer): number[] { const user_ids = user_pill.get_user_ids(pill_widget); const stream_user_ids = stream_pill.get_user_ids(pill_widget); diff --git a/web/src/stream_create_subscribers.ts b/web/src/stream_create_subscribers.ts index a9076ec9bf..7ed1f76e65 100644 --- a/web/src/stream_create_subscribers.ts +++ b/web/src/stream_create_subscribers.ts @@ -9,8 +9,10 @@ import type {ListWidget as ListWidgetType} from "./list_widget"; import * as people from "./people"; import {current_user} from "./state_data"; import * as stream_create_subscribers_data from "./stream_create_subscribers_data"; +import type {CombinedPillContainer} from "./typeahead_helper"; import * as user_sort from "./user_sort"; +let pill_widget: CombinedPillContainer; let all_users_list_widget: ListWidgetType; export function get_principals(): number[] { @@ -27,8 +29,7 @@ function add_user_ids(user_ids: number[]): void { } function add_all_users(): void { - const user_ids = stream_create_subscribers_data.get_all_user_ids(); - add_user_ids(user_ids); + add_subscribers_pill.append_user_group_from_name("role:everyone", pill_widget!); } function soft_remove_user_id(user_id: number): void { @@ -46,10 +47,14 @@ function sync_user_ids(user_ids: number[]): void { redraw_subscriber_list(); } -function build_pill_widget({$parent_container}: {$parent_container: JQuery}): void { +function build_pill_widget({ + $parent_container, +}: { + $parent_container: JQuery; +}): CombinedPillContainer { const $pill_container = $parent_container.find(".pill-container"); const get_potential_subscribers = stream_create_subscribers_data.get_potential_subscribers; - add_subscribers_pill.create_without_add_button({ + return add_subscribers_pill.create_without_add_button({ $pill_container, get_potential_subscribers, onPillCreateAction: add_user_ids, @@ -90,7 +95,7 @@ export function build_widgets(): void { const $simplebar_container = $add_people_container.find(".subscriber_list_container"); - build_pill_widget({$parent_container: $add_people_container}); + pill_widget = build_pill_widget({$parent_container: $add_people_container}); stream_create_subscribers_data.initialize_with_current_user(); const current_user_id = current_user.user_id;