topics: Fix unnecessary hashchange calls while using with operator.

Previously, while using "with" operator, even when the narrow
was not updated by the operator, hash change requests were
made.

This commit introduces a new variable - "narrow_requires_hash_change"
which determines whether the narrow was updated and thus
requires a hash change or not.

Fixes #30862

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;

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 (
+                        if (filter.narrow_requires_hash_change) {
-                            !filter.requires_adjustment_for_moved_with_target &&
-                            filter.has_operator("with")
-                        ) {
                             // 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({

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) {