Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

narrow_state: Remove narrowed_to_topic function. 

This was only being used in one place in compose_closed_ui
to create the label for the closed composebox. But it
only checked if the `channel` and `topic` filters existed,
while `stream_sub` can return `undefined` for a few other
reasons. To ensure that we're catching the undefined sub
while also avoiding duplicate work, it makes sense to just
call `stream_sub()` directly.

Fixes this bug:
https://zulip.sentry.io/issues/5367251929/events/40073ecf007a4a9798e728061a576377/?project=450455688282112
This commit is contained in:
evykassirer 2024-05-23 14:45:54 -07:00 committed by Tim Abbott
parent 852637d3b3
commit 825fa6f0e1
3 changed files with 4 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
web/src/compose_closed_ui.ts
View File

@ -1,5 +1,4 @@
import $ from "jquery";
import assert from "minimalistic-assert";
import * as compose_actions from "./compose_actions";
import {$t} from "./i18n";
@ -28,7 +27,6 @@ export function get_recipient_label(message?: ComposeClosedMessage): string {
// actual message objects with fake objects containing just a
// couple fields, both those constructed here and potentially
// passed in.
if (message_lists.current === undefined) {
return "";
}
@ -38,12 +36,9 @@ export function get_recipient_label(message?: ComposeClosedMessage): string {
// For empty narrows where there's a clear reply target,
// i.e. stream+topic or a single direct message conversation,
// we label the button as replying to the thread.
if (narrow_state.narrowed_to_topic()) {
const stream = narrow_state.stream_sub();
assert(stream !== undefined);
const stream_id = stream.stream_id;
const topic = narrow_state.topic();
assert(topic !== undefined);
const stream_id = narrow_state.stream_sub()?.stream_id;
const topic = narrow_state.topic();
if (stream_id !== undefined && topic !== undefined) {
return format_stream_recipient_label(stream_id, topic);
} else if (narrow_state.pm_ids_string()) {
const user_ids = people.user_ids_string_to_ids_array(narrow_state.pm_ids_string()!);

8
web/src/narrow_state.ts
View File

@ -138,6 +138,7 @@ export function stream_sub(
return undefined;
}
const stream_operands = current_filter.operands("channel");
if (stream_operands.length !== 1) {
return undefined;
}
@ -357,13 +358,6 @@ export function narrowed_by_stream_reply(current_filter: Filter | undefined = fi
return terms.length === 1 && current_filter.operands("channel").length === 1;
}
export function narrowed_to_topic(current_filter: Filter | undefined = filter()): boolean {
if (current_filter === undefined) {
return false;
}
return current_filter.has_operator("channel") && current_filter.has_operator("topic");
}
export function is_for_stream_id(stream_id: number, filter?: Filter): boolean {
// This is not perfect, since we still track narrows by
// name, not id, but at least the interface is good going

6
web/tests/narrow_state.test.js
View File

@ -74,7 +74,6 @@ test("narrowed", () => {
assert.ok(!narrow_state.narrowed_by_reply());
assert.ok(!narrow_state.narrowed_by_pm_reply());
assert.ok(!narrow_state.narrowed_by_topic_reply());
assert.ok(!narrow_state.narrowed_to_topic());
assert.ok(!narrow_state.narrowed_by_stream_reply());
assert.equal(narrow_state.stream_sub(), undefined);
@ -83,7 +82,6 @@ test("narrowed", () => {
assert.ok(!narrow_state.narrowed_by_reply());
assert.ok(!narrow_state.narrowed_by_pm_reply());
assert.ok(!narrow_state.narrowed_by_topic_reply());
assert.ok(!narrow_state.narrowed_to_topic());
assert.ok(narrow_state.narrowed_by_stream_reply());
set_filter([["dm", "steve@zulip.com"]]);
@ -91,7 +89,6 @@ test("narrowed", () => {
assert.ok(narrow_state.narrowed_by_reply());
assert.ok(narrow_state.narrowed_by_pm_reply());
assert.ok(!narrow_state.narrowed_by_topic_reply());
assert.ok(!narrow_state.narrowed_to_topic());
assert.ok(!narrow_state.narrowed_by_stream_reply());
set_filter([
@ -102,7 +99,6 @@ test("narrowed", () => {
assert.ok(narrow_state.narrowed_by_reply());
assert.ok(!narrow_state.narrowed_by_pm_reply());
assert.ok(narrow_state.narrowed_by_topic_reply());
assert.ok(narrow_state.narrowed_to_topic());
assert.ok(!narrow_state.narrowed_by_stream_reply());
set_filter([["search", "grail"]]);
@ -110,7 +106,6 @@ test("narrowed", () => {
assert.ok(!narrow_state.narrowed_by_reply());
assert.ok(!narrow_state.narrowed_by_pm_reply());
assert.ok(!narrow_state.narrowed_by_topic_reply());
assert.ok(!narrow_state.narrowed_to_topic());
assert.ok(!narrow_state.narrowed_by_stream_reply());
set_filter([["is", "starred"]]);
@ -118,7 +113,6 @@ test("narrowed", () => {
assert.ok(!narrow_state.narrowed_by_reply());
assert.ok(!narrow_state.narrowed_by_pm_reply());
assert.ok(!narrow_state.narrowed_by_topic_reply());
assert.ok(!narrow_state.narrowed_to_topic());
assert.ok(!narrow_state.narrowed_by_stream_reply());
});