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!
This is just a basic Dropbox webhook integration. It just
notifies a user when something has changed, it does not
specify what changed. Doing so would require storing data,
as Dropbox API was created mainly for file managers, not
integrations like this.
Closes#5672
Such payloads are generated when a GitLab repository has merge
request approvals enabled and a project member approves a merge
request. Approving is not the same as merging.
We now ignore payloads where payload['push']['changes'] is empty,
because an empty push doesn't really convey any useful information.
I couldn't find a way to replicate the action that would generate
such a payload, so I took one of our existing payloads and editted
out payload['push']['changes'] myself, so this payload is not
authentic.
Payloads that don't have a payload['object_attributes']['action']
attribute are generated when GitLab sends a test payload to verify
if the webhook was set up successfully. In this case, we should
send a message notifying that the webhook was configured
successfully.
The "subdomain" label is redundant, to the extent it's even
accurate -- this is really just the URL we want to display,
which may or may not involve a subdomain. Similarly "external".
The former `external_api_path_subdomain` was never a path -- it's a
host, followed by a path, which together form a scheme-relative URL.
I'm not quite convinced that value is actually the right thing in
2 of the 3 places we use it, but fixing that can start by giving an
accurate name to the thing we have.
This commit migrates all webhooks to use check_send_stream_message
instead of check_send_message. The only two webhooks that still
use check_send_message are our yo and teamcity webhooks. They
both use check_send_message for private messages.
TRELLO_MESSAGE_TEMPLATE and TRELLO_SUBJECT_TEMPLATE are
redundant. This commit removes them. Now, subjects don't end
in periods. And where a period is necessary in the message body,
one is appended at the end of the specific template for that
message.
I feel like getting notifications about a board's background being
changed isn't very useful information and could interrupt the flow
of other important information such as Card changes or movement,
so I think we should not support this event and
should simply ignore such payloads in the future.
The older fixture for this event assumed that the "assignee" key
had a value of '{}' if no one was assigned to a PR anymore.
However, that is no longer true because testing with requestbin
showed that in the latest JSON payload for this event, the key
"assignee" now has the value of 'null' (None when converted to
Python) when a user is unassigned from a PR. The current code
didn't handle this correctly. This commit makes sure it does!
Its unclear as to whether the old fixture was simply wrong or
whether GitHub changed its payloads in any way.
Update Email, Beanstalk, Hubot, JIRA, and Trello integrations
links.
The Hubot integrations section (/integrations#hubot-integrations)
was removed in an earlier redesign of /integrations. This commit
replaces the link with the hubot-scripts organization on
Github, which displays the comprehensive list of all integrations
available via Hubot.
Fixes#5875.
If an incoming payload contained a unicode character, it raised
a UnicodeEncodeError, because the message template was an str. Now,
the message template is unicode, so it can be formatted to include
unicode characters, should the incoming payloads contain any.
Eeshan Garg
Ricky
neiljp (Neil Pilgrim)
Alena Volkova
Tim Abbott
Rishi Gupta
nyan-salmon
Sumana Harihareswara
Josh Mandel
Privisus
Patrick Grave
Abijith10
infinitelooped
fredfishgames
Viraat Chandra
Sivagiri Visakan
Kiy4h
Xavier Cooney
MadElf1337
rht
Rhea Parekh
greysome
Angelika Serwa
Marco Burstein
Callum Fraser
Ivche1337
Arseny Chekanov
derAnfaenger
Sampriti Panda
Tommy Ip
Greg Price
Steve Howell
Marco Matarazzo
Jack Zhang
Greg Price
Vishnu Ks
Kenji Okimoto
Sadjad Fouladi
Nick Sweeting
Rohitt Vashishtha