Commit Graph

119 Commits

Author SHA1 Message Date
Eeshan Garg 2393342e03 webhooks/jira: Handle anomalous payloads properly.
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.
2021-12-28 10:56:25 -08:00
Shlok Patel 033e61a055 webhooks/github: Include discussion ID in messages.
We add discussion id and url in the comments and highlighted title to
the body of disscussion message to make it more meaningful and accessible.

Fixes #19938.
2021-10-26 17:52:57 -07:00
Shlok Patel 0678e2610f webhooks: Add support for GitHub discussions messages.
We aim to use Zulip topics thoughtfully in displaying messages from
discussions, as well as linking to the discussion in every message so
that it's easy to view them.

Fixes #19938.
2021-10-22 13:40:05 -07:00
PIG208 987f859681 webhooks: Detect view function with default naming convention. 2021-07-24 15:10:09 -07:00
PIG208 e373df88e0 webhooks: Make github webhook support event filtering system. 2021-07-13 16:47:18 -07:00
PIG208 5ecbfecd77 webhook: Rename FIXTURE_DIR_NAME to WEBHOOK_DIR_NAME.
Since FIXTURE_DIR_NAME is the name of the folder that contains the view
and tests modules of the webhook and another folder called "fixtures" that
store the fixtures, it is more appropriate to call it WEBHOOK_DIR_NAME,
especially when we want to refer to the view module using this variable.
2021-06-29 17:01:54 -07:00
Anders Kaseorg 544bbd5398 docs: Fix capitalization mistakes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-05-10 09:57:26 -07:00
Adam Birds 49b920d33a integrations: Fix bug with github multiple reviews requested on PR.
Fixes a bug in which multiple of the same messages are posted when
multiple reviewers are added to a PR.

Fixes #18393.
2021-05-08 07:56:48 -07:00
Adam Birds c72ef7be12 integrations: Move `get_setup_webhook_message` to `common.py`.
Move `get_setup_webhook_message` to
`zerver/lib/webhooks/common.py` so multiple integrations can use this
rather than just those which import `zerver/lib/webhooks/git.py`. Also
added the documentation for this.
2021-05-06 11:18:35 -07:00
Adam Birds 545cd961f4 integrations: Add docs for GitHub Actions integration.
I have added a documentation page for the GitHub Actions integration to
`/integrations/doc/github-actions` with a link to the Zulip GitHub
Actions repository.

Tweaked by tabbott to add cross-links with the main GitHub integration.
2021-04-15 16:42:31 -07:00
Tushar Upadhyay 6250902edc
webhooks: Add support for more GitHub pull_request events.
Add support for the locked, unlocked, auto-merge-enabled, and auto-merge-disabled actions 
for the pull_request event.

Fixes #16258.
2021-03-01 14:16:08 -08:00
Anders Kaseorg 6e4c3e41dc python: Normalize quotes with Black.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-02-12 13:11:19 -08:00
Anders Kaseorg 11741543da python: Reformat with Black, except quotes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-02-12 13:11:19 -08:00
Anders Kaseorg 5028c081cb python: Merge concatenated string literals that Black would uglify.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-02-12 13:11:19 -08:00
cozyrohan 16d1ab3d5f webhooks/github: Fix repeating description for edits and updates.
This change updates the GitHub Integration webhook
get_opened_or_update_pull_request_body method so that
the description is only printed if it actually changes.
If the update event is a result of some other
attribute update, such as an asignee change, then the
description is not included in the message sent to
the zulip stream.

Fixes #16345
2021-01-04 14:34:17 -08:00
Anders Kaseorg 72d6ff3c3b docs: Fix more capitalization issues.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-10-23 11:46:55 -07:00
Alex Vandiver 3f6e4ff303 webhooks: Move the extra logging information into a formatter.
This clears it out of the data sent to Sentry, where it is duplicative
with the indexed metadata -- and potentially exposes PHI if Sentry's
"make this issue public" feature is used.
2020-09-11 16:43:29 -07:00
Alex Vandiver 4917391133 webhooks: Derive payload from request itself. 2020-09-10 17:47:22 -07:00
Alex Vandiver a1f5f6502c webhooks: In logger, pull user from request, rather than parameter.
request.user is set by validate_api_key, which is called by
webhook_view and authenticated_rest_api_view.
2020-09-10 17:47:22 -07:00
Alex Vandiver d04db7c5fe webhooks: Remove repetitive argument to UnsupportedWebhookEventType.
The name of the webhook can be added by the webhook decorator.
2020-09-10 17:47:21 -07:00
Alex Vandiver cf6ebb9c8d webhooks: Rename api_key_only_webhook_view to webhook_view.
There are no other types of webhook views; this is more concise.
2020-09-10 17:47:21 -07:00
Alex Vandiver 9ea9752e0e webhooks: Rename UnexpectedWebhookEventType to UnsupportedWebhookEventType.
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.
2020-09-10 17:47:21 -07:00
Alex Vandiver 8016769613 webhooks: Move UnexpectedWebhookEventType into zerver.lib.exceptions.
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.
2020-09-10 17:47:21 -07:00
Steve Howell 5bff66b450 github webhook: Always send messages for team edits.
If there are unsupported keys, we still log an error,
but we now also send a message to the stream.  (This
is a good tradeoff for the github webhook, since users
can just turn off notifications if they find it spammy.
Also, we intend to support "repository" soon.)

