We ran into a bug in production caused by two issues:
- Some users came from orgs that didn't have a website and since
the URL field was required, they submitted invalid URLs.
- We didn't properly respond to invalid form submissions, which
led to UnboundLocalError exceptions in another part of the
code.
This commit solves this by doing the following:
- We now allow blank URLs and have a convenient placeholder text
label that tells users that they may leave the URL field blank.
- This commit refactors the code such that invalid form submissions
result in an informative error message about what exactly went
wrong.
We do not allow any user to edit the system user groups (including
renaming, deleting, adding or removing members, etc.) from the
API. These user groups will change only by the code when a new
user is added or role of a user is changed.
This is implemented by rejecting access_user_group_by_id always
except the case when it is use to get the user group for sending
email and push notifications, as we would need to send notifications
to the mentioned user group.
We make the description parameter in create_user_group as keyword-only
to improve readability. We would also keep the is_system_group
parameter which will be added in future keyword-only.
We were using incorrect element as we changed the input from
select to radio buttons in 439bc2920a.
This code was originally added in 6cb03ea78e (though in a
different file) and was removed from sometime and then added
again in e7311cdf5d. But in the meantime the input was
changed to radio, which resulted in incorrect code.
We were using emojiset_spinner element in destroy_indicator, but
there is no such element and emoji-settings-status element is used
in make_indicator, so destroy_indicator should also use the same
element.
This was added originally in 6cb03ea78e (though in a different
file) and was removed for sometime and then readded in e7311cdf5d.
But between these two commits, the setting was changed to be a
radio element and thus the spinner elements were also changed.
Tuples cannot be deserialized from JSON.
While we do use these validators for other things, like event
dictionaries, we have migrated the API away from using those. The
last use was removed in 4f3d5f2d87
Signed-off-by: Anders Kaseorg <anders@zulip.com>
These changes are all independent of each other; I just didn’t feel
like making dozens of commits for them.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The footer was being covered by a bunch of invisible divs, rendering
most of the links non-functional.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Recommonmark is no longer maintained, and MyST-Parser is much more
complete.
https://myst-parser.readthedocs.io/
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Commit 30eaed0378 (#15001) incorrectly
inserted a different section between the anchor and the heading.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The auth attempt rate limit is quite low (on purpose), so this can be a
common scenario where a user asks their admin to reset the limit instead
of waiting. We should provide a tool for administrators to handle such
requests without fiddling around with code in manage.py shell.
Calling `email.save()` is only needed if we altered `email.address`;
it is unnecessary if we called `email.users.add(...)` which will have
done its own INSERT.
This fixes two bugs: the most obvious is that there is a race where a
ScheduledEmail object could be observed in the window between creation
and when users are added; this is a momentary instance when the object
has no users, but one that will resolve itself.
The more subtle is that .save() will, if no records were found to be
updated, _re-create_ the object as it exists in memory, using an
INSERT[1]. Thus, there is a race with `deliver_scheduled_emails`
between when the users are added, and when `email.save()` runs:
1. Web request creates ScheduledEmail object
2. Web request creates ScheduledEmailUsers object
3. deliver_scheduled_emails locks the former, preventing updates.
4. deliver_scheduled_emails deletes both objects, commits, releasing lock
5. Web request calls `email.save()`; UPDATE finds no rows, so it
re-creates the ScheduledEmail object.
6. Future deliver_scheduled_emails runs find a ScheduledEmail with no
attending ScheduledEmailUsers objects
Wrapping the logical creation of both of these in a single transaction
avoids both of these races.
[1] https://docs.djangoproject.com/en/3.2/ref/models/instances/#how-django-knows-to-update-vs-insert
Only clear_scheduled_emails previously took a lock on the users before
removing them; make deliver_scheduled_emails do so as well, by using
prefetch_related to ensure that the table appears in the SELECT. This
is not necessary for correctness, since all accesses of
ScheduledEmailUser first access the ScheduledEmail and lock it; it is
merely for consistency.
Since SELECT ... FOR UPDATE takes an UPDATE lock on all tables
mentioned in the SELECT, merely doing the prefetch is sufficient to
lock both tables; no `on=(...)` is needed to `select_for_update`.
This also does not address the pre-existing potential deadlock from
these two use cases, where both try to lock the same ScheduledEmail
rows in opposite orders.
No codepath except tests passes in more than one user_profile -- and
doing so is what makes the deduplication necessary.
Simplify the API by making it only take one user_profile id.
On mobile, when the sidebar is toggled, the following three issues
are encountered:
- When none of the sidebar menus are expanded, the sidebar has no
scrollbar, which is expected. But if you scroll, the background
content scrolls, which is a bug.
- When some of the sidebar menus are expanded such that the content
overflows and is "scrollable", once you get to the end of the
sidebar content, the background content keeps scrolling in a weird
way.
- If the mobile screen is wide enough, if you scroll the sidebar
content, it scrolls as expected. But if you move the pointer to
the side of the background content that is still visible, you
can scroll the background content even though it should be fixed.
This commit fixes all of the above issues.
This fixes a bug where email notifications were sent for wildcard
mentions even if the `enable_offline_email_notifications` setting was
turned off.
This was because the `notification_data` class incorrectly considered
`wildcard_mentions_notify` as an indeoendent setting, instead of a wrapper
around `enable_offline_email_notifications` and `enable_offline_push_notifications`.
Also add a test for this case.
Eeshan Garg
Sahil Batra
Anders Kaseorg
Mateusz Mandera
Alex Vandiver
Suyash Vardhan Mathur
Abhijeet Prasad Bodas
PIG208