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".
Rewritten in significant part by tabbott to actually be correct.
One particularly nasty thing the original webhook integration did is
do `current_time = time.time()` at the top of the `view.py` function
-- that means that code ran at import time, not runtime.
Eeshan Garg
Tim Abbott
neiljp (Neil Pilgrim)
Balaji2198
Ricky
Alena Volkova