diff --git a/web/e2e-tests/message-basics.test.ts b/web/e2e-tests/message-basics.test.ts index 55a94cce81..dbe8dcf781 100644 --- a/web/e2e-tests/message-basics.test.ts +++ b/web/e2e-tests/message-basics.test.ts @@ -209,39 +209,6 @@ async function search_silent_user(page: Page, str: string, item: string): Promis await expect_home(page); } -async function expect_non_existing_user(page: Page): Promise { - await common.get_current_msg_list_id(page, true); - await page.waitForSelector(".empty_feed_notice", {visible: true}); - const expected_message = "This user does not exist!"; - assert.strictEqual( - await common.get_text_from_selector(page, ".empty_feed_notice"), - expected_message, - ); -} - -async function expect_non_existing_users(page: Page): Promise { - await common.get_current_msg_list_id(page, true); - await page.waitForSelector(".empty_feed_notice", {visible: true}); - const expected_message = "One or more of these users do not exist!"; - assert.strictEqual( - await common.get_text_from_selector(page, ".empty_feed_notice"), - expected_message, - ); -} - -async function search_non_existing_user(page: Page, str: string, item: string): Promise { - await page.click(".search_icon"); - await page.waitForSelector(".navbar-search.expanded", {visible: true}); - // Close the "in: home" pill - await page.click(".navbar-search .pill-close-button"); - await common.select_item_via_typeahead(page, "#search_query", str, item); - // Enter to trigger search - await page.keyboard.press("Enter"); - await expect_non_existing_user(page); - await un_narrow(page); - await expect_home(page); -} - async function search_tests(page: Page): Promise { await search_and_check( page, @@ -292,24 +259,6 @@ async function search_tests(page: Page): Promise { ); await search_silent_user(page, "sender:emailgateway@zulip.com", ""); - - await search_non_existing_user(page, "sender:dummyuser@zulip.com", ""); - - await search_and_check( - page, - "dm:dummyuser@zulip.com", - "", - expect_non_existing_user, - "Invalid user - Zulip Dev - Zulip", - ); - - await search_and_check( - page, - "dm:dummyuser@zulip.com,dummyuser2@zulip.com", - "", - expect_non_existing_users, - "Invalid users - Zulip Dev - Zulip", - ); } async function expect_all_direct_messages(page: Page): Promise { diff --git a/web/src/search.ts b/web/src/search.ts index 49eafbc92c..0e4f44e328 100644 --- a/web/src/search.ts +++ b/web/src/search.ts @@ -210,7 +210,12 @@ export function initialize({on_narrow_search}: {on_narrow_search: OnNarrowSearch if (e.key === "Escape" && $search_query_box.is(":focus")) { exit_search({keep_search_narrow_open: false}); } else if (keydown_util.is_enter_event(e) && $search_query_box.is(":focus")) { - narrow_or_search_for_term({on_narrow_search}); + const text_terms = Filter.parse(get_search_bar_text()); + if (text_terms.every((term) => Filter.is_valid_search_term(term))) { + narrow_or_search_for_term({on_narrow_search}); + } else { + $("#search_query").addClass("shake"); + } } }); @@ -287,7 +292,11 @@ function reset_searchbox(): void { assert(search_pill_widget !== null); search_pill_widget.clear(); search_input_has_changed = false; - search_pill.set_search_bar_contents(narrow_state.search_terms(), search_pill_widget); + search_pill.set_search_bar_contents( + narrow_state.search_terms(), + search_pill_widget, + set_search_bar_text, + ); } function exit_search(opts: {keep_search_narrow_open: boolean}): void { diff --git a/web/src/search_pill.ts b/web/src/search_pill.ts index 1f811fa483..63c6abaddd 100644 --- a/web/src/search_pill.ts +++ b/web/src/search_pill.ts @@ -1,4 +1,4 @@ -import assert from "minimalistic-assert"; +import $ from "jquery"; import {Filter} from "./filter"; import * as input_pill from "./input_pill"; @@ -39,8 +39,12 @@ type SearchPill = export type SearchPillWidget = InputPillContainer; -export function create_item_from_search_string(search_string: string): SearchPill { +export function create_item_from_search_string(search_string: string): SearchPill | undefined { const search_terms = Filter.parse(search_string); + if (!search_terms.every((term) => Filter.is_valid_search_term(term))) { + // This will cause pill validation to fail and trigger a shake animation. + return undefined; + } const description_html = Filter.search_description_as_html(search_terms); return { display_value: search_string, @@ -60,6 +64,9 @@ export function create_pills($pill_container: JQuery): SearchPillWidget { get_text_from_item: get_search_string_from_item, split_text_on_comma: false, }); + // We don't automatically create pills on paste. When the user + // presses enter, we validate the input then. + pills.createPillonPaste(() => false); return pills; } @@ -96,42 +103,53 @@ const user_pill_operators = new Set(["dm", "dm-including", "sender"]); export function set_search_bar_contents( search_terms: NarrowTerm[], pill_widget: SearchPillWidget, - set_search_bar_text?: (text: string) => void, + set_search_bar_text: (text: string) => void, ): void { pill_widget.clear(); let partial_pill = ""; + const invalid_inputs = []; + for (const term of search_terms) { - if (user_pill_operators.has(term.operator) && term.operand !== "") { - const user_emails = term.operand.split(","); - const users = user_emails.map((email) => { - const user = people.get_by_email(email); - assert(user !== undefined); - return user; - }); - append_user_pill(users, pill_widget, term.operator, term.negated ?? false); - continue; - } const input = Filter.unparse([term]); + // If the last term looks something like `dm:`, we // don't want to make it a pill, since it isn't isn't // a complete search term yet. // Instead, we keep the partial pill to the end of the // search box as text input, which will update the // typeahead to show operand suggestions. - if ( - set_search_bar_text !== undefined && - input.at(-1) === ":" && - term.operand === "" && - term === search_terms.at(-1) - ) { + if (input.at(-1) === ":" && term.operand === "" && term === search_terms.at(-1)) { partial_pill = input; continue; } + + if (!Filter.is_valid_search_term(term)) { + invalid_inputs.push(input); + continue; + } + + if (user_pill_operators.has(term.operator) && term.operand !== "") { + const users = term.operand.split(",").map((email) => { + // This is definitely not undefined, because we just validated it + // with `Filter.is_valid_search_term`. + const user = people.get_by_email(email)!; + return user; + }); + append_user_pill(users, pill_widget, term.operator, term.negated ?? false); + continue; + } + pill_widget.appendValue(input); } pill_widget.clear_text(); - if (set_search_bar_text !== undefined) { - set_search_bar_text(partial_pill); + + const search_bar_text_strings = [...invalid_inputs]; + if (partial_pill !== "") { + search_bar_text_strings.push(partial_pill); + } + set_search_bar_text(search_bar_text_strings.join(" ")); + if (invalid_inputs.length) { + $("#search_query").addClass("shake"); } } diff --git a/web/tests/search.test.js b/web/tests/search.test.js index d031db7506..4348dc70a9 100644 --- a/web/tests/search.test.js +++ b/web/tests/search.test.js @@ -11,6 +11,7 @@ const search_suggestion = mock_esm("../src/search_suggestion"); const search = zrequire("search"); const search_pill = zrequire("search_pill"); +const stream_data = zrequire("stream_data"); function stub_pills() { const $pill_container = $("#searchbox-input-container.pill-container"); @@ -25,6 +26,14 @@ set_global("getSelection", () => ({ let typeahead_forced_open = false; +const verona = { + subscribed: true, + color: "blue", + name: "Verona", + stream_id: 1, +}; +stream_data.add_sub(verona); + run_test("initialize", ({override, override_rewire, mock_template}) => { const $search_query_box = $("#search_query"); const $searchbox_form = $("#searchbox_form"); @@ -211,7 +220,11 @@ run_test("initialize", ({override, override_rewire, mock_template}) => { for (const pill of pills) { pill.$element.remove = noop; } - search_pill.set_search_bar_contents(terms, search.search_pill_widget); + search_pill.set_search_bar_contents( + terms, + search.search_pill_widget, + $search_query_box.text, + ); }; terms = [