This saves about 8% of the runtime of our total response middleware,
or equivalently close to 2% of the total Tornado response time. Which
is pretty significant given that we're not sure anyone is using statsd
in production.
It's also useful outside Tornado, but the effect is particularly
significant because of how important Tornado performance is.
This avoids parsing these functions on every request, which was
adding roughly 350us to our per-request response times.
The overall impact was more than 10% of basic Tornado response
runtime.
We'll use this in the push-notifications code, in a context where
there should definitely already be UserMessage rows if everything's
gone normally... but explicitly checking at the top seems like the
right pattern from a secure-coding perspective.
When a bunch of messages with active notifications are all read at
once -- e.g. by the user choosing to mark all messages, or all in a
stream, as read, or just scrolling quickly through a PM conversation
-- there can be a large batch of this information to convey. Doing it
in a single GCM/FCM message is better for server congestion, and for
the device's battery.
The corresponding client-side logic is in zulip/zulip-mobile#3343 .
Existing clients today only understand one message ID at a time; so
accommodate them by sending individual GCM/FCM messages up to an
arbitrary threshold, with the rest only as a batch.
Also add an explicit test for this logic. The existing tests
that happen to cause this function to run don't exercise the
last condition, so without a new test `--coverage` complains.
Adblock Plus's "Block social media icons tracking" setting blocked
integration logos for social media platforms from loading, so the logos
are renamed to bypass this.
Fixes#11590.
This change was prompted by a possible bug on Clubhouse's end. In
general, if a branch is added, it also prompts a workflow state
transition in its primary story.
However, our webhook error logs show an erroneous payload where
the same branch is added to another story, but there is no
workflow state transition. Also, both the stories are grouped in
the same payload, which confused/broke our code. Ideally, this
shouldn't happen and is most likely a bug on Clubhouse's end.
In most cases, changes included in Clubhouse payloads never
pertain to more than one parent entity (stories) simultaneously,
and we usually operate under the assumption that the changes
included therein are related to each other in terms of their
parent object (story or epic) and not a child object (the GitHub
branch).
A check suite is a collection of check runs. We care a lot more
about the outcomes of check runs in this case because check_run
payloads are a lot more informative than check_suite payloads.
(And in any case, the check_suite events are primarily for notifying
tools like CI to run checks).
We only support notifications for events where a check run has
completed. Notifications for when a check run has been queued or
is in progress are not very informative and may be too noisy.
We do not anticipate our UI for showing stream descriptions looking
reasonable for multi-line descriptions, so we should just ban creating
them.
Given the frontend changes, multi-line descriptions are only likely to
show up from importing content from other tools, in which case
replacing newlines with spaces is cleaner than the alternative.
This change should help people discover to distinguish
silent mentions in text as a part of Zulip syntax while
differentiating them from regular mentions.
This optimizes test-backend by skipping webhook
tests when run in default mode.
Tweaked by tabbott to extend the documentation and update the CI
commands.
The payloads for this event are missing some important details
about the Project's changes, such as the name of the project,
the card's column name, etc. Without such details, the resultant
notifications would not be useful at all!
ACCOUNT_ACTIVATION_DAYS doesn't seems to be used anywhere.
INVITATION_LINK_VALIDITY_DAYS seems to do it's job currently.
(It was only ever used in very early Zulip commits).
We were using a hardcoded relative path, which doesn't work if you're
not running this from the root of the Zulip checkout.
As part of fixing this, we need to make `LOCAL_UPLOADS_DIR` an
absolute path.
Fixes#11581.
Since da8f4bc0e back in August, this control flow has caused
`flags.active_mobile_push_notification` to be cleared if we don't send
these `remove` messages at all, and if we send them directly to GCM...
but not if we send them via the Zulip notification bouncer.
As a result, on a server configured to send `remove` notification-messages
via the bouncer, we accumulate "active" messages and never clear them.
If the user then does `mark_all_as_read`, we end up sending a `remove`
for each of those messages again, and all in one giant burst. We've
seen puzzling bursts of hundreds of removals pass through the bouncer
since turning on removals on chat.zulip.org; it's likely many of them
are caused by this bug.
This issue was made more acute with f4478aad5, which unconditionally
enabled removals.
Test added by tabbott.
This is the only server-side change required for the FCM migration!
Optionally, at some point in the future we might choose to migrate
to the new ("v1") API which FCM also offers. Nothing revolutionary
but there are some nice things about it:
https://firebase.google.com/docs/cloud-messaging/migrate-v1
The client-side fix to make these not a problem was in release
16.2.96, of 2018-08-22. We've been sending them from the
development community server chat.zulip.org since 2018-11-29.
We started forcing clients to upgrade with commit fb7bfbe9a,
deployed 2018-12-05 to zulipchat.com.
(The mobile app unconditionally makes a request to a route on
zulipchat.com to check for this kind of forced upgrade, so that
applies to mobile users of any Zulip server.)
So at this point it's long past safe for us to unconditionally
send these. Hardwire the old `SEND_REMOVE_PUSH_NOTIFICATIONS`
setting to True, and simplify it out.
We've for a while had logic to set plan_type to LIMITED when importing
into Zulip Cloud; we need corresponding logic to set it to SELF_HOSTED
when importing into a self-hosted server.
Fixes#11541.
For Google auth, the multiuse invite key should be stored in the
csrf_state sent to google along with other values like is_signup,
mobile_flow_otp.
For social auth, the multiuse invite key should be passed as params to
the social-auth backend. The passing of the key is handled by
social_auth pipeline and made available to us when the auth is
completed.
For internal stream messages, most of the time, we have access to
a Stream object. For the few corner cases where we don't, it is a
much cleaner approach to have a separate function that accepts a
stream name than having one multi-option helper that accepts both
names and objects.
The bot's details (such as ID and name) are used to format a
connection label in Zapier's UI. This allows users to distinguish
between different Zapier-to-Zulip connections set up with different
bots.
Our html collects extra spaces in a couple of places. The most prominent is
paragraphs that look like the following in the .md file:
* some text
continued
The html will have two spaces before "continued".
This changes the border-radius to 6px for the tabbed display, which is not
in line with the current Zulip style for border-radius (4px). However 6px
really looks a lot better for this (possibly because it's a bigger box than
most of our other boxes?)
I was hoping this would make things faster... it does, but sadly only
by about 70ms, 5% of this file's test runtime.
It sure does make this file rather less action-at-a-distance, though,
as well as fixing some duplication.
If we make a practice on the Zulip server of always explicitly setting
the desired priority, then when an old server doesn't set the priority
we can reasonably have the bouncer make a guess.
That is, this allows a Zulip server to now set the `priority`; but if
it doesn't, we use upstream's default value, which has the same effect
as we've always previously had by not setting it at all.
But when this is deployed to the push notifications bouncer server, it
does allow another server to set priority when pushing notifications
through the bouncer.
If the caller has access to a Stream object, it is wasteful to
query a database for a stream by ID or name. In addition, not
having to go through stream names eliminates various classes of
possible bugs involved with re-fetching the Stream object by name.
If the caller has access to a Stream object, it is wasteful to
query a database for a stream by ID or name. In addition, not
having to go through stream names eliminates various classes of
possible bugs involved with getting a Stream object back.
The name for_stream_name is more appropriate here. The name
for_stream is more suitable for a function that takes in a Stream
object, which we're about to add.
Our hash-naming of production assets interacted badly with the "look
at files in a directory" algorithm used to determine what sound
options exist for the "notification sound" feature. For lack of a
better solution, we fix this by excluding files with an extra `.` in
their name.
Extracts out common tests so that future social-auth backends can
be tested without duplicating tests. I have been careful to not
change any testing logic.
Add all the stop words to page_params, reading from the
`zulip_english.stop` database, with caching to avoid loading the file
on every page load.
Part of #10592.
This causes changing the email_address_visibility field to actually
modify what user_profile.email values are generated for users, both on
user creation and afterwards as email addresses are edited.
The overall feature isn't yet complete, but this brings us pretty close.
The original commit made a number of well-meaning but suboptimal product
changes to the invoice events, such as threading them under the invoice id
rather than the customer id. It also seems to be causing 500s for
invoiceitem events, though I'm not sure why.
This reverts commit 65489b0391.
We had disabled reference style links in bugdown, however,
we hadn't disabled them in marked. This commit rectifies
that and adds test cases for the same.
Fixes#11350.
This helps keep the realm.json small and easy to process; previously,
almost the entire size of that file was the analytics data.
We implement this by refactoring the analytics Config objects into a
separate subroutine that writes to a separate file, plus the
corresponding import code.
Manual testing was performed by exporting the 'analytics' realm, and
importing back to a newly created 'test' realm. The 'test' realm was
then exported and the json files were inspected. The data appeared
consistent with no abnormalities.
Fixes: #11220.
We eliminated use of this function in outgoing_webhook.py in
bdc95b5d72.
Tweaked by tabbott to also eliminate code only used for that mock.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This commit does the following three things:
1. Update stream model to accomodate rendered description.
2. Render and save the stream rendered description on update.
3. Render and save stream descriptions on creation.
Further, the stream's rendered description is also sent whenever the
stream's description is being sent.
This is preparatory work for eliminating the use of the
non-authoritative marked.js markdown parser for stream descriptions.
This adds a new API for sending basic analytics data (number of users,
number of messages sent) from a Zulip server to the Zulip Cloud
central analytics database, which will make it possible for servers to
elect to have their usage numbers counted in published stats on the
size of the Zulip ecosystem.
This is primarily a feature for onboarding, where an organization
administrator might send a bunch of random test messages as part of
joining, but then want a pristine organization when their users later
join.
But it can theoretically be used for other use cases (e.g. for
moderation or removing threads that are problematic in some way).
Tweaked by tabbott to handle corner cases with
is_history_public_to_subscribers.
Fixes#10912.
This replaces the current usage of stream names with stream ids.
This commit also removes the `traditional` attribute from the invite
form as now we are sending stream_ids as an argument; this was the
only place in the codebase we used traditional=true, and it's great to
have it removed.
Ever since we implemented support for stream IDs in Addressee,
Addressee.stream_name() can now return None. This commit ensures
that _internal_prep_message only calls ensure_stream when
Addressee.stream_name() is not None.
This commit also contains the following auxiliary changes:
* Adds a custom exception, StreamWithIDDoesNotExist for when
a stream with a given ID does not exist because the error
message returned by StreamDoesNotExist only makes with stream
names, not IDs.
* Adds a new helper, get_stream_by_id_in_realm, which is similar
to get_user_profile_by_id_in_realm (introduced in #10391).
* Adds a helper, validate_stream_id_with_pm_notification, which
returns the Stream object associated with a given ID and also
handles PM notifications to the bot owner if the message was
sent by a bot and if the stream does not exist or has no
subscribers.
* Modifies the message sent by send_pm_if_empty_stream to
accommodate stream IDs.
Note that all of the above changes are required before check_message
can be modified to support stream IDs.
We've had this code oscillated a few times; the original comparison
was added as part of Stride import but broke HipChat import.
c34a8f2e69 fixed HipChat import but
regressed Stride.
This change fixes this for both HipChat + Stride.
Tim Abbott
Rishi Gupta
Greg Price
synicalsyntax
Anders Kaseorg
Eeshan Garg
Challa Venkata Raghava Reddy
Rohitt Vashishtha
Raymond Akornor
sahil839
Vishnu Ks
Abdelhadi Dyouri
savish
anhhuynh
Harshit Bansal
YashRE42
Wyatt Hoodes
Hemanth V. Alluri
kunal-mohta
sameerchoubey
Shubham Dhama
Shoumorup
Pragati Agrawal