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.