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 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.
This will allow us to re-use this logic later, when we add support for
re-checking notification settings just before sending email/push
notifications to the user.
Also, since this is essentially part of the notifiability logic,
this better belongs to `notification_data.py` and this change will
hopefully reduce the reading complexity of the message-send codepath.
This commits update the code to use user-level email_address_visibility
setting instead of realm-level to set or update the value of UserProfile.email
field and to send the emails to clients.
Major changes are -
- UserProfile.email field is set while creating the user according to
RealmUserDefault.email_address_visbility.
- UserProfile.email field is updated according to change in the setting.
- 'email_address_visibility' is added to person objects in user add event
and in avatar change event.
- client_gravatar can be different for different users when computing
avatar_url for messages and user objects since email available to clients
is dependent on user-level setting.
- For bots, email_address_visibility is set to EVERYONE while creating
them irrespective of realm-default value.
- Test changes are basically setting user-level setting instead of realm
setting and modifying the checks accordingly.
Previously, user objects contained delivery_email field
only when user had access to real email. Also, delivery_email
was not present if visibility setting is set to "everyone"
as email field was itself set to real email.
This commit changes the code to pass "delivery_email" field
always in the user objects with its value being "None" if
user does not have access to real email and real email otherwise.
The "delivery_email" field value is None for logged-out users.
For bots, the "delivery_email" is always set to real email
irrespective of email_address_visibility setting.
Also, since user has access to real email if visibility is set
to "everyone", "delivery_email" field is passed in that case
too.
There is no change in email field and it is same as before.
This commit also adds code to send event to update delivery_email
field when email_address_visibility setting changes to all the
users whose access to emails changes and also changes the code to
send event on changing delivery_email to users who have access
to email.
This commit adds time restriction on moving messages between streams
using the move_messages_between_streams_limit_seconds setting in the
backend. There is no time limit for admins and moderators.
We now use the newly added move_messages_within_stream_limit_seconds
setting to check for how long the user can edit the topic replacing
the previously used 3-day limit. As it was previously, there is no
time limit for admins and moderators.
This commit renames parse_message_content_edit_or_delete_limit
to parse_message_time_limit_setting and also renames
MESSAGE_CONTENT_EDIT_OR_DELETE_LIMIT_SPECIAL_VALUES_MAP to
MESSAGE_TIME_LIMIT_SETTING_SPECIAL_VALUES_MAP.
We do this change since this function and object will also be
used for message move limit and it makes sense to have a more
generic name.
This commit extracts a function to parse message time limit type settings
and to set it if the new setting value is None.
This function is currently used for message_content_edit_limit_seconds and
message_content_delete_limit_seconds settings and will be used for
message_move_limit_seconds setting to be added in further commits.
When 'resolve|unresolve' and 'move stream' actions occurs in
the same api call, 'This topic was marked as resolved|unresolved'
notification is not sent.
Both 'topic moved' and 'topic resolved' notification should be generated.
This commit updates the logic of when and where to send
'topic resolve|unresolve' notification. Unlike previous logic, notification
may be sent even in the case 'new_stream' is not None.
In general, 'topic resolved|unresolved' notification is sent to
'stream_being_edited'. In this particular case ('new_stream' is not None),
notification is sent to the 'new_stream' after check.
Test case is included.
Fixes: #22973
Black 23 enforces some slightly more specific rules about empty line
counts and redundant parenthesis removal, but the result is still
compatible with Black 22.
(This does not actually upgrade our Python environment to Black 23
yet.)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We change the do_create_user function to use transaction.atomic
decorator instead of using with block. Due to this change, all
send_event calls are made inside transaction.on_commit.
Some other changes -
- Remove transaction.atomic decorator from send_inital_realm_messages
since it is now called inside a transaction.
- Made changes in tests which tests message events and notifications
to make sure on_commit callbacks are executed.
This commit changes the do_reactivate_user such that the complete function
is called inside an atomic transaction and events are called after the
transaction is commited using on_commit helper. This is a prep commit
for unsubscribing the bots of unaccessible private streams when reactivating
them.
A missed message email notification, where the message is the welcome
message sent by the welcome bot on account creation, get sent when
the user somehow not focuses the browser tab during account creation.
No missed message email or push notifications should be sent for the
messages generated by the welcome bot.
'internal_send_private_message' accepts a parameter
'disable_external_notifications' and is set to 'True' when the sender
is 'welcome bot'.
A check is introduced in `trivially_should_not_notify`, not to notify
if `disable_external_notifications` is true.
TestCases are updated to include the `disable_external_notifications`
check in the early (False) return patterns of `is_push_notifiable` and
`is_email_notifiable`.
One query reduced for both `test_create_user_with_multiple_streams`
and `test_register`.
Reason: When welcome bot sends message after user creation
`do_send_messages` calls `get_active_presence_idle_user_ids`,
`user_ids` in `get_active_presence_idle_user_ids` remains empty if
`disable_external_notifications` is true because `is_notifiable` returns
false.
`get_active_presence_idle_user_ids` calls `filter_presence_idle_user_ids`
and since the `user_ids` is empty, the query inside the function doesn't
get executed.
MissedMessageHookTest updated.
Fixes: #22884
This commit makes all the parameters after 'content' in
'internal_send_*', 'internal_prep_*' and '_internal_prep_*'
a mandatory keyword argument to increase code readability.
For alert words, we currently don't send email/push notifications --
only desktop notifications. Thus, we don't need to consider alert words
here, since desktop notifications do not utilize the presence status
calculated at this stage.
Tested manually that alert word desktop notifications work as expected.
When we implement email/push notifications for alert words (issues #5137
and #13127), we can add new fields like
`notifications_data.alert_word_email_notify`, similar to the existing
`notifications_data.wildcard_mention_email_notify`, which will allow us
to keep the alert word notifiability check inside the dataclass, similar
to how the mentions checks are done currently. So, even when that
feature is implemented, the code which this commit removes would be
unnecessary.
This commit renames "can_edit_topic_of_any_message" function
in models.py to "can_move_messages_to_another_topic" and
"user_can_edit_topic_of_any_message" function in settings_data.js
to "user_can_move_messages_to_another_topic".
This change is done since topic editing permission does not
depend on message sender now and messages are considered same
irrespective of whether the user who is editing the topic had sent
the message or not. This also makes the naming consistent with
what we use for the label of this setting in webapp and how we
describe this action in help documentation.
This commit changes the topic edit permssions to not depend whether the user
editing the message had sent the message or it was sent by someone else.
We only do backend changes in this commit and frontend changes will be done
in further commits.
Previously, we always allowed topic edits when the user themseleves had
sent the message not considering the edit_topic_policy and the 3-day time
limit. But now we consider all messages as same and editing is allowed only
according to edit_topic_policy setting and the time limit of 3 days in
addition for users who are not admins or moderators.
We change the topic and stream edit permssions to not depend on
allow_message_editing setting in the API and are allowed even
if allow_message_editing is set to False based on other settings
like edit_topic_policy and can_move_message_between_streams.
Fixes a part of #21739.
This solves the problem that resolving a topic with a long name (>60
characters) will cause the topic name to be truncated, and thus the edit
message code path thinks that the topic is being moved in addition to
being resolved.
We store the pre-truncation topic and use it to check against the
original topic when determining whether a topic is being moved while
getting (un)resovled or not.
Fixes#23482
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We intended to send both the "topic was resolved" and the "topic was
moved here" notification when resolving and moving a topic at the same
time in #22312.
The previous implementation did not work as expected and it was only
sending the "topic was moved here" notification.
This removes the check for old_topic and new_topic that have
RESOLVED_TOPIC_PREFIX stripped in maybe_send_resolve_notifications, so
that the notification will be sent regardless if the topic name without
the prefix stays the same or not.
Note that weird topic handling ("✔ ✔✔ some topic") in the comments
was added in e231a03eff is unaffected. In case of confusion, the lstrip
check is not essential to detecting topic being unresolved/resolved.
As we mainly have that handled in the latter part of the helper.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This is a follow-up to d201229df8.
do_get_invites_controlled_by_user queries for Confirmations when finding
multiuse invites controlled by a user. This means that a revoked
multiuse invite cannot really be fetched here, because
do_revoke_multi_use_invite deletes the Confirmation object when revoking
the invitations. However, having a defensive assert here should be
useful to make this doesn't secretly break in the future if the query
used changes or if there are unexpected revoked multiuse invites with an
existing Confirmations for any (buggy) reason.
This allows us to revoke MultiUseInvites by changing their .status
instead of deleting them (which has been deleting the helpful tracking
information on PreregistrationUsers about which MultiUseInvite they came
from).
We do not create historical UserMessage rows, for messages that didn't
have one, while marking messages as read and simply ignore those messages.
We do so because there is no user of creating UserMessage rows and it just
wastes storage.
Note that we still allow to mark messages from unsubscribed streams as
read but only those which have UserMessage rows for them to handle the
case when the unread messages were not marked as read while unsubscribing
from the stream due to some race condition. In such cases, messages
will not be included in the unread count shown in "All messages" menu
(and stream is anyways not present in the left sidebar), but the message
border on the left is green if viewing the stream after unsusbcribing it.
So, to avoid the confusion for users, the messages will be marked as read
when user scrolls down.
Zulip's unread messages design has an invariant that all unread stream
messages must be in streams the user is subscribed to. For example, We
do not include the unread messages from unsubscribed streams in the
"unread_msgs" data structure in "/register" response and we mark all
unread messages as read when unsubscribing a user from a stream.
Previously, the mark as unread endpoint allowed violating that
invariant, allowing you to mark messages in any stream as unread.
Doing so caused the "message_details" data structures sent with
"update_message_flags" events to not contain messages from
unsubscribed streams, even though those messages were present in the
set of message IDs. These malformed events, in turn, caused exceptions
in the frontend's processing of such an event.
This change is paired with a separate UI change to not offer the "Mark
as unread" feature in such streams; with just this commit, that will
silently fail.
With some additions to the tests by tabbott.
This guarantees that the Realm is always non-None when we hit the
codepath is_static_or_current_realm_url via
do_change_stream_description, so that we can properly skip rewritting
some images.
Fixes#19405
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
I don't think this is used anywhere outside of tests, but we should have
this logic correct. If this function is used to send a message from a
user to a cross-realm bot, the message.realm should be the realm of the
user.
In the normal case, where a user send a message to a cross-realm bot
through the API is already handled correctly, this bug is unrelated.
Previously we did not send notification for topic-only edits.
Now, we add backend support for sending notification to topic-only
edits as well.
We would add support for this in webapp in further commits since
message edit UI will be updated as well. We just make sure that no
notifications are sent when editing topic using pencil icon in
message header.
We also change the API default for moving a topic to only notify the
new location, not the old one; this matches the current defaults in
the web UI.
Includes many tests.
We also update the puppeteer tests to test only content edit as
we are going to change the UI to not allow topic editing from
message edit UI. Also fixing the existing tests to pass while
doing topic edits is somewhat complex as notification message
is also sent to new topic by default.
Fixes#21712.
Co-authored-by: Aman Agrawal <amanagr@zulip.com>
Co-authored-by: Tim Abbott <tabbott@zulip.com>
The previous commit did this for revoking sessions. send_events should
be handled similarly too, to correctly handle calling do_deactivate_user
inside a transaction.
This commit adds the OPTIONAL .realm attribute to Message
(and ArchivedMessage), with the server changes for making new Messages
have this set. Old Messages still have to be migrated to backfill this,
before it can be non-nullable.
Appropriate test changes to correctly set .realm for Messages the tests
manually create are included here as well.
zerver/migrations/0240_usermessage_migrate_bigint_id_into_id.py needs
to be updated to account for Django 4.1 creating AutoField as an
identity column rather than a serial column.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Previously, we included all three message edit related settings
("allow_message_editing", "message_content_edit_limit_seconds" and
"edit_topic_policy") in the event data and api response irrespective
of which of these settings were changed. Now, we only include changed
settings and separate events are sent for each setting if more than
one of them is changed.
Note that the previous typed in event_schema.py for
`message_content_edit_limit_seconds` incorrectly did not allow `None`
as a value, which is used to encode no limit.
This refactors and renames user_ids_muting_topic to accept a parameter
'visibility_policy' and fetch user IDs that have a specific
visibility_policy(provided as the parameter) set for a topic.
Fourth step in making user status `away` a deprecated way to access
`presence_enabled` for clients supporting older servers, and
checkpoint commit prior to deleting the `status` field from the
UserStatus model.
Part of transitioning from 'unavailable' user status feature to
'invisible mode' user presence feature.
When a user toggles a status update for `away=True|False`, we now update
their `presence_enabled` setting to match (`away!=presence_enabled`).
First step of making user status `away` updates a deprecated way to
access presence_enabled for clients supporting older servers, and
checkpoint commit before migrating users with a current UserStatus
of `status=AWAY` to have their `presence_enabled` set to `False`.
Note that when user status `away` is updated, we now send 4 events:
user_status, user_settings, presence, and update_global_notifications.
Also, this means that these updates change the UserPresence.status
value, which impacts the test for importing and exporting user
information.
Part of transitioning from 'unavailable' user status feature to
'invisible mode' user presence feature.
We need to move this function to a separate actions file specifically
for `user_status` because otherwise we will have a circular import
between `actions/user_settings.py` and `actions/presence.py` in an
upcoming commit.
Prep commit for migrating "unavailable" user status feature to
"invisible" user presence feature.
To allow `custom_profile_field` to display in user profile popover,
added new boolean field "display_in_profile_summary" in its model class.
In `custom_profile_fields.py`, functions are edited as per conditions,
like currently we can display max 2 `custom_profile_fields` except
`LONG_TEXT` and `USER` type fields.
Default external account custom profile fields made updatable for only
this new field, as previous they were not updatable.
Fixes part of: #21215
This commit adds do_change_can_remove_subscriber_group function for
changing can_remove_subscribers_group field of a stream. We also add
can_remove_subscribers_group_id field to stream and subscription
objects.
This function will be helpful for writing tests in next commit.
We would add API and UI support to change this setting in further
commits.
Previously, an active production Zulip server would experience a class
of deadlocks caused by two or more concurrent bulk update operations
on the UserMessage table.
This is because UPDATE ... SET ... WHERE statements that execute in
parallel take row-level UPDATE locks as they get results; since the
query plans may result in getting rows in different orders between two
queries, this can result in deadlocks.
Some databases allow ORDER BY on their UPDATE ... WHERE statements;
PostgreSQL does not. In PostgreSQL, the answer is to do a sub-select
with an ORDER BY ... FOR UPDATE to ensure consistent ordering on row
locks.
We do this all code paths using bitand or bitor as part of bulk
editing message flags, which should ensure that these concurrent
operations obtain row level locks on the table in the same order.
Fixes#19054.
This is preparatory commit for #18941.
Importing `do_delete_message` from `message_edit.py` was causing a
circular import error. In order to avoid that, we create a separate
message_delete.py file which has all the functions related to deleting
messages.
The tests for deleting messages are present in
`zerver/tests/test_message_edit.py`.
Fixes a part of #18941
In zerver.management.commands.logout_all_users,
we pass a values queryset containing the ids into
this function, which is not actually a list. This
broadens the type annotation so that the ValuesQuerySet
is accepted.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This uses a more specific type `_StrPromise` to replace `Promise`
providing typing information for lazy translation strings.
In places where the callee evaluates the `_StrPromise` object in all
cases we simply force the evaluation with `str()`. This includes
`JsonableError` that ends up handled by the error handler middleware,
and `internal_send_stream_message` that depends on `check_stream_topic`,
requiring the `topic` to be evaluated anyway. In other siuations, the
callee is expected to be able to handle `StrPromise` explicitly.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Whether we sent a resolve topic notification or not may be useful in
the caller. It was originally intended to be used in #21712, but may
only be relevant for future logging.
Part of #21712.
This will have no real effect in most situations. However, a user
moves a topic to another stream while also adding/removing the
resolved-topic checkmark from the topic name, then the "This topic was
resolved" notificaiton will now appear just before the "This topic was
moved" notification rather than just after.
This is likely slightly less confusing to users, since the topic
having been moved from somewhere else is likely the most salient fact
to a reader.
We expect to change things to not send both notifications in an
upcoming commit.
This refactoring helps with #21712.
Our seat count calculation is different for guest user than normal users
(a number of initial guests are free, and additional marginal guests are
worth 1/5 of a seat) - so these checks we apply when a user is being
invited or signing up need to know whether it's a guest or non-guest
being added.
In Zulip 2.1.0, the `is_muted` stream subscription property was
added and replaced the `in_home_view` property. But the server has
still only been sending subscription update events with the
`in_home_view` property.
Updates `do_change_subscription_property` to send a subscription
update event for both `is_muted` and `in_home_view`, so that
clients can fully migrate away from using `in_home_view` allowing
us to eventually remove it completely.
This is a prep commit such that we can avoid duplicate code when we
unsubscribe bots for inaccessible private streams when changing owner
or reactivating them.
This commit changes the code to consider zero as an invalid value for
message_content_edit_time_limit_seconds. Now to represent the setting that
user can edit the message anytime, the setting value will be "None" in
database and "unlimited" will be passed to API from clients.
Adds an API endpoint for accessing read receipts for other users, as
well as a modal UI for displaying that information.
Enables the previously merged privacy settings UI for managing whether
a user makes read receipts data available to other users.
Documentation is pending, and we'll likely want to link to the
documentation with help_settings_link once it is complete.
Fixes#3618.
Co-authored-by: Tim Abbott <tabbott@zulip.com>
We now use MEMBERS_GROUP_NAME instead of writing
the actual group name at multiple places, so that we
can have all the group names coded at one place only.
We now use FULL_MEMBERS_GROUP_NAME instead of
writing the actual full members system group
name at multiple places, so that we can have
all the group names coded at one place only.
I found the previous model for computing what settings to use for
streams increasingly difficult to understand, which is generally a
recipe for future bugs.
Refactor to have a clear computation of what complete permissions
state the client is requesting, validate that state, and then pass
that state to the do_change_stream_permission.
We now allow changing access to history of the stream by only passing
"history_public_to_subscribers" parameter. Previously, "is_private"
parameter was also required to change history_public_to_subscribers
otherwise the request was silently ignored.
We also raise error when only history_public_to_subscribers parameter
is passed with value False without "is_private: True" for a public
or web-public stream since we do not allow public streams with
protected history.
This commit removes the unnecessary assertion statements in
do_change_stream_permission for case when "is_web_public" is
True, since we already check those cases in the view function
update_stream_backend and this is the only place from where
do_change_stream_permission is called.
We aim to remove other assertions also from there as mentioned
in the comment and instead check the values in caller itself.
We now send a new user_topic event while muting and unmuting topics.
fetch_initial_state_data now returns an additional user_topics array to
the client that will maintain the user-topic relationship data.
This will support any future addition of new features to modify the
relationship between a user-topic pair.
This commit adds the relevent backend code and schema for the new
event.
We separate the permission checks for content and topic edits
by changing the can_edit_topic_or_content to can_edit_topic
and use it only for checking topic edits and check content
edits separately in check_update_message itself. There is no
change in behavior as of this PR, there will be more changes
as per #21739.
This is a prep commit for #21739. The permission checks for
them are essentially separate except the one that message
sender is allowed to edit content and topic irresepctive of
edit_topic_policy setting, and this will too be changed in
future commit and so it will be better to have these checks
separate for readability.
We can also probably create a new function for checking content
edits but currently we only check the sender is same as the use
who is editing and it does not make sense to have a separate
function for just one check. We can do so in future in case we
do some more refactoring for #21739.
Lauryn Menard
Alex Vandiver
Abhijeet Prasad Bodas
Sahil Batra
Anders Kaseorg
Prakhar Pratyush
Zixuan James Li
Mateusz Mandera
Tim Abbott
Julia Bichler
Kartik Srivastava
yogesh sirsat
Christopher Chong
madrix01
Adam Sah
Dinesh