diff --git a/help/import-from-mattermost.md b/help/import-from-mattermost.md index 2d02f3087b..d7a32d9493 100644 --- a/help/import-from-mattermost.md +++ b/help/import-from-mattermost.md @@ -187,9 +187,11 @@ Replace `` and `` with the appropriate values below. {!import-self-hosted-server-tips.md!} ``` + cd /tmp + tar -xf mattermost_data.tar.gz cd /home/zulip/deployments/current ./scripts/stop-server - ./manage.py convert_mattermost_data /tmp/mattermost_data.tar.gz --output /tmp/converted_mattermost_data + ./manage.py convert_mattermost_data /tmp/mattermost_data --output /tmp/converted_mattermost_data ./manage.py import '' /tmp/converted_mattermost_data/ ./scripts/start-server ``` @@ -197,9 +199,11 @@ Replace `` and `` with the appropriate values below. Alternatively, to import into a custom subdomain, run: ``` + cd /tmp + tar -xf mattermost_data.tar.gz cd /home/zulip/deployments/current ./scripts/stop-server - ./manage.py convert_mattermost_data /tmp/mattermost_data.tar.gz --output /tmp/converted_mattermost_data + ./manage.py convert_mattermost_data /tmp/mattermost_data --output /tmp/converted_mattermost_data ./manage.py import /tmp/converted_mattermost_data/ ./scripts/start-server ``` diff --git a/pyproject.toml b/pyproject.toml index a00bea9e53..920ad81d4a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -114,6 +114,7 @@ select = [ "EXE", # shebang "F", # flakes "FLY", # string formatting + "FURB", # refurbishing "G", # logging format "I", # import sorting "INT", # gettext diff --git a/scripts/log-search b/scripts/log-search index 2e52a8c4be..0bc488d29c 100755 --- a/scripts/log-search +++ b/scripts/log-search @@ -147,7 +147,7 @@ NGINX_LOG_LINE_RE = re.compile( (?P \S+ ) \s+ (?P \S+ ) """, - re.X, + re.VERBOSE, ) PYTHON_LOG_LINE_RE = re.compile( @@ -178,7 +178,7 @@ PYTHON_LOG_LINE_RE = re.compile( ) \s+ via \s+ (?P .* ) \) """, - re.X, + re.VERBOSE, ) diff --git a/tools/lib/provision_inner.py b/tools/lib/provision_inner.py index a4aa1d417f..09f513e25a 100755 --- a/tools/lib/provision_inner.py +++ b/tools/lib/provision_inner.py @@ -98,7 +98,7 @@ def setup_shell_profile(shell_profile: str) -> None: def write_command(command: str) -> None: if os.path.exists(shell_profile_path): with open(shell_profile_path) as shell_profile_file: - lines = [line.strip() for line in shell_profile_file.readlines()] + lines = [line.strip() for line in shell_profile_file] if command not in lines: with open(shell_profile_path, "a+") as shell_profile_file: shell_profile_file.writelines(command + "\n") diff --git a/tools/run-mypy b/tools/run-mypy index 88d4991867..2a90b69004 100755 --- a/tools/run-mypy +++ b/tools/run-mypy @@ -82,7 +82,7 @@ mypy_args += ["--", *python_files, *pyi_files] rc = subprocess.call([mypy_command, *mypy_args]) if rc != 0: - print("") + print() print("See https://zulip.readthedocs.io/en/latest/testing/mypy.html for debugging tips.") sys.exit(rc) diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 13f7160b52..ac058643d6 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -72,7 +72,7 @@ EXEMPT_FILES = make_set( "web/src/compose_closed_ui.ts", "web/src/compose_fade.ts", "web/src/compose_notifications.ts", - "web/src/compose_popovers.js", + "web/src/compose_popovers.ts", "web/src/compose_recipient.ts", "web/src/compose_reply.ts", "web/src/compose_send_menu_popover.js", diff --git a/tools/test-js-with-puppeteer b/tools/test-js-with-puppeteer index e81df9135b..0ac53d9485 100755 --- a/tools/test-js-with-puppeteer +++ b/tools/test-js-with-puppeteer @@ -140,7 +140,7 @@ or report and ask for help in chat.zulip.org""", file=sys.stderr, ) if os.environ.get("GITHUB_ACTIONS"): - print("", file=sys.stderr) + print(file=sys.stderr) print( """ Screenshots generated on failure are extremely helpful for understanding @@ -150,7 +150,7 @@ below to download and view the generated screenshots. """, file=sys.stderr, ) - print("", file=sys.stderr) + print(file=sys.stderr) else: print( "It's also worthy to see screenshots generated on failure stored under var/puppeteer/*.png" diff --git a/tools/total-contributions b/tools/total-contributions index 02a6f94a29..244748f1ae 100755 --- a/tools/total-contributions +++ b/tools/total-contributions @@ -1,10 +1,10 @@ #!/usr/bin/env python3 import argparse import os -import pathlib import subprocess import sys from collections import defaultdict +from pathlib import Path from typing import Dict, List bot_commits = 0 @@ -36,7 +36,7 @@ def retrieve_log(repo: str, revisions: str) -> List[str]: def find_path(repository: str) -> str: - return str(pathlib.Path().resolve().parents[0] / repository) + return str(Path.cwd().parent / repository) def process_repo( diff --git a/web/e2e-tests/message-basics.test.ts b/web/e2e-tests/message-basics.test.ts index 80655dc593..a98b2f4ba7 100644 --- a/web/e2e-tests/message-basics.test.ts +++ b/web/e2e-tests/message-basics.test.ts @@ -347,16 +347,16 @@ async function test_search_venice(page: Page): Promise { await page.waitForSelector(await get_stream_li(page, "Verona"), {visible: true}); await page.click("#streams_header .left-sidebar-title"); - await page.waitForSelector(".input-append.notdisplayed"); + await page.waitForSelector(".stream_search_section.notdisplayed"); } async function test_stream_search_filters_stream_list(page: Page): Promise { console.log("Filter streams using left side bar"); - await page.waitForSelector(".input-append.notdisplayed"); // Stream filter box invisible initially + await page.waitForSelector(".stream_search_section.notdisplayed"); // Stream filter box invisible initially await page.click("#streams_header .left-sidebar-title"); - await page.waitForSelector("#streams_list .input-append.notdisplayed", {hidden: true}); + await page.waitForSelector("#streams_list .stream_search_section.notdisplayed", {hidden: true}); // assert streams exist by waiting till they're visible await page.waitForSelector(await get_stream_li(page, "Denmark"), {visible: true}); diff --git a/web/src/add_subscribers_pill.ts b/web/src/add_subscribers_pill.ts index 2691e37009..ad690b23c8 100644 --- a/web/src/add_subscribers_pill.ts +++ b/web/src/add_subscribers_pill.ts @@ -99,6 +99,39 @@ export function create({ return pill_widget; } +export function create_without_add_button({ + $pill_container, + get_potential_subscribers, + onPillCreateAction, + onPillRemoveAction, +}: { + $pill_container: JQuery; + get_potential_subscribers: () => User[]; + onPillCreateAction: (pill_user_ids: number[]) => void; + onPillRemoveAction: (pill_user_ids: number[]) => void; +}): CombinedPillContainer { + const pill_widget = input_pill.create({ + $container: $pill_container, + create_item_from_text, + get_text_from_item, + }); + function get_users(): User[] { + const potential_subscribers = get_potential_subscribers(); + return user_pill.filter_taken_users(potential_subscribers, pill_widget); + } + + pill_widget.onPillCreate(() => { + onPillCreateAction(get_pill_user_ids(pill_widget)); + }); + pill_widget.onPillRemove(() => { + onPillRemoveAction(get_pill_user_ids(pill_widget)); + }); + + set_up_pill_typeahead({pill_widget, $pill_container, get_users}); + + return 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/compose_popovers.js b/web/src/compose_popovers.ts similarity index 91% rename from web/src/compose_popovers.js rename to web/src/compose_popovers.ts index 83080d1bf1..9d0c8c9351 100644 --- a/web/src/compose_popovers.js +++ b/web/src/compose_popovers.ts @@ -1,4 +1,5 @@ import $ from "jquery"; +import assert from "minimalistic-assert"; import * as tippy from "tippy.js"; import render_compose_control_buttons_popover from "../templates/popovers/compose_control_buttons/compose_control_buttons_popover.hbs"; @@ -6,13 +7,12 @@ import render_mobile_message_buttons_popover from "../templates/popovers/mobile_ import * as compose_actions from "./compose_actions"; import * as giphy_state from "./giphy_state"; -import * as narrow_state from "./narrow_state"; import * as popover_menus from "./popover_menus"; import * as popovers from "./popovers"; import * as rows from "./rows"; import {parse_html} from "./ui_util"; -export function initialize() { +export function initialize(): void { // compose box buttons popover shown on mobile widths. // We want this click event to propagate and hide other popovers // that could possibly obstruct user from using this popover. @@ -25,16 +25,11 @@ export function initialize() { // actions target: ".mobile_button_container", placement: "top", + theme: "popover-menu", onShow(instance) { popover_menus.popover_instances.compose_mobile_button = instance; popover_menus.on_show_prep(instance); - instance.setContent( - parse_html( - render_mobile_message_buttons_popover({ - is_in_private_narrow: narrow_state.narrowed_to_pms(), - }), - ), - ); + instance.setContent(parse_html(render_mobile_message_buttons_popover())); }, onMount(instance) { const $popper = $(instance.popper); @@ -58,7 +53,7 @@ export function initialize() { // Destroy instance so that event handlers // are destroyed too. instance.destroy(); - popover_menus.popover_instances.compose_mobile_button = undefined; + popover_menus.popover_instances.compose_mobile_button = null; }, }); @@ -68,6 +63,7 @@ export function initialize() { popover_menus.register_popover_menu(".compose_control_menu_wrapper", { placement: "top", onShow(instance) { + assert(instance.reference instanceof HTMLElement); const parent_row = rows.get_closest_row(instance.reference); let preview_mode_on; // If the popover is opened from a message edit form, we want to @@ -91,7 +87,7 @@ export function initialize() { }, onHidden(instance) { instance.destroy(); - popover_menus.popover_instances.compose_control_buttons = undefined; + popover_menus.popover_instances.compose_control_buttons = null; }, }); } diff --git a/web/src/emoji_picker.js b/web/src/emoji_picker.js index da684e195f..386d149f84 100644 --- a/web/src/emoji_picker.js +++ b/web/src/emoji_picker.js @@ -239,7 +239,7 @@ export function is_emoji_present_in_text(text, emoji_dict) { } function filter_emojis() { - const $elt = $(".emoji-popover-filter").expectOne(); + const $elt = $("#emoji-popover-filter").expectOne(); const query = $elt.val().trim().toLowerCase(); const message_id = Number($(".emoji-search-results-container").attr("data-message-id")); const search_results_visible = $(".emoji-search-results-container").is(":visible"); @@ -423,7 +423,7 @@ function get_next_emoji_coordinates(move_by) { function change_focus_to_filter() { const $popover = $(emoji_popover_instance.popper); - $popover.find(".emoji-popover-filter").trigger("focus"); + $popover.find("#emoji-popover-filter").trigger("focus"); // If search is active reset current selected emoji to first emoji. if (search_is_active) { current_section = 0; @@ -457,12 +457,12 @@ export function navigate(event_name, e) { const $emoji_map = $popover.find(".emoji-popover-emoji-map").expectOne(); const $selected_emoji = get_rendered_emoji(current_section, current_index); - const is_filter_focused = $(".emoji-popover-filter").is(":focus"); + const is_filter_focused = $("#emoji-popover-filter").is(":focus"); // special cases if (is_filter_focused) { // Move down into emoji map. - const filter_text = $(".emoji-popover-filter").val(); - const is_cursor_at_end = $(".emoji-popover-filter").caret() === filter_text.length; + const filter_text = $("#emoji-popover-filter").val(); + const is_cursor_at_end = $("#emoji-popover-filter").caret() === filter_text.length; if (event_name === "down_arrow" || (is_cursor_at_end && event_name === "right_arrow")) { $selected_emoji.trigger("focus"); if (current_section === 0 && current_index < 6) { @@ -489,7 +489,7 @@ export function navigate(event_name, e) { // goes to beginning) with something reasonable and // consistent (cursor goes to the end of the filter // string). - $(".emoji-popover-filter").trigger("focus").caret(Number.POSITIVE_INFINITY); + $("#emoji-popover-filter").trigger("focus").caret(Number.POSITIVE_INFINITY); scroll_util.get_scroll_element($emoji_map).scrollTop(0); scroll_util.get_scroll_element($(".emoji-search-results-container")).scrollTop(0); current_section = 0; @@ -526,7 +526,7 @@ export function navigate(event_name, e) { } function process_keypress(e) { - const is_filter_focused = $(".emoji-popover-filter").is(":focus"); + const is_filter_focused = $("#emoji-popover-filter").is(":focus"); const pressed_key = e.which; if ( !is_filter_focused && @@ -538,7 +538,7 @@ function process_keypress(e) { e.preventDefault(); e.stopPropagation(); - const $emoji_filter = $(".emoji-popover-filter"); + const $emoji_filter = $("#emoji-popover-filter"); const old_query = $emoji_filter.val(); let new_query = ""; @@ -599,8 +599,8 @@ function register_popover_events($popover) { emoji_select_tab(scroll_util.get_scroll_element($emoji_map)); }); - $(".emoji-popover-filter").on("input", filter_emojis); - $(".emoji-popover-filter").on("keydown", process_enter_while_filtering); + $("#emoji-popover-filter").on("input", filter_emojis); + $("#emoji-popover-filter").on("keydown", process_enter_while_filtering); $(".emoji-popover").on("keypress", process_keypress); $(".emoji-popover").on("keydown", (e) => { // Because of cross-browser issues we need to handle Backspace @@ -615,6 +615,7 @@ function register_popover_events($popover) { function get_default_emoji_popover_options() { return { + theme: "popover-menu", placement: "top", popperOptions: { modifiers: [ @@ -804,7 +805,7 @@ function register_click_handlers() { } }); - $("body").on("click", ".emoji-popover-filter", () => { + $("body").on("click", "#emoji-popover-filter", () => { reset_emoji_showcase(); }); diff --git a/web/src/filter.ts b/web/src/filter.ts index 7b00503995..8fee434d7d 100644 --- a/web/src/filter.ts +++ b/web/src/filter.ts @@ -726,6 +726,45 @@ export class Filter { return true; } + static adjusted_terms_if_moved(raw_terms: NarrowTerm[], message: Message): NarrowTerm[] | null { + if (message.type !== "stream") { + return null; + } + + assert(typeof message.display_recipient === "string"); + assert(typeof message.topic === "string"); + + const adjusted_terms = []; + let terms_changed = false; + + for (const term of raw_terms) { + const adjusted_term = {...term}; + if ( + Filter.canonicalize_operator(term.operator) === "channel" && + !util.lower_same(term.operand, message.display_recipient) + ) { + adjusted_term.operand = message.display_recipient; + terms_changed = true; + } + + if ( + Filter.canonicalize_operator(term.operator) === "topic" && + !util.lower_same(term.operand, message.topic) + ) { + adjusted_term.operand = message.topic; + terms_changed = true; + } + + adjusted_terms.push(adjusted_term); + } + + if (!terms_changed) { + return null; + } + + return adjusted_terms; + } + equals(filter: Filter, excluded_operators?: string[]): boolean { return _.isEqual( filter.sorted_terms(excluded_operators), diff --git a/web/src/giphy.js b/web/src/giphy.js index 4f141d857e..b71b4db2eb 100644 --- a/web/src/giphy.js +++ b/web/src/giphy.js @@ -155,6 +155,7 @@ function toggle_giphy_popover(target) { popover_menus.toggle_popover_menu( target, { + theme: "popover-menu", placement: "top", onCreate(instance) { instance.setContent(ui_util.parse_html(render_giphy_picker())); diff --git a/web/src/message_events.js b/web/src/message_events.js index 84b4ae26a9..c63000c6f2 100644 --- a/web/src/message_events.js +++ b/web/src/message_events.js @@ -288,6 +288,7 @@ export function update_messages(events) { } } // The event.message_ids received from the server are not in sorted order. + // Sorts in ascending order. event_messages.sort((a, b) => a.id - b.id); if ( @@ -307,6 +308,18 @@ export function update_messages(events) { drafts.rename_stream_recipient(old_stream_id, orig_topic, new_stream_id, new_topic); } + // Remove the stream_topic_entry for the old topics; + // must be called before we call set message topic. + const num_messages = event_messages.length; + if (num_messages > 0) { + stream_topic_history.remove_messages({ + stream_id: old_stream_id, + topic_name: orig_topic, + num_messages, + max_removed_msg_id: event_messages[num_messages - 1].id, + }); + } + for (const moved_message of event_messages) { if (realm.realm_allow_edit_history) { /* Simulate the format of server-generated edit @@ -335,23 +348,6 @@ export function update_messages(events) { } moved_message.last_edit_timestamp = event.edit_timestamp; - // Remove the Recent Conversations entry for the old topics; - // must be called before we call set_message_topic. - // - // TODO: Use a single bulk request to do this removal. - // Note that we need to be careful to only remove IDs - // that were present in stream_topic_history data. - // This may not be possible to do correctly without extra - // complexity; the present loop assumes stream_topic_history has - // only messages in message_store, but that's been false - // since we added the server_history feature. - stream_topic_history.remove_messages({ - stream_id: moved_message.stream_id, - topic_name: moved_message.topic, - num_messages: 1, - max_removed_msg_id: moved_message.id, - }); - // Update the unread counts; again, this must be called // before we modify the topic field on the message. unread.update_unread_topics(moved_message, event); diff --git a/web/src/message_view.js b/web/src/message_view.js index e5d3fad2b5..c20e27dedb 100644 --- a/web/src/message_view.js +++ b/web/src/message_view.js @@ -233,18 +233,24 @@ function try_rendering_locally_for_same_narrow(filter, opts) { return false; } - // If the difference between the current filter and the new filter - // is just a `near` operator, or just the value of a `near` operator, - // we can render the new filter without a rerender of the message list - // if the target message in the `near` operator is already rendered. - const excluded_operators = ["near"]; - if (!filter.equals(current_filter, excluded_operators)) { - return false; - } - if (filter.has_operator("near")) { const target_id = Number.parseInt(filter.operands("near")[0], 10); - if (!message_lists.current?.get(target_id)) { + const target_message = message_lists.current?.get(target_id); + if (!target_message) { + return false; + } + + const adjusted_terms = Filter.adjusted_terms_if_moved(filter.terms(), target_message); + if (adjusted_terms !== null) { + filter = new Filter(adjusted_terms); + } + + // If the difference between the current filter and the new filter + // is just a `near` operator, or just the value of a `near` operator, + // we can render the new filter without a rerender of the message list + // if the target message in the `near` operator is already rendered. + const excluded_operators = ["near"]; + if (!filter.equals(current_filter, excluded_operators)) { return false; } @@ -256,7 +262,7 @@ function try_rendering_locally_for_same_narrow(filter, opts) { } message_lists.current.data.filter = filter; - update_hash_to_match_filter(filter); + update_hash_to_match_filter(filter, "retarget message location"); return true; } @@ -402,38 +408,6 @@ export function show(raw_terms, opts) { if (id_info.target_id && filter.has_operator("channel") && filter.has_operator("topic")) { const target_message = message_store.get(id_info.target_id); - function adjusted_terms_if_moved(raw_terms, message) { - const adjusted_terms = []; - let terms_changed = false; - - for (const term of raw_terms) { - const adjusted_term = {...term}; - if ( - Filter.canonicalize_operator(term.operator) === "channel" && - !util.lower_same(term.operand, message.display_recipient) - ) { - adjusted_term.operand = message.display_recipient; - terms_changed = true; - } - - if ( - Filter.canonicalize_operator(term.operator) === "topic" && - !util.lower_same(term.operand, message.topic) - ) { - adjusted_term.operand = message.topic; - terms_changed = true; - } - - adjusted_terms.push(adjusted_term); - } - - if (!terms_changed) { - return null; - } - - return adjusted_terms; - } - if (target_message) { // If we have the target message ID for the narrow in our // local cache, and the target message has been moved from @@ -447,7 +421,10 @@ export function show(raw_terms, opts) { // The stream name is invalid or incorrect in the URL. // We reconstruct the narrow with the data from the // target message ID that we have. - const adjusted_terms = adjusted_terms_if_moved(raw_terms, target_message); + const adjusted_terms = Filter.adjusted_terms_if_moved( + raw_terms, + target_message, + ); if (adjusted_terms === null) { blueslip.error("adjusted_terms impossibly null"); @@ -485,7 +462,10 @@ export function show(raw_terms, opts) { !narrow_matches_target_message && (narrow_exists_in_edit_history || !realm.realm_allow_edit_history) ) { - const adjusted_terms = adjusted_terms_if_moved(raw_terms, target_message); + const adjusted_terms = Filter.adjusted_terms_if_moved( + raw_terms, + target_message, + ); if (adjusted_terms !== null) { show(adjusted_terms, { ...opts, diff --git a/web/src/rows.ts b/web/src/rows.ts index 76508ebbd5..6358fbb2eb 100644 --- a/web/src/rows.ts +++ b/web/src/rows.ts @@ -119,14 +119,14 @@ export function get_message_id(elem: HTMLElement): number { return message_id; } -export function get_closest_group(element: string): JQuery { +export function get_closest_group(element: HTMLElement): JQuery { // This gets the closest message row to an element, whether it's // a recipient bar or message. With our current markup, // this is the most reliable way to do it. return $(element).closest("div.recipient_row"); } -export function get_closest_row(element: string): JQuery { +export function get_closest_row(element: HTMLElement): JQuery { return $(element).closest("div.message_row"); } diff --git a/web/src/stream_create_subscribers.ts b/web/src/stream_create_subscribers.ts index 25ec7af67c..a9076ec9bf 100644 --- a/web/src/stream_create_subscribers.ts +++ b/web/src/stream_create_subscribers.ts @@ -9,10 +9,8 @@ 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[] { @@ -33,16 +31,35 @@ function add_all_users(): void { add_user_ids(user_ids); } -function remove_user_ids(user_ids: number[]): void { - stream_create_subscribers_data.remove_user_ids(user_ids); +function soft_remove_user_id(user_id: number): void { + stream_create_subscribers_data.soft_remove_user_id(user_id); + redraw_subscriber_list(); +} + +function undo_soft_remove_user_id(user_id: number): void { + stream_create_subscribers_data.undo_soft_remove_user_id(user_id); + redraw_subscriber_list(); +} + +function sync_user_ids(user_ids: number[]): void { + stream_create_subscribers_data.sync_user_ids(user_ids); redraw_subscriber_list(); } function build_pill_widget({$parent_container}: {$parent_container: JQuery}): void { const $pill_container = $parent_container.find(".pill-container"); const get_potential_subscribers = stream_create_subscribers_data.get_potential_subscribers; - - pill_widget = add_subscribers_pill.create({$pill_container, get_potential_subscribers}); + add_subscribers_pill.create_without_add_button({ + $pill_container, + get_potential_subscribers, + onPillCreateAction: add_user_ids, + // It is better to sync the current set of user ids in the input + // instead of removing user_ids from the user_ids_set, otherwise + // we'll have to have more complex logic of when to remove + // a user and when not to depending upon their group, channel + // and individual pills. + onPillRemoveAction: sync_user_ids, + }); } export function create_handlers($container: JQuery): void { @@ -56,24 +73,14 @@ export function create_handlers($container: JQuery): void { e.preventDefault(); const $elem = $(e.target); const user_id = Number.parseInt($elem.attr("data-user-id")!, 10); - remove_user_ids([user_id]); + soft_remove_user_id(user_id); }); - const button_selector = ".add_subscribers_container button.add-subscriber-button"; - function add_users({pill_user_ids}: {pill_user_ids: number[]}): void { - add_user_ids(pill_user_ids); - // eslint-disable-next-line unicorn/no-array-callback-reference - const $pill_widget_button = $container.find(button_selector); - $pill_widget_button.prop("disabled", true); - pill_widget.clear(); - } - - add_subscribers_pill.set_up_handlers({ - get_pill_widget: () => pill_widget, - $parent_container: $container, - pill_selector: ".add_subscribers_container .input", - button_selector, - action: add_users, + $container.on("click", ".undo_soft_removed_potential_subscriber", (e) => { + e.preventDefault(); + const $elem = $(e.target); + const user_id = Number.parseInt($elem.attr("data-user-id")!, 10); + undo_soft_remove_user_id(user_id); }); } @@ -100,6 +107,9 @@ export function build_widgets(): void { is_current_user: user.user_id === current_user_id, disabled: stream_create_subscribers_data.must_be_subscribed(user.user_id), img_src: people.small_avatar_url_for_person(user), + soft_removed: stream_create_subscribers_data.user_id_in_soft_remove_list( + user.user_id, + ), }; return render_new_stream_user(item); }, diff --git a/web/src/stream_create_subscribers_data.ts b/web/src/stream_create_subscribers_data.ts index a0371bc95c..be9a165ee0 100644 --- a/web/src/stream_create_subscribers_data.ts +++ b/web/src/stream_create_subscribers_data.ts @@ -1,11 +1,15 @@ +import _ from "lodash"; + import * as people from "./people"; import type {User} from "./people"; import {current_user} from "./state_data"; let user_id_set: Set; +let soft_remove_user_id_set: Set; export function initialize_with_current_user(): void { user_id_set = new Set([current_user.user_id]); + soft_remove_user_id_set = new Set(); } export function sorted_user_ids(): number[] { @@ -24,7 +28,7 @@ export function get_all_user_ids(): number[] { export function get_principals(): number[] { // Return list of user ids which were selected by user. - return [...user_id_set]; + return _.difference([...user_id_set], [...soft_remove_user_id_set]); } export function get_potential_subscribers(): User[] { @@ -42,6 +46,9 @@ export function add_user_ids(user_ids: number[]): void { const user = people.maybe_get_user_by_id(user_id); if (user) { user_id_set.add(user_id); + // Re-adding a user explicitly will not undo the soft remove on their row. + // e.g If `Iago` was added as part of a group and crossed out. + // Now, adding another group with Iago as part of it should not undo the soft remove. } } } @@ -50,5 +57,28 @@ export function add_user_ids(user_ids: number[]): void { export function remove_user_ids(user_ids: number[]): void { for (const user_id of user_ids) { user_id_set.delete(user_id); + undo_soft_remove_user_id(user_id); } } + +export function sync_user_ids(user_ids: number[]): void { + // Current user does not have their pill in their input + // box, so we need to make sure that we don't delete + // it unnecessarily while syncing. + if (user_id_set.has(current_user.user_id)) { + user_ids.push(current_user.user_id); + } + user_id_set = new Set(user_ids); +} + +export function soft_remove_user_id(user_id: number): void { + soft_remove_user_id_set.add(user_id); +} + +export function undo_soft_remove_user_id(user_id: number): void { + soft_remove_user_id_set.delete(user_id); +} + +export function user_id_in_soft_remove_list(user_id: number): boolean { + return soft_remove_user_id_set.has(user_id); +} diff --git a/web/src/stream_topic_history.ts b/web/src/stream_topic_history.ts index 328a8ee36d..dc3157b81b 100644 --- a/web/src/stream_topic_history.ts +++ b/web/src/stream_topic_history.ts @@ -185,18 +185,14 @@ export class PerStreamHistory { maybe_remove(topic_name: string, num_messages: number): void { const existing = this.topics.get(topic_name); - if (!existing || existing.count === 0) { + if (!existing) { return; } if (existing.count <= num_messages) { this.topics.delete(topic_name); - if (!is_complete_for_stream_id(this.stream_id)) { - // Request server for latest message in topic if we - // cannot be sure that we have all messages in the topic. - update_topic_last_message_id(this.stream_id, topic_name); - return; - } + // Verify if this topic still has messages from the server. + update_topic_last_message_id(this.stream_id, topic_name); } existing.count -= num_messages; diff --git a/web/src/stream_ui_updates.js b/web/src/stream_ui_updates.js index 436161326f..65006e2314 100644 --- a/web/src/stream_ui_updates.js +++ b/web/src/stream_ui_updates.js @@ -408,16 +408,11 @@ export function enable_or_disable_add_subscribers_elements( stream_creation = false, ) { const $input_element = $container_elem.find(".input").expectOne(); - const $add_subscribers_button = $container_elem - .find('button[name="add_subscriber"]') - .expectOne(); const $add_subscribers_container = $(".edit_subscribers_for_stream .subscriber_list_settings"); $input_element.prop("contenteditable", enable_elem); - $add_subscribers_button.prop("disabled", !enable_elem); if (enable_elem) { - $add_subscribers_button.css("pointer-events", ""); $add_subscribers_container[0]?._tippy?.destroy(); $container_elem.find(".add_subscribers_container").removeClass("add_subscribers_disabled"); } else { @@ -437,5 +432,13 @@ export function enable_or_disable_add_subscribers_elements( .find(".add_all_users_to_stream_btn_container") .addClass("add_subscribers_disabled"); } + } else { + const $add_subscribers_button = $container_elem + .find('button[name="add_subscriber"]') + .expectOne(); + $add_subscribers_button.prop("disabled", !enable_elem); + if (enable_elem) { + $add_subscribers_button.css("pointer-events", ""); + } } } diff --git a/web/styles/app_components.css b/web/styles/app_components.css index a85f9fa985..05a0c1f591 100644 --- a/web/styles/app_components.css +++ b/web/styles/app_components.css @@ -385,6 +385,11 @@ div.overlay { margin: 0; } +/* TODO: Once all layouts using this button + are modernized, the Font Awesome icon + should be replaced with a Zulip icon, + and its formatting should have no extra + space around its viewbox in SVG. */ .clear_search_button { &:hover { color: hsl(0deg 0% 0%); diff --git a/web/styles/app_variables.css b/web/styles/app_variables.css index d0f7698569..e155605e69 100644 --- a/web/styles/app_variables.css +++ b/web/styles/app_variables.css @@ -60,13 +60,21 @@ /* This represents the space in the sidebar reserved for symbols like the #; labels like the stream name go to the right of this. */ --left-sidebar-privacy-icon-column-size: 19px; - /* The full topic indentation includes 4px of indent in addition to - the above (and another 5px of padding not measured here) */ + /* 13px at 14px/1em */ + --left-sidebar-topic-resolve-width: 0.9286em; + /* At legacy sizes, the full indentation to the + left of the topic name was 25px of gutter, + plus 13px for the topic-resolution checkmark. + That works out to 38px (25px + 13px), which + we here express as pixels, as that is always + the amount of space to the left of the topic + name. However, CSS subtracts the em-unit width + of the topic-resolution checkmark to prevent + the the topic name from being shifted to the + right. */ --left-sidebar-topic-indent: calc( - var(--left-sidebar-far-left-gutter-size) + - var(--left-sidebar-privacy-icon-column-size) + 4px + 38px - var(--left-sidebar-topic-resolve-width) ); - --left-sidebar-topic-resolve-width: 13px; /* space direct message / stream / topic names from unread counters and @ mention indicators by 3px on the right */ --left-sidebar-before-unread-count-padding: 3px; @@ -409,6 +417,7 @@ --color-message-content-container-border: hsl(0deg 0% 0% / 10%); --color-message-content-container-border-focus: hsl(0deg 0% 57%); --color-compose-control-button-background-hover: hsl(0deg 0% 0% / 5%); + --color-compose-focus-ring: var(--color-outline-focus); /* Text colors */ --color-text-default: hsl(0deg 0% 20%); @@ -684,7 +693,6 @@ --color-background-modal: hsl(212deg 28% 18%); --color-background-invitee-emails-pill-container: hsl(0deg 0% 0% / 20%); --color-unmuted-or-followed-topic-list-item: hsl(236deg 33% 90%); - --color-outline-focus: hsl(0deg 0% 67%); --color-recipient-bar-controls-spinner: hsl(0deg 0% 100%); --color-background-search: hsl(0deg 0% 20%); --color-background-search-option-hover: hsl(0deg 0% 30%); @@ -782,6 +790,7 @@ --color-message-content-container-border: hsl(0deg 0% 0% / 80%); --color-message-content-container-border-focus: hsl(0deg 0% 100% / 27%); --color-compose-control-button-background-hover: hsl(0deg 0% 100% / 5%); + --color-compose-focus-ring: hsl(0deg 0% 67%); /* Text colors */ --color-text-default: hsl(0deg 0% 100% / 75%); diff --git a/web/styles/compose.css b/web/styles/compose.css index ebb5a5e96d..c5d96f3c70 100644 --- a/web/styles/compose.css +++ b/web/styles/compose.css @@ -490,7 +490,7 @@ } &:focus-visible { - outline-color: var(--color-outline-focus); + outline-color: var(--color-compose-focus-ring); } } @@ -1096,7 +1096,7 @@ textarea.new_message_textarea { } &:focus-visible { - outline-color: var(--color-outline-focus); + outline-color: var(--color-compose-focus-ring); } } @@ -1308,11 +1308,6 @@ textarea.new_message_textarea { } } -.compose_mobile_stream_button i, -.compose_mobile_direct_message_button i { - margin-right: 4px; -} - /* Class for send-area buttons, such as Drafts and the send-adjacent vdots */ .send-control-button { @@ -1335,7 +1330,7 @@ textarea.new_message_textarea { } &:focus-visible { - outline-color: var(--color-outline-focus); + outline-color: var(--color-compose-focus-ring); } &:hover { @@ -1381,7 +1376,7 @@ textarea.new_message_textarea { will handle the dimension change, so there won't be any movement of the vdots in this state. */ outline: 0; - border: 2px solid var(--color-outline-focus); + border: 2px solid var(--color-compose-focus-ring); } @media ((width >= $sm_min) and (width < $mc_min)) { diff --git a/web/styles/dark_theme.css b/web/styles/dark_theme.css index 5506a01745..0d9b2d8e9e 100644 --- a/web/styles/dark_theme.css +++ b/web/styles/dark_theme.css @@ -306,9 +306,9 @@ } } - & textarea.new_message_textarea { - &.invalid, - &.invalid:focus { + #message-content-container { + &:has(textarea.new_message_textarea.invalid), + &:has(textarea.new_message_textarea.invalid:focus) { border-color: hsl(3deg 73% 74%); box-shadow: 0 0 2px hsl(3deg 73% 74%); } @@ -411,7 +411,7 @@ color: inherit; } - .dropdown-list-search .dropdown-list-search-input:focus { + .popover-filter-input-wrapper .popover-filter-input:focus { background-color: hsl(225deg 6% 7%); border: 1px solid hsl(0deg 0% 100% / 50%); box-shadow: 0 0 5px hsl(0deg 0% 100% / 40%); diff --git a/web/styles/left_sidebar.css b/web/styles/left_sidebar.css index 1e5806d1a5..7ec1f4efeb 100644 --- a/web/styles/left_sidebar.css +++ b/web/styles/left_sidebar.css @@ -39,7 +39,8 @@ li.show-more-topics { & a { - font-size: 12px; + /* 12px at 14px/1em */ + font-size: 0.8571em; } } @@ -68,21 +69,15 @@ li.show-more-topics { padding: 0; font-weight: normal; - .input-append.topic_search_section { - padding: 2px 0 2px - calc( - var(--left-sidebar-topic-indent) - - var(--left-sidebar-topic-resolve-width) - ); - margin-bottom: 3px; - margin-left: 3px; - - & input { - width: calc(100% - 50px); - } + .topic_search_section { + margin: 3px 0; .clear_search_button { - margin-left: -1px; + grid-area: row-content; + justify-self: self-end; + /* Override app-component positioning. */ + position: static; + padding-right: 4px; } } @@ -279,7 +274,8 @@ li.show-more-topics { align-items: baseline; & a { - font-size: 12px; + /* 12px at 14px/1em */ + font-size: 0.8571em; } .unread_count { @@ -716,7 +712,8 @@ li.top_left_scheduled_messages { .dm-box, .subscription_block, .topic-box, -.searching-for-more-topics { +.searching-for-more-topics, +.topic_search_section { display: grid; align-items: center; /* This general pattern of elements applies to every single row in the left @@ -875,10 +872,14 @@ li.top_left_scheduled_messages { .topic-box, .searching-for-more-topics { grid-template-columns: - 25px var(--left-sidebar-topic-resolve-width) minmax(0, 1fr) minmax( - 0, - max-content - ) + var(--left-sidebar-topic-indent) var(--left-sidebar-topic-resolve-width) + minmax(0, 1fr) minmax(0, max-content) + 30px 0; +} + +.topic_search_section { + grid-template-columns: + var(--left-sidebar-topic-indent) 0 minmax(0, 1fr) minmax(0, max-content) 30px 0; } @@ -909,6 +910,10 @@ li.top_left_scheduled_messages { } } +.topic-list-filter { + grid-area: row-content; +} + .searching-for-more-topics img { height: 16px; grid-area: row-content; @@ -916,7 +921,8 @@ li.top_left_scheduled_messages { .sidebar-topic-check { grid-area: starting-anchor-element; - font-size: 15px; + /* 15px at 14px/1em */ + font-size: 1.0714em; } .stream-markers-and-controls, @@ -1179,7 +1185,7 @@ li.topic-list-item { width: var(--left-sidebar-header-icon-width); } - .input-append { + .stream_search_section { grid-area: filter-box; display: flex; justify-content: stretch; @@ -1188,7 +1194,7 @@ li.topic-list-item { line-height: 20px; white-space: nowrap; - & input { + .stream-list-filter { /* Use the border-box model so flex can do its thing despite whatever padding and border we specify. */ @@ -1331,14 +1337,6 @@ li.topic-list-item { overflow: hidden; } -.topic-list-filter { - /* Input width = 100% - 30px right-margin - 6px right-padding */ - /* To keep the right edge of input along with its borders inline with other - topic items we consider to subtract the space given for right margin of - other items, and right padding of input element. */ - width: calc(100% - 36px); -} - .zero_count { visibility: hidden; } @@ -1370,7 +1368,8 @@ li.topic-list-item { display: block; text-decoration: none; color: inherit; - font-size: 12px; + /* 12px at 14px/1em */ + font-size: 0.8571em; & span { display: block; diff --git a/web/styles/popovers.css b/web/styles/popovers.css index bb3541e7dd..1efd3a95be 100644 --- a/web/styles/popovers.css +++ b/web/styles/popovers.css @@ -919,31 +919,6 @@ ul { outline: 1px solid hsl(0deg 100% 50%); } - .search-box { - display: flex; - position: sticky; - padding: 2px; - - & input { - flex-grow: 1; - margin: 5px; - border-radius: 3px; - } - - .clear_search_button { - position: absolute; - top: 5px; - right: 3px; - font-size: 16px; - - &:focus { - .fa-remove { - outline: 2px solid var(--color-outline-focus); - } - } - } - } - .giphy-scrolling-container { overflow: auto; height: 200px; diff --git a/web/styles/reactions.css b/web/styles/reactions.css index aeb0a31a59..0909b23a63 100644 --- a/web/styles/reactions.css +++ b/web/styles/reactions.css @@ -171,32 +171,6 @@ .emoji-popover { width: 250px; - .emoji-popover-top { - position: relative; - - padding: 8px 10px; - margin-bottom: 0; - - background-color: var(--color-background-emoji-picker-popover); - - border-radius: 3px 3px 0 0; - - .fa-search { - position: absolute; - color: hsl(0deg 0% 73%); - top: 15px; - left: 17px; - z-index: 3; - } - - .emoji-popover-filter { - margin: auto; - padding-left: 22px; - width: calc(100% - 22px - 8px); - font-size: 90%; - } - } - .emoji-popover-category-tabs { /* Flex needed here to work around #7511 (90% zoom issues in firefox) */ display: flex; diff --git a/web/styles/subscriptions.css b/web/styles/subscriptions.css index 2114f1fe27..0c162150c3 100644 --- a/web/styles/subscriptions.css +++ b/web/styles/subscriptions.css @@ -201,6 +201,10 @@ h4.user_group_setting_subsection_title { th.user-remove-actions { min-width: 80px; } + + .strikethrough { + text-decoration: line-through; + } } } } diff --git a/web/styles/zulip.css b/web/styles/zulip.css index ec699719eb..a405c52c61 100644 --- a/web/styles/zulip.css +++ b/web/styles/zulip.css @@ -2222,24 +2222,24 @@ body:not(.hide-left-sidebar) { overscroll-behavior: contain; } -.dropdown-list-container { - .dropdown-list-search { - display: flex; +.popover-filter-input-wrapper { + display: flex; - .dropdown-list-search-input { - background: var(--color-background-widget-input); - color: var(--color-text-dropdown-input); - width: 100%; - margin: 4px 4px 2px; + .popover-filter-input { + background: var(--color-background-widget-input); + color: var(--color-text-dropdown-input); + width: 100%; + margin: 4px 4px 2px; - &:focus { - background: hsl(0deg 0% 100%); - border: 1px solid hsl(229.09deg 21.57% 10% / 80%); - box-shadow: 0 0 6px hsl(228deg 9.8% 20% / 30%); - } + &:focus { + background: hsl(0deg 0% 100%); + border: 1px solid hsl(229.09deg 21.57% 10% / 80%); + box-shadow: 0 0 6px hsl(228deg 9.8% 20% / 30%); } } +} +.dropdown-list-container { .dropdown-list-wrapper { /* Sync with `max-height` in dropdown_widget. */ max-height: 210px; diff --git a/web/templates/dropdown_list_container.hbs b/web/templates/dropdown_list_container.hbs index 50986dc0a4..3954746ae6 100644 --- a/web/templates/dropdown_list_container.hbs +++ b/web/templates/dropdown_list_container.hbs @@ -1,6 +1,6 @@
-
- +
+
    diff --git a/web/templates/filter_topics.hbs b/web/templates/filter_topics.hbs index 0010f05e33..6ba373a143 100644 --- a/web/templates/filter_topics.hbs +++ b/web/templates/filter_topics.hbs @@ -1,4 +1,4 @@ -
    +
    +
    +
    {{! We need a container we can replace diff --git a/web/templates/left_sidebar.hbs b/web/templates/left_sidebar.hbs index cb6de50754..4414c34f3a 100644 --- a/web/templates/left_sidebar.hbs +++ b/web/templates/left_sidebar.hbs @@ -178,7 +178,7 @@
    -
    +
    -
    + {{#if (not hide_add_button)}} +
    + +
    + {{/if}}
    diff --git a/web/templates/stream_settings/new_stream_user.hbs b/web/templates/stream_settings/new_stream_user.hbs index 49154d5d63..ef6ab654d6 100644 --- a/web/templates/stream_settings/new_stream_user.hbs +++ b/web/templates/stream_settings/new_stream_user.hbs @@ -1,13 +1,17 @@ - {{> ../user_display_only_pill display_value=full_name}} + {{> ../user_display_only_pill display_value=full_name strikethrough=soft_removed}} {{#if email}} - {{email}} + {{email}} {{else}} - {{t "(hidden)"}} + {{t "(hidden)"}} {{/if}} - + {{#if soft_removed}} + + {{else}} + + {{/if}} diff --git a/web/templates/stream_settings/new_stream_users.hbs b/web/templates/stream_settings/new_stream_users.hbs index 976790eb84..0ff15ad0fd 100644 --- a/web/templates/stream_settings/new_stream_users.hbs +++ b/web/templates/stream_settings/new_stream_users.hbs @@ -1,6 +1,6 @@
    - {{> add_subscribers_form}} + {{> add_subscribers_form hide_add_button=true}}

    @@ -12,7 +12,7 @@
    -

    {{t 'Subscribers' }}

    +

    {{t 'Subscribers preview' }}

    diff --git a/web/templates/user_display_only_pill.hbs b/web/templates/user_display_only_pill.hbs index 34f55ca212..a3b6abdd8b 100644 --- a/web/templates/user_display_only_pill.hbs +++ b/web/templates/user_display_only_pill.hbs @@ -3,7 +3,7 @@ {{#if img_src}} {{/if}} - + {{display_value}} {{#if is_current_user}}{{t '(you)'}}{{/if}} {{~#if should_add_guest_user_indicator}} ({{t 'guest'}}){{~/if~}} diff --git a/web/tests/filter.test.js b/web/tests/filter.test.js index 8abc942d0c..daff0cbda5 100644 --- a/web/tests/filter.test.js +++ b/web/tests/filter.test.js @@ -2204,3 +2204,59 @@ run_test("equals", () => { ), ); }); + +run_test("adjusted_terms_if_moved", () => { + // should return null for non-stream messages + let raw_terms = [{operator: "channel", operand: "Foo"}]; + let message = {type: "private"}; + let result = Filter.adjusted_terms_if_moved(raw_terms, message); + assert.strictEqual(result, null); + + // should return null if no terms are changed + raw_terms = [{operator: "channel", operand: "general"}]; + message = {type: "stream", display_recipient: "general", topic: "discussion"}; + result = Filter.adjusted_terms_if_moved(raw_terms, message); + assert.strictEqual(result, null); + + // should adjust channel term to match message's display_recipient + raw_terms = [{operator: "channel", operand: "random"}]; + message = {type: "stream", display_recipient: "general", topic: "discussion"}; + let expected = [{operator: "channel", operand: "general"}]; + result = Filter.adjusted_terms_if_moved(raw_terms, message); + assert.deepStrictEqual(result, expected); + + // should adjust topic term to match message's topic + raw_terms = [{operator: "topic", operand: "random"}]; + message = {type: "stream", display_recipient: "general", topic: "discussion"}; + expected = [{operator: "topic", operand: "discussion"}]; + result = Filter.adjusted_terms_if_moved(raw_terms, message); + assert.deepStrictEqual(result, expected); + + // should adjust both channel and topic terms when both are different + raw_terms = [ + {operator: "channel", operand: "random"}, + {operator: "topic", operand: "random"}, + ]; + message = {type: "stream", display_recipient: "general", topic: "discussion"}; + expected = [ + {operator: "channel", operand: "general"}, + {operator: "topic", operand: "discussion"}, + ]; + result = Filter.adjusted_terms_if_moved(raw_terms, message); + assert.deepStrictEqual(result, expected); + + // should not adjust terms that are not channel or topic + raw_terms = [ + {operator: "channel", operand: "random"}, + {operator: "topic", operand: "random"}, + {operator: "sender", operand: "alice"}, + ]; + message = {type: "stream", display_recipient: "general", topic: "discussion"}; + expected = [ + {operator: "channel", operand: "general"}, + {operator: "topic", operand: "discussion"}, + {operator: "sender", operand: "alice"}, + ]; + result = Filter.adjusted_terms_if_moved(raw_terms, message); + assert.deepStrictEqual(result, expected); +}); diff --git a/web/tests/stream_create_subscribers_data.test.js b/web/tests/stream_create_subscribers_data.test.js index 98ffc3dfa9..f8e9174615 100644 --- a/web/tests/stream_create_subscribers_data.test.js +++ b/web/tests/stream_create_subscribers_data.test.js @@ -78,3 +78,71 @@ test("must_be_subscribed", () => { assert.ok(!stream_create_subscribers_data.must_be_subscribed(me.user_id)); assert.ok(!stream_create_subscribers_data.must_be_subscribed(test_user101.user_id)); }); + +test("sync_user_ids", () => { + // sync_user_ids should not remove current user if already present. + stream_create_subscribers_data.initialize_with_current_user(); + stream_create_subscribers_data.sync_user_ids([test_user101.user_id, test_user102.user_id]); + assert.deepEqual(stream_create_subscribers_data.sorted_user_ids(), [ + me.user_id, + test_user101.user_id, + test_user102.user_id, + ]); + + // sync_user_ids should not add current user if already not present. + stream_create_subscribers_data.remove_user_ids([me.user_id]); + stream_create_subscribers_data.sync_user_ids([test_user101.user_id, test_user102.user_id]); + assert.deepEqual(stream_create_subscribers_data.sorted_user_ids(), [ + test_user101.user_id, + test_user102.user_id, + ]); +}); + +test("soft remove", () => { + stream_create_subscribers_data.initialize_with_current_user(); + stream_create_subscribers_data.add_user_ids([test_user101.user_id, test_user102.user_id]); + + stream_create_subscribers_data.soft_remove_user_id(test_user102.user_id); + // sorted_user_ids should still have all the users. + assert.deepEqual(stream_create_subscribers_data.sorted_user_ids(), [ + me.user_id, + test_user101.user_id, + test_user102.user_id, + ]); + assert.deepEqual(stream_create_subscribers_data.get_principals(), [ + me.user_id, + test_user101.user_id, + ]); + assert.ok(stream_create_subscribers_data.user_id_in_soft_remove_list(test_user102.user_id)); + assert.ok(!stream_create_subscribers_data.user_id_in_soft_remove_list(test_user101.user_id)); + + // Removing a user_id should also remove them from soft remove list. + stream_create_subscribers_data.remove_user_ids([test_user102.user_id]); + assert.ok(!stream_create_subscribers_data.user_id_in_soft_remove_list(test_user102.user_id)); + assert.deepEqual(stream_create_subscribers_data.sorted_user_ids(), [ + me.user_id, + test_user101.user_id, + ]); + assert.deepEqual(stream_create_subscribers_data.get_principals(), [ + me.user_id, + test_user101.user_id, + ]); + + // Undo soft remove + stream_create_subscribers_data.soft_remove_user_id(test_user101.user_id); + assert.deepEqual(stream_create_subscribers_data.sorted_user_ids(), [ + me.user_id, + test_user101.user_id, + ]); + assert.deepEqual(stream_create_subscribers_data.get_principals(), [me.user_id]); + + stream_create_subscribers_data.undo_soft_remove_user_id(test_user101.user_id); + assert.deepEqual(stream_create_subscribers_data.sorted_user_ids(), [ + me.user_id, + test_user101.user_id, + ]); + assert.deepEqual(stream_create_subscribers_data.get_principals(), [ + me.user_id, + test_user101.user_id, + ]); +}); diff --git a/web/tests/stream_topic_history.test.js b/web/tests/stream_topic_history.test.js index a2c0c42945..42b33b95a2 100644 --- a/web/tests/stream_topic_history.test.js +++ b/web/tests/stream_topic_history.test.js @@ -214,15 +214,23 @@ test("server_history", () => { history = stream_topic_history.get_recent_topic_names(stream_id); assert.deepEqual(history, ["hist2", "hist1"]); - // We can try to remove a historical message, but it should - // have no effect. + // Removing message from a topic fetched from server history, will send + // query to the server to get the latest message id in the topic. + let update_topic_called = false; + stream_topic_history.set_update_topic_last_message_id((stream_id, topic_name) => { + assert.equal(stream_id, 66); + assert.equal(topic_name, "hist2"); + update_topic_called = true; + }); stream_topic_history.remove_messages({ stream_id, topic_name: "hist2", num_messages: 1, }); + assert.equal(update_topic_called, true); history = stream_topic_history.get_recent_topic_names(stream_id); - assert.deepEqual(history, ["hist2", "hist1"]); + assert.deepEqual(history, ["hist1"]); + stream_topic_history.set_update_topic_last_message_id(noop); // If we call back to the server for history, the // effect is always additive. We may decide to prune old diff --git a/zerver/lib/compatibility.py b/zerver/lib/compatibility.py index 912345454a..1c237cc91e 100644 --- a/zerver/lib/compatibility.py +++ b/zerver/lib/compatibility.py @@ -43,7 +43,7 @@ def is_outdated_server(user_profile: Optional[UserProfile]) -> bool: def pop_numerals(ver: str) -> Tuple[List[int], str]: - match = re.search(r"^( \d+ (?: \. \d+ )* ) (.*)", ver, re.X) + match = re.search(r"^( \d+ (?: \. \d+ )* ) (.*)", ver, re.VERBOSE) if match is None: return [], ver numerals, rest = match.groups() @@ -94,9 +94,9 @@ def version_lt(ver1: str, ver2: str) -> Optional[bool]: def find_mobile_os(user_agent: str) -> Optional[str]: - if re.search(r"\b Android \b", user_agent, re.I | re.X): + if re.search(r"\b Android \b", user_agent, re.IGNORECASE | re.VERBOSE): return "android" - if re.search(r"\b(?: iOS | iPhone\ OS )\b", user_agent, re.I | re.X): + if re.search(r"\b(?: iOS | iPhone\ OS )\b", user_agent, re.IGNORECASE | re.VERBOSE): return "ios" return None diff --git a/zerver/lib/markdown/include.py b/zerver/lib/markdown/include.py index 394181064e..b5bf68b162 100644 --- a/zerver/lib/markdown/include.py +++ b/zerver/lib/markdown/include.py @@ -27,7 +27,7 @@ class IncludeExtension(Extension): class IncludeBlockProcessor(BlockProcessor): - RE = re.compile(r"^ {,3}\{!([^!]+)!\} *$", re.M) + RE = re.compile(r"^ {,3}\{!([^!]+)!\} *$", re.MULTILINE) def __init__(self, parser: BlockParser, base_path: str) -> None: super().__init__(parser) diff --git a/zerver/lib/user_agent.py b/zerver/lib/user_agent.py index b80dc15dfc..28ec2720d4 100644 --- a/zerver/lib/user_agent.py +++ b/zerver/lib/user_agent.py @@ -9,7 +9,7 @@ pattern = re.compile( (/ (?P [^/ ]* ))? ([ /] .*)? $""", - re.X, + re.VERBOSE, ) diff --git a/zerver/lib/zcommand.py b/zerver/lib/zcommand.py index a92b40a301..7af12cd0c7 100644 --- a/zerver/lib/zcommand.py +++ b/zerver/lib/zcommand.py @@ -26,18 +26,18 @@ def process_zcommands(content: str, user_profile: UserProfile) -> Dict[str, Any] if command == "ping": return {} - elif command == "night": - if user_profile.color_scheme == UserProfile.COLOR_SCHEME_NIGHT: + elif command == "dark": + if user_profile.color_scheme == UserProfile.COLOR_SCHEME_DARK: return dict(msg="You are still in dark theme.") return dict( msg=change_mode_setting( setting_name="dark theme", switch_command="light", setting="color_scheme", - setting_value=UserProfile.COLOR_SCHEME_NIGHT, + setting_value=UserProfile.COLOR_SCHEME_DARK, ) ) - elif command == "day": + elif command == "light": if user_profile.color_scheme == UserProfile.COLOR_SCHEME_LIGHT: return dict(msg="You are still in light theme.") return dict( diff --git a/zerver/management/commands/create_default_stream_groups.py b/zerver/management/commands/create_default_stream_groups.py index 8565d5cb3f..00d92d0586 100644 --- a/zerver/management/commands/create_default_stream_groups.py +++ b/zerver/management/commands/create_default_stream_groups.py @@ -64,4 +64,4 @@ Create default stream groups which the users can choose during sign up. print(default_stream_group.description) for stream in default_stream_group.streams.all(): print(stream.name) - print("") + print() diff --git a/zerver/management/commands/deactivate_user.py b/zerver/management/commands/deactivate_user.py index 63f0f42ee8..af9f4bbaf1 100644 --- a/zerver/management/commands/deactivate_user.py +++ b/zerver/management/commands/deactivate_user.py @@ -35,7 +35,7 @@ class Command(ZulipBaseCommand): print(f"{user_profile.delivery_email} has the following active sessions:") for session in user_sessions(user_profile): print(session.expire_date, session.get_decoded()) - print("") + print() print( f"{user_profile.delivery_email} has {get_active_bots_owned_by_user(user_profile).count()} active bots that will also be deactivated." ) diff --git a/zerver/management/commands/delete_old_unclaimed_attachments.py b/zerver/management/commands/delete_old_unclaimed_attachments.py index a83e81a7c3..00a9f2317a 100644 --- a/zerver/management/commands/delete_old_unclaimed_attachments.py +++ b/zerver/management/commands/delete_old_unclaimed_attachments.py @@ -61,15 +61,15 @@ class Command(ZulipBaseCommand): if options["for_real"]: do_delete_old_unclaimed_attachments(delta_weeks) - print("") + print() print("Unclaimed files deleted.") if options["clean_up_storage"]: - print("") + print() self.clean_attachment_upload_backend(dry_run=not options["for_real"]) if not options["for_real"]: - print("") + print() raise CommandError("This was a dry run. Pass -f to actually delete.") def clean_attachment_upload_backend(self, dry_run: bool = True) -> None: diff --git a/zerver/management/commands/register_server.py b/zerver/management/commands/register_server.py index 8772ebbf68..edb8ae629a 100644 --- a/zerver/management/commands/register_server.py +++ b/zerver/management/commands/register_server.py @@ -100,7 +100,7 @@ class Command(ZulipBaseCommand): tos_prompt = input( "Do you want to agree to the Zulip Terms of Service and proceed? [Y/n] " ) - print("") + print() if not ( tos_prompt.lower() == "y" or tos_prompt.lower() == "" or tos_prompt.lower() == "yes" ): diff --git a/zerver/migrations/0041_create_attachments_for_old_messages.py b/zerver/migrations/0041_create_attachments_for_old_messages.py index 6b8d593a19..838a21be8f 100644 --- a/zerver/migrations/0041_create_attachments_for_old_messages.py +++ b/zerver/migrations/0041_create_attachments_for_old_messages.py @@ -11,7 +11,7 @@ attachment_url_re = re.compile(r"[/\-]user[\-_]uploads[/\.-].*?(?=[ )]|\Z)") def attachment_url_to_path_id(attachment_url: str) -> str: path_id_raw = re.sub(r"[/\-]user[\-_]uploads[/\.-]", "", attachment_url) # Remove any extra '.' after file extension. These are probably added by the user - return re.sub(r"[.]+$", "", path_id_raw, flags=re.M) + return re.sub(r"[.]+$", "", path_id_raw, flags=re.MULTILINE) def check_and_create_attachments(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: diff --git a/zerver/migrations/0371_invalid_characters_in_topics.py b/zerver/migrations/0371_invalid_characters_in_topics.py index 90dbb5c812..4cade25f5d 100644 --- a/zerver/migrations/0371_invalid_characters_in_topics.py +++ b/zerver/migrations/0371_invalid_characters_in_topics.py @@ -32,7 +32,7 @@ def fix_topics(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None # Nothing to do if there are no messages. return - print("") + print() while lower_bound < max_id: print(f"Processed {lower_bound} / {max_id}") with connection.cursor() as cursor: diff --git a/zerver/migrations/0375_invalid_characters_in_stream_names.py b/zerver/migrations/0375_invalid_characters_in_stream_names.py index ac497755a1..9769df97f5 100644 --- a/zerver/migrations/0375_invalid_characters_in_stream_names.py +++ b/zerver/migrations/0375_invalid_characters_in_stream_names.py @@ -30,7 +30,7 @@ def fix_stream_names(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) - if len(realm_ids) == 0: return - print("") + print() for realm_id in realm_ids: print(f"Processing realm {realm_id}") realm_stream_dicts = Stream.objects.filter(realm_id=realm_id).values("id", "name") diff --git a/zerver/migrations/0383_revoke_invitations_from_deactivated_users.py b/zerver/migrations/0383_revoke_invitations_from_deactivated_users.py index f2d9def5cc..e57d2ba9fa 100644 --- a/zerver/migrations/0383_revoke_invitations_from_deactivated_users.py +++ b/zerver/migrations/0383_revoke_invitations_from_deactivated_users.py @@ -44,7 +44,7 @@ def revoke_invitations(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) return confirmation_ids - print("") + print() for realm_id in Realm.objects.values_list("id", flat=True): deactivated_user_ids = UserProfile.objects.filter( is_active=False, realm_id=realm_id diff --git a/zerver/migrations/0423_fix_email_gateway_attachment_owner.py b/zerver/migrations/0423_fix_email_gateway_attachment_owner.py index 53333c3b5c..e91a9b9e5e 100644 --- a/zerver/migrations/0423_fix_email_gateway_attachment_owner.py +++ b/zerver/migrations/0423_fix_email_gateway_attachment_owner.py @@ -37,7 +37,7 @@ def fix_email_gateway_attachment_owner( if len(orphan_attachments) == 0: return - print("") + print() print(f"Found {len(orphan_attachments)} email gateway attachments to reattach") for attachment in orphan_attachments: # We look for the message posted by "Internal" at the same diff --git a/zerver/migrations/0501_delete_dangling_usermessages.py b/zerver/migrations/0501_delete_dangling_usermessages.py index 56e1995964..f7c9ebc7e5 100644 --- a/zerver/migrations/0501_delete_dangling_usermessages.py +++ b/zerver/migrations/0501_delete_dangling_usermessages.py @@ -152,7 +152,7 @@ def log_extra_usermessage_rows(apps: StateApps, schema_editor: BaseDatabaseSchem ) else: log_file = sys.stderr - print("", file=log_file) + print(file=log_file) stack.enter_context(redirect_stdout(log_file)) for message in messages: @@ -189,7 +189,7 @@ def log_extra_usermessage_rows(apps: StateApps, schema_editor: BaseDatabaseSchem for um in ums: read = "(read)" if um.flags & 1 else "(unread)" print(f" {um.user_profile.delivery_email} {read}") - print("") + print() class Migration(migrations.Migration): diff --git a/zerver/models/users.py b/zerver/models/users.py index d9a3fc5379..703d94c37e 100644 --- a/zerver/models/users.py +++ b/zerver/models/users.py @@ -73,9 +73,9 @@ class UserBaseSettings(models.Model): twenty_four_hour_time = models.BooleanField(default=False) starred_message_counts = models.BooleanField(default=True) COLOR_SCHEME_AUTOMATIC = 1 - COLOR_SCHEME_NIGHT = 2 + COLOR_SCHEME_DARK = 2 COLOR_SCHEME_LIGHT = 3 - COLOR_SCHEME_CHOICES = [COLOR_SCHEME_AUTOMATIC, COLOR_SCHEME_NIGHT, COLOR_SCHEME_LIGHT] + COLOR_SCHEME_CHOICES = [COLOR_SCHEME_AUTOMATIC, COLOR_SCHEME_DARK, COLOR_SCHEME_LIGHT] color_scheme = models.PositiveSmallIntegerField(default=COLOR_SCHEME_AUTOMATIC) # Information density is established through diff --git a/zerver/openapi/markdown_extension.py b/zerver/openapi/markdown_extension.py index d5310c8d06..1cac209885 100644 --- a/zerver/openapi/markdown_extension.py +++ b/zerver/openapi/markdown_extension.py @@ -170,7 +170,7 @@ def render_javascript_code_example( ) -> List[str]: pattern = rf'^add_example\(\s*"[^"]*",\s*{re.escape(json.dumps(function))},\s*\d+,\s*async \(client, console\) => \{{\n(.*?)^(?:\}}| *\}},\n)\);$' with open("zerver/openapi/javascript_examples.js") as f: - m = re.search(pattern, f.read(), re.M | re.S) + m = re.search(pattern, f.read(), re.MULTILINE | re.DOTALL) if m is None: return [] function_source_lines = dedent(m.group(1)).splitlines() diff --git a/zerver/tests/test_settings.py b/zerver/tests/test_settings.py index fbb0486234..5a4870349e 100644 --- a/zerver/tests/test_settings.py +++ b/zerver/tests/test_settings.py @@ -474,11 +474,11 @@ class ChangeSettingsTest(ZulipTestCase): self.login("hamlet") result = self.client_patch( - "/json/settings/display", dict(color_scheme=UserProfile.COLOR_SCHEME_NIGHT) + "/json/settings/display", dict(color_scheme=UserProfile.COLOR_SCHEME_DARK) ) self.assert_json_success(result) hamlet = self.example_user("hamlet") - self.assertEqual(hamlet.color_scheme, UserProfile.COLOR_SCHEME_NIGHT) + self.assertEqual(hamlet.color_scheme, UserProfile.COLOR_SCHEME_DARK) def test_changing_setting_using_notification_setting_endpoint(self) -> None: """ diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 3c7c42aa5e..75bf35ded0 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -1298,7 +1298,7 @@ class UserProfileTest(ZulipTestCase): do_change_user_setting(cordelia, "emojiset", "twitter", acting_user=None) do_change_user_setting(cordelia, "timezone", "America/Phoenix", acting_user=None) do_change_user_setting( - cordelia, "color_scheme", UserProfile.COLOR_SCHEME_NIGHT, acting_user=None + cordelia, "color_scheme", UserProfile.COLOR_SCHEME_DARK, acting_user=None ) do_change_user_setting( cordelia, "enable_offline_email_notifications", False, acting_user=None @@ -1342,8 +1342,8 @@ class UserProfileTest(ZulipTestCase): self.assertEqual(cordelia.timezone, "America/Phoenix") self.assertEqual(hamlet.timezone, "") - self.assertEqual(iago.color_scheme, UserProfile.COLOR_SCHEME_NIGHT) - self.assertEqual(cordelia.color_scheme, UserProfile.COLOR_SCHEME_NIGHT) + self.assertEqual(iago.color_scheme, UserProfile.COLOR_SCHEME_DARK) + self.assertEqual(cordelia.color_scheme, UserProfile.COLOR_SCHEME_DARK) self.assertEqual(hamlet.color_scheme, UserProfile.COLOR_SCHEME_AUTOMATIC) self.assertEqual(iago.enable_offline_email_notifications, False) diff --git a/zerver/tests/test_zcommand.py b/zerver/tests/test_zcommand.py index 1501b96935..020c35561c 100644 --- a/zerver/tests/test_zcommand.py +++ b/zerver/tests/test_zcommand.py @@ -27,7 +27,7 @@ class ZcommandTest(ZulipTestCase): user.color_scheme = UserProfile.COLOR_SCHEME_LIGHT user.save() - payload = dict(command="/night") + payload = dict(command="/dark") result = self.client_post("/json/zcommand", payload) response_dict = self.assert_json_success(result) self.assertIn("Changed to dark theme", response_dict["msg"]) @@ -39,10 +39,10 @@ class ZcommandTest(ZulipTestCase): def test_day_zcommand(self) -> None: self.login("hamlet") user = self.example_user("hamlet") - user.color_scheme = UserProfile.COLOR_SCHEME_NIGHT + user.color_scheme = UserProfile.COLOR_SCHEME_DARK user.save() - payload = dict(command="/day") + payload = dict(command="/light") result = self.client_post("/json/zcommand", payload) response_dict = self.assert_json_success(result) self.assertIn("Changed to light theme", response_dict["msg"]) diff --git a/zerver/tornado/sharding.py b/zerver/tornado/sharding.py index f12b571d92..f876e06d9c 100644 --- a/zerver/tornado/sharding.py +++ b/zerver/tornado/sharding.py @@ -17,7 +17,8 @@ if os.path.exists("/etc/zulip/sharding.json"): data, # backwards compatibility ) shard_regexes = [ - (re.compile(regex, re.I), port) for regex, port in data.get("shard_regexes", []) + (re.compile(regex, re.IGNORECASE), port) + for regex, port in data.get("shard_regexes", []) ]