This commit refactors the code to find the
lines to be first, and then test all the
lines that contained commands.
This was done to avoid duplication of
the code for the other case, where the
.md file won't exist, as those are soon
to be deleted in favour of a common template.
Soon, the .md files of each endpoint would
be removed and be auto-generated from OpenAPI
data.
So, instead of using the files directly,
we should check from the list of endpoints and
open the files from there.
A follow-up to change logic when the .md files
get deleted should be done.
This changes the button text from "Reply" to "Reply to selected
message". Here's the thinking:
* The title "Reply" was a little confusing/inconsistent with the
button's label.
* If you're hovering over the button, it's because you want more
information about what it does -- not just a repeat of the button's
label.
* The "Message foo > bar" content of the button already cleanly
expresses what the button will do if you click it right now.
* The hover text "Reply to selected message (r)" explains to you what
that button's role is in all situation, not just with the current
selection, and thus documents the concept. And it also gives you
clarify if you're thinking "but how do I reply to something in
Zulip?" and try hovering over the buttons at the bottom to find out.
This locks the message row while a reaction is being added/removed,
which will handle race conditions caused by deleting the message
at the same time.
We make sure that events work happens outside the transaction,
so that in case there's some problem with the queue processor, the
locks aren't held for too long.
As a nice side-effect, we also handle race conditions from double
adding reactions, because once the message is locked, a duplicate
request will wait till the earlier transaction commits, and hence
will not throw `IntegrityErrors`s (rather, will be handled in our
safety check in the /views code itself), which earlier had to be
handled explicitly.
This locks the message while creating a submessage, which
will handle race conditions caused by deleting the message
simultaneously.
We make sure that events work happens outside the transaction,
so that in case there's a problem with the queue processor,
the locks aren't held for too long.
Further commits will start locking the message rows while
adding related fields like reactions or submessages,
to handle races caused by deleting the message itself at the
same time.
The message locking implemented then will create a possibility
of deadlocks, where the related field transaction holds a lock
on the message row, and the message-delete transaction holds a
lock on the database row of the related field (which will also
need to be deleted when the message is deleted), and both
transactions wait for each other.
To prevent such a deadlock, we lock the message itself while
it is being deleted, so that the message-delete transaction
will have to wait till the other transaction (which is about
to delete the related field, and also holds a lock on the
message row) commits.
https://chat.zulip.org/#narrow/near/1185943 has more details.
Further commits will hook some `send_event` calls to `on_commit`.
With those changes, these will never be executed in tests, because
transactions never get commited with `TestCase`, which the
`ZulipTestCase` is a subclass of.
We want to make sure that these events are actually sent for testing
purposes, hence this change.
There's no need to actually capture the callbacks, because the
events are already thoroughly tested.
Further commits will hook `send_event` calls to `on_commit`
in some cases. This change will make it easier to test such
situations.
We don't need to actually capture the callbacks, because the
events sent are already tested via the list in which they are
captured by `tornado_redirected_to_list`.
This commit updates the docs as the setting for invites
is separated into two UI elements - a checkbox for
'are invites required to join organization' and a
dropdown for setting 'who can invite new users'.
This commit divides the user_invite_restriction setting dropdown to
a checkbox and a dropdown.
The checkbox is used for 'realm_invite_required' setting and dropdown
for 'realm_invite_to_realm_policy'.
This separation of UI elements is fine as these two settings are
separate in database also and also helps in removing excess if-else
conditions and switch cases.
There was a lot of duplicate code in test_sync_realm_settings where
each value in common_policy_values was being tested for different
policies (create_stream_policy and invite_to_stream_policy).
This commit deduplicates is by using a for-loop for testing all
common_policy values and extracting the code as a function to test
different policies.
We do not require values of realm_create_stream_policy,
realm_invite_to_stream_policy, realm_private_message_policy
and realm_wildcard_mention_policy in the organization settings
templates, as we handle the dropdown values of these settings
in javascript code (settings_org.js) only and these values
are not used anywhere in templates.
Previously the logic to show save button considered only topic
and content edit. This commit fixes it to show the save button
when only stream edit is possible and content and topic edits
are not.
This commit only introduces a simple fix, in long-term we would
want to change the logic of get_editability to include stream
edits as well but it would also require discussion on when to
show the edit icons at different places, so it is better to do
it in a separate commit later.
This commit fixes a bug where moving messages between streams was
not allowed for non-admins when allow_community_topic_editing was
set to false and move_messages_between_streams_policy was set to
Realm.POLICY_MEMBERS_ONLY.
The bug is fixed by calling can_edit_content_or_topic only when
topic or content edit is there and not in the case where only
message is moved from one stream to another.
This commit extracts the logic of checking the message edit permissions,
like whether the sender is same as user, whether it is a (no topic)
message or whether community topic editing is allowed, into a separate
function.
This is a prep commit for fixing a bug where permission to move messages
between streams is affected by permission of editing topics.
Previously when enforcing the check to do not allow editing topics
after a certain time, we were checking whether 'content is None' and
considering it as that if content is None then there must be topic
edit.
But after adding support for moving messages between streams it can be
the case that we are neither changing topic nor content and just moving
streams, and the original code raises error if this is done after the
time limit of editing topics, which is wrong.
This commit fixes this by actually checking 'topic_name is not None'.
The message-editing section of settings is moved from "Organization
Settings" to "Organization Permissions", which feels like a more
natural place for these settings.
Previously, we had this complicated layering where the right sidebar
logic would display "Last active: foo" but the user popovers would
just display "foo", which doens't make any sense, since the two
settings have equal context about the string.
We deduplicate that and also arrange that the "Last active:" prefix is
used when it's not clear what we're talking about; i.e. all the values
except for "Active now".
There is no clear reason to not use a button element here. According
to the spec pharasing content, which includes the <span> element,
are allowed in the button element.
Manually tested both buttons to make sure it works and made sure all
the selectors are updated by grepping all the selector classes/id in
the handlebars templates that are parents of the button or are
present on the button.
(One of the jQuery handler code got reformatted due to it fitting
the line limit due to one character deletion for the selector)
Soon, each endpoint won't necessarily have a .md
file, but would generate API doc directly from
OpenAPI data using a template.
So, the lists of endpoints to be tested should not
be taken from the .md files, but from the REST
endpoints available in the sidebar.
This commit also adds a missing test for an invalid
article being accessed in the URL of an API page.
The current logic to get API pages' title using
OperationID should be used when the first line
of the file explicitly mentions so.
In cases where the files didn't begin with `#` but also
didn't need to get title from OpenAPI summary,
this logic fails and causes Server error.
This particularly happens when the article is invalid,
and the `missing.md` file doesn't need title to be
generated, but doesn't start with `#` either.
This commit fixes the logic of using the generated title and covers the bug.
This should help with #17425, where messages with lots of LaTeX are
lost, due to the large expansion factor.
This isn't a total fix for this - large messages with lots of LaTeX
can still end up larger than 1MB, and rendering could timeout, but
this fix should help significantly.
1MB is still small enough that I don't expect we'll run into any DOS
problems - my testing didn't show any problems rendering messages that
contain ~1MB of LaTeX.
This will offer users who are self-hosting to adjust
this value. Moreover, this will help to reduce the
overall time taken to test `test_markdown.py` (since
this can be now overridden with `override_settings`
Django decorator).
This is done as a prep commit for #18641.
Suyash Vardhan Mathur
Tim Abbott
Abhijeet Prasad Bodas
sahil839
m-e-l-u-h-a-n
Priyank Patel
Vishnu KS
Wesley Aptekar-Cassels
seiwailai
Alex Vandiver
Mateusz Mandera
akshatdalton
Riken Shah
Robert Imschweiler