This is a bit of an experiment to see how this plays
in the field:

    * will customers notice the change?
    * will Sentry reports look any different?
2020-09-03 10:44:39 -07:00
Steve Howell c6b9a23c17 github webhooks: Fix message for unsupported team payloads.
The main thing fixed here is that we weren't turning
on our keys into a list.  And then I refined the message
a bit more, including sorting the keys.

I also avoid the unnecessary "else".
2020-09-03 10:44:39 -07:00
Steve Howell 8785790a27 github tests: Add test_team_edited_error_handling.
This doesn't test much interesting yet, but it
will soon.
2020-09-03 10:44:39 -07:00
Steve Howell cc2dbefc60 mypy: Use better types for EVENT_FUNCTION_MAPPER.
The EVENT_FUNCTION_MAPPER maps a string event name
to a function handler.  Before this we circumvented
mypy checks with a call to get_body_function_based_on_type,
which specified Any as the type of our event function.

Now the types are rigorous.

This change was impossible without the recent commit
to introduce the Helper class.
2020-09-03 10:44:39 -07:00
Steve Howell 4de2b78c25 github refactor: Add Helper class.
The Helper class will soon grow, but the immediate
problem it solves is the need to jankily inspect
the parameters of our get_*_body function.

Most of the changes were handled by an ad hoc
munge.py script.

The substantive changes were adding the Helper
class and passing it in.

And then the linter discovered a place where
the optional include_title parameter wasn't used
(which is one of the reasons to avoid the janky
inspect-signature technique).

As a side note, none of the include_title parameters
needed a default value of False, as we always passed
in an explicit value.

We test cover both sides of include_title, which
you can verify by hard coding it to either True or
False (and seeing the relevant failures), although I
suspect most individual codepaths
only test one value, based on whether "topic" is in
the fixture or not.

Finally, I know Helper is not a great name, but I
intend to evolve the class a bit before deciding
whether a more descriptive name is helpful here.
(For example, an upcoming commit will add a
log_unexpected helper method.)
2020-09-03 10:44:39 -07:00
Steve Howell ead7cbea40 github refactor: Handle header_event explicitly.
We get the header_event one level up the call
stack now, too.

It's somewhat annoying that we have our own
concept of "event" here, instead of just returning
our event handlers directly, or just calling them
directly, but it's a bit non-trivial to fix that
right away.

In passing, I remove the strange OR for "ping",
which is already a key in EVENT_FUNCTION_MAPPER.
2020-09-03 10:44:39 -07:00
Steve Howell 0d9b1817f9 github refactor: Use early-exit idiom.
We also comment a bit more explicitly about the
None case.
2020-09-03 10:44:39 -07:00
Steve Howell 5c916135c9 github webhooks: Avoid string interpolation.
We know the event explicitly here.
2020-09-03 10:44:39 -07:00
Steve Howell 425db931a8 github webhook: Explicitly ignore team actions. 2020-09-03 10:44:39 -07:00
Steve Howell 294fd59983 github webhook: Ignore more pull_request actions.
See https://github.com/zulip/zulip/issues/16258 for
possible follow up here.

We now ignore the following two new pull_request
actions (as well as the three existing ones
from before):

    approved
    converted_to_draft

