This removes a bit of complexity. If a piece of
settings code needs to render a stream with
subscribers, it just asks for it.
We no longer have the brittle, action-at-a-distance
mechanism of mutating the subscriber count on to
the stream_data version of a sub.
Stream subs are pretty small, so making copies of
them is cheap, and the blueslip timings from the
previous commit can help confirm that.
There is some discussion of putting `subscriber_count`
on the Stream model, which may eventually get us
away from tracking it in `peer_data.js`, but we will
cross that bridge when we get there. See
https://github.com/zulip/zulip/issues/17101 for
more details.
The weekly stream traffic is a better tiebreaker
for stream typeaheads than subscriber count, as
it's more directly a measure of a stream's current
relevance.
Normally stream traffic and subscriber counts are
closely correlated, but a good example for me is
the #twitter feed on czo, which only has 80 subscribers,
but which gets more traffic than our #integrations
stream (with 16k subscribers). I would rather
see #twitter win the tiebreaker (if it even got
to the tiebreaker).
The main motivation behind this fix, though, is
to break our dependency on peer_data, which has
some upcoming changes that will introduce some
performance tradeoffs, and I want one less place
to audit.
Also, it will be easier long term to share this
code with mobile if we don't require mobile
to pull in our peer_data dependency. (The webapp
has different forces than mobile that dicate
our data structures.)
We use day_old calculated based on day instead of hours to
render last seen values. This fixes us incorrectly quoting
anything 24 - 48 hours ago as Yesterday and
incorrectly quoting `time` that are Yesterday
but < 24 hours ago in 'x hours ago' format.
We were adding `expanded` class to left-sidebar when searching
for streams even if the left-sidebar was not in the popover state.
This cased confusion with popovers.any_active returning true,
when actually it is not.
topic_generator previously included an entire lazy generator
combinator library that was used four times. These straightforward
equivalent loops might not be as fun but they are way simpler.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Commit f0f6138f01 deleted the
translation for "Saved. Please <a class='reload_link'>reload</a> for
the change to take effect." which caused puppeteer test 16-settings to
fail as it had hardcoded the translation for German. This commit
changes the expected text and hence fixes the failing puppeteer test.
After this change all peer_data functions consistently
use stream_id rather than some "sub" object whose
data type is complicated by all sort of fields that
don't really concern how we track subscribers.
The goal here is to make all our peer_data functions
basically work in id space. Passing a full `sub`
to these functions is a legacy of when subscriber
info was attached to a full stream "sub" object,
but we don't care about anything sub-related
(color, description, name, etc.) when we are
dealing with subscriptions.
When callers pass in stream_id, you can be more
confident in a quick skim of the code that we're
not mutating anything in the "sub".
This de-clutters stream_data a bit. Since our
peer data is our biggest performance concern,
I want to contain any optimizations to a fairly
well-focused module.
The name `peer_data` is a bit of a compromise,
since we already have `subs.js` and we use
`sub` as a variable name for stream records
throughout our code, but it's consistent with
our event nomenclature (peer/add, peer/remove)
and it's short while still being fairly easy
to find with grep.
This sets us up to use better system-wide data structures
for tracking subscribers.
Basically, instead of storing subscriber data on the
"sub" objects in stream_data.js, we instead have a
parallel data structure called stream_subscribers.
We also have stream_create, stream_edit, and friends
use helper functions rather than accessing
sub.subscribers directly.
In commit ebea17b9a6,
we added an extra fetch to get accurate data for the top
items in recent topics table.
But the `narrow` parameter wasn't passed to the endpoint,
this resulted in fetching the user's overall message
history including the muted streams/topics which aren't
required by the recent topics table.
`operators` can be replaced as we set the same value for
the `narrow_state` module and the narrowed message list's
filter, when activating the narrow.
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.
Steve asked me to remove this, since the tictactoe game was always
intended as a proof of concept. Now that we have poll and todo
widgets, the sample code for tictactoe has much less value.
We replace the content and type in test_widgets.py to maintain
coverage.
This fixes a bug where the autocomplete for topics
deleted all the text content, if the topic jump is used
without entering any text.
The topic typeahead is automatically set up, on entering
the ">" key for stream completions. Therefore there is a
case where the user can select a typeahead item without
entering any text.
Thus the token length will be 0 and `beginning.slice(0, -0)` returns
"" instead of the `beginning` string. The case is only relevant for
"topic_list" completion as we don't set up the typeahead for empty
strings.
Fix this by reverting a hunk of
48f5e5179a, adding a test.
Fixes#16599.
Co-authored-by: Rohitt Vashishtha <aero31aero@gmail.com>
Refactor test_video_link_compose_clicked into seperate tests for:
No video provider.
Jitsi as the provider.
Zoom as the provider.
BigBlueButton as the provider.
While working on shifting toward native browser time zone APIs
(#16451), it was found that all but very recent Chrome and Node
versions reject certain legacy timezone aliases like US/Pacific
(https://crbug.com/364374).
For now, we only canonicalize the timezone property returned in user
objects and not the timezone setting itself.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We now can send an implied matrix of user/stream tuples
for peer_add and peer_remove events.
The client code basically does this:
for stream_id in event['stream_ids']:
for user_id in event['user_ids']:
update_sub(stream_id, user_id)
We used to send individual events, which gets real
expensive when you are creating new streams. For
the case of copy-to-stream case, we should see
events go from U to 1, where U is the number of users
added.
Note that we don't yet fully optimize the potential
of this schema. For adding a new user with lots
of default streams, we still send S peer_add events.
And if you subscribe a bunch of users to a bunch of
private streams, we only go from U * S to S; we can't
optimize it down to one event easily.
Upstream has slightly changed the whitespace around stashes. Take
this opportunity to clean up the extra blank lines we were outputting.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Using web_public_guest for anonymous users is confusing since
'guest' is actually a logged-in user compared to
web_public_guest which is not logged-in and has only
read access to messages. So, we rename it to
web_public_visitor.
Since the denmark stream stub has subscribed = true,
we include the current user id in it's subscribers set.
We remove user `me` from sweden stream stub for the same
reason.
For streams in which only full members are allowed to post,
we block guest users from posting there.
Guests users were blocked from posting to admin only streams
already. So now, guest users can only post to
STREAM_POST_POLICY_EVERYONE streams.
This is not a new feature but a bugfix which should have
happened when implementing full member stream policy / guest users.
For the lines of code that I changed here, we were
getting field reports that the below code
was getting `undefined`:
emoji.all_realm_emojis.get(r.emoji_code)
It's not really clear to me how this could happen,
but we definitely should fail softly here. We
still report it as an error, but we let the function
return and don't trigger a TypeError.
If there's a legitimate reason for realms to delete
realm emojis, we should either downgrade this to a
warning or consider a strategy of back-fixing messages
when realm emojis get deleted.
We rename all_everyone_warn_threshold to
wildcard_mention_large_stream_threshold as we would
be adding wildcard_mention_policy and this
constant will also be used to show error
in case when wildcard_mention_policy is set
to admins only.
In c563cdba61 we imported the generated
pygments data from outside `/shared` folder. This had a couple of
problems:
* Using `require` was the wrong way to do the import in ES6 modules.
* Since we get the data from outside `/shared`, clients like
zulip-mobile would not receive it - this case had to be handeled.
Here, we fix the above problems by receiving the data when initializing
through fenced_code.initialize, and when the pygments data structure is
empty (for zulip-mobile) we fallback to the old header structure without
the data-code-language tag.
Also, this commit does a small refactor to improve the way we fetch
canonicalized_alias from pygments_data.
Tests amended.
The failures saying incorrect password were caused due to
change in focus. Some actual code of ours calls focus on
the modal when opened but puppeteer was starting to type
before this occurs due to which the test was only able
to enter a part of string.
This was happening with change full name too but less
frequently as it's a relatively shorter string.
As a fix, we wait till the focus is on the modal and then
start typing.
Instead of prohibiting ‘return undefined’ (#8669), we require that a
function must return an explicit value always or never. This prevents
you from forgetting to return a value in some cases. It will also be
important for TypeScript, which distinguishes between undefined and
void.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
It would conflict with the stream_id variable after migration to an
ES6 module, and adds no real convenience over stream_sub().
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit enables keyboard support for user info popovers for
navigating through popover options using up/down keys.
We add get_user_info_popover_items function, whose implementation
is different from other similar functions. Instead of using
popover_data.$tip we directly use $("div.user-info-popover")
because when we open the popover of bot owner from the bot
popover, the element which opens the popover is removed from
DOM and popover_data is undefined.
This commit replaces the "Reply mentioning user" option with "Copy mention
syntax" for user info popovers that are not opened from a message.
Clicking on "Copy mention syntax" will copy the mention syntax of user to
clipboard.
This change is done because user popovers not opened from message are not
linked to any message.
Previously, compose_ui.autosize_textarea didn't work while editing
messages in many cases (uploading files, typeaheads, keydown handling,
etc.).
Refactored the autosize_textarea function in compose_ui to work
while editing messages too and added appropriate argument for the
introduced function parameter at all occurences of the function
use.
Also, updated the corresponding test cases.
On uploading a few files from markdown_preview mode of compose box and
then switching back to edit mode, the compose box doesn't get resized.
It even doesn't allow to scroll through the content.
Fixed this by switching back to the edit mode everytime user uploads
some file in markdown_preview mode as there's no use of staying in
markdown_preview mode anyways after uploading a file as the preview
doesn't get updated.
Also, updated the corresponding test cases.
Fixes: #16296.
This mimics the backend logic for adding the data-attribute -
to know what Pygments language was used to highlight the code
block - in locally echoed messages.
New test added checks our logic for canonicalizing pygments alias
(for both frontend and backend).
Other fixtures and tests amended.
We need this information in the frontend to:
* Display the 'view in playground' option for locally echoed messages.
* When we add a UI settings for realm admins to configure their
playground choices, we'll need to use these canonicalized aliases
for displaying the option.
Hence, this tweaks the tool which generates pygments_data.json to contain
the data we need.
Bumping major PROVISION_VERSION since folks need to provision in both
directions.
Tests amended.
Wait for disable_stream_notifications selector to be visible
before clicking as it could cause flakes if the test tries
to click without it being visible.
Added a stronger validation of waiting for text "Verona" to
appear but that didn't really seem to have worked though it
seemed like fixing the flake by passing ~600 runs.
So, change the puppeteer click to a click through evaluate
as we had experiences where page.click() didn't work sometimes.
Though this has passed 1000 runs on CI, I'm not very certain
if this fixed it as this test passed 1000 times with my previous
PR fixing the same flake.
Currently, compose box placeholder text for PMs only gets updated
when the focus shifts to it.
With this change, the text is now also updated if recipients are
added or removed.
Fixes#15897.
The code to run single files was added
in c15695e514,
and it's just kinda strange code.
We already do a lot of file logic in Python
to check for line-coverage, so it's easier
to just have all the logic in Python.
This adds a new feature--you can now specify
the actual file:
./tools/test-js-with-node frontend_tests/node_tests/people.js
(This is helpful if you just want to use
shell autocomplete.)
Another minor change is that if you specify
individual files, we won't sort them. This is
important when you're trying to hunt down test
leaks.
Finally, we have a nicer message if we can't find
the file.
Because `util` is so late in the alphabet, this
leak never surfaced in practice, but I tried running
the node tests in reverse, and this leak came
up if you ran `util` before `stream_list`. I guess
it's nice that `stream_list` actually exercises
the difference between a dumb sort and an
Intl-aware sort.
It's possible that we should just assume that
Intl.Collator is always available at this point,
which would eliminate the need for this test.
There is good reason to do this (explanation is bit long!). With the
TypeScript migration, and the require and ES6 migrations that come
with it, we use require instead of set_global which loads the entire
module. Suppose we have a util module, which is used by some other
module, say message_store, and util is being required in message_store
since it is removed from window. Then, if a test zrequires
message_store first, and then zrequires the util module qand mocks one
of its methods, it will not be mocked for the message_store
module. The reason is:
1. zrequire('message_store') leads to require('util').
2. zrequire('util') removes the util module from cache and it is
reloaded. Now the util module in message_store and the one in
the test will be different and any updates to it in tests won't
be reflected in the actual code.
Which can lead to confusion for folks writing tests. I'll mention this
can be avoided doing zrequire('util') first but...that is not ideal.
And, since there was one outlier test that relied on this behavior,
we add the namespace.reset_module function.
Fixes#16252.
icon* classes are used by bootstrap for displaying glyphicons.
We removed these classes in our custom version of bootstrap 2.1.1;
but since our reset to v2.3.2, they have been added again and hence
any classes starting with icon* in zulip will have to be renamed.
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.
This completes the remaining work required to support
addition of all members of another stream.
This allows the creation of stream pills on pasting
the #streamname and copying it from the stream pill.
The user pills uses email ids instead.
And also allows creating stream pills when the user
hides the typeahead.
Tested by commenting out the "set_up_typeahead_on_pills"
line in `stream_edit.js`.
A `node_tests/stream_pill.js` file has been created
for the node tests and the other half of the coverage
check takes place in `node_tests/stream_edit.js`.
This fixes the regression introduced in the pervious
commit to regain the 100% line coverage in `user_pill.js`
as well as `stream_pill.js`.
The new `stream_edit.js` mainly tests for:
* The stream related queries of the typeahead in `user_pill.js`
* The "Add subscribers" event handlers.
* The event handler which displays the settings for a stream.
Now that all casper tests have been migrated to
puppeteer, there's no need for having casper
related things.
Removed the casperjs package and removed/replaced
casper in few places with puppeteer.
Only removed few of them which I'm confident
about. Also didn't make any changes in docs
as it would be easier to remove them while
adding puppeteer docs.
check_messages_test should have table set to zfilt
instead of zhome as we are narrowed to only starred
messages.
Found it as the test failed with the previous commit.
The next adds a few tests which heavily rely on
check_messages_sent. There were some weird errors,
this fixed those. I think the errors were due to
us navigating multiple times and this function
not waiting for the messages to become visible.
These `waitForSelector`s appear just after page loads.
Though they worked most of the times, a few clicks weren't
getting registered because of these selectors not appearing
and thus causing flakes as the modal takes time to appear.
Adding visible: true asserts that it's visible and not just present.
In few rare cases the click on display settings section wasn't working
which was causing the test to stay in the "your account" settings section.
This lead to a waitFor fail.
The screenshot in this failed CircleCI build suggests the above.
https://app.circleci.com/pipelines/github/chdinesh1089/zulip/525/workflows/cd77e269-6a3e-4283-b765-d1c4584ccf35/jobs/1807/artifacts
This and the previous commit along with the changes to prevent
logging out of user on changing password were tested by running
this test 1200 times and all of them passed!
This handles a rare race condition that occurs when the session hash
is not updated by the backend during the password change process.
This mostly occurs in puppeteer tests, but could occur to a user.
In list_render.js, [...list] requires list to be an array, and
widget.set_sorting_function(...opts.init_sort) requires init_sort to
be an array.
This allows the Node tests to pass in Babel strict mode. We currently
use loose mode for performance, and so we should test in loose mode as
well; but we must never depend on loose mode for correctness, since
individual Babel transformations may stop being applied as our browser
support baseline improves.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Clicking on the copy-to-clipboard button triggers the clipboard.js
API to dynamically set the text to be copied. This text is the
actual code content from the sibling <code> element (extracted
though jQuery text() method).
The html structure would now look like:
<div class="codehilite">
<pre>
<button> The copy button </button>
<span></span>
<code>......</code>
</pre>
</div>
Additionally, this preserves the original code formatting of
the codeblock during copy-paste.
Tests amended.
Fixes: #15208
`update_message_flags` events used `operation` instead of `op`, the
latter being the standard field used in other events. So add `op`
field to `update_message_flags` and mark `operation` as deprecated,
so that it can be removed later.
This flake occurs because the Verona dropdown menu couldn't be
clicked, in rare cases, because puppeteer would click it too
quickly before it appears and then fails. To fix this we wait for it
to fully appear and then click it. Around 1000 runs
passed without a failure.
The error the flake caused was:
TimeoutError: waiting for selector
"#org-submit-notifications[data-status="unsaved"]" ...
The dispatch test here really only cares that values
get passed on.
Note that the dispatch code ignores the email field, because
we only send subscription/update events to the user
whose subscription has changed.
This fixes a bug with the original frontend-side implementation for
has: filters, where it would incorrectly not match content in cases
where the message's nesting structure did not have an outer tag.
Bug was introduced in 02ea52fc18.
Fixes#16118.
This commit adds "role" field to the Subscription objects passed to
clients. This is important preparation for being able to work on the
frontend for this feature.
The dispatch for presence is a trivial one-liner,
so the test just makes sure three important parameters
get passed along.
We will eventually want to use the fixtures data in
other presence-related tests, but for now the only
goal is to make it pass the schema checks.
Also add helper functions needed.
`select_item_via_typeahead` has been ported from casper
and is exactly same in puppeteer to as I couldn't find
any better way for that purpose.
I had a misconception with hidden and visible options
and thought `hidden: false` was same as `visible: true`
and other way too.
But `hidden: false` or `visible: false` does nothing
more than checking if the selector exists.
Also, to mention, `visible: false`'s were fixed in
33e19fa7d1
We need to replace 'visible: false' with 'hidden: true' to wait
for elements to get hidden. Using 'visible: false' just checks
whether the selector exists or not and does not check whether
the element is hidden or not.
Since our Webpack config passes pre-minified JS files to
script-loader, they can’t be used as modules. Use the normal
unminified version, letting Webpack minify it and give us source maps.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We also just make the test express what's actually
happening in the code; we just pass the entire
"exports" section of the event to the settings code
and let it do its thing.
We follow the naming convention.
I also arbitrarily assign the "op" of
"add" to the attachment event, even
though we don't meaningfully test it.
The situation with attachment from the
dispatch test point of view is that
we just want to test that the one line
of code that calls into attachments_ui
(for all three ops) does get dispatched
correctly. We eventually want to get
deeper coverage there, but attachments_ui
wasn't written in the most test-friendly
way. I think it might actually be easy
to fix up attachments_ui to make it a
bit easier to test, but it's out of the
scope of my current PR.
The benefit here is check-node-fixtures
now gives a more concrete plan for
moving schemas to event_schema.py.
We extract test_realm_emojis, and we make
the name of the event more explicit (adding
the __update suffix).
We also add the "op" of "update" here, which
is sort of a quirk of the api, since we don't
actually have alternatives like add/remove,
and therefore the current frontend code doesn't
look at the "op", and thus the original tests
never had to provide a correct value for it.
We move this function from `user_pill.js` to `pill_typeahead.js`.
The function has also been renamed to `set_up`.
The move was made because there are plans to update the pills
typeahead (i.e. to include user-groups/streams in the results).
Thus this function should not belong in `user_pill.js`.
This commit allows skipping over any disabled tabs
that are in the middle when using the left or right
arrow keys.
We also add `enable_tab` to the `components` API.
After the latest message in a stream is deleted, we should update
the max_message_id in the stream.
Removed false comment in message_util.get_messages_in_topic
this method only takes 2ms for 10,000 messages loaded locally.
Fixes#15992.
If the last message of the topic was deleted, we update the stored
message_id in the topic history so that the topic order in topic_list
is updated correctly.
When messages weren't locally echoed,
`wait_for_full_processes_message` fails to assert that the message
is being sent. It was figured out by tabbott that the messages
aren't locally echoed because of content no loading completely.
So, this commit changes the `log_in` function to wait till
the selector '#zhome .message_row' is visible which indicates
that the cotent is loaded.
Removed the waitForNavigation in the `log_in` as it would
become a redundant check after this change.
Also removed this same check present in 03-compose.js which
becomes reduntant as we already do it in `log_in`.
This test changes user password causing all subsequent
tests to fail. Since rechanging the password would be
a redudant test/task or having manually entering the
new password in every test after this aren't good ideas,
this commit makes the settings test run in the end by
renaming it be numbered 16. It is assumed that we'll
end up having 16 tests seeing the number of tests in
casper and considering 03 and 02 from casper are being
combined as 02-message-basics.js. Though 03 of casper
has not yet been added in message-basics test it will
soon be added.
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>
Commit 114cc1ec25 (#15949) introduced a
subtle bug because sortablejs provides both a CJS module and an ES
module that expose different interfaces to CJS require() under
Webpack. This difference will disappear when we convert
settings_profile_fields to an ES module.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Steve Howell
Rohitt Vashishtha
Aman Agrawal
Anders Kaseorg
Riken Shah
YashRE42
Ryan Rehman
Tim Abbott
Vishnu KS
sahil839
Greg Rowe
Sumanth V Rao
Dinesh
Anders Kaseorg
Priyansh Garg
neiljp (Neil Pilgrim)
Priyansh Garg
Vinit Singh
Priyank Patel
Mohit Gupta
orientor
Sahil Batra