This kind of payload that's loaded from json in the body of the request
is not only used for webhooks, but also in the push bouncer, and may get
used elsewhere too - so a general name is better.
This converts most webhook integration views to use @typed_endpoint instead
of @has_request_variables, rewriting REQ parameters. For these
webhooks, it simply requires switching the decorator, rewriting the
type annotation of payload/message to WebhookPayload[WildValue], and
removing the REQ default that defines the to_wild_value converter.
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>
Adds request as a parameter to json_success as a refactor towards
making `ignored_parameters_unsupported` functionality available
for all API endpoints.
Also, removes any data parameters that are an empty dict or
a dict with the generic success response values.
We recently ran into a payload in production that didn't contain
an event type at all. A payload where we can't figure out the event
type is quite rare. Instead of letting these payloads run amok, we
should raise a more informative exception for such unusual payloads.
If we encounter too many of these, then we can choose to conduct a
deeper investigation on a case-by-case basis.
With some changes by Tim Abbott.
It looks like this ritual was born when a type comment wasn’t working
because it was mistyped without the colon.
Signed-off-by: Anders Kaseorg <anders@zulip.com>'
Any exception is an "unexpected event", which means talking about
having an "unexpected event logger" or "unexpected event exception" is
confusing. As the error message in `exceptions.py` already explains,
this is about an _unsupported_ event type.
This also switches the path that these exceptions are written to,
accordingly.
8e10ab282a moved UnexpectedWebhookEventType into
`zerver.lib.exceptions`, but left the import into
`zserver.lib.webhooks.common` so that webhooks could continue to
import the exception from there.
This clutters things and adds complexity; there is no compelling
reason that the exception's source of truth should not move alongside
all other exceptions.
Fixes#2665.
Regenerated by tabbott with `lint --fix` after a rebase and change in
parameters.
Note from tabbott: In a few cases, this converts technical debt in the
form of unsorted imports into different technical debt in the form of
our largest files having very long, ugly import sequences at the
start. I expect this change will increase pressure for us to split
those files, which isn't a bad thing.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Automatically generated by the following script, based on the output
of lint with flake8-comma:
import re
import sys
last_filename = None
last_row = None
lines = []
for msg in sys.stdin:
m = re.match(
r"\x1b\[35mflake8 \|\x1b\[0m \x1b\[1;31m(.+):(\d+):(\d+): (\w+)", msg
)
if m:
filename, row_str, col_str, err = m.groups()
row, col = int(row_str), int(col_str)
if filename == last_filename:
assert last_row != row
else:
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
with open(filename) as f:
lines = f.readlines()
last_filename = filename
last_row = row
line = lines[row - 1]
if err in ["C812", "C815"]:
lines[row - 1] = line[: col - 1] + "," + line[col - 1 :]
elif err in ["C819"]:
assert line[col - 2] == ","
lines[row - 1] = line[: col - 2] + line[col - 1 :].lstrip(" ")
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Generated by pyupgrade --py36-plus --keep-percent-format, but with the
NamedTuple changes reverted (see commit
ba7906a3c6, #15132).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Generated by `pyupgrade --py3-plus --keep-percent-format` on all our
Python code except `zthumbor` and `zulip-ec2-configure-interfaces`,
followed by manual indentation fixes.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
For event types that we don't yet support, like worklog_created (and
likely many more in the future), it doesn't make sense to call a
function that only parses issue events correctly.
I'm not sure what causes some Jira webhook events to not include the
metadata that other events do, but it's definitely a format sent by
real installations of Jira (likely a very old version, since this has
fields missing from what modern Jira does) and we've seen it in
production.
The best we can do is encourage users to upgrade Jira for better data.
Atlassian announced that it will no longer provide information about
comments along with their "issue" type event payloads for Jira. So
we must now update the Jira integration to appropriately respond to
"comment" type events (the reason why we didn't do this before was
that initially, the "comment" type event payloads didn't contain
sufficient information about their issues, but this payload has
since been improved).
Note: This commit does *not* remove support for the older "issue"
type event payloads where information about comments was included.
This way we can maintain compatibility with old self-hosted versions
of self hosted Jira (2016 and before).
Source:
https://developer.atlassian.com/cloud/jira/platform/change-notice-
removal-of-comments-from-issue-webhooks/
Fixes#13012
If the event key is None, the handler content_func never gets
defined, which leads to an UnboundLocalError. This can be easily
avoided by having a dedicated function that handles the case for
when the event key is None.
This commit modifies the regex used when parsing JIRA's full links of
the form `[text|link]` so that if you have two in a message, Zulip
markup conversion doesn't think that the first link extends to the
closing `]` of the second link.
comment_created payloads may not contain the required issue data
to format a useful notification, therefore, it is better to handle
issue comments through issue_updated events (which we already do).
Fixes: #11995.
A key part of this is the new helper, get_user_by_delivery_email. Its
verbose name is important for clarity; it should help avoid blind
copy-pasting of get_user (which we'll also want to rename).
Unfortunately, it requires detailed understanding of the context to
figure out which one to use; each is used in about half of call sites.
Another important note is that this PR doesn't migrate get_user calls
in the tests except where not doing so would cause the tests to fail.
This probably deserves a follow-up refactor to avoid bugs here.
Anders Kaseorg
Mateusz Mandera
Zixuan James Li
abdullahm1
Hari Prashant Bhimaraju
Lauryn Menard
Eeshan Garg
PIG208
Alex Vandiver
Steve Howell
Anders Kaseorg
Tim Abbott
Hemanth V. Alluri
Wyatt Hoodes
Eeshan Garg
David Wood
sameerchoubey
Anders Kaseorg