For filter values which don't exist or are invalid in some
way, we return false to show user that there are no messages
in the filter user is trying to render. Our previous behaviour
was to show all the messages and ignore the filter which
isn't good.
The only downside of this is that it makes it harder to control the
order of these tests; which isn't that important. And the structure
of naming each with its test order fundamentally requires renaming
files when adding/deleting tests, so if we want to control the default
test order, we'd be better off doing that by just hardcoding a list in
the test runner code.
Previously we were liable to have false positives in our tests here
because we did not reset the visible state for these selectors, this
commit adds a helper and relevant calls to it in order to prevent such
false positives.
This commit changes some fragile selectors (like
`a[href=#link]`) to more stable selectors because they
are more prone to break from doing something normal
like adding another link in the app.
It also solves an inconsistency in `07-navigation.ts`,
where the subscription overlay was opened by clicking
on the header stream instead of the menu list.
It also fixes a rare flake (in `07-navigation.ts`), where
the close button of subscription overlay was not clicked
due to a delay in the opening. The delay was caused by
clicking the header stream to open subscription overlay
which caused unnecessary loading of the stream
setting(Verona).
As we are using the 'navigate_to' function to navigate
the links on the left sidebar, It'd be more clear to rename
the function to 'navigate_using_left_sidebar'.
Also adding '#left-sidebar' when selecting the element,
to be sure it will select the element from the left sidebar.
We recently added the commit to add the log-out call
after each test (52706908b).
This commit cleans that approach by using
just one log-out call after the test is executed at
`common.ts`
This commit adds waitForFunction to wait till the background mouse events
are enabled after closing the modal in the settings test.
This change is needed to avoid the failure that will be caused after we
change the code to handle re-enabling of mouse events only at one place
using 'hidden.bs.modal' event of bootstrap, as this event is fired only
after the modal is completely hidden, and we would want the mouse events
to be enabled before using clicks in further tests.
We now consistently set the PM counts for the right
sidebar toggle in unread_ui, similar to what we
do for the overall counts in the left sidebar toggle.
(Use a thin window to see the code in action.)
This breaks a dependency cycle.
In passing I improve the test coverage for the
actual job that pm_list still does (updating its
own total count in the "Private Messages" section).
This data structure has never been one that we actually render into
the DOM; instead, its role is to support clicking into view that
contain muted streams and topics quickly.
This downgrade makes that situation much more explicit, and is also
useful refactoring to help simpify the upcoming changes in #16746.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
I lift this function out of message_store to
break some dependencies, and it's also more
consistent with the rest of the codebase:
alert_words.process_message
pm_conversations.process_message
recent_topics.process_messages
recent_senders.process_message_for_senders
We can do further cleanup to make these names
consistent (and possibly have them all work in
bulk), but that's out of the scope of the current PR.
We move the message_store.add_message_metadata function
(and all its dependencies) into a new module called
message_helper and rename the function to process_new_message.
(It does a bit more than adding message metadata, such
as updating our message store.)
We also have a "protected" interface now in message_store,
so that message_helper can access the message store:
update_message_cache
get_cached_message
Because update_message_cache is identical to
the former create_mock_message, we just renamed it
in the tests.
Most callers should use these functions:
message_helper.process_new_message (setting)
message_store.get (getting)
It's slightly annoying that the setter interface
is in a different module than the getter interface,
but that's how you break a bunch of dependencies.
We also extract the tiny message_user_ids class:
user_ids()
add_user_ids()
All the code moves here are pretty trivial, and
the code that was moved maintains 100% line
coverage.
The module name `message_helper` is not ideal, but it's a single
function and it'll save time to just do the topology change now and
leave thinking through the right name to later.
When migrating from Casper to Puppeteer, some tests
were missed for adding log-out calls at the end.
This commit adds log-out calls to those missed tests.
Also, As we are resetting the test database after each
run (from 99f8be6a12) it will better to log out because we are
not resetting Tornado's internal state. It'd help us prevent
any future flakes.
Before this we did not have remove event in server_events_dispatch.js
for the user group delete event even though server had. This was
leading to blueslip errors. Extracted the logic which was used in
success() of channel.del for user_groups into the remove case in
server_events_dispatch. Also removed the redundant reload call as
we already do that in server events.
This is a mostly verbatim extraction.
I re-phrased one line of code to work around a lint
false alarm. (Look for `preamble` in the diff.)
There are about 8 lines missing coverage here, so
the new module might be a good candidate to get
100% line coverage on.
Before this change, you would need to remove 74
edges from our dependency graph to make it
acyclic. Now it's 72.
This change does not impact the overall complexity
of our dependency graph (at least in terms of the
number of edges that we would need to remove to get
a tree), but it does clarify the picture a bit.
* `op` (operation) field, added in f6fb88549f, was never intended for
`custom_profile_fields` event. This commit removes the `op` as it doesn't
have any use in the code.
* As a part of cleanup, this also eliminates the schema check warnings
for `custom_profile_fields` event, mentioned in #17568.