If you have a group PM where some users have
three-digit user_ids and some with four-digit
user_ids (or similar), a huddle could effectively
be ignored when determining the order of
search search suggestions.
Basically, we need a way to canonically sort
user_ids in "huddle" strings, and it's somewhat
arbitrary whether you sort lexically or sort
numerically, but you do need to be consistent
about it.
And JS is not exactly helpful here:
> [99, 101].sort()
[ 101, 99 ]
This is a pretty obscure bug with pretty low
user-facing consequences, and it was never
reported to us as far as I know, but the fix
here is pretty straightforward.
We have had similar bugs of slightly more consequence
in the past. The reason this bug has shown
up multiple times in our codebase is that every
component that deals with huddles has slightly
different forces that determine how it wants
to serialize the huddle. It's just one of those
annoying things. Plus, bugs with group PMs
do tend to escape detection, since most people
spend most of their time either on streams
or in 1:1 PMs.
This is a pure code extraction. The current
code is buggy with respect to user_ids with
different lengths of digits, i.e. it does
a naive lexical sort instead of a numerical
sort. We'll fix that in the next commit.
We already have a loading indicator for fetching older
messages. Thus it makes sense to implement the same
for displaying newer messages.
We set the display of `bottom-messages-logo` to none,
to prevent displaying two loading indicators during
the initial message load.
Fixes#15060.
I consolidate most of our users toward the top
of the file, so that we don't have to clutter
up individual tests. This also avoids some
confusion where charles/maria got repeated
in different tests with different ids.
I also introduce a couple four-digit ids to
try to expose more bugs related to sorting.
Note that it's still easy to keep tests
isolated here, as we have always been able
to cheaply re-initialize `people.js` and then
add individual users back.
There are still some tests where it makes
sense to just declare users locally, especially
if we are mutating their data.
There are a few minor incidental cleanups here,
mostly involving replacing hard coded ids
with things like `maria.user_id`.
This creates a little bit of noise in some
tests where we don't care about users, but
it's worth avoiding confusion about which
users exist at which time. Also the noisy
aspects here may actually catch regressions.
Finally, if the noise gets annoying, we can
do things like rename "Ted" not to collide
with the "Test" stream.
Using "bob" as the current user was a bad
choice, as our convention is to use "me" or
"myself" or "alice" for the current user.
It also particularly complicated the tests
around Group PMs.
Now we have both "bob" and "myself", which
makes the intentions of the tests a little
more clear.
This commit adds code to live update the message edit history.
Message edit history is fetched and rendered again if the edit
history modal is open.
This also adds 'data-message-id' attribute to 'message-history'
when opening history modal element which is used for checking
whether the history modal opened is of the message which is
edited.
Fixes#15051.
This code generates the timestamp string to be shown to the user
from the given timestamp in unix format using moment.js.
We also render the timestamp in a pill.
Previously, we handled this code only in message_list_view.js.
Now we support rendering stream descriptions and some dynamic
elements can be rendered in them, so we extract this new module
and use it in both the places.
This adds support for syntax like: !time(Jun 7 2017, 6:30 PM) so that
everyone sees the time in their own local timezone. This can be used
when scheduling online meetings, etc.
This adds some hardcoded values for timezones, because of there
being no sureshot way of determining the timezone easily. However,
since the main way of using the feature should be a typeahead for
entering the time, this shouldn't be cause of much concern.
Fixes#5176.
We wrap the [reset] anchor tag in a button so that we can set 'disabled'
attribute on it. We change the styles to hide the [reset] button and the
pencil icon when the widget is disabled.
We also need to call `e.preventDefault()` in the event handler since now
the anchor tag behaves as a button.
Add methods to extract recent topics from received messages.
Process new messages as they are received.
Use new messages received from the server to extract recent_topics.
Node tests added.
Previously, we tried to read the value from page_params, which was just
a hack to make the calling code look cleaner. We now remove that hack
and thus, our dependency on page_params existing. Now, if the caller
does not specify a default value, we'll use the null-value.
This also creates a new init() function to cleanly wrap the code that
makes changes to the opts passed to the widget.
JSON.parse behaves as we want for numbers but for strings, we would
throw an error like 'unexpected token at position 0'. This meant we
couldn't read back the value set by `$input.data('val', 'text')`.
We had removed this function from the codebase when we switched to
using dropdown_list_widget. This was accidentally left as it is when
making that change.
Previously, we handled these updates in server_events_dispatch
and could accidentally call widget.render() before initializing
the widget.
Original report: https://chat.zulip.org/#narrow/near/875608.
The sync_realm_settings function ensures that if the settings are
not open, any updates are a noop.
When a user changes its avatar image, the user's avatar in popovers
wasn't being correctly updated, because of browser caching of the
avatar image. We added a version on the request to get the image in
the same format we use elsewhere, so the browser knows when to use the
cached image or to make a new request to the server.
Edited by Tim to preserve/fix sort orders in some tests, and update
zulip_feature_level.
Fixes: #14290
* Remove old topic and reprocess both old and new topic to ensure
that we are correctly storing the last_msg_id of users in the
topic. Also, Handle topic's stream (& topic) edit updates.
* Add function to get all messages in a topic in message_utils.js.
* Send topic edit event to recent_senders.
* Add func get sorted list of recent_senders to topic.
The function will be useful to handle topic edits in Recent Topic UI.
We no longer use `/json/users` in the codepath
for bot settings (admin side).
We also specifically don't load human users when
we load bots, so you no longer have to pay for
the server round trip as a side effect of loading
bots. Instead, there is a dedicated `set_up_bots`
entry point.
We also get the bot ids directly from `bot_data` now.
This commit, to some degree, builds on the prior commit
that had us hydrate data from `people.js` instead
of the payload from `/json/users`.
We want to move toward having list consumers
pass us in a list of ids that we hydrate later
in the process. This should help live-update
scenarios. The next commit will describe the
benefits in a bit more detail, using the
concrete example of our bot settings table
in the org settings.
A slightly longer-term goal here is to be
able to ask `list_render` to re-render a particular
id, and this moves us closer to that. But even
before that, this change should eliminate a class
of bugs dealing with stale data, such as when
you manually patch a list (with direct jQuery
hacks) but then later go to sort/filter the rows.
We will now re-hydrate the items in those scenarios.
For the below payloads we want `owner_id` instead
of `owner`, which we should deprecate. (The
`owner` field is actually an email, which is
not a stable key.)
page_params.realm_bots
realm_bot/add
realm_bot/update
IMPORTANT NOTE: Some of the data served in
these payloads is cached with the key
`bot_dicts_in_realm_cache_key`.
For page_params, we get the new field
via `get_owned_bot_dicts`.
For realm_bot/add, we modified
`created_bot_event`.
For realm_bot/update, we modified
`do_change_bot_owner`.
On the JS side, we no longer
look up the bot's owner directly in
`server_events_dispatch` when we get
a realm_bot/update event. Instead, we
delegate that job to `bot_data.js`.
I modified the tests accordingly.
The prior version of "me" confusingly had the same
user_id as one of our bots, so I fixed that.
I also avoid using a test email of 'owner@zulip.com',
which is confusing for earlier tests where I haven't
established "me" as the actual owner of any bots.
Since production testing of `message_retention_days` is finished, we can
enable this feature in the organization settings page. We already had this
setting in frontend but it was bit rotten and not rendered in templates.
Here we replaced our past text-input based setting with a
dropdown-with-text-input setting approach which is more consistent with our
existing UI.
Along with frontend changes, we also incorporated a backend change to
handle making retention period forever. This change introduces a new
convertor `to_positive_or_allowed_int` which only allows positive integers
and an allowed value for settings like `message_retention_days` which can
be a positive integer or has the value `Realm.RETAIN_MESSAGE_FOREVER` when
we change the setting to retain message forever.
This change made `to_not_negative_int_or_none` redundant so removed it as
well.
Fixes: #14854
This adds a way to keep track of max_message_id of a
stream and fetch it using the method get_max_message_id().
This will be useful for sorting streams by most recent
activity which will be implemented in the upcoming commit.
Essentially rewritten by tabbott to have a coherent tracking system,
and provide documentation.
Part of #10794.
When switching from Private Messages narrow to
All messages narrow, stream list max-height was not
correctly updated. Stream list max-height was calculated
before new height were updated by browser for
All message narrow.
Inshort:
Stream list max-height was being updated before the browser could
render height for `#global_filters`. Calling resize after narrow
completes removes this issue.
This exists in all versions of the desktop app that we still support,
and will eventually let us delete a bit of annoying compatibility code
from the desktop app’s injected JavaScript.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Previously, the message and event APIs represented the user differently
for the same reaction data. To make this more consistent, I added a
user_id field to the reaction dict for both messages and events. I
updated the front end to use the user_id field rather than the user
dict. Lastly, I updated front end and back end tests that used user
info.
I primarily tested this by running my local Zulip build and
adding/removing reactions from messages.
Fixes#12049.
In passing, we also get coverage for
list_render.get.
This test also implicitly demonstrates that
you can call `widget.sort` directly from some
external event handler; in other words, you
are not locked into the widget's conventions
for setting up <th> tags.
The check here was too late, and it should
have given a blueslip error. We obviously
don't expect these errors at runtime; this
is a convenience for developers creating
new widgets.
This covers how we wire up the filter, and it
covers using `filterer` instead of `predicate`.
I also go away from the strange length-based
predicate that I had in the original test.
The intention behind the original test was
to show that filters could be more than simple
string-matching, but that was just a strange
way to demonstrate it.
This commit:
- Switches margin for padding on the search closed icon, to ensure we
cover the region to the right of icon as clickable area.
- Applies the click handler that initiates the search to the second
last element of the navbar:
- This will most commonly be the narrow_description element, but may
also be the entire navbar eg in the case of "ALL" or "starred".
Applying this change to user names in "group-pm-with: ..." based
narrows is a little questionable, but there are no other triggers
on these names so this change makes sense for now.
- The narrow_description may also contain links, which need to be
handled correctly so that the behave like links should. We work
around the onClick on the narrow_description, by applying a
handler to <a> tags and invoking stopPropagation.
- We also add CSS to change the cursor to a pointer to make the
search icon change color on hover over the clickable area to
indicate that the search box can be opened with a single click.
- However, since <a> tags are handled differently, we add a hover
listener which makes sure it behaves appropriately. We also increase
the vertical padding of the <a> tags so they cover the entire
vertical navbar region.
We shouldn't add redundant data to page_params. Since we already have
page_params.realm_notifications_stream_id, we can use that value instead
of creating page_params.notifications_stream.
We, however, still need the name of the notifications stream to render
it in templates. Thus we create stream_data.get_notifications_stream().
This commit removes most of the duplicate logic for the stream selection
dropdowns for the settings: `realm_signup_notifications_stream_id` and
`realm_notifications_stream_id`.
We also make minot changes to DropdownListWidget to accomodate the stream
rendering of the format: `#stream_name`.
We finally switch to using stream_ids instead of stream_name everywhere
which makes reading data from page_params simpler.
'get_active_message_people` function is added which returns active
users who have sent the messages that are currently showing up in
the feed.
typeahead fetches the users from 'get_active_message_people` instead
of `get_message_people` and thus shows only active users in the
mention typeahead and excludes deactivated users.
Fixes#14310
This commit makes it so that inline (recipient bar) topic edits follow
a different path from full message row edits in `message_edit.js`.
This commit:
- deletes `.save()` endpoint and replaces all calls to it with
`.save_message_row_edit()` and `.save_inline_topic_edit()`
- deletes `.end()` endpoint and replaces all calls to it with calls to
either ".end_message_row_edit()" and ".end_inline_topic_edit()".
Some extraneous zrequires were added in
3bc818b9f7
This is not a huge deal, but it makes it
appear as if data modules are dependent
on things that they don't really care
about. The tests should provide a bit
of signal on how "deep" an object's
dependencies go.
The set_up_muted_topics_ui and templates have been
refactored to use list_render.
This is done to support filtering and sorting of
the muted stream topics.
This also includes the addition of a new Date muted header.
The div containing options for filtering streams was placed in the
centre. Aligned it towards the right. Had to pass a special check
variable in subs.js:540 to add the specific class for this purpose.
This was a specific scenario where this sort of CSS was to be added,
hence had to make a specific case.
Also, fixed the bottom border color of the search streams bar for night
mode.
Previously, we would always pick up the stream and topic name from
compose_state. This would work for message edits as well when the
composebox was open.
Now, if we are in a message edit, we get the stream and topic of the
message being edited before falling back on trying to populate using
the composebox state.
Fixes#14545.
This commit changes the code to show user according to emails based
on email_address_visibilty_values and the type of user.
1. email_address_visibility = admins,members and guests
Typeaheads are shown according to original emails.
2. email_address_visibility = admins only
Typeaheads are shown according to original email to admins which
were previously shown according to system-generated email of
form "user10@zulipdev.com".
For non-admins, typeaheads are not shown according to emails as
they are not visible in the typeahead itself to non-admins.
3. email_address_visibility = nobody
Typeaheads are not shown according to emails for all type of users.
This commit moves the get_visible_email function to people.js
as this function will be used in other places and people.js seems
relevant file for this.
Tests are added to get full coverage.
We change the user facing interface to allow specifying expected
number of error messages (default=1). Now an average test can look
like:
```
// We expect 3 error messages;
blueslip.expect('error', 'an error message', 3);
throwError();
throwError();
throwError();
blueslip.reset();
```
This change adds a toggle widget to the "add streams" page that
lets the user change the sort order of the streams list. So far,
this supports sorting by stream name, by number of subscribers,
or by estimated weekly traffic.
Some uploads can be rejected in the frontend, like when the file
size is too big, without sending the file to the server. Remove the
'Uploading file...' message from the compose box in such cases.
Now, the system uses word='' and an editing=True for rendering an
form for addition of alert words. This is a very vulnerable
way to implement said feature and this commit fixes that.
The addition form has been moved to alert_word_settings.hbs
thereby rendering it only once but always. Now, we do not have
to manually add an empty word and editing for the form to be
rendered.
As part of refactoring, the editing parameter has also been
removed as there is no purpose left.
This updates the logged-in top navbar to display the stream/message
name, number of users, and description. It also replaces the search
bar with a search icon that expands into a full-width search bar.
Co-authored-by: Max Nussenbaum <max@maxnuss.com>
Fixes: #164.
Fixes: #5198.
In ee0d4541b4, we renamed the topic_date
-> stream_topic_history, and in the process renamed some local object
properties from .name => .topic_name, and accidentally change the
type for the data from the server as well.
The test fixtures were incorrectly migrated in the same way, so we fix
that as well.
`stream_topic_history` is a more appropriate name as this
module will contain information about last message of a
stream in upcoming commits. Function and variable names
are changed accordingly like:
* topic_history() -> per_stream_history()
* get_recent_names() -> get_recent_topic_names()
* name -> topic_name
If a file cannot be added for upload because of restrictions in frontend
we call cancelAll immediately in 'info-visible' callback. This would
prevent files that are already added to be cancelled but does not cancel
files that are yet to be added. So we use break to prevent any more files
from being added.
Calling uppy.cancelAll() when a batch of uploads is completed
result in the cancelation of any other batch of uploads that is
in progress. This case happens when a user uploads some files
and then tries to upload another bunch of files before the existing
upload is completed.
The form for entering alert words has been moved above the list
of words.
The list of words will be presented alphabetically rather than
time of addition.
We already know which list widget a `<th>`
tag is associated with when we set up the
event handler, so it's silly to read data
from the DOM to find that widget again
when the handler runs.
This commit eliminates a whole class of possible
errors and busy work.
For some widgets we now avoid duplicate redraw
events from this old pattern:
widget = list_render.create(..., {
}).init();
widget.sort(...);
The above code was wasteful and possibly
flicker-y due to the fact that `init` and
`sort` both render.
Now we do this:
widget = list_render.create(..., {
init_sort: [...],
});
For other widgets we just clean up the need
to call `init()` right after `create()`.
We also allow widgets to pass in `sort_fields`
during initialization (since you may want to
have `init_sort` use a custom sort before the
first render.)
Finally, we make the second and third calls
eliminate the prior updates from the previous
widget. This can prevent strange bugs with
double-reversing columns (although that's
been prevented in a better way with a recent
commit), as well as avoiding double work
with sorting.
This code has always been kind of convoluted
and buggy, starting with the first
sorting-related commit, which put filtering
before sorting for some reason:
3706e2c6ba
This should fix bugs like the fact that
changing filter text would not respect
reversed sorts.
Now the scheme is simple:
- external UI actions set `meta` values like
filter_value, reverse_mode, and
sorting_function, as needed, through
simple setters
- use `hard_redraw` to do a redraw and
trigger external actions
- all filtering/sorting/reverse logic on
the *data* happens in a single, simple
function called `filter_and_sort`
We put this in `scroll_util` to make it more likely
we will eventually unify this with other scrolling
logic. (A big piece to move is ui.get_scroll_element,
but that's for another PR.)
And then the other tactical advantage is that we get
100% line coverage on it.
I changed the warning to an error, since I don't
think we ever expect scrolling at the `body` level,
and I don't bother with the preview node.
I pushed this risk commit to the end of
a PR that had a bunch of harmless prep
commits at the front, and I didn't make
it clear enough that the last commit (this
one) hadn't been tested thoroughly.
For the list_render widget, we can simplify
the intialization pretty easily (avoid
extra sorts, for example), but the cache aspects
are still tricky on subsequent calls.
For some widgets we now avoid duplicate redraw
events from this old pattern:
widget = list_render.create(..., {
}).init();
widget.sort(...);
The above code was wasteful and possibly
flicker-y due to the fact that `init` and
`sort` both render.
Now we do this:
widget = list_render.create(..., {
init_sort: [...],
});
For other widgets we just clean up the need
to call `init()` right after `create()`.
We also allow widgets to pass in `sort_fields`
during initialization (since you may want to
have `init_sort` use a custom sort before the
first render.)
We had a bug where if your peer mentioned you in
message, but then edited the message not to mention
you, the latter wouldn't reset your unread counts
for "Mentions". And the same problem would happen
vice versa.
The fix basically extracts `update_message_for_mention`
and makes sure it handles all combinations of
unread/mentioned flags, instead of assuming
any invariants about which directions of change
are possible.
And then we call that new function from
`message_events.js` whenever we get message
edit events.
Fixes#14544
We use a somewhat more realistic message, mostly
to prep for testing some mention/unread stuff in
a subsequent commit.
We also set message booleans.
Unfortunately, `recent_senders` is kind of awkward
for checking a single message, since its only
public API is for sorting. I don't bother with it.
But I do check the `topic_data` interaction.
The function message_send_error was messing up
on calls to message.get when we were passing in
string versions of `local_id`. Now we pass in
float ids.
This fixes a traceback where we tried to set
`.failed_request` on to an `undefined` value
that we had instead expected to be a locally
echoed message from our message store.
We stop using `local_id_counter`, which was just noise,
and instead we just make the test more realistic:
- Use 123.04 for our local id on the message that
we're simulating sending.
- Use 127 as the message id that the server gives
us back in the success payload.
We still stub echo functions, but for
one of our stubs (`try_deliver_locally`)
we now exercise one its actual callees
in the stub (`echo.insert_local_message`).
And we're still stubbing some callees
of `echo.insert_local_message`, since
that has all kinds of unwanted side
effects, too.
The main piece we want from
`insert_local_message`, for now,
is somewhat realistic handling of
our local message ids.
We also add a little sanity check
that our timestamp does get plumbed
through to `local_message.insert_message`.
Option is added to video_chat_provider settings for disabling
video calls.
Video call icon is hidden in two cases-
1. video_chat_provider is set to disabled.
2. video_chat_provider is set to Jitsi and settings.JITSI_SERVER_URL
is none.
Relevant tests are added and modified.
Fixes#14483
This adds a new realm setting: default_code_block_language.
This PR also adds a new widget to specify a language, which
behaves somewhat differently from other widgets of the same
kind; instead of exposing methods to the whole module, we
just create a single IIFE that handles all the interactions
with the DOM for the widget.
We also move the code for remapping languages to format_code
function since we want to preserve the original language to
decide if we override it using default_code_clock_language.
Fixes#14404.
This is a prep commit for changes to the top navbar, it adds helpers
to filter.js which will help control the behavior of some aspects of
the redesigned navbar.
Modified by tabbott to add comments, internationalization tags on the
strings, support streams:public, and change various title strings.
We fix this by adding a more expressive data function, with tests, for
whether a filter is on UserMessage data, which would mean that
streams:public could never add additional matches.
We now use `assert.throws()` to test that we're
properly calling `blueslip.fatal`.
In order to not break line coverage here, we have
to remove an unreachable `return` in `stream_data.js`.
Usually we test `fatal` for line coverage reasons.
Most places where we use `blueslip.fatal` fall in
these categories:
* the code is theoretically unreachable, but
we have `blueslip.fatal` for defensive reasons
* we have some upstream bug that we should just
fix
* the code should recover gracefully and just
use blueslip.errors()
It's possible that we should eliminate `blueslip.fatal`
from our API and just throw errors when really important
invariants get broken. This will make it more obvious
to somebody reading the code that we're not going to
continue after the call, and `blueslip` already knows
how to catch exceptions and report them.
When we redraw the left sidebar, we need to tell the
topic list to clear its data structures (and do other
stuff like hiding its popover), since we are clearing
its parent container.
The commit f0e18b3b3e
introduced this regression in late January 2020.
That commit made topic_list use a vdom to avoid
unnecessary updates. Before that, topic_list did
a lot of brute-force redraws, which covered up the
fact that we weren't having stream_list telling it
when the rug was being pulled out from under it.
The boundary between stream_list and topic_list
has always been kind of complicated code, since
topic lists get embedded into the stream list.
The main interactions, though, are basically:
* topic_zoom.clear_topics() - you're leaving
a narrow that may or may not be zoomed
* topic_list.clear() - you're about to redraw
stream items in the unzoomed stream list
* topic_list.rebuild(stream_li, stream_id) -
you're building or updating a topic list
for the newly active stream
Fixes#14465
In case of video embeds, the previous logic used
`data-src-fullsize` or `src` as a key to look
for the metadata of video in `lightbox.open()`,
but while parsing, the key used while storing
the metadata was the video ID.
This doesn't make any sense because video's data
could never be accessed from `asset_map` and we
always needed to lookup the DOM for this.
This commit fixes this by using $img.attr('src')
as a key for `asset_map` for both, images and
videos. Since `src` is the link of preview image
in case of video embeds, it will always uniquely
determine the video ID and we won't loose
anything with the change in how videos handle
things.
Part of #14152.
This changes the payload that is used
to populate `page_params` for the webapp,
as well as responses to the once-every-50-seconds
presence pings.
Now our dictionary of users only has these
two fields in the value:
- activity_timestamp
- idle_timestamp
Example data:
{
6: Object { idle_timestamp: 1585746028 },
7: Object { active_timestamp: 1585745774 },
8: Object { active_timestamp: 1585745578,
idle_timestamp: 1585745400}
}
We only send the slimmer type of payload
to clients that have set `slim_presence`
to True.
Note that this commit does not change the format
of the event data, which still looks like this:
{
website: {
client: 'website',
pushable: false,
status: 'active',
timestamp: 1585745225
}
}
This adds a new test_realm_integer, replacing test_realm_boolean for
testing integer fields like realm_create_stream_policy,
realm_invite_to_stream_policy, and realm_invite_required in dispatch.js.
Fixes#12284
Fields like realm_email_address_visibility and realm_bot_creation_policy
were strings instead of integers in page_params obeject in
settings_org.js tests.
Also use values struct defined in settings_config.js and setting_bots.js
instead of direct values for improving readability.
If folks use an overly broad selector for message rows,
they will accidentally include drafts from the drafts
dialog, which won't have zids. More specific selectors
will be more efficient and possibly prevent strange
behaviors.
For testing convenience, we extract the message.
The UI in the `#settings/notifications` page is updated similarly
to what is done in the `update_global_notifications` path present
in the `server_events_dispatch` file.
This setting is being overridden by the frontend since the last
commit, and the security model is clearer and more robust if we don't
make it appear as though the markdown processor is handling this
issue.
Co-authored-by: Tim Abbott <tabbott@zulipchat.com>
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
While we could fix this issue by changing the markdown processor,
doing so is not a robust solution, because even a momentary bug in the
markdown processor could allow cached messages that do not follow our
security policy.
This change ensures that even if our markdown processor has bugs that
result in rendered content that does not properly follow our policy of
using rel="noopener noreferrer" on links, we'll still do something
reasonable.
Co-authored-by: Tim Abbott <tabbott@zulipchat.com>
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This function returns a list of objects to create a
list_render object, and each item contains the streams
whose atleast one notification setting differs from the
default set by the user.
This is done by comparing the global settings in the
`#settings/notifications` page with those settings
present in the subscribed streams.
Work towards #9228.
This flag was used to delay unread count updates while the bankruptcy
modal was visible. Now that bankrupcty is no longer a modal, we don't
need this flag at all.
Starred messages from muted topics were not shown in the starred
messages view. Condition for muting_enabled is modified accordingly
such that the starred messages from muted topics is shown in the
starred messages narrowed view.
Node tests are updated accordingly.
Fixes#13548
We've noticed that many production organizations don't set either an
organization description or profile picture, even large open source
organizations that could definitely take advantage of this feature.
This adds a top-of-page banner that bugs organization administrators
to add an organization description and profile picture, generally
starting on the second login (as we only do it on page load after
notifications are configured).
Significantly tweaked by tabbott to get the right user experience.
Fixes#14019.
I think we could write this test better, but it's not a big deal for
this to break in the rare even that we change/remove one of the 2
strings it interacts with.
We now only use `page_params.realm_default_streams` during
initialization, and then after that we use `stream_data`
APIs to get default stream ids and related info. (And
for the event that replace the data, we just update our
internal data structures as well.)
Long term we should have the server just send us ids here,
since we are now hydrating info from stream data in all places.
We only used get_default_stream_names() in a
test, so now it's being replaced with a function
that just gets ids.
We'll have use for get_default_streams_ids()
in an upcoming commit.
Now if a default stream gets deleted, we just
redraw the table. We always have a small number
of default streams, and the way that we were removing
rows without the actual consent of `list_render` was
really janky (and just a vestige of pre-list-render
code that never got fully ported).
This also makes us consistent with how we handle
added streams (i.e. just call
`update_default_streams_table`).
ASIDE:
Ideally we will update `list_render` at some point to
have an API for adding and removing elements. It does
allow you now to call `data()` to reset its data, but
for now we just build a new `list_render` object every
time.
We stopped needing this with
0329b67048
(Dec 2016).
The function sets `bot.can_admin`,
which was only used in `bot_data.get_editable`.
We removed two tests (and then put back
some test setup that needed to leak down
to the last test).
This is code simplification motivated
by a recent bug that we fixed with some
server changes, but which was really
caused in some sense by our client code
using an overly finicky
condition to check falsiness.
For cross-realm bots, the value of
`user.bot_owner_id` may be `null`, or it
may simply be `undefined`, depending
on whether the server passes `None`
or simply omits the field.
We don't want out client code to be
coupled to that rather arbitrary
decision.
We were doing a `!== null` check instead
of checking for falsiness, which led to
blueslip errors in the past. Because a
bot owner id could be plausibly 0, a falsiness
check would be brittle in a different way.
Now we avoid that ugliness by calling
`get_bot_owner_user`, which either returns
an object or `undefined`.
And then the caller can just do a concise
check for whether `bot_owner` exists.
And we also fix up the crufty code that
was putting `bot_owner_full_name` on to
the object instead of using a local.
We have a bug report for this again, although
it might be on an old branch.
Fixes#13621.
Instead of having logical expressions in templates, it's always preferred
to calculating them in javascript and pass the results as a context. It
also enhances the readability of templates and testing of such logic is
easier in js over templates.
The use case for this are small or fixed tables, which do not need
filtering support. Thus we are able to not include the unnecessary
search input inside the html parent container.
It is not used at present, but will be required when we refactor
the settings pages.
We also split out exports.validate_filter function for
unit testing the above condition.
Before this commit, the reactions code would
take the `message.reactions` structure from
the server and try to "collapse" all the reactions
for the same users into the same reactions,
but with each reaction having a list of user_ids.
It was a strangely denormalized structure that
was awkward to work with, and it made it really
hard to reason about whether the data was in
the original structure that the server sent or
the modified structure.
Now we use a cleaner, normalized Map to keep
each reaction (i.e. one per emoji), and we
write that to `message.clean_reactions`.
The `clean_reactions` structure is now the
authoritatize source for all reaction-related
operations. As soon as you try to do anything
with reactions, we build the `clean_reactions`
data on the fly from the server data.
In particular, when we process events, we just
directly manipulate the `clean_reactions` data,
which is much easier to work with, since it's
a Map and doesn't duplicate any data.
This rewrite should avoid some obscure bugs.
I use `r` as shorthand for the clean reaction
structures, so as not to confuse it with
data from the server's message.reactions.
It also avoids some confusion where we use
`reaction` as a var name for the reaction
elements.
Fixes#14254
You can test this on dev:
* do "-stream:Verona" in the search bar (the minus
sign negates the search here)
* reload the browser
You should see the same search (all streams besides Verona).
We had this API:
people.add_in_realm = full-fledged user
people.add = not necessarily in realm
Now the API is this:
people.add = full-fledged user
people._add_user = internal API for cross-realm bots
and deactivated users
I think in most of our tests the distinction between
people.add() and people.add_in_realm() was just an
accident of history and didn't reflect any real intention.
And if I had to guess the intention in 99% of the cases,
folks probably thought they were just creating ordinary,
active users in the current realm.
In places where the distinction was obviously important
(because a test failed), I deactivated the user via
`people.deactivate`.
For the 'basics' test in the people test suite, I clean
up the test setup for Isaac. Before this commit I was
adding him first as a non-realm user then as a full-fledged
user, but this was contrived and confusing, and we
didn't really need it for test coverage purposes.
We want to move more logic to stream_data to facilitate
testing.
Both before and after this commit, we essentially build a
new list of users for typeahead, but now the new list
excludes subscribed users. We can do even better than
this in a follow-up commit.
Before this commit, presence used get_realm_count()
to determine whether a realm was "small" (and thus
should show all human users in the buddy list, even
humans that had not been active in a while).
The `get_realm_count` function--despite a very wrong,
misleading comment--was including bots in its count.
The new function truly counts only active humans
(and no bots).
Because we were overcounting users before this change,
we should technically adjust `BIG_REALM_COUNT` down
by some amount to reflect our original intention there
on the parameter. I'm leaving it alone for now, though,
since we've improved the performance of the buddy list
over time, and it's probably fine if a few "big" realms
get re-classified as small realms (and show more users)
by virtue of this change.
(Also note that this cutoff value only affects the
"normal" view of the buddy list; both small realms
and large realms will show long-inactive users if you
do searches.)
Fixes#14215
Given that can_mark_messages_read is called whenever the blue box
cursor stops on a message and that it is calculated purely on the
basis of sorted_term_types, it makes sense to cache the result.
If you were in the "Starred messages" narrow and
your pointer was on a message with the stream/topic
of "social/lunch", we wouldn't move you to the unread
messages for that topic.
I fixed this by removing the code that looked at
the current message's topic. Instead, we only look
at the active narrow to figure out the "next" topic
to go to.
Fixes#14120.
Original email address is shown to admin users in subscriber list when
email_address_visibilty is set to "Admins only" by passing delivery_email
at required places. Email address are not shown to non-admin users when
visibility is set to "Admins only".
Tweaked by tabbott to fix a few bugs and dead code.
Fixes a part of #13541.
This intent is that we'll be able to reuse this when editing streams
as well.
* Rename method: filter_with_new_topic to filter_with_new_param.
* Fix tests and method calls.
This extends our email address visibility settings to deny access to
user email addresses even to organization administrators.
At the moment, they can of course change the setting (which leaves an
audit trail), but in the future only organization owners will be able
to change that setting.
While we're at this, we rewrite the settings_data.js test to cover all
the cases in a more consistent way.
Fixes#14111.
The file populates `windows.i18n`, so now
the file name matches our convention.
Note that the module really just initializes
`i18next` and then does this:
window.i18n = i18next;
It doesn't really add any functionality to
third party library.
Before this test, we were validating the behavior
of `i18next`, but we weren't validating our light
layer that sits on top of `i18next`, which currently
resides in the slightly misnamed `translations.js`
file.
The translations module is now so small that I'll
just quote it verbatim here:
import i18next from 'i18next';
i18next.init({
lng: 'lang',
resources: {
lang: {
translation: page_params.translation_data,
},
},
nsSeparator: false,
keySeparator: false,
interpolation: {
prefix: "__",
suffix: "__",
},
returnEmptyString: false, // Empty string is not a valid translation.
});
window.i18n = i18next;
We now just do `zrequire('translations')` to initialize
the `i18next` library, which allows us to have simpler
test setup and to actually exercise the above call to
`i18next.init`.
This change now gives us 100% line coverage of `translations.js`,
which of course isn't that hard to acheive (see above).
This extracts a new module with three
functions, which we will test with 100%
line coverage:
- show_email
- email_for_user_settings
- get_time_preferences
The first two break several dependencies
in the codebase on `settings_org.js`. The
`get_time_preferences` breaks an annoying
dependency on `page_params` within people.
The module is pretty cohesive, in terms that
all three functions are just light wrappers
around `page_params` and/or `settings_config`.
Now all the modules that want to call show_email()
only have to require `settings_data`, instead of
having a dependency on the much heavier
`settings_org.js` module.
I also make some of the unit tests here be more
full-stack, where instead of stubbing show_email,
I basically just toggle `page_params.is_admin`.
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.
This follows the convention of other code calling into
add_sub_to_table of checking whether the stream settings overlay is
open (and thus in the DOM) before trying to rerender it.
We add these two functions to the API,
so that we no longer have `alert_words_ui`
using private data from `alert_word`:
alert_words.has_alert_word()
alert_words.get_word_list()
And to initialize the data, we have a proper
`initialize` method that is passed in only
the parameters that it needs from `ui_init`.
(We also move the step of deleting `alert_words`
from `page_params` to the `ui_init` module.)
Because it's a bit less cumbersome to initialize
`alert_words`, we now just it directly in the
node tests for `alert_words_ui`.
This is follow up to da79fd206a
I accidentally skipped over pm_conversations. Same
ideas as the bigger previous commit--we pass in params
to the initialize function and do the delete cleanup
within ui_init.
This cleans up the handoff of page_params
data between ui_init and modules that
take over ownership of page_params-derived
data.
Read the long comment in ui_init for a bit
more context.
Most of this diff is actually test cleanup.
And a lot of the diff to "real" code is
just glorified `s/page_params/params/`
in the `initialize` functions.
One little oddity is that we don't actually
surrender ownership of `page_params.user_id`
to `people.js`. We could plausibly sweep
the rest of the codebase to just use
`people.my_user_id()` consistently, but it's
not a super high priority thing to fix,
since the value never changes.
The stream_data situation is a bit messy,
since we consume `page_params` data in the
initialize() function in addition to the
`params` data we "own". I added a comment
there and intend to follow up. I tried
to mostly avoid the "word soup" by extracting
three locals at the top.
Finally, I don't touch `alert_words` yet,
despite it also doing the delete-page-params-data
dance. The problem is that `alert_words`
doesn't have a proper `initialize()`. We
should clean that up and have it use a
`Map` internally, too.
This gives them cache-compatible URLs, and also avoids some extra
copies of the sprite sheet images.
Comments on the Octopus emoji added by tabbott.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This is not always a behavior-preserving translation: _.defaults
mutates its first argument. However, the code does not always appear
to have been written to expect that.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This is not always a behavior-preserving translation: _.extend mutates
its first argument. However, the code does not always appear to have
been written to expect that.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This moves some code from settings_display.js
into the new module settings_config.js.
Extracting this module breaks some dependencies
on settings_display.js (which has some annoying
transitive dependencies, including jQuery).
In particular this isolates stream_data from
from settings_display.js.
Two of the three structures that we moved here
weren't even directly used by settings_display.js,
since we do a lot of rendering in the modules
admin.js and setting.js.
We make get_all_display_settings() a function
to avoid a require-time dependency on page_params.
Breaking the dependencies simplifies a few
node tests.
Most of the node test complexity came from the
following commit in March 2019:
5a130097bf
The commit itself seems harmless enough, but
dependencies can have a somewhat "viral" nature,
where making stream_data depend on settings_display
caused us to modify four different node tests.
This refactoring is the first step toward sharing
our markdown code with mobile. This focuses on
the Zulip layer, not the underlying third party `marked`
library.
In this commit we do a one-time initialization to
wire up the markdown functions, but after further
discussions with Greg, it might make more sense
to just pass in helpers on every use of markdown
(which is generally only once per sent message).
I'll address that in follow-up commits.
Even though it looks like a pretty invasive change,
you will note that we barely needed to modify the
node tests to make this pass. And we have pretty
decent test coverage here.
All of the places where we used to depend on
other Zulip modules now use helper functions that
any client (e.g. mobile) can configure themselves.
Or course, in the webapp, we configure these from
modules like people/stream_data/hash_util/etc.
Even in places where markdown used to deal directly with
data structures from other modules, we now use functions.
We may revisit this in a future commit, and we might
just pass data directly for certain things.
I decided to keep the helpers data structure completely flat,
so we don't have ugly nested names like
`helpers.emoji.get_emoji_codepoint`. Because of this,
some of the names aren't 1:1, which I think is fine.
For example, we map `user_groups.is_member_of` to
`is_member_of_user_group`.
It's likely that mobile already has different names
for their versions of these functions, so trying for
fake consistency would only help the webapp. In some
cases, I think the webapp functions have names that
could be improved, but we can clean that up in future
commits, and since the names aren't coupled to markdown
itself (i.e. only the config), we will be less
constrained.
It's worth noting that `marked` has an `options`
data structure that it uses for configuration, but
I didn't piggyback onto it, since the `marked`
options are more at the lexing/parsing layer vs.
the app-data layer stuff that our helpers mostly
help with.
Hopefully it's obvious why I just put helpers in
the top-level namespace for the module rather than
passing it around through multiple layers of the
parser.
There were a couple places in markdown where we
were doing awkward `hasOwnProperty` checks for
emoji-related stuff. Now we use the Python
principle of ask-forgiveness-not-permission and
just handle the getters returning falsy data. (It
should be `undefined`, but any falsy value is
unworkable in the places I changed, so I use
the simpler, less brittle form.)
We also break our direct dependency on
`emoji_codes.json` (with some help from the
prior commit).
In one place I rename streamName to stream_name,
fixing up an ancient naming violation that goes
way back to before this code was even extracted
away from echo.js. I didn't bother to split this
out into a separate commit, since 2 of the 4
lines would be immediately re-modified in the
subsequent commit.
Note that we still depend on `fenced_code`
via the global namespace, instead of simply
requiring it directly or injecting it. The
reason I'm postponing any action there is that
we'll have to change things once we move
markdown into a shared library. (The most
likely outcome is that we'll rename/move both files
at the same time and fix the namespace/require
details as part of that commit.)
Also the markdown code still relies on `_` being
available in the global namespace. We aren't
quite ready to share code with mobile yet, but the
underscore dependency should not be problematic,
since mobile already uses underscore to use the
webapp's shared typing_status module.
This mostly moves logic into people.js.
The people functions added here are glorified
two-liners.
One thing that changes here is that we
are a bit more rigorous about duplicate
names.
The code is slightly awkward, because this
commit preserves the strange behavior
that if 'alice|42' doesn't match on
the user with the name "alice" and user_id
"42", we instead look for a user whose
name is "alice|42". That seems like a
misfeature to me, but there's a test for
it, so I want to check with Tim that it's not
intentional behavior before I simplify
the code.
We add this API to emoji.js, so that markdown
doesn't need to look at internal data structures
(or even need to understand any kind of record
format for results).
Here are the functions:
get_realm_emoji_url()
get_emoji_name()
get_emoji_codepoint()
We use the API now in markdown, which eliminates
the need for the markdown parser to require
the emoji JSON file.
Each function has a simple docstring:
get_emoji_name('1f384') === 'holiday_tree'
get_emoji_codepoint('avocado') === '1f951'
get_realm_emoji_url('shrug') === '/user_avatars/2/emoji/images/31.png'
Also we have simple test coverage for the API
(including tests that verify the docstrings).
This name was misleading, because we weren't
actually setting realm_filters (that's what
`page_params.realm_filters = realm_filters`
is for); we were instead updating our
realm filter rules.
These functions were just shims that were
used in the somewhat painful migration from
subject_* to topic_*.
The commit 4572be8c27
fixed it so that the client never needs to
deal with "subject_links".
So now we just go back to simpler code:
message.topic_links = links
links = message.topic_links
I am not quite ready to declare victory on
the subject/topic migration, but we are super
close. In this commit I bump a blueslip
warning to a blueslip error, so that we'll
be notified of any codepath that is still
using the janky fall-back-to-subject defensive
code here.
If we go a couple days without any errors, then
we can remove the blueslip warning and the
defensive code immediately and then inline
the callers at our leisure. I wouldn't be
wildly against keeping these wrappers in some
parts of the code, but that debate is out of
the scope of this immediate fix, and I haven't
thought hard about it yet.
We can basically sweep set_message_topic() now,
if we wanted to, since it's truly just a one-liner.
(At one point it was encapsulating something
like `message.subject = foo`).
This required a tiny change to compose_fade
test setup.
Most of this logic is specific to markdown
message processing, so we move the code to
markdown.js.
The only responsibility that we leave with
`emoji.js` is to provide us with a list
of translations (regex and replacement text).
But now `markdown.js` actually (directly) executes
those translations against Zulip messages
as part of its preprocessing.
This should simplify the upcoming mobile conversion.
Instead of mobile needing to duplicate this fairly
complex function, they will just need to pass
us in a list similar to `emoji_translations` inside
of `emoji.js`. That code has a comment that shows
what the data structure looks like.
I am 99% sure we can rely on trimRight() and
trim() being available in all browsers that
we support. I verified in FF.
This removes the util dependency from both
modules touched here.
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.
We used to have a block of code doing this just in the presence
endpoint because that's where we'd had error-handling problems with it
not being present, but it seems more correct for it to run
unconditionally on all HTTP requests.
This requires adding a dependency of channel on reload_state, which we
record in the webpack configuration for now.
webpack optimizes JSON modules using JSON.parse("{…}"), which is
faster than the normal JavaScript parser.
Update the backend to use emoji_codes.json too instead of the three
separate JSON files.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
I believe we can remove these and rely on
other parts of our testing/code-review
to ensure template quality.
These tests never really exercised our
app code, as evidenced by us not regressing
any of the 100%-line-coverage files.
We have a couple other ways that we verify
the correct format of the templates:
- webpack (can they compile?)
- check-templates (are they nicely indented?)
For deep testing, we have Casper, which
exercises most of our most important templates
in some meaningful way.
I think it's pretty rare that we get bugs
now that are directly caused by bad templates,
and an even smaller subset of them would
have been caught by the node tests.
If that trend changes in the future, I would prefer to
just do something "greenfield" to address
any common problems rather than resurrect
this code, but we could always resurrect it
from git.
The template node tests did check a little bit of
detail about which fields are there, but not
in an integrated way, so that aspect of the tests
wasn't very useful either.
This effectively reverts the following
commit from May 2019:
be527905ca
The implementation of closest() was a bit
buggy and complex. It's easy enough
to just stub the method yourself. We may
want to eventually re-implement it, but we
should follow the template of parent/set_parent.
If you fail to stub `closest` zjquery gives
a fairly helpful error message:
Error: You must create a stub for $("link-stub").closest
We stub out jquery elements rather than giving
the illusion of having real DOM.
Also, we make it so that the message_store
interaction has an assertion attached to it.
In the next commit we're going to change what the
server sends for the following:
- page_params
- server responses to /json/users/me/presence
We will **not** yet be changing the format of the data
that we get in events when users update their presence.
It's also just a bit in flux what our final formats
will be for various presence payloads, and different
optimizations may lead us to use different data
structures in different payloads.
So for now we decouple these two things:
raw_info: this is intended to represent a
snapshot of the latest data from the
server, including some data like
timestamps that are only used
in downstream calculations and not
user-facing
exports.presence_info: this is calculated
info for modules like buddy_data that
just need to know active vs. idle and
last_active_date
Another change that happens here is we rename
set_info_for_user to update_info_for_event,
which just makes it clear that the function
expects data in the "event" format (as opposed
to the format for page_params or server
responses).
As of now keeping the intermediate raw_info data
around feels slightly awkward, because we just
immediately calculate presence_info for any kind
of update. This may be sorta surprising if you
just skim the code and see the various timeout
constants. You would think we might be automatically
expiring "active" statuses in the client due to
the simple passage of time, but in fact the precise
places we do this are all triggered by new data
from the server and we re-calculate statuses
immediately.
(There are indirect ways that clients
have timing logic, since they ask the
server for new data at various intervals, but a
smarter client could simply expire users on its
own, or at least with a more efficient transfer
of info between it and the server. One of
the thing that complicates client-side logic
is that server and client clocks may be out
of sync. Also, it's not inherently super expensive
to get updates from the server.)
The important details for the test setup here
are just the number of users who are active.
We don't need to simulate the currently awkward
way of populating this data.
The _.each calls with an inline function expression have already been
converted to for…of loops. We could do that here, but using .forEach
when we’re just reusing an existing function seems like a good
guideline.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
predicate is expected to return a function, not a boolean. The
boolean true was causing _.filter to match items with a property named
"true", which is definitely not what was intended. Matching no items
is probably also not intended, but matching every item causes the test
to fail.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
We just get the stream_name from the sub struct now.
This mostly affects node tests.
The only place in real code where we called add_sub()
was when we initialized data from the server.
We now require all of our unit tests to handle
blueslip errors for warn/error/fatal. This
simplifies the zblueslip code to not have any
options passed in.
Most of the places changed here fell into two
categories:
- We were just missing a random piece of
setup data in a happy path test.
- We were testing error handling in just
a lazy way to ensure 100% coverage. Often
these error codepaths were fairly
contrived.
The one place where we especially lazy was
the stream_data tests, and those are now
more thorough.
This saves a tiny bit of bandwidth, but more
importantly, it protects us against races for
stream name changes. There's some argument that
if the user is thinking they're sending to
old_stream_name, and unbeknownst to them, the
stream has changed to new_stream_name, then we
should fail. But I think 99% of the time the
user just wants the message to go that stream
despite any renames.
In order to verify the blueslip error, we
had to turn on error checking, which required
a tiny fix to a place where we left out
a stream_id for add_sub.
We avoid complicated code to update unread counts
by just using vdom.js.
One small change here is that if click on "more
topics", we replace it with the spinner instead
of putting the spinner after it. This saves us
a redraw under the new scheme.
Babel strict generates more code for [...x] than you’d like, while
Babel loose mode assumes x is an array.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
We had a plan at some point to use this to display a phone icon or
something for users who would receive push notifications if you
messaged them. IT's not clear that feature was a good idea in any
case, but it certainly shouldn't be synced as presence data; it would
change >100x less often than the rest of presence and so should likely
be synced differently, maybe as a property on user. So it's best to
delete this prototype.
The “Smileys & People” category has been split into “Smilys & Emotion”
and “People & Body”.
Also, fix generate_sha1sum_emoji to read the emoji-datasource-google
version from yarn.lock, since package.json only gives a version range.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
When quoting a message with fenced code blocks without a language,
we used to have ambiguity in which '```' fence terminates the quote.
This commit adds explicitly non-interfering fences, which fixes the
above issue as well as makes the raw message easier to quickly read.
Fixes#12446.
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.
This flag affects page_params and the
payload you get back from POSTs to this
url:
users/me/presence
The flag does not yet affect the
presence events that get sent to a
client.
This is defensive code for the scenario that we
have a user_id in presence but not people. This is
unlikely to occur by the time that we actually render
the buddy list, which is the context for this code.
We have previously been reporting an error here via
the people code, but we add an additional warning.
Also, we filter the user_id from the result.
This reverts commit d84646f091 (which
incorrectly assumed in unread_topic_counter that the messages were
present in the message store), while fixing the type confusion problem
by using IntDict for stream_id keys.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
stream_count and topic_count in the actual code have been IntDict
since commit 9ba1829243 (#13569).
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Fixes type confusion in unread_topic_counter, which uses stream IDs as
keys.
Since unread_topic_counter calls message_store.get now, update the
mocks so that message_store.get knows about our mocked messages.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Previously the sender was not included in display_recipient when
a private message was locally echoed. This broke the copy conversation
link functionality, if the user try to copy the link immedeatly after
sending the message. This issue is present only during local echo.
This was fixed by including the recipient of the user during
local echo.
Fixes#13547.
Edited the warning to clearly state that most members/most stream members
will be notified on using wildcard mentions, along with the specific
mention (e.g. @ALL, @everyone and @stream).
Did a separate check for all wildcard mentions in util.js and stored the
corresponding mention in wildcard_mention inside compose.js.
Fixes: #13636
For few settings like `waiting_period_threshold` it makes sense to have the
"value" attribute of option to have a value other than the actual setting
value because multiple settings are depending upon this dropdown, so
handling them in JS code makes more sense. But for many settings (which has
integer values), we have followed a wrong trend over the time of
representing every new dropdown with human-readable values and manually
handling them in JS Code, where it makes more sense to use actual setting
value. The result of which is code has become less concise, sensible and
less likely to be mistaken.
We now use vdom-ish techniques to track the
list items for the pm list. When we go to update
the list, we only re-render nodes whose data
has changed, with two exceptions:
- Obviously, the first time we do a full render.
- If the keys for the items have changed (i.e.
a new node has come in or the order has changed),
we just re-render the whole list.
If the keys are the same since the last re-render, we
only re-render individual items if their data has
changed.
Most of the new code is in these two modules:
- pm_list_dom.js
- vdom.js
We remove all of the code in pm_list.js that is
related to updating DOM with unread counts.
For presence updates, we are now *never*
re-rendering the whole list, since presence
updates only change individual line items and
don't affect the keys. Instead, we just update
any changed elements in place.
The main thing that makes this all work is the
`update` method in `vdom`, which is totally generic
and essentially does a few simple jobs:
- detect if keys are different
- just render the whole ul as needed
- for items that change, do the appropriate
jQuery to update the item in place
Note that this code seems to play nice with simplebar.
Also, this code continues to use templates to render
the individual list items.
FWIW this code isn't radically different than list_render,
but it's got some key differences:
- There are fewer bells and whistles in this code.
Some of the stuff that list_render does is overkill
for the PM list.
- This code detects data changes.
Note that the vdom scheme is agnostic about templates;
it simply requires the child nodes to provide a render
method. (This is similar to list_render, which is also
technically agnostic about rendering, but which also
does use templates in most cases.)
These fixes are somewhat related to #13605, but we
haven't gotten a solid repro on that issue, and
the scrolling issues there may be orthogonal to the
redraws. But having fewer moving parts here should
help, and we won't get the rug pulled out from under
us on every presence update.
There are two possible extensions to this that are
somewhat overlapping in nature, but can be done
one a time.
* We can do a deeper vdom approach here that
gets us away from templates, and just have
nodes write to an AST. I have this on another
branch, but it might be overkill.
* We can avoid some redraws by detecting where
keys are moving up and down. I'm not completely
sure we need it for the PM list.
If this gets merged, we may want to try similar
things for the stream list, which also does a fairly
complicated mixture of big-hammer re-renders and
surgical updates-in-place (with custom code).
BTW we have 100% line coverage for vdom.js.
Now that we have the type situation of having anchor support passing a
string, this is a much more natural way to implement
use_first_unread_anchor.
We still support the old interface to avoid breaking compatibility
with legacy versions of the mobile apps.
This makes the code more readable, by just passing the anchor through
without changing its field name back and forth.
There's no reason for this parameter to involve parsing and integer --
it should be a number in all incoming code paths.
The feature is used for editing stream descriptions as well, and in
any case, what's important is that it's a content-editable widget (aka
a form of input box).
We now give "slight smile" precedence over
"small airplane" if you type "sm".
More generally, we favor popularity over prefix
matches for emoji matches, as long as the popular
emoji matches on any of its pieces.
This extracts get_emoji_matcher and all the
functions it depended on, most of which were
in composebox_typeahead.js.
We also move remove_diacritics out of the people
module.
This is the first major step for #13728.
We now only go the server if both of these
conditions are true:
- our message data seems incomplete for
the stream
- we haven't already fetched history
This function will make more sense when we start
tracking api calls that retrieve topic history.
The unit tests here are kinda duplicating what we
have in the stream_data tests. If we move the
function out of stream_data, we can kill off the
tests there, but for now I think a bit of duplicate
testing is fine here.
This is mostly for tactical reasons. It's hard to
get 100% test coverage on topic_list.js, but it
should be easy to get 100% test coverage on this
very important function.
I considered just moving this code into topic_data.js,
but it just didn't feel quite right. I feel like
this is a pretty core piece of code that's nice
to be by itself and not be near other complicated
code that does stuff like build widgets or talk
to servers. (And, again, it's not just the actual
code here, which is pretty small, it's the unit
tests, which are inherently verbose to exercise
all the edge cases.)
This test mostly tests how we glue everything
together, but I want to change that in an upcoming
commit.
Also, the data stuff that it tests is now better
covered by the test recent tests I added.
The only place we ever set active-sub-filter is
right after we build the template, so there is
no reason to have it be a separate step.
(I made a similar fix to pm_list recently, and
this helps set the stage for doing vdom-like
stuff.)
We may revisit this in the future, but similar to is:private, the
current Zulip user experience makes users expect that in the
is:mentioned view, they should really be able to mark messages as
read.
Further, the practice use case for not marking them as read is very
low, since it's rare for someone to have so many mentions that
revisiting the mentions view isn't sufficient to see everything that
needs their attention.
Previously, is_exactly() had already been repalced with can_bucket_by().
This commit removes is_exactly() and replaces its usage in our tests
with can_bucket_by().
We now incorporate people.get_message_people() in our
logic for compose/PM typeaheads. This not only gives
users better results in some cases, but it will also
improve performance for large realms in some cases.
A recent commit removed test coverage for the
actual filtering/sorting of mention typeaheads
when you did a non-silent method. This commit
now tests that important step again.
Note that we also had (and still have) tests
that make sure the is_silent flag is set
correctly by get_candidates.
We don't have a true full-stack test, but those
can be quite tricky to set up and maintain.
This is relatively unobtrusive, and we don't send
anything to the server.
But any user can now enter blueslip.timings in the
console to see a map of how long things take in
milliseconds. We only record one timing per
event label (i.e. the most recent).
It's pretty easy to test this by just clicking
around. For 300 users/streams most things are
fast except for:
- initialize_everything
- manage streams (render_subscriptions)
Both do lots of nontrivial work, although
"manage streams" is a bit surprising, since
we're only measuring how long to build the
HTML from the templates (whereas the real
time is probably browser rendering costs).
This change sets us up to optimize how we
filter users in the admin user settings.
See #13554 for more context on the user
facing issues.
This fix is basically three related things:
- Add filterer options to list_render.
- Add helper method to people.js.
- Use filterer in settings_users.js.
The filter "callback" was only a "callback" in the
most general sense of the word.
It's just a filter predicate that returns a bool.
This is to prepare for another filtering option,
where the caller can filter the whole list
themselves. I haven't figured out what I will name
the new option yet, but I know I want to make the
two options have specific names.
We are already providing callbacks everywhere, so
it would be nice to eliminate some dead code.
This also speeds things up ever so slightly (no
longer type-checking the option every time through
the loop).
We also split out exports.filter to make unit testing
easier. The function seems kinda silly now, being so
small, but I hope to add another filtering option soon.
Zulip has had a small use of WebSockets (specifically, for the code
path of sending messages, via the webapp only) since ~2013. We
originally added this use of WebSockets in the hope that the latency
benefits of doing so would allow us to avoid implementing a markdown
local echo; they were not. Further, HTTP/2 may have eliminated the
latency difference we hoped to exploit by using WebSockets in any
case.
While we’d originally imagined using WebSockets for other endpoints,
there was never a good justification for moving more components to the
WebSockets system.
This WebSockets code path had a lot of downsides/complexity,
including:
* The messy hack involving constructing an emulated request object to
hook into doing Django requests.
* The `message_senders` queue processor system, which increases RAM
needs and must be provisioned independently from the rest of the
server).
* A duplicate check_send_receive_time Nagios test specific to
WebSockets.
* The requirement for users to have their firewalls/NATs allow
WebSocket connections, and a setting to disable them for networks
where WebSockets don’t work.
* Dependencies on the SockJS family of libraries, which has at times
been poorly maintained, and periodically throws random JavaScript
exceptions in our production environments without a deep enough
traceback to effectively investigate.
* A total of about 1600 lines of our code related to the feature.
* Increased load on the Tornado system, especially around a Zulip
server restart, and especially for large installations like
zulipchat.com, resulting in extra delay before messages can be sent
again.
As detailed in
https://github.com/zulip/zulip/pull/12862#issuecomment-536152397, it
appears that removing WebSockets moderately increases the time it
takes for the `send_message` API query to return from the server, but
does not significantly change the time between when a message is sent
and when it is received by clients. We don’t understand the reason
for that change (suggesting the possibility of a measurement error),
and even if it is a real change, we consider that potential small
latency regression to be acceptable.
If we later want WebSockets, we’ll likely want to just use Django
Channels.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
As the part of making notification stream settings to change using
"save/discard" widget instead of immediate saving, we need to access the
stream id which is being selected at the moment.
For "New stream notifications" and "New user notifications" it is more
intuitive to just use the new system for showing success/saving status
feedback.
Extracting the function makes it a bit easier to
test and use in a generic way.
Also, I wanted this to live in stream_data, so that
it's easier to find if we change how we model
subscriber data.
Finally, I use _.every to do the subset check
instead of `_.difference`, since _.difference
is actually N-squared:
_.difference = restArguments(function(array, rest) {
rest = flatten(rest, true, true);
return _.filter(array, function(value){
return !_.contains(rest, value);
});
});
And we don't actually want to build a list only
to check that it's zero vs. nonzero length.
We now do this, which short circuits as soon
as it finds any key that is only in sub1:
return _.every(sub1.subscribers.keys(), (key) => {
return sub2_set.has(key);
});
First, there are no more convoluted signals.
We also simplify the parameter to just the "mentioned"
object corresponding to either a user or a broadcast
mention.
For the user group scenario, this has always been dead
code, which you only realized when you got to the comment
at the bottom. Now we actually do nothing.
And I moved the relevant commment to the
the typeahead code (with new wording).
I also moved the is_silent check to the caller. I don't
feel too strongly about that either way. It's kind of silly
to call a function only to give that function an additional
responsibility to worry about. On the other hand, I see
the logic of that function enforcing everything. I went
with the former for now.
Arguably we should have a warning for silent mentions,
since doing a silent mention of somebody not on a stream
is a good indication of a typo. I do understand the use
case, but the user can always ignore the warning. Anyway,
we have decent test coverage on this.
This isn't really an extraction; it's more giving
a name to an anonymous function and moving it to
higher module scope.
We convert this to an ordinary function call, which
allows us to move it out of intialize().
Since there's just one simple parameter now (linked_stream),
we can avoid some error checking.
We also avoid the comment that describes the function,
since it now has a name.
And then one minor tweak is to do the inexpensive
`invite_only` higher in the function. This will be
a nice speedup when you link to really large public
streams.
The unit tests are also a bit easier to read now--less
setup and more explicit names.
Previously, if you tried to invite a user whose account had been
deactivated, we didn't provide a clear path forward for reactivating
the users, which was confusing.
We fix this by plumbing through to the frontend the information that
there is an existing user account with that email address in this
organization, but that it's deactivated. For administrators, we
provide a link for how to reactivate the user.
Fixes#8144.
The sort_recipients helper is used for many different
typeaheads, such as compose PMs, compose mentions,
and some settings-related code.
We now avoid unnecessary sorting steps in cases
where we have plenty of results in the top buckets
(such as users who match on prefix).
This change should not have any user-facing
implications.
This method is a bit complex, but I think it's
worthwhile to force PM autocompletes and mention
autocompletes through the same code path.
We also kill off this method:
typeahead_helper.sort_people_and_user_groups
For historical reasons pm_list was handling just
one possible edge case of where is:private was
combined with other search terms, namely the
pm-with operator.
The code was correct in realizing the is:private
was redundant there, but now we handle that
upstream in Filter.fix_operators (see previous
commit).
Now we just look for any is:private term.
This change makes these two functions more alike:
- get_search_result
- get_search_result_legacy
To test the UI modify zerver/views/home.py by
replacing `settings.SEARCH_PILLS_ENABLED` with
`True`. I only did a quick sanity check, since
any bugs with the new system are more likely due
to bitrot than any changes I have made here.
The history is this:
Tim cloned the code (before the smaller
helpers were extracted):
db4f6e278f
In 8b153f6452
Shubham removed get_operator_subset_suggestions but
accidentally left a `concat` statement in that got
misapplied to the previous suggestions:
- suggestions = get_operator_subset_suggestions(operators);
result = result.concat(suggestions);
The error there was carried over in some recent changes,
but this commit fixes that strangeness.
In 73e4f3b3fa
Shubham made this change, which makes sense only for
pills, and this code remains intact.
- if (operators.length > 0) {
- last = operators.slice(-1)[0];
+ if (query_operators.length > 0) {
+ last = query_operators.slice(-1)[0];
+ } else {
+ // If query_operators = [] then last will remain
+ // {operator: '', operand: '', negated: false}; from above.
+ // `last` has not yet been added to operators/query_operators.
+ // The code below adds last to operators/query_operators
+ operators.push(last);
+ query_operators.push(last);
}
Mohit made a couple changes to both old and new.
Anders made a couple non-substantive changes related to
the ES6 migration.
Steve (me) made several structural changes to the code. For
some of them I only changed the legacy code, not the pills
code. I didn't fix Shubham's mistake until this change.
Now the two functions should look similar except in the places
where they are intentionally different. I also added a comment
explaining the get_operator_subset_suggestions difference.
Fixes#13609
I want to be able to easily test this without
having to simulate all the jQuery side effects.
This simply preserves the old logic, which seems
to handle one edge case without handling every
possible edge case. The edge cases aren't super
important here, though, since the only thing it affects
is bolding "Private Messages", and when to do that
is somewhat up to personal tastes.
Having said that, we could definitely improve
this code and possibly should move some of this
logic to either narrow_state.js or filter.js.
Instead of doing various ad-hoc calculations of
which PM is "active" and plumbing it through various
functions and then updating it via jQuery instead of
just the template, we now just calculate `is_active`
in `_build_private_messages_list` with a little
helper function.
This test mostly tests logic that I'm about
to remove in subsequent commits, and it's a bit
messy.
This commit removes 100% line coverage, but I
will restore that a few commits later.
In 3cfc3ca24b I removed
the feature that limited PM conversations to five or
less (including the active conversation), but I
didn't clean up this parameter. I think lint was
confused by the fact that we did mutate it.
I am wondering if this started out as an experiment
and was never fully polished before the push? Or
maybe I was just careless. Anyway, I don't
think were any symptoms here--it was just dead code
that we didn't need.
This fixes a rebase issue between the int_dict introduction and use
for people.js with the introduce of filter_values on dict.js and use
inside people.js.
Note that we haven't fully swept this for Dict,
since some dicts are keyed by strings. For
example PM counts can have a huddle like
"101,102,103" as a key.
This should be slightly more performant, and we
often call this function N times, such as when
rendering the buddy list.
There's a minor change to pm_list to avoid
an unnecessary computation on huddles that would
otherwise trigger a blueslip warning for the
huddles case.
When we are pulling data from message.display_recipient
for private messages, the user_id field is always
called 'id', not 'user_id', so we can simplify
some defensive code.
This required lots of manual testing:
- search/navigate user presence
- send PM and mention user
- pay attention to compose fade
- send stream msg and mention user
- open Private Messages in top-left and click
- test unread counts
- invite user who already has account
- search for users in search bar
- check user settings
- User Groups
- Users
- Deactivated Users
- Bots
- create a bot
- mention user groups
- send group PM then click on lower right
- view/edit/create streams
If there are still pieces of code that don't convert
ids to ints, the code should still work but report
blueslip errors.
I try to mostly convert user_ids to ints in the callers,
since often the callers are dealing with small amounts
of data, like user ids from huddles.
We only ever show 3 or 4 people in search suggestions
(possibly w/a couple variations, like pm-with/sender/etc.),
so we can try to search a smaller subset of people
before going through the entire realm.
We use message_store.user_ids() for this, since you
typically want to search messages for people that
have sent messages recently, and we already sort
based on PM conversations.
This should avoid some memory allocations.
We also use build_person_matcher to avoid
repeating the same logic over and over
again to process the query into termlets.
We also remove people.get_all_persons() and
people.person_matches_query().
This looks like simple code cleanup, but it's more
than that.
The code cleanup here is that we don't have three
callbacks to get a list of typeaheads for bootstrap.
Instead, we just have one function that does all the
main work.
And then the speedup comes from the fact we no longer
need to remove diacritics from the query for every
time through our loop of seeing if a person matches
the query.
It's a bit subtle to see in the diff, but these are
the relevant lines:
const matcher = exports.get_person_or_user_group_matcher(query);
const filtered_results = _.filter(people_and_groups, matcher);
Before this, bootstrap was doing $.grep, and we'd have
to reinitialize the matcher for every person.
If you profile this before and after, you'll see that
remove_diacritics gets called fewer times.
To profile this, you want to loads lots of users into
your DB and try to autocomplete "Extra", as in "Extra1 User".
If you try to autocomplete something else, then my patch
won't really help, and `remove_diacritics` will still
show up as expensive. Because it is that expensive a function.
This may seem silly now, since we are returning a function
that still dispatches over all flavors of search for
every item, but subsequent commits will make it obvious
why I'm doing this.
We want to do our own matching of items, rather than
just giving a callback to bootstrap, which does $.grep
on all the items.
Doing our own matching gives us flexibility for future
improvements like custom data structures for searching
through big amounts of data. Even in the short term
we can speed up searches by pulling expensive operations
outside the grep/filter call.
This architecture has been in place for our search
bar since ~2014.
The benchmark is commented out. It takes only a few
milliseconds to run, so there may be no reason not
to always run it. It doesn't test correctness, so
it would arguably inflate line coverage, but set/get
are obviously covered elsewhere.
We now require the actual tests to explicitly
to zrequire Dict, rather than magically adding this.
In one case, the use of Dict was clearly just for
the test (not the app), so I converted that an ordinary
JS object (see timerender.js).
We have ~5 years of proof that we'll probably never
extend Dict with more options.
Breaking the classes into makes both a little faster
(no options to check), and we remove some options
in FoldDict that are never used (from/from_array).
A possible next step is to fine-tune the Dict to use
Map internally.
Note that the TypeScript types for FoldDict are now
more specific (requiring string keys). Of course,
this isn't really enforced until we convert other
modules to TS.
We had a potentially nasty bug where we
weren't guaranteeing that all/stream/everyone
collated in consistent ways inside of
`compare_people_for_relevance`, which can
send certain types of sort algorithms into
an infinite loop. I doubt this ever happened
in practice, but it's obviously worth fixing.
Now we also have a clear tiebreaker between
any two all/everyone/stream mentions, which
is the idx field.
Finally, this should be a bit more efficient.
We don't have people named "all". Instead, we
create pseudo person objects with email/full_name
of "all" (along with some other fields). The tests
now reflect this.
We want to mostly deprecate this function (see
the comment I added), so I gave it a more specific
name.
Ideally I'd just fix `stream_create`, but it does
use this function in a couple places, and it's helpful
to reuse the same sort here. In one place stream_create
actually unshifts the "me" user back to the top of the
list, which makes sense for its use case.
If two user_ids in a recent huddle have ids
that sort lexically differently than numerically,
such as 7 and 66, then we were creating two
different buckets in pm_conversations.
This regression was introduced in
263ac0eb45 on
November 21, 2019.
Instead of having our callers pass in a possibly
non-canonical version of a user_ids_string, just
have them pass in a list.
The next commit will canonicalize the sort.
The server may send us ids in the order
[11, 2], instead of [2, 11]. We don't want
to rely on server behavior, regardless, for
the sort.
Our tests now show we process that data.
The current code is is still buggy and causes
us to show the same huddle two different times
for situations where the lexical sort doesn't
match the numerical sort.
This happens on czo often, where Tim is user
7, and his id sorts lexically after ids like
58, 622, 4444, etc.
This should make any operation on subscribed
streams faster (we won't need to filter out
unsubscribed streams every time).
I started writing this before I realized we
had a bug where we call `subscribed_streams`
in a nested loop.
After fixing the bugs, this is not as much of
a bottleneck, but it's still a speedup in many
important places:
* build left sidebar
* every keystroke in search bar
* first keystroke in making #stream_links
* every keystroke in compose stream box
The streams settings code is kinda complicated.
It does a non-deterministic sort of the "others"
bucket when you add elements to the left panel.
They get hidden, anyway. Our values() call now
puts subscribed streams first. It never guaranteed
order, but putting subscribed streams first is
probably a good behavior for most situations.
This defers O(N*S) operations, where
N = number of streams
S = number of subscribers per stream
In many cases we never do an O(N) operation on
a stream. Exceptions include:
- checking stream links from the compose box
- editing a stream
- adding members to a newly added stream
An operation that used to be O(N)--computing
the number of subscribers--is now O(1), and we
don't even pay O(N) on a one-time basis to
compute it (not counting the cost to build the
array from JSON, but we have to do that).
Calling `set_filter_out_inactives` is expensive, since we
count up the number of subscribed streams, which iterates
through all your streams, creates a new list of subscribed
streams, then counts them.
In my dev setup, I created 700 streams, and this shaved
about 700ms off of the initial call to `build_stream_list`.
If we aren't showing users emails, then we don't
want to use emails in the search.
And if we are showing users emails, we want to
search on the email that's displayed to them.
For admins this will be delivery_email.
For regular users we arguably shouldn't search
on emails either, since it mostly causes confusion,
but this commit just preserves the current
behavior for those users (unless `show_email` is
false).
We want to be able to unit test this value,
since it's conditional on several factors:
- am I an admin?
- can non-admins view emails?
- do we have delivery_email for the user?
I'm mocking show_email in the tests, since the
show_email code is in `settings_org` and
kind of hard to unit test. It's not impossible,
but it's too much for this commit. (Either
we need to extract it out to a nice file or
deal with mocking jQuery. That module is
mostly data-oriented, so it would be nice
to have something like `settings_config` that
is actually pure data.)
This was duplicate code. I'm moving it to people
for pragmatic reasons--it's hard to unit test stuff
in settings_users.js due to all the jQuery.
It's also nice to have all people-related search
code in one place, just for auditing purposes.
Once we have max_items results, stop trying
to get more items.
This should really help large realms when
you do a search on streams that turns up
more than N streams (where N is about 12).
We won't even bother to find people.
This isn't a huge speedup, but it's an easy
code change.
We remove the two-liner highlight_with_escaping,
which was only called in one place, and when
we inline it into the caller, we can pull the
first line, which builds the regex, out of the
loop.
The streams:all adveritsement notice in search should only appear
after we've already received the response from the server, to avoid a
mix of problems ranging from misplaced loading indicator to scrolling
issues to the notice just being distracting while you're waiting for
the server to return results.
We need to add a pre_scroll_cont parameter to the message_fetch API,
since adding this notice would otherwise potentially throw off the
scroll positioning logic for which message to select.
Fixes#13441.
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.
In 1fe4f795af, we added the
wildcard_mentions_notify setting, which controls whether wildcard
mentions should be treated as mentions for the purposes of
notifications. The original implementation focused on the more
important area of email/push notifications, and neglected to address
desktop notifications for wildcard mentions.
This change makes the wildcard_mentions_notify flag behave correctly
for desktop/sound notifications, including unit tests.
Fixes#13073.
Adds required API and front-end changes to modify and read the
wildcard_mentions_notify field in the Subscription model.
It includes front-end code to add the setting to the user's "manage
streams" page. This setting will be greyed out when a stream is muted.
The PR also includes back-end code to add the setting the initial state of
a subscription.
New automated tests were added for the API, events system and front-end.
In manual testing, we checked that modifying the setting in the front end
persisted the change in the Subscription model. We noticed the notifications
were not behaving exactly as expected in manual testing; see
https://github.com/zulip/zulip/issues/13073#issuecomment-560263081 .
Tweaked by tabbott to fix real-time synchronization issues.
Fixes: #13429.
If a message begins with /me, we do not have any cases where the
rendered content would not begin with `<p>/me`. Thus, we can safely
remove the redundant checks both on the backend and frontend.
In e42c3f7418, we made the assumption
that compose_pm_pill.get_recipient() would return no users for stream
messages. It turns out, due to the confusing name of
compose_state.recipient (which we just renamed to
compose_state.private_message_recipient), this assumption was wrong.
As a result, when composing a stream message using the reply hotkeys,
we'd end up sending typing notiifcations to the person who sent the
message we're replying to as though a PM was being composed.
We fix this by avoiding passing an (expected to be unused) value for
private_message_recipient to compose_state.start.
The compose_state.recipient field was only actually the recipient for
the message if it was a private_message_recipient (in the sense of
other code); we store the stream in compose_state.stream instead.
As a result, the name was quite confusing, resulting in the
possibility of problematic correctness bugs where code assumes this
field has a valid value for stream messages. Fix this by changing it
to compose_state.private_message_recipient for clarity.
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 simple change switches us to take advantage of the
server-maintained data for the pm_conversations system we implemented
originally for mobile use.
This should make it a lot more convenient to find historical private
message conversations, since one can effectively scroll infinitely
into the history.
We'll need to do some profiling of the backend after this is deployed
in production; it's possible we'll need to add some database indexes,
denormalization, or other optimizations to avoid making loading the
Zulip app significantly slower.
Fixes#12502.
message_id, rather than timestamps, is our standard way to sort by
time. And this refactor is important because we're about to start
using data from the server to populate this data structure.
This change makes it possible for users to control the notification
settings for wildcard mentions as a separate control from PMs and
direct @-mentions.
This commit was automatically generated by `tools/lint --only=eslint
--fix`, after an `.eslintrc.json` change.
A half dozen files were removed from the changes by tabbott pending
further work to ensure we avoid breaking valuable PRs with merge
conflicts.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Hovering over user names (and user circles for PM List) now displays
Name, Status Message and Last online time in a js tooltip.
Hovering over group names displays the names of all group members.
Unavailable users are shown as "Last active: Today".
Hovering on a user circle in the Buddy List results in a js tooltip
with Active/Idle/Offline/Unavailable for
green/orange/white/white-with-line.
Resolves#11607.
When strings are tagged for translation using `tr this`, the strings
were passed into the frontend i18n as-is (including new line and tab
characters that are not functional in the text, existing just to
format the HTML files reasonably).
This did not match the algorithm used in `manage.py makemessages` for
extracting strings for translation, which (correctly) removed that
whitespace to provide a good experience for translators. The fix is
for the `tr this` implementation to use that same whitespace-stripping
algorithm.
Tested manually by checking if those strings that were not translated
earlier were translated, and also fixed an automated test that had the
wrong result, which should help prevent regressions.
Fixes#13389.
ES6 and TS modules don’t insert themselves into `window`, so our tests
shouldn’t insert them either. Since the test `window` behaves like
`global` now, we can rely on legacy modules that do insert themselves
to do it themselves.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
These should work consistently with how the individual user setting
works; see the last commit.
With changes from tabbott to fix real-time sync.
Fixes#12553.
This commit was originally automatically generated using `tools/lint
--only=eslint --fix`. It was then modified by tabbott to contain only
changes to a set of files that are unlikely to result in significant
merge conflicts with any open pull request, excluding about 20 files.
His plan is to merge the remaining changes with more precise care,
potentially involving merging parts of conflicting pull requests
before running the `eslint --fix` operation.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Because of the separate declarations, ESLint would convert them to
`let` and then trigger the `prefer-const` error.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Even though this variable was only assigned once, it was accessed
before its initialization, so it couldn’t be converted directly to
`let` or `const`. Use `let` with an explicit `null` to make it
clearer what’s going on and satisfy ESLint. (Why not `undefined`?
There’s an ESLint rule against that too.)
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This feels a bit more semantically appropriate: it more clearly says
"here's some information: there is no (relevant) recipient", rather
than "no information available". (Both `null` and `undefined` in JS
can have either meaning, but `undefined` especially commonly means
the latter.)
Concretely, it ensures a bit more explicitness where the value
originates: a bare `return;` becomes `return null;`, reflecting the
fact that it is returning a quite informative value.
Also make the implementation more explicit about what's expected here,
replacing truthiness tests with `!== null`. (A bit more idiomatic
would be `!= null`, which is equivalent when the value is well-typed
and a bit more robust to ill-typing bugs. But lint complains about
that version.)
It'd already been the case for some while that calling `stop` had the
same effect as calling `update` (previously `handle_text_input`) with
a falsy recipient. With the API changes in the previous few commits,
this becomes quite natural to make explicit in the API.
This was named after when it gets called from the UI, rather than
after what it can be expected to do.
Naming it after what it's meant to do -- and giving a summary line to
expand on that -- provides a more helpful semantic idea for reasoning
about the function. Doubly so for using the function in a different
client with its own UI, like the mobile app.
The main motivation for this change is to simplify this interface
and make it easier to reason about.
The case where it affects the behavior is when
is_valid_conversation() returns false, while current_recipient
and get_recipient() agree on some truthy value.
This means the message-content textarea is empty -- in fact the
user just cleared it, because we got here from an input event on
it -- but the compose box is still open to some PM thread that we
have a typing notification still outstanding for.
The old behavior is that in this situation we would ignore the
fact that the content was empty, and go ahead and prolong the
typing notification, by updating our timer and possibly sending a
"still typing" notice.
This contrasts with the behavior (both old and new) in the case
where the content is empty and we *don't* already have an
outstanding typing notification, or we have one to some other
thread. In that case, we cancel any existing notification and
don't start a new one, exactly as if `stop` were called
(e.g. because the user closed the compose box.)
The new behavior is that we always treat clearing the input as
"stopped typing": not only in those cases where we already did,
but also in the case where we still have the same recipients.
(Which seems like probably the common case.)
That seems like the preferable behavior; indeed it's hard to see
the point of the "compose_empty" logic if restricted to the other
cases. It also makes the interface simpler.
Those two properties don't seem like a coincidence, either: the
complicated interface made it difficult to unpack exactly what
logic we actually had, which made it easy for surprising wrinkles
to hang out indefinitely.
All these cases are meant to simulate having a user actually typing a
message to some actual recipients, so the `conversation_is_valid`
parameter would be true.
We make this change so that in an upcoming change that eliminates this
parameter, the adjustments to the test cases can be highly regular and
we don't have to introduce a new wrinkle to correspond to these values
being false.
The real purpose these two callbacks serve is exactly what an ordinary
parameter is perfect for:
* Each has just one call site, at the top of the function.
* They're not done for side effects; the point is what they return.
* The function doesn't pass them any arguments of its own, or
otherwise express any internal knowledge that doesn't just as
properly belong to its caller.
So, push the calls to these callbacks up into the function's caller,
and pass in the data they return instead.
This greatly simplifies the interface of `handle_text_input` and of
`typing_status` in general.
Users generally don't expect wildcard mentions in muted streams and
topics to be treated as a mention, either for the purposes of desktop
notifications or the unread mention counts.
This fixes the unread mention counts part of the issue.
Fixes part of #13073.
This adds the general machinery required, and sets it up for the file
`typing_status.js` as a first use case.
Co-authored-by: Anders Kaseorg <anders@zulipchat.com>
When a user performs a search that might contain historical public
streams messages that the user has access to (but doesn't because
we're searching the user's own personal history), we add a notice
above the first search result to let the user know that not all
messages may have been searched.
Fixes#12036.
This ensures that typing '```java' and pressing enter would result in
getting dropped into a java codeblock instead of javascript codeblock.
We implement this by pushing the exact match of a query to be pushed to
the top of the returned matches in `sort_languages`.
With some comments added by tabbott in the tests explaining the
current reasoning.
Fixes#13109.
This changes the availability icon for bot users to user_circle_green;
previously it was accidentally defaulting to user_circle_empty, making
it appear that bots were never available.
Fixes#13149.
As it turns out, our rerender_the_whole_thing function (used whenever
we were adding messages and discovered that the resulting message list
would be out-of-order) was just broken and scrolled the browser to a
random location.
This caused two user-facing bugs:
* On very fast networks, if two users sent messages at very close to
the same time, we could end up with out-of-order message deliveries,
triggering this code path, which was intended to silently correct
the situation, but failed.
* In some narrows to streams with muted topics in the history but some
recent traffic, the user's browser-cached history might have some
gaps that mean the server fetch we do after narrowing discovers the
history is out-of-order, again triggering the
rerender_the_whole_thing code path.
The fix is to just remove that function, adding a new option to the
well-tested rerender_preserving_scrolltop (which has explicit logic to
preserve the scroll position) instead.
Fixes#12067. Likely also fixes#12498.
This brings us in line, and also allows us to style these more like
unordered lists, which is visually more appealing.
On the backend, we now use the default list blockprocessor + sane list
extension of python-markdown to get proper list markup; on the
frontend, we mostly return to upstream's code as they have followed
CommonMark on this issue.
Using <ol> here necessarily removes the behaviour of not renumbering
on lists written like 3, 4, 7; hopefully users will be OK with the
change.
Fixes#12822.
Fixes#13134 as the last commit in the series for this issue.
Solves the "The (?) should just be a target=_blank link to
/help/message-a-stream-by-email." part of the issue.
As a result, a bunch code managing the email hint popup can be deleted,
together with a node test for that.
There was a bug where the success banner stuck
around even after the export completed. We now
nicely fade and remove the banner upon a successful
population of the export in the table.
Fixes: #13045
Add ability to search entire message history of all public streams at
once. It includes all subscibed, non subscribed public streams messages
and even historical public stream messages sent before user had joined
an organization or stream.
Fixes#8859.
If a user was active within the last 90 days,
show number of days (23 Days ago).
If the user was active more than 90 days ago and in the same year,
then show MMM DD (Mar 15).
In any other case show MMM DD YYYY (Nov 10 2018),
Change timerender.js test to accomodate changes.
Delete trailing newlines from all files, except
tools/ci/success-http-headers.txt and tools/setup/dev-motd, where they
are significant, and static/third, where we want to stay close to
upstream.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This reverts commit 76e50af78e.
Empirically, this caused weird issues with the cursor jumping around,
so more investigation is required into the right way to fix it.
Although SimpleBar automatically sets itself up on elements with a
`data-simplebar` attribute, sometimes we try to set event listeners
before that happens. Create the SimpleBar early in that case.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
When you press enter on a typeahead and start typing, your cursor is
placed at the end of the textbox, whereas we want it to be placed at
the end of the typeahead immediately. This causes some characters to
appear at the end of the message before you again get to typing from
where you left off.
To fix, we use the change event triggered on typeahead completion to
reposition the cursor instead of using a setTimeout().
Fixes#12621.
We implement 3 changes:
1. Partial Stream Typeahead
In addition to regular stream completion, we do partial completion
of stream typeahead on pressing '>'. We use our custom addition to
typeahead.js: this.trigger_selection to start topic_list typeahead.
Implements: `#stream na|` (press >) => `#**stream name>|`.
2. Topic Jump Typeahead
'topic_jump' typeahead moves the cursor from just ahead of a
completed stream-mention to just after the end of the mention
text and is triggered by typing '>' after the stream mention.
This typeahead merely uses the regex matching and event hooks of
the typeahead library instead of displaying any text completions.
Implements: `#**stream name** >|` => `#**stream name>|`.
3. Topic List Typeahead
'topic_list' typeahead shows the list of recent topics of a stream
and if your current text doesn't match one of them, also shows you
the current query text, allowing you to create mentions for topics
that do not exist yet.
Implements: `#**stream name>someth|` => `#**stream name>something** |`.
At the end of this commit, we support the following mechanisms to
complete the stream-topic mention:
1. Type "#denmar|".
2. Press Enter to get "#**Denmark** |".
3. Press > to get "#**Denmark>|".
4. Type topic name and press enter.
OR
1. Type "#denmar|".
2. Type > to get "#**Denmark>|".
3. Type topic name and press enter.
Both result in the final inserted syntax: "#**Denmark>topic name**".
Documentation is still pending.
Fixes#4836.
In this refactor, we extract two functions in unread.js. Which one to
use depends on whether res has already been fetched or not.
This also adds node tests to maintain coverage of unread.js.
Tweaked by tabbott for cleaner variable names and tests.
This change is long overdue. After implementing this much more robust
system and deploying it on chat.zulip.org, we hesitated to make
load_server_counts the default behavior in master, because of data
anomalies present for many existing users (basically messages far back
in their history that they had never read, on streams they believed
themselves caught up on), which would have been confusing for many
users.
However, because the mobile apps have been using this data set for a
long time, we've likely cleared out the anomalies from active users'
data set. And for older users, they're going to come back to
approximately infinite unread messages anyway, so the data anomalies
are unlikely to be important.
Fixes#7096.
This icon should only show when the user is not an admin and either the
realm or server settings have disabled name changes. Previously the icon
always showed for admin users.
The count_span element is parented by a .selectable_sidebar_block element
which is parented by the li element that the class is supposed to be added
to. Thus, use the parents() jQuery method for locating the li parent so
that the class gets added to the correct element.
This commit adds a new setting to the user's notification settings that
will change the behaviour of the unread count in the title bar and
desktop application.
When enabled, the title bar will show the count of unread private messages
and mentions. When disabled, the title bar will act as before, showing
the total number of unread messages.
Fixes#1736.
The approach taken here is basically use user IDs in operator that
support it when sending the request for fetching the messages
(see comments in code for more details).
Combined with work in the desktop app, this makes it possible for the
desktop app to clearly indicate to other users whether the current
user is active on the system and thus would see a desktop
notification, not just whether they are active in the current Zulip
window.
Essentially rewritten by tabbott to add unit tests and consider the
desktop app data authoritative.
Add new custom profile field type, External account.
External account field links user's social media
profile with account. e.g. GitHub, Twitter, etc.
Fixes part of #12302
Not all our errors actually happen in the contexts we were
wrapping (e.g. `setTimeout` and `_.throttle`). Also this fixes the
neat Firefox inspector feature that shows you where your event
handlers for a given DOM element actually live.
Using this "semi-modern" browser event means that Safari 9 and older
and IE10 and older may not have our browser error reporting active;
that seems fine giving the vanishing market share of those browsers.
https://blog.sentry.io/2016/01/04/client-javascript-reporting-window-onerror
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The comment that jQuery “doesn’t have” this was nonsense: jQuery
supports every event the browser does.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
After migration to an ES6 module, `suppress_unread_counts` would no
longer be mutable from outside the module.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
After migration to an ES6 module, `server_furthest_read` would no
longer be mutable from outside the module.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Fix the .get_suggestions and .get_suggestions_legacy
to correctly handle search terms in group PM and treat
it as search term by not concatenating it at end of pm-with
email list operand.
We reuse the link regexes we use elsewhere inn markdown
for parsing links in topic names and add a button to open
them in new tabs similar to our behavior with linkifiers
in topic names.
Fixes#12391.
Right now we have buttons for "New conversation" and "New private message"
in different views, but both buttons do the same thing.
The current state is confusing for new users, since there is already a lot
of terminology one needs to learn in order to understand the Zulip
conversation model. It's very plausible a user would think a "conversation"
is something different from a "private message" or a "topic".
With the help of `check_property_changed` function now we collect the data
whose values are changed from the current one. Currently this optimizes
only for those elements whose values are collected by
`populate_data_for_request` function i.e. it doesn't optimize data
collected by `get_complete_data_for_subsection`.
This is a preliminary commit which refactors `populate_data_for_request`
function, now this function traverse on all "property elements" of a given
subsection, but get the data only of those properties which have
`setting-widget-type` data attribute. Therefore, it doesn't change the
functionality of this function and overall changes don't make any
difference. In upcoming commits, we're going to use `input_elem` as an
argument to `check_property_changed` function, so that only those elements
whose values are changed are sent to the backend.
This commit disables "Mobile notifications" if `push_notifications` are not
enabled. It also adds a tooltip explaining why this is disabled.
Fixes#12208
Add `name` attribute to stream settings `input` elements
and change `id` attribute corresponding to name.
`name` attribute stores value of settings name which is
stored in frontend data sets.
Some search queries always return empty because of how we handle search,
this adds text that ensures users trying bad searches realize that they
are doing so.
This fixes an issue where one could end up with a `(` in the markdown
syntax for a link after copy-pasting this, which doesn't work in
markdown.
Fixes#12579.
Rename notification property `enable_stream_sounds` to
`enable_stream_audible_notifications` to match with other
notification property patterns.
Fixes part of #12304
Prior to this commit, we'd put up the green "Enable desktop
notifications" bar on page load AND the first time a desktop
notification worthy message was received, it would attempt to notify,
automatically triggering a browser permission popup (the same one as
clicking the green bar results in).
Now, desktop notifications are not attempted at all until the green
bar is clicked. Additionally Firefox and Webkit browser-specific
checks are made more uniform and done at the same point.
Tested written by YashRE42.
Fixes#11504.
This tests was added to make sure we catch subtle bug related to
comparing new_recipient and current_recipient. When we changed the
recipient to use arrays instead of string to use new user IDs based
api we encoured this bug and out testing suite couldn't detect this.
API changes:
* The behaviour of Date.toLocaleTimeString() reverts to pre 8.0.0,
this only affects automated tests. Lots of other API changes but
we didn't use any of those.
* The internal sorting algorithm changed which causes one of our own
compare function to miss coverage.
This adds a setting to control Zulip's default behavior of sorting to
bottom and graying out inactive streams. The previous logic is still
the default "automatic", but this gives users more control. See the
models.py comment for details.
Fixes#11524.
We have had a longtime bug where the state of pinned streams would not
update properly from the greyed out/inactive state to the active state
when a first message arrived to them.
After some discussion, we determined that likely the right fix for
this is to simply configure pinned streams to never be marked as
inactive; that's more in line with the intended user experience.
Fixes#8201.
Lightbox previews for youtube playlists use the "current" video in the playlist
for the preview. The open link for such previews is incorrectly set to the first
video alone, and not the playlist. This commit fixes the bug by linking to the
original URL for lightbox preview is being shown, instead of computing the URL.
This commit separates the `waiting_period_threshold` setting from
the `create_stream_policy` setting, adding a new setting that the user
can use to select a waiting period threshold.
Both the invite to stream policy and create stream policy now have
three options: admins only, members and admins, or members after
waiting period/admins.
In email hidden case (that is when `email_address_visibilty` is set to
everyone), for "non admins", this commit hides emails from:
- compose box user typeahead.
- PM user typeahead
In email hidden case, for admins, email is shown in user typeaheads.
In email hidden case (that is when `email_address_visibilty` is set to
everyone), for "non admins", this commit hides emails from:
- user popover
- custom profile popover
In email hidden case, for admins, email is shown in both user popovers and
custom profile popovers.
Along with this, we refactored settings_org.populate_auth_methods to use
HTML function after rendering all auth methods rows rather than appending
each row individually, which actually is a good practice.
Also in this commit, to compare `current_val` and `changed_val` in
`check_property_changed` function of the property
`realm_authentication_methods`, which are objects, and we found here
https://stackoverflow.com/a/1144249 that there is no easy way to do so. So
I followed this approach,
```js
JSON.stringify(obj1) === JSON.stringify(obj2)
```
but before converting them to string we want the same order of keys, so we
used `sort_object_by_key` to sort `current_val` by keys and
`get_auth_method_table_data` always return `changed_val` having keys
sorted.
Since these refactor were closely related we kept them as a single commit
here.
Fixes: #11954.
We now use a Proxy to wrap zjquery elements, so
that we can detect callers trying to invoke methods
(or access attributes) that do not exist. We try
to give useful error messages in those cases.
The main impact here is that we force lots of tests
to explicitly stub `length`.
Also, we can't do equality checks on zjquery
objects any more due to the proxy object, but the
easy workaround is to compare selectors. (This
is generally an unnecessary technique, anyway.)
The proxy wrapper is fairly straightforward, and
we just have a few special cases for things like
"inspect" that happen when you try to print out
objects.
With perfectScrollbar, we needed to call a function from JavaScript to
enable a scrollbar on a new element, but simplebar has a much simpler
default API one can do by using data-simplebar attributes in the HTML.
So we can delete all the scrollbar creation/deletion code.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Instead of deleting and rebuilding #private-container every time its
contents need to be updated, just replace its contents. This
eliminates some scrollbar flashing.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
perfect-scrollbar replaces both the appearance and the behavior of the
scrollbar, and its emulated behavior will never feel native on most
platforms. SimpleBar customizes the appearance while preserving the
native behavior.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Send at most 1k message ids in a single read flag request to avoid locking
large number of rows in server database in a single request and avoid long
processing time.
Fixes#11956.
This commit migrates the Subscription's notification fields from a
BooleanField to a NullBooleanField where a value of None means to
inherit the value from user's profile.
Also includes a migrations to set the corresponding settings to None
if they match the user profile's values. This migration helps us in
getting rid of the weird "Apply to all" widget that we offered on
subscription settings page.
The mobile apps can't handle None appearing as the stream-level
notification settings, so for backwards-compatibility we arrange to
only send True/False to the mobile apps by applying those defaults
server-side. We introduce a notification_settings_null value within a
client_capabilities structure that newer versions of the mobile apps
can use to request the new model.
This mobile compatibility code is pretty effectively tested by the
existing test_events tests for the subscriptions subsystem.
If MAX_FILE_UPLOAD_SIZE is set to 0, then UI elements like the upload
icon in the compose and message edit UI and "Attachments" menu in
"/#settings" are not displayed.
A different error message is also displayed if a user tries to drag and
drop or paste a file into the compose message box.
Fixes#12152.
This adds three bools to message_container object which calculate bools
where the "(EDITED)" label should appear:
* `edited_in_left_col` -- when label appears in left column.
* `edited_alongside_sender` -- when label appears alongside sender info.
* `edited_status_msg` -- when label appears for a "/me" message.
We use the new bools and remove the complicated if else statements
from the templates for the "(EDITED)" label.
This also allows us to add a unit test to verify the logic.
This commit renames the `create_stream_permission` field in the
templates to `create_stream_policy`, matching the field used in the
database model. This matches what `invite_to_stream_policy` does and
will be clearer when the `waiting_period_threshold` is split into its
own field.
This commit replaces the `create_stream_by_admins_only` setting with a
new `create_stream_policy` setting, which mirroring the structure of
the existing `invite_to_stream_policy`.
This is important preparation for migrating the waiting period feature
to be its own independent setting.
Fixes#12236.
Fixes#12251.
Previously when disabling name changes in server settings, instead
of realm settings, the name edit button did not get disabled.
Changing name resulted in a message stating `no changes made`.
Fixes#12132.
Realm setting to disable avatar changes is already present.
The `AVATAR_CHANGES_DISABLED` setting now follows the same
2-setting model as `NAME_CHANGES_DISABLED`.
This is useful when syncing avatars from an integrated LDAP/active
directory.
The upload avatar and delete avatar buttons are hidden if avatar
changes are disabled and the user is a non-admin.
If the user has a gravatar set, then the user will not be able to
upload an image as their avatar if avatar changes are disabled.
Part of #12132.
This commit creates a new organization setting that determines whether
a user can invite other users to streams. Previously this was linked
to the waiting period threshold, but this was both not documented and
overly limiting.
With significant tweaks by tabbott to change the database model to not
involve two threshhold fields, edit the tests, etc.
This requires follow-up work to make the create stream policy setting
work how this code implies it should.
Fixes#12042.
In handlebars and settings_org.js, the subsection in which
`realm_message_retention_days` property lies doesn't agree, and this wasn't
observed for a long time as it's disabled, still to make things right, in
this commit we have moved the logic which handles the collection of data
from `other_permission` to `other_settings` as it makes more sense there.
This is a small patch to fix the error message an admin would receive if
they tried to change bot info and owner from the "bots" setting of the
organization settings panel.
The alphabetic sorting of lists in the User/Organization settings was
changed in fa0a5ec to be case insensitive.
This commit makes changes to the list_render.js test to verify that the
sorting of these lists is indeed case insensitive.
We no longer store handlers as an array of functions,
and instead we assume that code will only ever set up
one handler per sel/event or sel/event/child. This is
almost always a sane policy for the app itself.
We also try to improve error handling when devs write
incorrect tests.
The only tests that required changes here are the
activity tests, which were a little careless about how
data got reset between tests.
Apparently, our use of JavaScript string `.replace()` here was buggy,
because replace() has several special escape sequences starting with
`$` if they appear in the replacement content string. We can work
around this through something of a hack, which is to pass a function
as the second argument to replace, which seems cleaner than replacing
all $s with $$s.
Thanks to Shreya for the report.
Now that we have a scroll container for the PM list,
it doesn't make much sense to limit the number to
five.
We may resurrect this feature if "more conversations"
actually fetches more conversations, but it doesn't
currently.
We also may soon make it easy to limit PMs to just
unread messages, which will make the max-5 feature
perhaps less necessary, and we don't want to make
the UI overly complicated.
This is the follow-up of PR #10267.
Here, we add typeahead for slash commands `/me` and `/poll` in compose box.
The slash typeahead will open only when `/` is the first character and all
slash commands will be displayed when one types `/`, as this helps a lot in
discoverability. This also adds a description of what the slash command
does in the typeahead.
When new PMs came in, we would do a little
animation to show you the incoming message.
Unfortunately, it was broken and would animate
too many things. (The code looks at a single
var to see if PM counts changed, but there are
actually mulitple PM counts. We could fix that,
but we decided that this feature just isn't
worth the complexity.)
We still animate incoming mentions.
Fixes#11868.
This makes the "more topics" option which appears below the list of
known topics in the left sidebar appear only when it's possible there
are actually more topics to be displayed. Two specific cases it
resolves completely include:
* Newly created realms; this widget was a common source of confusion
for new organization administrators.
* Newly created streams.
There are still some corner cases this doesn't handle, e.g. if you
just joined a private stream with protected history, but there isn't
as easy a fix for those.
Essentially rewritten by tabbott to fix code duplication and comment
extensively.
Fixes#10265.
As a follow up of commit (bf1c9420df), this
commit removes the `build_realm_day_mode_logo_widget` and
`build_realm_night_mode_logo_widget` function , and changes
`build_realm_logo_widget` to take single argument `is_night` and depending
on this argument, corresponding `day mode` or `night_mode` widget is
handled.
This changes the "new private message" button to be instead "new
conversation" when looking at PMs, to avoid confusion that the button
was the right thing to do to reply to the current private message
conversation.
Fixes#11679.
Even though there are only ever zero or one active
topic widgets in our current sidebar, it's almost the
same amount of code to just manage them with a Dict.
Also, we can more easily do possible future features
like setting streams to be always-open.
This moves the configuration of widget type from settings_org to instead
live in respective HTML templates, via `data-widget-setting-type` and we
also remove `get_subsection_property_types` and refactor function
`populate_data_for_request` accordingly.
Fixes: #11708.
This fixes the bug where the `Saved` state button faded out almost
instantly (that is actually 300 ms) and `Discard` button fades out
along with `Saved` state button; the key problem here was that the
setTimeout intended to fade was actually delaying the transition from
"saving" to "saved".
Now, first of all, we use `setTimeOut` function to fadeout elements giving
fadeout_delay time as `800 ms` and we hide discard button during `saving`
state. Also, when `Discard` button is selected, `Save changes` and `Dicard`
fade out simultaneously.
Fixes: #11737.
If you click on the avatar, we now show the menu
right next to the avatar. The current behavior
is particularly funny for long names. (I confirmed
this with Rishi.)
Adds possibility for users to use | as an OR-operator (besides ,)
when searching for other users.
This is a thing reasonable folks might try, and | in the thing to
search for isn't a realisitic possibility, so there's no real downside
to adding this.
Fixes#4109.
In this commit, I've added a feature to unstar all the starred
messages. This is useful, e.g., for folks who are using starred
messages to keep track of things they should come back when next at
their desktop.
The event flow is the standard one for a feature with a confirmation modal:
(1) User clicks on unstar all messages.
(2) We display a confirmation modal; if the user confirms, we send a
request to the backend to clear all starred messages.
(3) The events system sends that UI update back to us, removing the
stars from the UI.
Fixes#11401.
This commit deduplicates the code for `build_realm_logo_widget` and
`build_realm_night_logo_widget`. It deduplicates the common code for
`build_realm_day_mode_logo_widget` & `build_realm_night_mode_logo_widget`
into tthe function `build_realm_logo_widget`.
Add a background highlight to vote count button if currently
logged in user votes on that option.
Tweaked by tabbott to use better variable names and Rishi for better
styling.
This is a pure data function, so it shouldn't be in popovers.js file
(Steve Howell added test coverage here, and tabbott removed an
accidental functional change.)
The correct behavior here is that we want to ensure there is
whitespace in between the syntax being added and the content on either
side. Our smart_insert logic handled this for the cases that were
common with inserting emoji (etc.), but didn't handle the more complex
cases with "quote and reply".
Fixes#11702.
This new helper allows us to do the same operation
on every message in our message_store. We will
use this in a future commit to clear the `is_tall`
flags on all messages, after a resize.
We should be somewhat cautious about using this,
but simple operations should be really fast, even
if you have lots of messages in the store.
Previously, if you were in the process of editing the last message in
a narrow and a new message came in, we'd rerender that second-to-last
message, causing your editing widget to lose focus (and thus the next
few keys you typed to be interpreted as keyboard shortcuts, which
had a good chance of resulting in your navigating somewhere random).
This rerendering was essentially unnecessary; the only change to state
going into the rendering process was the next_is_same_sender CSS class
being toggled on the messagebox in the message. So, at most, we
should have been just toggling that CSS class (and this commit makes
us do precisely that).
It seems like we could further improve this code by just removing the
next_is_same_sender CSS class entirely and removing this block, but
I'm leaving that for follow-up work.
Fixes#11656.
This fixes an issue where closing stream search was not working if
user had not entered a search term and tried to close the search box
by clicking on the close icon; the problem was that we'd end up
re-opening the widget immediately after through event propagation.
Fixes: #11636.
Also adds tests to ensure that we do not accidentally overwrite
the 'beginning' variable that contains the message content upto
that point. These should prevent similar errors in the future.
The bug was added in 8119258c4d.
The bug here was that when we rerendered messages following local echo
through the echo.process_from_server code path, the eventual call to
_rerender_header() made the implicit assumption that all messages in a
message group had the same date. As a result, it created a totally
new/fake message group and called the rendering logic on that group
without calling the functions for setting up recipient row dates,
which would always result in no recipient bar date being added. This
bug was latent/invisible before, because when introduced, the locally
echoed messages were always being added to a recipient group from
today, where the recipient bar's date area was by default empty anyway.
This latent bug was revealed when we modified the structure of the app
to do date dividers between individual messages within a message
group, rather than strictly between message groups.
For consistency, we should keep all the code that works with
@mentions in markdown.js. In this case, message_list_view was
rewriting the contents of the mentions in cases where users'
names had been changed since we rendered their mention.
This change should help people discover to distinguish
silent mentions in text as a part of Zulip syntax while
differentiating them from regular mentions.
To test formatting we want a hard coded date, so we
can verify the date arithmetic with stable dates.
To make the test less brittle, we disable the
feature to remove old drafts.
This was an emergency fix. We should probably just
remove the last N drafts instead of having the 30-day
limit. Or we should have a better way to stub the cutoff
date.
This is mostly adding markup, calling some convenient
functions in buddy_data.js, and adjusting CSS.
To make the circles update dynamically, I mostly
orchestrate this though activity.js for now. It's
possible we'll want to adjust that eventually to
happen through something like a `presence_events`
dispatcher, but that's essentially what
a good part of `activity.js` does now.
We now have a function get_user_circle_class
that returns one of these values:
"user_circle_green"
"user_circle_orange"
"user_circle_empty"
And we put that in the templates.
And then CSS renders the circle of the appropriate
color.
The unit tests now explicitly capture whether
we are rendering the correct kind of circle.
This fixes a longstanding UI issue when you have way too many recent
private message conversations, as you can now scroll down the list to
find what you're looking for.
Fixes#5384.
This removes the left border extending the stream label from the
recipient bar in from the drafts in drafts modal. Those borders are
important in the message feed for containing several messages, but
here we're only ever going to show individual drafts, and this change
avoids potential color clashes with the blue box surrounding the
recipient blocks.
Show "sent to different narrow" notification and other such notification by
notifications.notify_local_mixes for non locally echoed message sent by
current client.
With significant new comments added by tabbott.
Fixes: #11488.
We swallow the error if our python_to_js_filter code is
unable to parse some python regex properly. This ensures
that the web app stays responsive.
We would fail to show an accurate local echo for these
regexes, however, the backend would act as the final
authority for handling the realm pattern conversion.
Since on replacing the first 'P<>' group, we remove this text from
the string, we have to make the RegExp start looking from index 0
again to properly convert later 'P<>' groups to JS regex syntax.
We want the search widget, when visible, to be
outside the scroll container for the stream list.
One obvious use case is if you start scrolling, and
then realize it might be less effort to search.
Also, for user search, it already worked this way.
We have to add a couple resizing hooks here, but
it's not necessary to change the actual resize
calculation, since we move the section inside
of #streams_header, which is already accounted
for.
The only markup change here is to add
a `stream_search_section` class. I don't
know why we use `notdisplayed` here instead of
jQuery, or what `input-append` is for, but I
considered them outside the scope of this change.
We can also remove some crufty CSS that was
compensating for it being inside the container.
This adds date dividers within a single message group when the only
reason we had previously been splitting apart two message groups is a
change of date. The overall effect is a cleaner message list user
experience.
The downside of this change would be that the recipient bars no longer
will always show a new date for date changes; to fix that, we rewrite
how the floating recipient bars both set the date field on the
floating recipient bar itself, as well as ensure that non-floating
recipient bars don't show duplicate dates.
In a future design update where we modify how message recipient bars
look, we may very well be able to simplify this logic by removing some
of the dynamic nature of the recipient bar calculations. But this is
a good implementation of what remains.
Tweaked significantly by tabbott from Steve Howell's original, both to
extract these changes from a larger PR as well as to modify the
first_visible_message logic to handle some tricky corner cases.
Fixes#10171.
Use the results of commit #73d26c8 to remove the method
`render_stream_description` in static/js/stream_data.js and instead
use the rendered_description attribute now being sent by the backend.
This will be a valuable optimization and a step towards removing the
need for the marked.js markdown parser and speeding up the client end.
In small screen sizes, when the user presses shortcut `w` to search
for another user, the hide_all function calls in the search code path
would hide the right sidebar, immediately after opening it, making the
hotkey basically unusable.
We fix this by extracting a separate hide method that hides all true
popovers, but not the user list sidebar.
Fixes#11463.
This code will correctly add video call link to the message
textarea based on whether 'Add video call' was selected from
message composition form or message edit form.
The implementation was semi-rewritten by tabbott to remove an
unnecessary global variable, with fixes for the unit tests from
showell.
Fixes#11188.
This is primarily a feature for onboarding, where an organization
administrator might send a bunch of random test messages as part of
joining, but then want a pristine organization when their users later
join.
But it can theoretically be used for other use cases (e.g. for
moderation or removing threads that are problematic in some way).
Tweaked by tabbott to handle corner cases with
is_history_public_to_subscribers.
Fixes#10912.
This function unlike `invite_streams()` returns an array of objects having
various info (name, stream_id, invite_only, default_stream) related to
streams rather than an array of names of streams.
We now compute the class that drives the tiny
green/orange/empty dot in the user popover using
the same logic as the buddy list.
This was broken in the early implementation of
set/clear-away, but it was never released.
Fixes#11413
This makes it possible to mention a user with a name like Gaël that
contains diacritics by typing e.g. "Gael", significantly reducing the
need to use a special keyboard to mention other users.
Fixes#11183.
The following elements in the top left corner
are major components of our app:
All messages
Private messages
Starred messages
Mentions
We can now find them directly:
$('.top_left_all_messages')
$('.top_left_private_messages')
$('.top_left_starred_messages')
$('.top_left_mentions')
Before this, we had to build up complicated selectors
like below:
exports.get_global_filter_li = function (filter_name) {
var selector = "#global_filters li[data-name='"
+ filter_name + "']";
return $(selector);
};
I don't think any newbie would know to grep for "global_filter",
and I've seen a PR where somebody added specific markup here
to "Private messages" because they couldn't grok the old scheme.
Another thing to note is that we still have a "home-link"
class for "All messages", which overlapped with portico
code that had the same name. (There were some inaccurate
comments in the code relating to the tab bar, but we don't
actually have a way to click to the home view in the tab
bar any more.) I'll eliminate that cruft in another commit.
For this commit the four elements still have the
"global-filter" class, since there's some benefit to being
able to style them all as a group, although we should give
it a nicer name in a subsequent commit.
Most of this PR is basic search/replace, but I did add a
two-line helper: `top_left_corner.update_starred_count`
We had initially designed the poll widget like a blog
post with comments beneath it but it makes more sense
to think of it as just a simple poll with options.
Instead of custom stubs, we now use zjquery. We also
limit a couple checks to the first call to
`show_empty_narrow_messages`, since it's the same
logic every time.
When you hover over a user that has set a user
status, we now show something like "out to lunch."
You can test this in the console by doing:
user_status.server_update({status_text: 'out to lunch'})
And then hover over your name in the buddy list.
The stubs here were kind of unnecessary, as the
real people module is lightweight and data setup
is pretty easy.
In passing I also removed the unnecessary `sed`
abbreviation.
The name `insert_user_into_list` is sort of misleading,
since we are often just redrawing the user's existing
item in the buddy list.
I chose `redraw_user` over `update_user` to emphasize
that we're just going to redraw it with whatever data
has been updated by the callers.
This adds a setting under "Notification" section of
"Organization settings" tab, which enables Organization administrator to
control whether the missed message emails include the message content or
not.
Fixes: #11123.
The commit f863a9b567 had modified
jquery.filedrop's paste method to exit early if any of the items in the
clipboardData is of the string kind. The early exit was added to prevent pasting
an image thumbnail for text copied from software like MS Word, instead of
pasting the actual copied text content. When copying an image in a (modern?)
Browser, though, the clipboard seems to contain a html `img` tag item, along
with the actual image file. This resulted in pastes being broken.
This commit modifies the condition checked for the early exit. We now actually
look at the html content in the clipboard to see if it is an `img` tag, in which
case we upload the image, instead of exiting early.
Closes#7130.
This commit takes away the ability for non-admin members to create
streams where only admins can post messages by hiding the option from
them.
Fixes#11290.
You can now pass in an info field with a value
like "out to lunch" to the /users/me/status,
and the server will include that in its outbound
events.
The semantics here are that both "away" and
"status_text" have to have defined values in order
to cause changes. You can omit the keys or
pass in None when values don't change.
The way you clear info is to pass the empty
string.
We also change page_params to have a dictionary
called "user_status" instead of a set of user
ids. This requires a few small changes on the
frontend. (We will add "status_text" support in
subsequent commits; the changes here just keep
the "away" feature working correctly.)
We had a bug where if you started typing a message
and then used quote/reply (after the fact), we
would overwrite the user's original message.
The bug was kind of subtle--the internal call
to "respond" to the message would select the message
text, and then `smart_insert` would replace the
selection, unless it was Firefox.
Note that we now also allow you to cross-post
replies, which is a plausible scenario, although
possibly unintentional at times, too. I'm erring
on the side of giving the user control here, but
I'll add a warning in the next commit. Our compose
fade feature should also prevent unintentional
mixes here, too.
NOTE: If you revert this commit, you want to revert
the immediately prior commit as well. The history
is that Ishan made some improvements to the widget,
but there were some minor bugs. I decided not
to squash the commits together so that the git
history is clear who did what. (In particular, I
want questions about the JS code to come to me if
somebody does `git blame`.)
Anyway...
This is a fairly significant rewrite of the polling
widget, where I clean up the overall structure of
the code (including things from before the prior
fix) and try to polish the prior commit a bit as
well.
There are a few new features:
* We tell "other" users to wait for the poll
to start (if there's no question yet).
* We tip the author to say "/poll foo" (as
needed).
* We add edit controls for the question.
* We don't allow new choices until there's
a question.
On the backend, we extend the BlockQuoteProcessor's clean function that
just removes '>' from the start of each line to convert each mention to
have the silent mention syntax, before UserMentionPattern is invoked.
The frontend, however, has an edge case where if you are mentioned in
some message and you quote it while having mentioned yourself above
the quoted message, you wouldn't see the red highlight till we get the
final rendered message from the backend.
This is such a subtle glitch that it's likely not worth worrying about.
Fixes#8025.
These mentions look like regular mentions except they do not
trigger any notification for the person mentioned. These are
primarily to be used when you make a bot take an action and
the bot mentions you, or when you quote a message that mentions
you.
Fixes#11221.
AFAIK I should this never fail, hence the blueslip.error line. But it
is failing in practice when rendering user groups after looking them
up by ID, and the error handling should definitely be softer.
In between releases, the following commit introduced
a bug where we agressively scroll to the top every
place we call `ui.update_scrollbar`:
092b73d0b7
The main symptoms were that the left and right sidebars
would go to the top for things like selecting a topic,
getting activity updates from the server, and resizing
the window. It was very jarring.
The recent commit looked innocuous--the root of the problem
was the original API expressed an intent to scroll to the
top, but didn't actually do it, so it was a bug in hiding.
There are **some** occasions where it's actually appropriate
to scroll to the top, mostly around search filtering, and
in those places we now call the new `ui.reset_scrollbar`
function.
This is a bit of an emergency fix, so particularly with
the settings stuff, we may get more reports of glitches here.
The important thing here is that you almost never want to
reset the scrollTop for sidebars.
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.
We now have two functions:
add_new_messages
add_old_messages
This is a lot easier on the eyes, and it will also
prevent us from exceeding line length in future commits.
We also remove an unneeded stub in the narrow_activate
tests.
This is the preferred way to check that a user
id belongs to the current user.
We have a recent bug where the current user's
circle doesn't turn green right away. It's not
clear this is the fix, though. (It's hard to
repro locally.)
This is mostly for testing purposes. The code
structure here is pretty stable--we will probably
always use level() here to either sort or
group users, and being able to test it directly
is nice, rather than bringing in all the other
machinery.
This makes it possible it include our standard markdown formatting in
one's custom profile fields, allowing for links, emphasis, emoji, etc.
Fixes#10131.
While we're at it, we remove the JSON parsing that was part of the
user field code path, since this function isn't responsible for
rendering user fields.
Previously, messages with more than one line did not parse '/me' at
the beginning of the message. Since there's a reasonable way to
render multi-line messages, this commit adds support for doing so.
This change does potentially break with the expected behavior of other
slash commands, but it seems worth providing useful functionality over
a blind focus on consistency.
Fixes#11025.
We instead get the specific fields from message
that we use. This is particularly helpful
for subject -> topic migration; we no longer
have to account for "subject" fields in
client-side templates.
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.
The `assert_message_groups_list_equal` and
`assert_message_list_equal` helpers were
always returning `true`, as they were doing
a bogus traversal of the data structures and
always comparing empty arrays, even when there
was real data.
To prevent this pitfall in the future, we assert
that the extracted data is truth-y, and for the
empty cases we just directly assert deep equality
to `[]`.
As part of giving the stream/topic fields in the
compose box longer ids, I broke the autocomplete
code that handles re-focusing the cursor after
a user hits enter. The worst symptom of this was
that we tried to send a message before compose
finished (although it wouldn't fully deliver the
message).
The new code should be a bit easier to grep for
if we rename these fields again, as we explicitly
use selector syntax.
This adds a new realm_logo field, which is a horizontal-format logo to
be displayed in the top-left corner of the webapp, and any other
places where we might want a wide-format branding of the organization.
Tweaked significantly by tabbott to rebase, fix styling, etc.
Fixing the styling of this feature's loading indicator caused me to
notice the loading indicator for the realm_icon feature was also ugly,
so I fixed that too.
Fixes#7995.
We are trying to carve room for a more specific
"user_status" concept, which refers to statuses
that users specifically set, like "I'm away".
So we call this function "update_presence_info",
which reflects that it's more about actual
"presence"--i.e. the user really is present
in the browser, even though the actual human
may not want to be disturbed.
The current user gets excluded from all non-empty
searches, even ones that match the user, since
it can look funny when the user's at the top of a
search, and you'd never need to search for yourself
(again, since the current user is at the top of
the buddy list).
Apparently, we didn't have one of these, and thus had a moderate
number of generally very old violations in the codebase. Fix this and
clear the ones that exist..
We now rely on set_up() methods to call their
own module-specific versions of maybe_disable_widgets()
in the codepath for admin_sections.load_admin_section().
And then for live updates, we just explicitly call
all four modules that support maybe_disable_widgets().
This should make switching between sections slightly faster,
and it also reduces the risk of module A messing with
module B's state. (Granted, we have lots of other ways
that modules can mess with each other's state.)
We had an anonymous callback for drafts that was
hard to read. It's much easier to flatten the code,
give functions actual names, and stub them as needed
in the unit tests.
The first bug fixed here has been around for a long
time--we were redundantly updating unread counts
indirectly via muting_ui.initialize(). The
unread counts also get updated in
unread_ui.initialize(), when we have more valid
state. (And it's worth noting here that the unread
counts get updated yet again once message fetches
complete.)
The second bug was a very recent regression from
my recent stream name -> stream id cleanup in the
muting system. We now depend on stream_data to
initialize muting data, so we need to initialize
muting.js slightly later in the process.
These fixes are intertwined, because they were both
somewhat caused by the anti-pattern of having
muting_ui.js initialize unread_ui.js and muting.js,
instead of doing more direct, fine-grained initialization
from ui_init.js.
Essentially we replace this code:
exports.update_muted_topics = function (muted_topics) {
muting.set_muted_topics(muted_topics);
unread_ui.update_unread_counts();
};
with this:
exports.initialize = function () {
exports.set_muted_topics(page_params.muted_topics);
};
And the modules load like this:
stream_data
...
muting
...
unread_ui
And we don't need any page-load initialization for muting_ui,
which is mostly used for Settings/Muted topics.
This function used to be called initialize_from_page_params(),
and we called it indirectly through `subs.js`.
Now we call it directly from `ui_init.js`, which gives us a
bit more control over how things are initialized. In fact,
this sets us up for the next commit, where I fix a recent
regression I introduced.
This is mostly about cleaning up the naming convention
for streams and topics, but it also adds a test that
specifically tests the muted-topic case (without any
other factors that would prevent a notification).
Before this commit, it was possible to change the
API for muting topics and get false positives, even
when the test setup was clearly broken.
The stream_list test that was fixed here was sort of
broken. It accomplished the main goal of verifying
what gets rendered, but now the data setup part is
more like the actual app code (and simpler, too).
This fixes the most core data structures inside of
muting.js. We still use stream names for incoming
data to set_muted_topics and outgoing data from
get_muted_topics.
This will make us more resilient to stream name changes.
Before, if you were logged on when a stream rename
occured, topics that were muted under that stream would
appear to be unmuted. (You could fix it with a reload,
but it can be jarring to have a bunch of unread messages
appear in your feed suddenly.)
Fixes#11033
Like the other similar commits, we were doing the same work in all
code paths, just with a much more error-prone approach.
We can also now remove the now-unused finish_initial_narrow function.
Like the other commits in this series, we were already doing this in
all of the callers of load_messages; this centralizes that logic in a
less ad-hoc feeling way.
We no longer use or need the start_initial_narrow function.
Previously, each individual caller of load_messages that passed
num_before > 0 would do its own manual management of fetch_status;
now, we just do it inside load_messages.
The stream/topic edit areas now have these ids:
#stream_message_recipient_stream
#stream_message_recipient_topic
They are pretty verbose, but being able to grep
for these without noise does have some value.
We can eliminate the janky `setup_page` methods
and just pass in section from `hashchanged`.
This sets us up to handle browser history more
nicely when you load '#settings' and we could essentially
redirect you to '#settings/your-account' (or similar
things). A future commit will address that.
We also use `launch` as the new entry point, which
is more consistent with other modules.
The prior name of this was a bit inaccurate, as we no
longer ever hide the menu item for non-admins. Also,
it belongs more naturally in `gear_menu.js` at this point.
Also, we remove one call to this, which was in a place
where it was no longer necessary.
We now run the code to disable widgets every time
we reload a section, which was the original intention
of the code, but the call to it only happened when
you first launched the page.
We also continue to run this logic for live updates
of is_admin, although it's worth noting that the
code still only handles the "demotion" case of going
from admin to non-admin. (If somebody makes you an
admin, you continue to need to reload to get
widgets enabled.)
This ensures the "account settings" UI for managing a user's own email
address uses the delivery email, since that's what users care most about.
Eventually, we'll need to add support for at least viewing both email
addresses in "account settings", but this is the right long-term
behavior.
This new setting is still hidden in the UI when not in the development
environment, because the feature isn't ready for production, but
merging this will help simplify future work on the feature.
Previously, Topic editing was offered in the UI even to message
senders and organizations admins only if the message was no more than
one day old. This was correct for the "community topic editing" case,
but not for message senders and organization admins.
While we're at it, this also centralizes some previously haphazard
logic to always call message_edit.is_topic_editable().
Tweaked significantly by tabbott to fix the logic.
Closes#10568.
The goal here was to enforce 100% coverage on
parse_narrow, but the code has an unreachable line
and is overly tolerant of bogus urls. This will
be fixed in the next commit.
Also adds relevant tests and documentation. We currently
do not narrow to a new topic, and instead just narrow to
the stream. Similarly, we do not narrow to a PM if any of
the recipients are invalid.
We ignore keystrokes like alt-left-arrow and alt-right-arrow,
so that the browser can do back/forward.
We may need to refine the handling of ctrl/alt/shift in the
future, but now we only support single-key operations.
Steve Howell
Vishnu KS
Ryan Rehman
sahil839
Rohitt Vashishtha
Aman Agrawal
Aman Agrawal
clarammdantas
Anders Kaseorg
Pragati Agrawal
Hashir Sarwar
Anders Kaseorg
rebtung
Steve Howell
YashRE42
vaibhavrajsingh2001
Roland Crosby
Divyanshu Agrawal
Abhishek-Balaji
Tim Abbott
YashRE42
Stefan Weil
Jagan
Wbert Adrian Castro Vera
shubhamgupta2956
Chris Heald
Tlazypanda
Mohit Gupta
Nat1405
Vinit Singh
Dinesh
Sophie
Matheus Melo
Thomas Ip
Greg Price
Mateusz Mandera
Wyatt Hoodes
Kanishk Kakar
Cynthia Lin
David Wood
Priyank Patel
vsvipul
Yashashvi Dave
Anders Kaseorg
Rishi Gupta
Seth Nickell
Vaibhav
Puneeth Chaganti
Aditya Bansal
Harshit Bansal
Shubham Padia
Hemanth V. Alluri
Nikhil-Vats
Challa Venkata Raghava Reddy
Siddharth Varshney
Casper
ruchit2801
varunvaruns9
overide
Natsu Kagami
Abhinav Singh
kunal-mohta
Shubham Dhama
ss62171
Marco Burstein
Cynthia Lin
Joshua Pan
onkar shedge