diff --git a/web/src/filter.ts b/web/src/filter.ts index 534721c9a4..9c13a05bdd 100644 --- a/web/src/filter.ts +++ b/web/src/filter.ts @@ -296,11 +296,13 @@ export class Filter { _predicate?: (message: Message) => boolean; _can_mark_messages_read?: boolean; requires_adjustment_for_moved_with_target?: boolean; + narrow_requires_hash_change: boolean; constructor(terms: NarrowTerm[]) { this._terms = terms; this.setup_filter(terms); this.requires_adjustment_for_moved_with_target = this.has_operator("with"); + this.narrow_requires_hash_change = false; } static canonicalize_operator(operator: string): string { @@ -1623,6 +1625,9 @@ export class Filter { const adjusted_terms = Filter.adjusted_terms_if_moved(this._terms, message); if (adjusted_terms) { + // If the narrow terms are adjusted, then we need to update the + // hash user entered, to point to the updated narrow. + this.narrow_requires_hash_change = true; this.setup_filter(adjusted_terms); } this.requires_adjustment_for_moved_with_target = false; diff --git a/web/src/message_view.js b/web/src/message_view.js index 0a8e4b0c10..67a14e8e9f 100644 --- a/web/src/message_view.js +++ b/web/src/message_view.js @@ -661,14 +661,12 @@ export function show(raw_terms, opts) { validate_filter_topic_post_fetch: filter.requires_adjustment_for_moved_with_target, cont() { - if ( - !filter.requires_adjustment_for_moved_with_target && - filter.has_operator("with") - ) { + if (filter.narrow_requires_hash_change) { // We've already adjusted our filter via // filter.try_adjusting_for_moved_with_target, and // should update the URL hash accordingly. update_hash_to_match_filter(filter, "retarget topic location"); + filter.narrow_requires_hash_change = false; } if (!select_immediately) { render_message_list_with_selected_message({ diff --git a/web/tests/filter.test.js b/web/tests/filter.test.js index 1cf85fb335..18e3b2149f 100644 --- a/web/tests/filter.test.js +++ b/web/tests/filter.test.js @@ -1821,6 +1821,60 @@ test("try_adjusting_for_moved_with_target", ({override}) => { assert.deepEqual(filter.terms(), [ {operator: "dm", operand: "user3@zulip.com", negated: false}, ]); + + // When message id attached to `with` operator is found locally, + // and is present in the same narrow as the original one, then + // no hash change is required. + terms = [ + {operator: "channel", operand: "Verona", negated: false}, + {operator: "topic", operand: "Test 2", negated: false}, + {operator: "with", operand: "17", negated: false}, + ]; + filter = new Filter(terms); + filter.try_adjusting_for_moved_with_target(); + assert.deepEqual(filter.narrow_requires_hash_change, false); + + // When message id attached to `with` operator is not found + // locally, but messages fetched are in same narrow as + // original narrow, then no hash change is required. + terms = [ + {operator: "channel", operand: "Verona", negated: false}, + {operator: "topic", operand: "Test 2", negated: false}, + {operator: "with", operand: "1", negated: false}, + ]; + filter = new Filter(terms); + filter.try_adjusting_for_moved_with_target(); + // now messages are fetched from server, and a single + // fetched message is used to adjust narrow terms. + filter.try_adjusting_for_moved_with_target(messages["17"]); + assert.deepEqual(filter.narrow_requires_hash_change, false); + + // When message id attached to `with` operator is found locally, + // and is not present in the same narrow as the original one, + // then hash change is required. + terms = [ + {operator: "channel", operand: "Verona", negated: false}, + {operator: "topic", operand: "Test 2", negated: false}, + {operator: "with", operand: "12", negated: false}, + ]; + filter = new Filter(terms); + filter.try_adjusting_for_moved_with_target(); + assert.deepEqual(filter.narrow_requires_hash_change, true); + + // When message id attached to `with` operator is not found + // locally, and messages fetched are in different narrow from + // original narrow, then hash change is required. + terms = [ + {operator: "channel", operand: "Verona", negated: false}, + {operator: "topic", operand: "Test 2", negated: false}, + {operator: "with", operand: "1", negated: false}, + ]; + filter = new Filter(terms); + filter.try_adjusting_for_moved_with_target(); + // now messages are fetched from server, and a single + // fetched message is used to adjust narrow terms. + filter.try_adjusting_for_moved_with_target(messages["12"]); + assert.deepEqual(filter.narrow_requires_hash_change, true); }); function make_private_sub(name, stream_id) {