As the issue above indicates, we may want to actually
support "approved" if we can find somebody to work
on the webhook.  (And then the issue goes a little
broader than what changed here.)
2020-09-03 10:44:39 -07:00
Steve Howell 5dea85a186 github tests: Extract test_ignored_pull_request_actions.
We consolidate the tests and remove the fixtures, which
just have a lot of noisy fields that we ignore.  Also,
pull_request__request_review_removed was named improperly.
2020-09-03 10:44:39 -07:00
Steve Howell 4c2787c35f github tests: Add test_ignored_events. 2020-09-03 10:44:39 -07:00
Steve Howell 040bf82122 github webhook: Remove unused exception class. 2020-09-03 10:44:39 -07:00
Steve Howell f429df3401 github tests: Extract verify_post_is_ignored. 2020-08-24 12:34:46 -07:00
Steve Howell dfe6960584 minor: Format IGNORED_EVENTS for github. 2020-08-24 12:34:46 -07:00
Steve Howell 388053db6b webhook tests: Rename main helper to check_webhook.
Almost all webhook tests use this helper, except a few
webhooks that write to private streams.

Being concise is important here, and the name
`self.send_and_test_stream_message` always confused
me, since it sounds you're sending a stream message,
and it leaves out the webhook piece.

We should consider renaming `send_and_test_private_message`
to something like `check_webhook_private`, but I couldn't
decide on a great name, and it's very rarely used.  So
for now I just made sure the docstrings of the two
sibling functions reference each other.
2020-08-24 12:34:46 -07:00
Steve Howell 00001a396b webhook tests: Shorten topic constants.
The "EXPECTED_" prefix and "_EVENTS" suffix
usually provided more noise than signal.

We also use module constants to avoid the "self."
noise.  It also makes it a bit more clear which
constants actually have to be in the class (e.g.
"FIXTURE_DIR_NAME") to do their job.
2020-08-24 12:34:46 -07:00
Till Schneidereit 6d6d43188d webhooks/github: Include full repository name in notification messages.
This changes the notification messages for events that currently just
include the string `"the repository"` to also include the full (`org/repo`)
name of the affected repository. Messages for the following events are
changed:
- `public`
- `star`
- `watch`
- `repository`
- `team_add`

Background: we're using the GitHub integration for org-wide notifications
for the [Bytecode Alliance Zulip](bytecodealliance.zulipchat.com/), and
having all messages just say "the repository" isn't ideal. Even now one
can hover over the link to see the repo's url, but it'd be much nicer if
the message just contained the full name.

I also changed the message for `star` to include a link to the repository,
same as the `watch` notification.
2020-06-29 13:12:16 -07:00
Anders Kaseorg 69c0959f34 python: Fix misuse of Optional types for optional parameters.
There seems to have been a confusion between two different uses of the
word “optional”:

• An optional parameter may be omitted and replaced with a default
  value.
• An Optional type has None as a possible value.

Sometimes an optional parameter has a default value of None, or None
is otherwise a meaningful value to provide, in which case it makes
sense for the optional parameter to have an Optional type.  But in
other cases, optional parameters should not have Optional type.  Fix
them.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-06-13 15:31:27 -07:00
Anders Kaseorg 365fe0b3d5 python: Sort imports with isort.
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>
2020-06-11 16:45:32 -07:00
Anders Kaseorg 69730a78cc python: Use trailing commas consistently.
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>
2020-06-11 16:04:12 -07:00
Anders Kaseorg 6480deaf27 python: Convert more "".format to Python 3.6 f-strings.
Generated by pyupgrade --py36-plus --keep-percent-format, with more
restrictions patched out.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-06-10 14:48:09 -07:00
Anders Kaseorg 8dd83228e7 python: Convert "".format to Python 3.6 f-strings.
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>
2020-06-08 15:31:20 -07:00
Anders Kaseorg 840cf4b885 requirements: Drop direct dependency on mock.
mock is just a backport of the standard library’s unittest.mock now.

The SAMLAuthBackendTest change is needed because
MagicMock.call_args.args wasn’t introduced until Python
3.8 (https://bugs.python.org/issue21269).

The PROVISION_VERSION bump is skipped because mock is still an
indirect dev requirement via moto.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-05-26 11:40:42 -07:00
Hemanth V. Alluri ab03659095 webhooks/github: Provide more detailed info for unexpected events.
Signed-off-by: Hemanth V. Alluri <hdrive1999@gmail.com>
2020-05-21 12:31:43 -07:00
Hemanth V. Alluri c8f4177872 webhooks/github: Add a support for team/edited events.
Signed-off-by: Hemanth V. Alluri <hdrive1999@gmail.com>
2020-05-16 14:59:08 -07:00
cestrell 297185cc12 integrations: Use new function in Github releases.
Use get_release_event_message from webhooks/git.py to format release
events using the newly implemented release message template.

Tweaked by tabbott to handle name=None.

Builds on #14746. Proposed in #14934.
2020-05-12 17:42:49 -07:00