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

narrow: Make `with` operator behaviour consistent with API behaviour. 

Previously, in presence of `dm` operators, the `with` operator
behaviour in the web client was not consistent with that of
the API. For instance, when `with` points to a stream message,
in presence of a `dm` term, then rather than narrowing to the
corresponding channel of `with` operand, the `with` term simply
used to be ignored, and narrowed to the corresponding `dm`
narrow.

Also, in cases when the `with` used to point to a direct message,
in presence of a `channel` term, then after correcting to the
right narrow, the `with` term used to be removed. The `with`
term would still be needed after correcting the narrow to
maintain consistency between channel and dm conversations.

This commit removes these inconsistencies of `with` in case
of `dm` operators, and makes it consistent to those mentioned
in the api documentation.
This commit is contained in:
roanster007 2024-08-03 01:41:21 +05:30 committed by Tim Abbott
parent 719518baba
commit bc384094d3
3 changed files with 66 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
web/src/filter.ts
View File

@ -402,7 +402,8 @@ export class Filter {
return orig_terms;
}
const updated_terms = [...orig_terms];
const updated_terms = orig_terms.filter((term: NarrowTerm) => term.operator !== "dm");
let channel_term = updated_terms.find(
(term: NarrowTerm) => Filter.canonicalize_operator(term.operator) === "channel",
);
@ -616,9 +617,9 @@ export class Filter {
"channels-public",
"channel",
"topic",
"with",
"dm",
"dm-including",
"with",
"sender",
"near",
"id",
@ -891,6 +892,8 @@ export class Filter {
this.cached_sorted_terms_for_comparison = undefined;
if (this.has_operator("channel")) {
this._sub = stream_data.get_sub_by_name(this.operands("channel")[0]!);
} else {
this._sub = undefined;
}
}
@ -1078,6 +1081,10 @@ export class Filter {
return true;
}
if (_.isEqual(term_types, ["dm", "with"])) {
return true;
}
if (_.isEqual(term_types, ["dm"])) {
return true;
}
@ -1303,6 +1310,7 @@ export class Filter {
}
if (
(term_types.length === 2 && _.isEqual(term_types, ["dm", "near"])) ||
(term_types.length === 2 && _.isEqual(term_types, ["dm", "with"])) ||
(term_types.length === 1 && _.isEqual(term_types, ["dm"]))
) {
const emails = this.operands("dm")[0]!.split(",");
@ -1475,7 +1483,6 @@ export class Filter {
fix_terms(terms: NarrowTerm[]): NarrowTerm[] {
terms = this._canonicalize_terms(terms);
terms = this._fix_redundant_is_private(terms);
terms = this._fix_redundant_with_dm(terms);
return terms;
}
@ -1488,16 +1495,6 @@ export class Filter {
return terms.filter((term) => Filter.term_type(term) !== "is-dm");
}
_fix_redundant_with_dm(terms: NarrowTerm[]): NarrowTerm[] {
// Because DMs can't move, the `with` operator is a noop on a
// DM conversation.
if (terms.some((term) => Filter.term_type(term) === "dm")) {
return terms.filter((term) => Filter.term_type(term) !== "with");
}
return terms;
}
_canonicalize_terms(terms_mixed_case: NarrowTerm[]): NarrowTerm[] {
return terms_mixed_case.map((term: NarrowTerm) => Filter.canonicalize_term(term));
}
@ -1613,6 +1610,7 @@ export class Filter {
if (
_.isEqual(term_type, ["channel", "topic", "with"]) ||
_.isEqual(term_type, ["channel", "topic"]) ||
_.isEqual(term_type, ["dm", "with"]) ||
_.isEqual(term_type, ["dm"])
) {
return true;

4
web/src/narrow_state.ts
View File

@ -304,7 +304,7 @@ export function _possible_unread_message_ids(
return unread.get_msg_ids_for_stream(sub.stream_id);
}
if (current_filter.can_bucket_by("dm")) {
if (current_filter.can_bucket_by("dm", "with") || current_filter.can_bucket_by("dm")) {
current_filter_pm_string = pm_ids_string(current_filter);
if (current_filter_pm_string === undefined) {
return [];
@ -349,7 +349,7 @@ export function narrowed_by_pm_reply(current_filter: Filter | undefined = filter
if (current_filter === undefined) {
return false;
}
const terms = current_filter.terms();
const terms = current_filter.terms().filter((term) => term.operator !== "with");
return terms.length === 1 && current_filter.has_operator("dm");
}

60
web/tests/filter.test.js
View File

@ -324,6 +324,19 @@ test("basics", () => {
assert.ok(filter.is_conversation_view());
assert.ok(!filter.is_conversation_view_with_near());
terms = [
{operator: "dm", operand: "joe@example.com,jack@example.com"},
{operator: "with", operand: "12"},
];
filter = new Filter(terms);
assert.ok(filter.contains_only_private_messages());
assert.ok(filter.can_mark_messages_read());
assert.ok(filter.supports_collapsing_recipients());
assert.ok(filter.can_apply_locally());
assert.ok(!filter.is_personal_filter());
assert.ok(filter.is_conversation_view());
assert.ok(!filter.is_conversation_view_with_near());
// "pm-with" was renamed to "dm"
terms = [{operator: "pm-with", operand: "joe@example.com"}];
filter = new Filter(terms);
@ -742,13 +755,6 @@ test("redundancies", () => {
];
filter = new Filter(terms);
assert.ok(filter.can_bucket_by("is-dm", "not-dm"));
terms = [
{operator: "dm", operand: "joe@example.com,"},
{operator: "with", operand: "12"},
];
filter = new Filter(terms);
assert.ok(filter.can_bucket_by("dm"));
});
test("canonicalization", () => {
@ -1561,12 +1567,25 @@ test("can_bucket_by", () => {
assert.equal(filter.can_bucket_by("channel"), false);
assert.equal(filter.can_bucket_by("channel", "topic"), false);
assert.equal(filter.can_bucket_by("dm"), false);
assert.equal(filter.can_bucket_by("dm", "with"), false);
terms = [{operator: "dm", operand: "foo@example.com,bar@example.com"}];
filter = new Filter(terms);
assert.equal(filter.can_bucket_by("channel"), false);
assert.equal(filter.can_bucket_by("channel", "topic"), false);
assert.equal(filter.can_bucket_by("dm"), true);
assert.equal(filter.can_bucket_by("dm", "with"), false);
assert.equal(filter.can_bucket_by("is-mentioned"), false);
assert.equal(filter.can_bucket_by("is-dm"), false);
terms = [
{operator: "dm", operand: "foo@example.com,bar@example.com"},
{operator: "with", operand: "7"},
];
filter = new Filter(terms);
assert.equal(filter.can_bucket_by("channel"), false);
assert.equal(filter.can_bucket_by("channel", "topic"), false);
assert.equal(filter.can_bucket_by("dm", "with"), true);
assert.equal(filter.can_bucket_by("is-mentioned"), false);
assert.equal(filter.can_bucket_by("is-dm"), false);
@ -1820,6 +1839,22 @@ test("try_adjusting_for_moved_with_target", ({override}) => {
assert.deepEqual(filter.requires_adjustment_for_moved_with_target, false);
assert.deepEqual(filter.terms(), [
{operator: "dm", operand: "user3@zulip.com", negated: false},
{operator: "with", operand: "2", negated: false},
]);
// When the narrow consists of `dm` operators, while the `with`
// operator corresponds to that of a channel topic message.
terms = [
{operator: "dm", operand: "iago@foo.com"},
{operator: "with", operand: "12"},
];
filter = new Filter(terms);
filter.try_adjusting_for_moved_with_target();
assert.deepEqual(filter.requires_adjustment_for_moved_with_target, false);
assert.deepEqual(filter.terms(), [
{operator: "channel", operand: "Scotland", negated: false},
{operator: "topic", operand: "Test 1", negated: false},
{operator: "with", operand: "12", negated: false},
]);
// When message id attached to `with` operator is found locally,
@ -1991,6 +2026,10 @@ test("navbar_helpers", () => {
{operator: "topic", operand: "pink"},
];
const dm = [{operator: "dm", operand: "joe@example.com"}];
const dm_with = [
{operator: "dm", operand: "joe@example.com"},
{operator: "with", operand: "12"},
];
const dm_group = [{operator: "dm", operand: "joe@example.com,STEVE@foo.com"}];
const dm_with_guest = [{operator: "dm", operand: "alice@example.com"}];
const dm_group_including_guest = [
@ -2221,6 +2260,13 @@ test("navbar_helpers", () => {
title: "Foo",
redirect_url_with_search: "#",
},
{
terms: dm_with,
is_common_narrow: true,
zulip_icon: "user",
title: properly_separated_names([joe.full_name]),
redirect_url_with_search: "#",
},
];
realm.realm_enable_guest_user_indicator = true;