In the case where a stream existed but had no subscribers, the error
message used to send to the owner always used `stream_name`, which
may have been None.
Switch to using `stream.name` rather than `stream_name` for this case.
This code is called in the hot path when Tornado is processing events.
As such, making this code performant is important. Profiling shows
that a significant portion of the time is spent calling asdict() to
serialize the UserMessageNotificationsData dataclass. In this case
`asdict` does several steps which we do not need, such as attempting
to recurse into its fields, and deepcopy'ing the values of the fields.
In our use case, these add a notable amount of overhead:
```py3
from zerver.tornado.event_queue import UserMessageNotificationsData
from dataclasses import asdict
from timeit import timeit
o = UserMessageNotificationsData(1, False, False, False, False, False, False, False, False, False, False, False)
%timeit asdict(o)
%timeit {**vars(o)}
```
Replace the `asdict` call with a direct access of the fields. We
perform a shallow copy because we do need to modify the resulting
fields.
Previously, we showed an empty message banner if the user tried
to send an empty message. We only showed it for users with
"ctrl+enter to send" because we thought it might be easy for a
user to press just enter accidentally.
However, this missed the case where the user clicks on the Enter
button. We want to show the user something in this case to tell
them that they're missing message content.
To avoid more complicated logic, this PR removes the banner
completely and changes the compose box border to red if the
user tries to send an empty message (for all cases).
The red line goes away as soon as the composebox has non-whitespace
characters.
This commit adds migration to fix extra_data field
of RealmAuditLog objects created on changing
can_remove_subscribers_group setting to add "property"
field since the same event type will now be used for
other group based stream settings that will be added
in future.
We add stream_permission_group_settings object which is
similar to property_types framework used for realm settings.
This commit also adds GroupPermissionSetting dataclass for
defining settings inside stream_permission_group_settings.
We add "do_change_stream_group_based_setting" function which
is called in loop to update all the group-based stream settings
and it is now used to update 'can_remove_subscribers_group'
setting instead of "do_change_can_remove_subscribers_group".
We also change the variable name for event_type field of
RealmAuditLog objects to STREAM_GROUP_BASED_SETTING_CHANGED
since this will be used for all group-based stream settings.
'property' field is also added to extra_data field to identify
the setting for which RealmAuditLog object was created.
We will add a migration in further commits which will add the
property field to existing RealmAuditLog objects created for
changing can_remove_subscribers_group setting.
This commit adds require_system_group parameter to
get_realm_user_groups_for_dropdown_list_widget.
We currently pass this parameter as "true" always,
but this will be needed in future when we will allow
to set groups other than system groups in settings.
While switching from a private stream in the stream editing UI to
the stream creation UI, Announce stream becomes disabled. The state
of Announce stream option should not be affected by where the create
stream UI is opened.
Made the privacy_type selector more specific since it was also
selecting the last opened stream privacy type.
Fixes: #24238.
We're changing the ping interval from 50s to 60s, because that's what
the mobile apps have hardcoded currently, and backwards-compatibility
is more important there than the web app's previously hardcoded 50s.
For PRESENCE_PING_INTERVAL_SECS, the previous value hardcoded in both
clients was 140s, selected as "plenty of network/other latency more
than 2 x ACTIVE_PING_INTERVAL_MS". This is a pretty aggressive value;
even a single request being missed or 500ing can result in a user
appearing offline incorrectly. (There's a lag of up to one full ping
interval between when the other client checks in and when you check
in, and so we'll be at almost 2 ping intervals when you issue your
next request that might get an updated connection time from that
user).
To increase failure tolerance, we want to change the offline
threshhold from 2 x ACTIVE_PING_INTERVAL + 20s to 3 x
ACTIVE_PING_INTERVAL + 20s, aka 140s => 200s, to be more robust to
temporary failures causing us to display other users as offline.
Since the mobile apps currently have 140s and 60s hardcoded, it should
be safe to make this particular change; the mobile apps will just
remain more aggressive than the web app in marking users offline until
it uses the new API parameters.
The end result in that Zulip will be slightly less aggressive at
marking other users as offline if they go off the Internet. We will
likely be able to tune ACTIVE_PING_INTERVAL downwards once #16381 and
its follow-ups are completed, because it'll likely make these requests
much cheaper.
As of the previous commit, the server provides these parameters in
page_params. The defaults match the values hard-coded in the webapp so
far - so get rid of the hard-coded values in favor of taking them from
page_params.
This old 300s value was meaningfully used in 2 places:
1. In the do_change_user_settings presence_enabled codepath when turning
a user invisible. It doesn't matter there, 140s is just since the
point is to make clients see this user as offline. And 140s is the
threshold used by clients (see the presence.js constant).
2. For calculating whether to set "offline" "status" in
result["presence"]["aggregated"] in get_presence_backend. It's fine
for this to become 140s, since clients shouldn't be looking at the
status value anymore anyway and just do their calculation based on
the timestamps.
This commit deduplicates template code for showing custom profile
fields in user info popover and full profile modal by extracting
it in a new file and then using that template file to render
the fields in user info popover and full profile modal.
This commit does not change the design or behavior and they are
same as before.
This makes use of the new case insensitive UNIQUE index added in the
earlier commit. With that index present, we can now rely solely on the
database to correctly identify duplicates and throw integrity errors as
required.
This will allow us to rely on the database to detect duplicate
`UserTopic`s (with the same `topic_name` with different cases)
and thus correctly throw IntegrityErrors when expected.
This is also important from a correctness point of view, since as
of now, when checking if topic is muted or requesting the backend for
muting a topic, the frontend does not check for case insensitivity.
There might exist duplicate UserTopics (in a case insensitive sense)
which need are removed before creating the new index.
The migration was tested manually using `./manage.py shell`.
In 141b0c4, we added code to handle races caused by duplicate muting
requests. That code can also handle the non-race condition, so we don't
require the first check.
The background-color and opacity is same for all
select elements in modals and settings in disabled
state, but due to the background of modal being
bright enough, the select element in modals would
not look disabled.
One possible solution could have been to set
"opacity: 1" but that changes opacity for text too
and makes it darker which is not the case for other
select elements.
So instead made the background slightly darker for
select elements inside the modal using hsla property.
It might not be exact same as the other select elements,
but it is still better than the previous behavior.
For role element in bot edit form, we used to set
opacity to 1 to fix this bug, and this commit removes
it as we have fixed it for all modals in general
Since we added settings_select class to select elements
in both stream settings and user or realm settings in
previous commits, we can have common CSS defined at only
one place.
This commit adds modal_select class to select elements in
modals, such that we can add CSS using this class and not
using "select" as selector so that we can easily add a
select element in future with different CSS if needed.
This commit adds settings_select class to select elements in
stream settings, such that we can add CSS using this class and
not using "select" as selector so that we can easily add a select
element in future with different CSS if needed.
This commit adds settings_select class to select elements in
user and organization settings, such that we can add CSS
using this class and not using "select" as selector so that
we can easily add a select element in future with different
CSS if needed.
Currently, when restoring drafts with long
messages no character limit or the compose
banner is shown.
We fix this by adding a check for text
overflow whenever any draft is restored.
We directly check if message feed is visible in the code instead
of indirectly checking so via recent topics. This helps read the
code clearly and would be helpful with the upcoming inbox view.
`#recent_topics_table` may not be present in the DOM when
this click handler is initialized which can cause this
it to not work, delegating it to body ensures it will always work.
Removes the initial check in `_internal_prep_message` of the length
of the message content because the `check_message` in the try block
will call `normalize_body` on the message content string, which
does a more robust check of the message content (empty string, null
bytes, length). If the message content length exceeds the value of
`settings.MAX_MESSAGE_LENGTH`, then it is truncated based on that
value. Updates associated backend test for these changes.
The removed length check would truncate the message content with a
hard coded value instead of using the value for
`settings.MAX_MESSAGE_LENGTH`.
Also, removes an extraneous comment about removing null bytes. If
there are null bytes in the message content, then `normalize_body`
will raise an error.
Note that the previous check had intentionally reduced any message over
the 10000 character limit to 3900 characters, with the code in
question dating to 2012's 100df7e349.
The 3900 character truncating rule was implemented for incoming emails
with the email gateway, and predated other features to help with
overly long messages (better stripping of email footers via Talon,
introduced in f1f48f305e, and
condensing, introduced in c92d664b44).
While we could preserve that logic if desired, it likely is no longer
a necessary or useful variation from our usual truncation rules.
This has always been in the global namespace. Pretending that it’s
scoped when it isn’t and giving it a short name will lead to
confusion.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Flatpickr had been unconditionally using the light theme in automatic
color scheme mode; this fixes it to follow the system preference like
the rest of the app.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Updates the descriptions of content parameters (optional and
required) to note that the maximum size of the message content
should be based on the `max_message_length` value returned by
the register endpoint.
Previously these descriptions had a hardcoded value of 10000
bytes as the maximum message size.
Also, updates the description of `max_message_length` to clarify
that the value represents Unicode code points.
The password parameter being passed in the `_do_test` helper
function for `TestAuthenticatedJsonPostViewDecorator` tests was
being ignored, as the user needs to be logged in. Removes the
parameter from the helper function and updates the success test
to use `assert_json_success` instead of just checking the status
code.
Also adds a test case for when a user is not logged in to confirm
that it returns an UnauthorizedError.
This reverts commit 851d68e0fc.
That commit widened how long the transaction is open, which made it
much more likely that after the user was created in the transaction,
and the memcached caches were flushed, some other request will fill
the `get_realm_user_dicts` cache with data which did not include the
new user (because it had not been committed yet).
If a user creation request lost this race, the user would, upon first
request to `/`, get a blank page and a Javascript error:
Unknown user_id in get_by_user_id: 12345
...where 12345 was their own user-id. This error would persist until
the cache expired (in 7 days) or something else expunged it.
Reverting this does not prevent the race, as the post_save hook's call
to flush_user_profile is still in a transaction (and has been since
168f241ff0), and thus leaves the potential race window open.
However, it much shortens the potential window of opportunity, and is
a reasonable short-term stopgap.
The post-delete signal on AlertWord clears the realm cache; when it is
called repeatedly, this results in re-fetching the realm object O(n)
times, where n scales by number of users in the database.
Disconnect this cache-clearing signal before removing the AlertWord
entries, and reconnect it afterwards. This is not thread-safe, but
this section is single-threaded. It is also probably unnecessary to
re-connect the signal, as rest of `./manage.py populate_db` does not
delete AlertWord objects, but cleanliness dictates doing the
re-connection.
This drops the time to repeatedly run:
python3 ./manage.py populate_db --num-messages=0 --extra-users=1000
...from 47 seconds to 36 seconds.
5db55c38dc switched from `ensure => present` to the more specific
`ensure => directory` on the premise that tarballs would result in
more than one file being copied out of them. However, we only extract
a single file from the wal-g tarball, and install it at the output
path. The new rule attempts to replace it with an empty directory
after extraction.
Switch back to `ensure => present` for the tarball codepath.
The Client.name field is only 30 characters long, but there is no
limit to the length of parsed User-Agent value which we may attempt to
store in it. This can cause requests with long user-agents to 500
when the creation of the Client row fails.
Truncate the name at 30 characters for the cache key, and passing
`name` to `get_or_create`.