This is a deceptively ugly diff. It makes
the actual code way more tidy.
I basically inlined some calls to mock_module
and put some statements in lexical order.
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>
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>
Previously, the `muting_enabled` property of
MessageListData class was used to indicate whether
some messages in the message list need to be
filtered due to topic muting, depending on the
narrow. For example, we exclude messages belonging
to muted topics from stream narrows, but not from
search narrows.
The name `muting_enabled` is a bit confusing, and hence is
changed to `excludes_muted_topics`.
It is also important that the name be specific, since
a similar new property will be added for user mutes
in future commits.
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>
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>
The reason for this change is that, this is where `Filter` and
actual tracking of what messages are contiguous lives. This
will be beneficial when we will to move to a model where we
cache `MessageListData` objects for a large number of views.
Explicitly stubbing i18n in 48 different files
is mostly busy work at this point, and it doesn't
provide much signal, since often it's invoked
only to satisfy transitive dependencies.
We now treat util like a leaf module and
use "require" to import it everywhere it's used.
An earlier version of this commit moved
util into our "shared" library, but we
decided to wait on that. Once we're ready
to do that, we should only need to do a
simple search/replace on various
require/zrequire statements plus a small
tweak to one of the custom linter checks.
It turns out we don't really need util.js
for our most immediate code-sharing goal,
which is to reuse our markdown code on
mobile. There's a little bit of cleanup
still remaining to break the dependency,
but it's minor.
The util module still calls the global
blueslip module in one place, but that
code is about to be removed in the next
few commits.
I am pretty confident that once we start
sharing things like the typeahead code
more aggressively, we'll start having
dependencies on util. The module is barely
more than 300 lines long, so we'll probably
just move the whole thing into shared
rather than break it apart. Also, we
can continue to nibble away at the
cruftier parts of the module.
This commit includes a new `stream_post_policy` setting,
by replacing the `is_announcement_only` field from the Stream model,
which is done by mirroring the structure of the existing
`create_stream_policy`.
It includes the necessary schema and database migrations to migrate
the is_announcement_only boolean field to stream_post_policy,
a smallPositiveInteger field similar to many other settings.
This change is done to allow organization administrators to restrict
new members from creating and posting to a stream. However, this does
not affect admins who are new members.
With many tweaks by tabbott to documentation under /help, etc.
Fixes#13616.
In 452e226ea2 and
648a60baf6, we changed how `search:`
narrows work to:
(1) Never mark messages as read inside searches (search:)
(2) Take you to the bottom, not the first unread, if a `near:` or
similar wasn't specified.
This is far better behavior for these use cases, because in these
narrows, you can't actually see all the context around the target
messages, so marking them as read is counterproductive. This is
especially important in `has:mention` where you goal is likely
specifically to keep track of which threads mentioning you haven't
been read. But in many other narrows, the current behavior is
effectively (1) setting the read bit on random messages and (2) if the
search term matches many messages in a muted stream with 1000s of
unreads, making it hard or impossible to find recent search matches.
The new behavior is that any narrow that is structurally a search of
history (including everything that that isn't a stream, topic,
pm-with, "all messages" or "private messages") gets that new behavior
of being unable to mark messages as read and narrows taking you to the
latest matching messages.
A few corner cases of interest:
* `is:private` is keeping the old behavior, because users on
chat.zulip.org found it confusing for `is:private` to not mark
messages as read when one could see them all. Possibly a more
complex answer is required here.
* `near:` narrows are getting the new behavior, even if it's a stream:
+ topic: narrow. This is debatable, but is probably better than
what was happening before.
Modified significantly by tabbott for cleanliness of implementation,
this commit message, and unit tests.
Fixes#9893. Follow-up to #12556.
Fixes commit id 648a60baf6. When
allow_use_first_unread_when_narrowing() is false last message of
narrow is shown in view.
Comments rewritten by tabbott to explain in detail what's happening.
This seems like a small change (apart from all the
test changes), but it fundamentally changes how
the app finds "topic" on message objects. Now
all code that used to set "subject" now sets "topic"
on message-like objects. We convert incoming messages
to have topic, and we write to "topic" all the way up
to hitting the server (which now accepts "topic" on
incoming endpoints).
We fall back to subject as needed, but the code will
emit a warning that should be heeded--the "subject"
field is prone to becoming stale for things like
topic changes.
This continues the effort to isolate "subject" references
to util calls.
Also, we fix a comment.
Finally, we use canonicalized operators in a switch
statement.
We now use narrow_state.filter() everywhere. The
two functions did the same thing, and I slightly
prefer the concise name, which was already in use
in lots of places.
Steve Howell
Anders Kaseorg
Abhijeet Prasad Bodas
Ryan Rehman
Steve Howell
Anders Kaseorg
Mohit Gupta