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.
There's probably follow-up work to do here to eliminate these
completely, but this dramatically shrinks the ~1 minute race window
that was previously present between import and test function being
called.
This commit:
* Restructures the doc to use a numbered-step format.
Note that there are no screenshots. I signed up for a
Fabric/Crashlyics account but you have to link an Android/iOS app
to even get to the settings panel, which seemed like too much work
just to get a screenshot.
However, the way we can verify (somewhat) the correctness of the
last step is that it is a paraphrase of the first paragraph of
Fabric's Webhook docs, which can be found here:
https://docs.fabric.io/apple/crashlytics/custom-web-hooks.html
This commit modifies the text to:
1. Removes unnecessary screenshots.
2. Use the numbered-style format.
3. I also removed the instructions for generating an access token.
I took a look at Dropbox's docs and you shouldn't need that
for a webhook setup. The whole point behind a webhook is that
one can get by without using OAuth.
4. Rearranges the instructions to only contain 4 steps. For
uncomplicated instructions, that seems to be the ideal number.
GitLab recently changed their event name for build notifications
from "Build Hook" to "Job Hook". Instead of just supporting the
latter, we should support both just in case people are running
older versions of GitLab.
At some point, GitLab decided to change the name of the event for
CI notifications from "Build Hook" to "Job Hook" and we started
running into errors in webhook-logger.log.
This commit:
* Removes the unnecessary screenshot. The UI is intuitive enough
and standalone instructions should suffice.
* Rearranges the instructions into 4 steps.
* Makes the wording more explicit.
This commit:
* Removes the unnecessary screenshot. The user should be able to
easily see the fields in question in this case.
* Wraps the text at 80 chars.
* Combines the instructions into 4 steps.
The docs for this can easily be combined into 4 steps. For
uncomplicated setups, 4 seems to be like a good number.
Again, I have no way of verifying the correctness of the instructions
here because Airbrake doesn't let you do anything till you specify
your credit card information, which I didn't want to.
This commit modifies the text to:
* Use number 1 for all steps and let Markdown take care of the rest.
* Removes the line that says this webhook is "experimental". It isn't
anymore.
This commit modifies the doc.md to:
* Use consistent language and style.
* Use the number 1 for all numbered steps and let Markdown take
care of the rest.
* Have detailed steps on how to get to the Integrations settings
instead of just linking to the page.
* Remove unnecessary screenshots.
This commit:
* Adds a missing step to the documentation.
* Replaces wording such as "Go to X" with "Click on X".
* Removes the unnecessary screenshots.
* Rearranges the doc to contain only 4 steps. For uncomplicated
setups, 4 seems to be the right number.
This commit:
* Removes the unnecessary screenshot.
* Reorders the instructions and combines them in to 4 steps.
* Improves the contents of the webhook-url-with-bot-email-indented.md
macro and makes it more consistent with create-bot-construct-url.md.
* Sets the recommended stream name to "commits", since that's what
the webhook function for Beanstalk expects in
zerver/webhooks/beanstalk/view.py. This allows us to use the
create-stream.md macro.
* Remove unnecessary screenshot. It doesn't help very much in this
case.
* Update text to instruct users to not leave the `Title` field
empty (it cannot be blank).
* Replace wording such as `Go to Settings` with `Click on Settings`.
* Combine the "fill out the form" and "click 'Save'" steps.
* Replace "Choose X on the left-hand side" with "Choose X".
* Replace "Remember to check the X" with "Check the X".
With minor fixes by eeshangarg!
Eeshan: I decided to remove the screenshot. It looks very old and
was blurry and the instructions were very screenshot-agnostic
anyway!
I couldn't update the screenshot because Airbrake doesn't even let
you use the free trial till you give them your credit card info,
which I didn't want to do!
Eeshan Garg
neiljp (Neil Pilgrim)
Tim Abbott
Balaji2198
Ricky
Alena Volkova
Rishi Gupta
nyan-salmon
Sumana Harihareswara
Josh Mandel
Privisus
Patrick Grave
Abijith10