Skip to content

Commit

Permalink
narrow: Add new "with" narrow operator.
Browse files Browse the repository at this point in the history
This commit adds a new narrow operator -- "with" which expects
a string operand of message_id, and returns all the messages
in the same channel and topic of the operand, with the first
unread message of the topic as anchor.

This is done as an effort to provide permalinks for topics in
zulip#21505.
  • Loading branch information
roanster007 committed May 23, 2024
1 parent 62dfd93 commit 756e993
Show file tree
Hide file tree
Showing 12 changed files with 358 additions and 17 deletions.
10 changes: 10 additions & 0 deletions api_docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ format used by the Zulip server that they are interacting with.

## Changes in Zulip 9.0

**Feature level 263**

* [`GET /messages`](/api/get-messages),
[`GET /messages/matches_narrow`](/api/check-messages-match-narrow),
[`POST /messages/flags/narrow`](/api/update-message-flags-for-narrow),
[`POST /register`](/api/register-queue):
Added support for a new [search/narrow filter](/api/construct-narrow),
`with`, which returns messages in the same channel and topic as that
of the operand of filter, with the first unread message as anchor.

**Feature level 262**:

* [`GET /users/{user_id}/status`](/api/get-user-status): Added a new
Expand Down
26 changes: 25 additions & 1 deletion api_docs/construct-narrow.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ important optimization when fetching messages in certain cases (e.g.
when [adding the `read` flag to a user's personal
messages](/api/update-message-flags-for-narrow)).

**Changes**: In Zulip 9.0 (feature level 250), support was added for
**Changes**: In Zulip 9.0 (feature level 263), narrows gained support for a new
filter - `with`, taking a message ID as operand. If the message corresponding to
the message ID exists and is visible to the user, then it replaces any "conversation"
filters — channel/stream, topic conversation — with filters corresponding to the
conversation the message is in. Else, the `with` operator is just ignored.

In Zulip 9.0 (feature level 250), support was added for
two filters related to channel messages: `channel` and `channels`. The
`channel` operator is an alias for the `stream` operator. The `channels`
operator is an alias for the `streams` operator. Both `channel` and
Expand Down Expand Up @@ -84,6 +90,24 @@ filters did.

### Message IDs

The `with` operator uses message ID, which must be encoded as a string
for its operand.

* `with:12345`: Search for the conversation (channel and topic) which
contains the message with ID `12345`.

**Changes** This is a new operand added in Zulip 9.0 (feature level 263),
in an effort to provide permalinks for topics.

```json
[
{
"operator": "with",
"operand": 12345
}
]
```

The `near` and `id` operators, documented in the help center, use message
IDs for their operands.

