The TeamCity webhook plugin supports multiple payload formats that
are customized to be used by different services such as Slack,
Flowdock, etc. We don't support such payloads, so we should ignore
them and stick to parsing only the generic ones. We should also
notify that bot owner about the error.
Now reading API keys from a user is done with the get_api_key wrapper
method, rather than directly fetching it from the user object.
Also, every place where an action should be done for each API key is now
using get_all_api_keys. This method returns for the moment a single-item
list, containing the specified user's API key.
This commit is the first step towards allowing users have multiple API
keys.
This is a follow-up in response to Tim's comments on #9951.
In instances where all messages from a BitBucket integration are
grouped under one user specified topic (specified in the URL), we
should include the title of the PR in the message body, since
the availability of a user-specified topic precludes us from
including it in the topic itself (which was the default behaviour).
This is a follow-up in response to Tim's comments on #9951.
In instances where all messages from a Gogs integration are
grouped under one user specified topic (specified in the URL), we
should include the title of the PR in the message body, since
the availability of a user-specified topic precludes us from
including it in the topic itself (which was the default behaviour).
This is a follow-up in response to Tim's comments on #9951.
In instances where all messages from a GitHub integration are
grouped under one user specified topic (specified in the URL), we
should include the title of the issue/PR in the message body, since
the availability of a user-specified topic precludes us from
including it in the topic itself (which was the default behaviour).
This is a follow-up in response to Tim's comments on #9951.
In instances where all messages from a Gitlab integration are
grouped under one user specified topic (specified in the URL), we
should include the title of the issue/MR in the message body, since
the availability of a user-specified topic precludes us from
including it in the topic itself (which was the default behaviour).
We already include the issue title in the topic. But if one chooses
to group all gitlab notifications under one topic, the message body
is misleading in the sense that only the Issue ID and the description
are displayed, not the title, which isn't super helpful if the topic
doesn't tell you the title either.
I think we should err on the side of always including the title in
the main message body, which is what this commit does.
Fixes#9913.
The only changes visible at the AST level, checked using
https://github.com/asottile/astpretty, are
zerver/lib/test_fixtures.py:
'\x1b\\[(1|0)m' ↦ '\\x1b\\[(1|0)m'
'\\[[X| ]\\] (\\d+_.+)\n' ↦ '\\[[X| ]\\] (\\d+_.+)\\n'
which is fine because re treats '\\x1b' and '\\n' the same way as
'\x1b' and '\n'.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This was technically a bug. For events that aren't unsupported
intentionally, the control should fall to the line that raises
UnknownWebhookEventType, and shouldn't be handled by anything else.
The events that are intentionally unsupported should be handled
more explicitly.
This is one of those weird webhooks where the
download-python-bindings.md macro doesn't work, because the user
only needs the bindings to run the one-time Trello script to register
the webhook and that script can be run from anywhere and doesn't need
to be "hosted" anywhere.
I spend a lot of time on this. One of our users had reported that
this webhook wasn't working at all. So I tested this with a local
ngrok instance and made sure that it was working. I also took this
opportunity to rewrite the docs for this, which were quite outdated.
With a few changes by Rishi Gupta!
This improves test coverage for a lot of our webhooks that relied
on ad-hoc methods to handle unexpected event types.
Note that I have deliberately skipped github_legacy, it isn't
advertised and is officially deprecated.
Also, I have refrained from making further changes to Trello, I
believe further improvements to test coverage should be covered
in separate per-webhook commits/PRs.
We ask our users to enable Snapshot notifications in Zulip via
Slack! But our Slack integration isn't exactly super robust and
I checked and our librato implementation isn't super smart about
handling snapshot payloads that come in via Slack.
Overall, this seems like a very poor solution, asking the user
to set up Slack in order to get the notifications in Zulip. So, I
thought we should get rid of at least the docs that suggest doing
this.
I also read librato/view.py and it wasn't clear to me how Slack
is supposed to act as an intermediate service here in a reliable
manner, which is another reason to not advertise this.
Epics are a way to further organize Pivotal Stories and are a
somewhat advanced feature that would take a significant amount of
work to properly implement. Unless we get requests for supporting
epics, I don't think we should support them.
We let Markdown increment the list step numbers, which is more
reliable than keeping track of numbered-steps manually.
Also, instead of linking to the CircleCI docs, we now have full
instructions for how to setup a webhook by modifying the circle.yml
file.
Ancient GitLab from several years ago doesn't include the
HTTP_X_GITLAB_EVENT header (and seems to have a different format), so
we should ignore its requests.
Might be good to document the version threshhold, but it's very hard
to tell from Googling what it is.
Rishi and I decided that it makes sense to get rid of the Facebook
integration for a few reasons, some of which are:
* The setup process is too complicated on Facebook's end. The users
will surely have to browse Facebook's huge API reference before even
having a vague idea of what they want.
* Slack chooses not to have a Facebook integration, but relies on
Zapier for it. Zaps that integrate with Facebook are much more
streamlined and the setup process isn't as much of a pain. Zapier's
Facebook Zaps are much more fine-tuned and there are different Zaps
for different parts of the FB API, a luxury that would likely span
2K+ lines of code on our end if we were to implement it from
scratch. So, I think we should relegate integration with Facebook to
Zapier as well!
* After thoroughly testing the setup process, we concluded that the
person who submitted the FB integration didn't really test it
thoroughly because there were some gaping holes in the docs (missing
steps, user permissions, etc.).
This one is one of the most tedious to set up and get working.
We now also rely on the Trello scripts available as part of the
`python-zulip-api/zulip` API package to make the setup process
easier.
This was a user-reported bug and a very subtle and painful one
to track down.
Previously, if payload['push']['changes'][i]['closed'] was True,
we assumed that a branch was removed. Looking at whether `closed`
was set to True or not was our way to tell whether a push removed
a branch or not.
However, this is wrong! `closed` being set to True can also mean
that the pull request associated with the branch was approved but
the branch itself was not deleted. According to the BitBucket docs,
the correct way to see if a branch is deleted is to check if `new`
is null.
This bug was leading to KeyErrors about not being able to find
the `commits` key, which shouldn't happen anymore!
webhook-errors.log file is cluttered with Stream.DoesNotExist
errors, which hides the errors that we actually need to see. So,
since check_message already sends the bot_owner a PM if the webhook
bot tries to send a message to a non-existent stream, we can ignore
such exceptions.
We filter out hidden comments out of Issue descriptions but this
breaks when description is null (which is unusual). So this commit
just checks to see if the description is None and if so, not to
filter anything out.
For a personal build, the teamcity webhook still sends a private
message using check_send_private_message since a personal build
should never trigger a public notification.
For a non-personal build, check_send_webhook_message is used,
which can either send a PM or a stream message based on whether
a stream is specified in the webhook URL or not.
We now only give users two options, to specify a stream and receive
public notifications for their goals, or to leave it out and receive
PMs and thus, keep their goals private. This simplifies the docs!
This fixes a regression in 93678e89cd
and a4979410f9, where the webhooks using
authenticated_rest_api_view were migrated to a new model that didn't
include setting a custom Client string for the webhook.
When restoring these webhooks' client strings, we also fix places
where the client string was not capitalized the same was as the
product's name.
This commit migrates all of our webhooks to use
check_send_webhook_message, except the following:
beeminder: Rishi wanted to wait on this one.
teamcity: This one is slightly more work.
yo: This one is PM-only. I am still trying to decide whether we
should have a force_private argument or something in
check_send_webhook_message.
facebook: No point in migrating this, will be removed as part of
#8433.
slack: Slightly more work too with the `channel_to_topics` feature.
Warrants a longer discussion.
These are the straightforward ones.
Note that there is a line in zerver.lib.test_classes.build_webhook_url
that lost test coverage. That's because most of our tests test using
stream messages so the webhook URLs being tested always have a query
parameter. So the line that accounts for there being no query
parameters never gets called, which is fine, but we should still
keep it.
This commit adds a generic function called check_send_webhook_message
that does the following:
* If a stream is specified in the webhook URL, it sends a stream
message, otherwise sends a PM to the owner of the bot.
* In the case of a stream message, if a custom topic is specified
in the webhook URL, it uses that topic as the subject of the
stream message.
Also, note that we need not test this anywhere except for the
helloworld webhook. Since helloworld is our default example for
webhooks, it is here to stay and it made sense that tests for a
generic function such as check_send_webhook_message be tested
with an actual generic webhook!
Fixes#8607.
Originally was going to centralize this in zerver/lib/request.pyi, but this
file is not visible at run-time, being only a stub. The matching request.py
file seemed inappropriate, as it doesn't actually use ViewFuncT.
Note that the "Save" button has no text in the Taiga webhook
setup UI. There is a small floppy disk symbol for saving which
is visible right beside the form fields. So I simply said,
"Save the form".
Eeshan Garg
Rishi Gupta
Adam Birds
Tim Abbott
Yago González
Anders Kaseorg
Lenny Jagielski
MaTaM673
Aditya Bansal
Preston Hansen
neiljp (Neil Pilgrim)
Balaji2198
Ricky
Alena Volkova