These mock objects did not buy any ease in testing, as
evident from the fact that this commit hardly contains
any changes to the tests themselves.
This commit also removes some unnecessary `filter: undefined`,
parameters sent to MessageList constructor.
This logic is already thoroughly tested in node tests for
the `MessageListData` class, and the `unmuted_messages`
method of `MessageList` only directly call the corresponding
method of the data class.
Use fully resolvable request paths because we need to be able to refer
to third party modules, and to increase uniformity and explicitness.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We now just use a module._load hook to inject
stubs into our code.
For conversion purposes I temporarily maintain
the API of rewiremock, apart from the enable/disable
pieces, but I will make a better wrapper in an
upcoming commit.
We can detect when rewiremock is called after
zrequire now, and I fix all the violations in
this commit, mostly by using override.
We can also detect when a mock is needlessly
created, and I fix all the violations in this
commit.
The one minor nuisance that this commit introduces
is that you can only stub out modules in the Zulip
source tree, which is now static/js. This should
not really be a problem--there are usually better
techniques to deal with third party depenencies.
In the prior commit I show a typical workaround,
which is to create a one-line wrapper in your
test code. It's often the case that you can simply
use override(), as well.
In passing I kill off `reset_modules`, and I
eliminated the second argument to zrequire,
which dates back to pre-es6 days.
Found by running the tests after
sed -i 's/\.with(/.toBeUsed().with(/g' frontend_tests/node_tests/*.js
Signed-off-by: Anders Kaseorg <anders@zulip.com>
When idle, we try to backfill messages and in the end reselect
the closest message in the list, which can be a unread message
if present.
When recent topics is open, we can backfill messages; but
shouldn't select the message_id otherwise it will mark the
message as read if the message is unread while triggering
`message_selected.zulip`.
We no longer export make_zjquery().
We now instead have a singleton zjquery instance
that we attach to global.$ in index.js.
We call $.clear_all_elements() before each module.
(We will soon get even more aggressive about doing
it in run_test.)
Test functions can still override $ with set_global.
A good example of this is copy_and_paste using the
real jquery module.
We no longer exempt $ as a global variable, so
test modules that use the zjquery $ need to do:
const $ = require("../zjsunit/zjquery");
This commit replaces all `with_stub` calls and
explicitly calls `make_stub` instead.
The `with_stub` helper does not add much clarity
hence we now use scoped stub objects instead.
This de-indents some blocks where scoping isn't
required, for example when there is a single
stub object inside a `run_test` function.
With this change, we also need to explicitly
assert `num_calls`.
We still need to write to these globals with set_global because the
code being tested reads from them, but the tests themselves should
never need to read from them.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit makes it so that MessageListData
methods always attempt to filter muted messages.
We later, in a new function
(`messages_filtered_for_topic_mutes`)
check if `excludes_muted_topics` is true or not,
and skip the filtering work if it isn't.
This new function consistently returns a new list.
This refactor will later allow us to write clean
and concise code as part of mute users.
This commit also refactors the muting tests
for MessageListData, which were earlier
spread across two `run_test` functions.
These tests should remain organized,
since similar tests will be added as part of
user mutes in future commits.
This is a minor refactor in the muting test in
`message_list`.
The `unmuted_messages` function filters out messages only
considering topic mutes, and not stream mutes.
The test previously made it look like we were testing
stream muting, by stubbing the `is_topic_muted` on the
basis of `stream_id`.
This also replaces the stub and uses real data instead.
The changes made in this commit are as follows:
* The `remove_messages` is moved to the `message_events.js`
file from `ui.js`.
* We refactor `MessageListData.change_message_id` to no
longer require an `opts` parameter as this function
just returns whether we need to rerender or not.
The blueslip error block can be removed since we made
the change to no long defer the data updates in
commit 3b5ba6b2c1,
this case can no longer occur.
The changes made in this commit are as follows:
* We remove the now unused `ui.find_message` which was added
in commit 1666403850.
* We change the function paramter to now accept message ids
instead of messages to eliminate redundant message ids to
message convertion as only the id is required.
* The remove method in MessageListData did not remove the
messages from the hash, it removed only from the items,
this fixes it.
* This commit also fixes a bug where messages are not added
to the current message list if an event is recieved where
messages are moved to this current narrow.
Only the message removal logic was present, which has been
refactored in this commit.
We were not updating the trailing bookend on deactivation of stream
if the user was narrowed to deactivated stream and this commit fixes
this.
For subscribed streams, we just show the trailing bookend with
content as 'This stream has been deactivated' and hide the
Unsubscribe button.
For unsubscribed streams, we change the content of trailing bookend
to 'This stream has been deactivated' and hide the Subscribe button.
Fixes#15999.
ES and TypeScript modules are strict by default and don’t need this
directive. ESLint will remind us to add it to new CommonJS files and
remove it from ES and TypeScript modules.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This is done to decouple our message view related update events
from MessageListData as there are plans to create multiple
MessageListData objects. Instead we update the `stored_messages`
which tracks the complete data for all messages.
This forces us to more explicitly document at the
top of the file what dependencies we are stubbing,
plus it's less magical.
Also, we may want to do occasional audits of
set_global to clean up places where we mock
things like stream_data, which are probably just
easier to use the real version of now that we
have cleaner APIs to set up stream data.
The modules most affected by this change are our
dispatch-oriented tests--basically, all the
modules that test handling of Zulip events
plus hotkey.js.
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
There is a bug and race issue that occurs when a message is selected
while we are in the process of reifying a locally echoed message,
raising the "Selected message id not in MessageList" error.
The code flow to get the exception is as follows:
* A user sends a message to the current narrow we are in.
* Before the new message event is received, we sent a message to
the same message list which renders it with a locally echoed id.
* One of the ways of getting the exception is to already have the
locally sent message selected, before receiving an acknowledgment
from the server.
* Thus the Message List Data's `selected_id` now points to the new
message id. The exception is raised on entering the `was_selected`
if block inside `message_list_view` which tries to re-select the
message.
Updating the `_rerender_message` code for this special case won't fix
the entire bug because, as mentioned above there are other ways of
getting the exception:
Ideally, after all our synchronous work (`echo.process_from_server`)
has completed we would expect the re-order and re-render work of the
`change_message_id` would occur first, due to the timer of the
setTimeout being set to 0.
However as evident from the race condition existing, this isn't always
the case. `change_message_id` function is responsible for 3 things:
updation, re-ordering and re-rendering.
The first one which is responsible for updating the message list's
local cache, occurs synchronously while for the latter two, they both
occur asynchronously.
Before the setTimeout which is responsible for the latter two actions,
is encountered the user might select the message by clicking or more
commonly by scrolling, which causes this message selection event to be
ahead of the setTimeout in the callback queue.
During this time frame, our race condition takes place.
And even though the message id is updated it's Message List is not
in the correct sort order, which leads to `closest_id` !== `id` in
`MessageList_select_id` being true and raising the exception.
Now, we only asynchronously call the re_render function, to guarantee
the data is always correct and UI updates should be done at the end.
Extended by tabbott to comment the setTimeout call.
Fixes#15346.
Tim Abbott
Steve Howell
Abhijeet Prasad Bodas
Anders Kaseorg
Aman Agrawal
ryanreh99
sahil839