Expand Down
32 changes: 32 additions & 0 deletions web/src/filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,20 @@ function message_matches_search_term(message: Message, operator: string, operand
// this is all handled server side
return true;

case "with": {
if (message.type !== "stream") {
return false;
}
const msg_id = Number.parseInt(operand, 10);
const msg = message_store.get(msg_id);
return (
msg !== undefined &&
msg.type === "stream" &&
msg.stream_id === message.stream_id &&
msg.topic === message.topic
);
}

case "id":
return message.id.toString() === operand;

Expand Down Expand Up @@ -484,6 +498,7 @@ export class Filter {
"channels-public",
"channel",
"topic",
"with",
"dm",
"dm-including",
"sender",
Expand Down Expand Up @@ -536,6 +551,8 @@ export class Filter {
return verb + "channels";
case "near":
return verb + "messages around";
case "with":
return verb + "conversation containing";

// Note: We hack around using this in "describe" below.
case "has":
Expand Down Expand Up @@ -750,6 +767,7 @@ export class Filter {
"channels-web-public",
"not-channels-web-public",
"near",
"with",
]);

for (const term of term_types) {
Expand Down Expand Up @@ -784,6 +802,14 @@ export class Filter {
// it is limited by the user's message history. Therefore, we check "channel"
// and "topic" together to ensure that the current filter will return all the
// messages of a conversation.
if (
_.isEqual(term_types, ["channel", "topic", "with"]) ||
_.isEqual(term_types, ["channel", "with"]) ||
_.isEqual(term_types, ["with"])
) {
return true;
}

if (_.isEqual(term_types, ["channel", "topic"])) {
return true;
}
Expand Down Expand Up @@ -1022,6 +1048,12 @@ export class Filter {
},
);
}
if (term_types.length === 3 && _.isEqual(term_types, ["channel", "topic", "with"])) {
const channel_name = this.operands("channel")[0];
this._sub = stream_data.get_sub_by_name(channel_name);
assert(this._sub !== undefined);
return this._sub.name;
}
if (term_types.length === 1) {
switch (term_types[0]) {
case "in-home":
Expand Down
92 changes: 85 additions & 7 deletions web/src/narrow.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ function create_and_update_message_list(filter, id_info, opts) {
});
}

// In case we have some already fetched messages, we add them to
// the msg_data, and build the list around it.
if (opts.fetched_message_ids !== undefined) {
const msgs = opts.fetched_message_ids.map((msg_id) => message_store.get(msg_id));
msg_data.add_messages(msgs);
}

msg_list = new message_list.MessageList({
data: msg_data,
});
Expand Down Expand Up @@ -171,6 +178,39 @@ function create_and_update_message_list(filter, id_info, opts) {
return {msg_list, restore_rendered_list};
}

function get_narrow_data_for_with_operator(fetched_messages, original_filter) {
let anchor;
const messages = fetched_messages.messages;
const msg_ids = messages.map((msg) => msg.id);

if (fetched_messages.anchor === LARGER_THAN_MAX_MESSAGE_ID) {
anchor = Math.max(...msg_ids);
} else {
anchor = fetched_messages.anchor;
}

// messages fetched with the `with` operator are always stream
// messages.
const msg = message_store.get(msg_ids[0]);
assert(msg.type === "stream");
const channel_name = msg.display_recipient;
const topic_name = msg.topic;

const adjusted_terms = [
{operator: "stream", operand: channel_name, negated: false},
{operator: "topic", operand: topic_name, negated: false},
{operator: "with", operand: `${anchor}`, negated: false},
];

const filter = new Filter(adjusted_terms);
const id_info = {target_id: anchor, local_select_id: anchor};
const is_narrow_updated =
!original_filter.has_operand("stream", channel_name) ||
!original_filter.has_operand("topic", topic_name);

return {filter, id_info, msg_ids, is_narrow_updated};
}

export function activate(raw_terms, opts) {
/* Main entry point for switching to a new view / message list.
Expand Down Expand Up @@ -207,7 +247,14 @@ export function activate(raw_terms, opts) {
if (raw_terms.length === 0) {
raw_terms = [{operator: "is", operand: "home"}];
}
const filter = new Filter(raw_terms);

// raw_terms with both `dm` and with operators are redundant, since dms
// don't have topics. Hence, we remove the `with` operator.
if (raw_terms.some((term) => term.operator === "dm")) {
raw_terms = raw_terms.filter((term) => term.operator !== "with");
}

let filter = new Filter(raw_terms);
const is_combined_feed_global_view = filter.is_in_home();
const is_narrowed_to_combined_feed_view = narrow_state.filter()?.is_in_home();
if (is_narrowed_to_combined_feed_view && is_combined_feed_global_view) {
Expand Down Expand Up @@ -277,7 +324,7 @@ export function activate(raw_terms, opts) {
const scope = Sentry.getCurrentHub().pushScope();
scope.setSpan(span);

const id_info = {
let id_info = {
target_id: undefined,
local_select_id: undefined,
final_select_id: undefined,
Expand All @@ -293,6 +340,12 @@ export function activate(raw_terms, opts) {
if (filter.has_operator("id")) {
id_info.target_id = Number.parseInt(filter.operands("id")[0], 10);
}
if (filter.has_operator("with")) {
const target_message_id = Number.parseInt(filter.operands("with")[0], 10);
if (message_store.get(target_message_id) !== undefined) {
id_info.target_id = target_message_id;
}
}

// Narrow with near / id operator. There are two possibilities:
// * The user is clicking a permanent link to a conversation, in which
Expand Down Expand Up @@ -401,7 +454,7 @@ export function activate(raw_terms, opts) {
return;
}
}
} else if (!opts.fetched_target_message) {
} else if (!opts.fetched_target_message && !filter.has_operator("with")) {
// If we don't have the target message ID locally and
// haven't attempted to fetch it, then we ask the server
// for it.
Expand Down Expand Up @@ -494,7 +547,7 @@ export function activate(raw_terms, opts) {
}
}

const {msg_list, restore_rendered_list} = create_and_update_message_list(
let {msg_list, restore_rendered_list} = create_and_update_message_list(
filter,
id_info,
opts,
Expand Down Expand Up @@ -567,7 +620,30 @@ export function activate(raw_terms, opts) {
}
message_fetch.load_messages_for_narrow({
anchor,
cont() {
cont(data, opts) {
if (filter.has_operator("with")) {
const updated_narrow_data = get_narrow_data_for_with_operator(
data,
filter,
);

if (updated_narrow_data.is_narrow_updated) {
filter = updated_narrow_data.filter;
id_info = updated_narrow_data.id_info;
opts.fetched_message_ids = updated_narrow_data.msg_ids;
opts.change_hash = true;

const updated_msg_list_data = create_and_update_message_list(
filter,
id_info,
opts,
);

msg_list = updated_msg_list_data.msg_list;
restore_rendered_list = updated_msg_list_data.restore_rendered_list;
handle_post_view_change(msg_list);
}
}
if (!select_immediately) {
render_message_list_with_selected_message({
id_info,
Expand Down Expand Up @@ -700,7 +776,7 @@ export function maybe_add_local_messages(opts) {
// Full-text search and potentially other future cases where
// we can't check which messages match on the frontend, so it
// doesn't matter what's in our cache, we must go to the server.
if (id_info.target_id) {
if (!filter.has_operator("with") && id_info.target_id) {
// TODO: Ideally, in this case we should be asking the
// server to give us the first unread or the target_id,
// whichever is first (i.e. basically the `found` logic
Expand All @@ -722,7 +798,9 @@ export function maybe_add_local_messages(opts) {
// first unread message, or the target_id (if any), whichever
// is earlier. See #2091 for a detailed explanation of why we
// need to look at unread here.
id_info.final_select_id = min_defined(id_info.target_id, unread_info.msg_id);
id_info.final_select_id = filter.has_operator("with")
? unread_info.msg_id
: min_defined(id_info.target_id, unread_info.msg_id);

if (!load_local_messages(msg_data)) {
return;
Expand Down
14 changes: 14 additions & 0 deletions web/src/narrow_state.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as blueslip from "./blueslip";
import {Filter} from "./filter";
import * as message_lists from "./message_lists";
import * as message_store from "./message_store";
import {page_params} from "./page_params";
import * as people from "./people";
import type {NarrowTerm} from "./state_data";
Expand Down Expand Up @@ -281,6 +282,19 @@ export function _possible_unread_message_ids(
return unread.get_msg_ids_for_stream(sub.stream_id);
}

if (
current_filter.can_bucket_by("channel", "topic", "with") ||
current_filter.can_bucket_by("channel", "with") ||
current_filter.can_bucket_by("with")
) {
const msg_id = Number.parseInt(current_filter.operands("with")[0], 10);
const msg = message_store.get(msg_id);
if (msg !== undefined && msg.type === "stream") {
return unread.get_msg_ids_for_topic(msg.stream_id, msg.topic);
}
return [];
}

if (current_filter.can_bucket_by("dm")) {
current_filter_pm_string = pm_ids_string(current_filter);
if (current_filter_pm_string === undefined) {
Expand Down
Loading

0 comments on commit 756e993

Please sign in to comment.