Statuspage.io webhook integration #7911

Rishabh570 · 2017-12-27T07:26:26Z

Fixes #5868 .

MadElf1337 · 2017-12-28T16:59:16Z

I'm doing the documentation, not coding

Rishabh570 · 2017-12-28T17:05:37Z

Okay, cool !!!

showell · 2018-01-07T13:42:40Z

I did a quick skim, but not a complete review. Overall things seem reasonable.

Can you put the world "Add" in your commit message, after the "integration: " prefix? We like verbs in the commit message.

Rishabh570 · 2018-01-07T13:49:47Z

@showell That's not my commit message..my commit message is "integration: Statuspage.io webhook integration"

Balaji2198 · 2018-01-07T15:24:29Z

Hi @Rishabh570. It will be better if your commit message looks like
integration: Add Statuspage.io webhook integration.
You can do that by git commit --amend.

eeshangarg

@Rishabh570: Thanks for working on this! Left a few comments! :)

eeshangarg · 2018-01-22T18:24:03Z

zerver/webhooks/statuspage/doc.md

+Get notifications from Statuspage.io in Zulip whenever any
+company that you subscribed to creates or updates an incident, or updates a component's
+status.
+


Replace the above with:

Get Zulip notifications for your Statuspage.io subscriptions!

eeshangarg · 2018-01-22T18:34:15Z

zerver/webhooks/statuspage/doc.md

+
+1. What company needs to do:
+Go to the **subscribers settings** in **notifications** tab and **enable** notifications delivery through **webhook**.
+


Okay, I am not sure what you mean by this step. For instance is **enable** something users click on or is it **webhook** that they are supposed to click on! I would recommend being more explicit here and making sure you get capitalisation for the buttons' text right! :)

@eeshangarg This setting is something company needs to set up as how they want notifications to be sent to their users (like by email, webhooks or sms)...company must select this if they want to send webhook notifications to their users...

Here is the screenshot of that setting : https://github.com/Rishabh570/zulip/blob/statuspage-webhook/static/images/integrations/statuspage/001.png

@Rishabh570: I see that, what I meant was that you don't need to bold things like **enable** unless they are actual physical sections or buttons on the website. And you need to make sure that stuff like **webhook** (which is an actual button in the screenshot) is properly capitalised. So, in this case, it would be **WEBHOOK**. :)

Ooh okay ! I'll change this..

eeshangarg · 2018-01-22T18:38:45Z

zerver/webhooks/statuspage/doc.md

+2. What users need to do:
+Go to the company's statuspage.io site(which is example.statuspage.io) and click **SUBSCRIBE TO UPDATES** button and
+provide there the Webhook URL.
+


Okay, so replace the above point with this:

In order to subscribe to a company's Statuspage.io notifications: 1. Go to the company's Statuspage.io site (for instance, `example.statuspage.io`). 2. Enter the webhook URL constructed above in the **Target Webhook** field. 2. Click on **SUBSCRIBE TO UPDATES**.

For future reference, with instructions such as these, try to make sure that they are in the correct order. For example, you can't click on SUBSCRIBE TO UPDATES without providing the webhook URL first, etc. :)

eeshangarg · 2018-01-22T18:41:15Z

zerver/webhooks/statuspage/view.py

+                           stream: str=REQ(default='statuspage-test'),
+                           topic: str=REQ(default='Statuspage')) -> HttpResponse:
+
+    status = payload["page"]["status_description"]


I would recommend using payload["page"]["status_indicator"]. Relying on the description to figure out what kind of event a payload corresponds to can be wrong. Descriptions can be anything and can change. :)

eeshangarg · 2018-01-22T18:48:43Z

zerver/webhooks/statuspage/view.py

+        new_status = payload["component_update"]["new_status"]
+        body = u"Component status changed :\n **{}** has changed status from **{}** to **{}**"
+        body = body.format(name, old_status, new_status)
+


Okay so:

Split out each if clause into a separate function. See how our GCI webhook does this.

Do not have a space before the :. So use Component status changed:. :)

I think the topic should be the status_description and the name of the incident, which would mean that you would not need things like Incident Report: or Component status changed:.

Rishabh570 · 2018-01-29T16:11:06Z

@eeshangarg Sorry for delay...had a vagrant issue. I would like you to take a look at view.py (created functions and called them inside if else). Thanx for reviewing btw :)

eeshangarg

@Rishabh570: Hey, I left a couple of more comments! Thanks for working on this! :)

eeshangarg · 2018-01-31T05:50:00Z

zerver/webhooks/statuspage/doc.md

+
+{!create-bot-construct-url.md!}
+
+What company needs to do:


Delete this line. :)

eeshangarg · 2018-01-31T05:52:14Z

zerver/webhooks/statuspage/doc.md

+
+What company needs to do:
+
+Go to the **subscribers settings** in **notifications** tab and enable notifications delivery through **WEBHOOK**.


Okay so a couple of points about this one:

Is subscribers settings an option on the third party website? If so, shouldn't it be capitalised like Subscriber's Settings? If not, then you do not need to bold the text. :)

Change it like this: If you're a company and you'd like to enable webhook notifications, go to the .... and so on. :)

Subscriber's settings are for company... I guess Subscriber's Settings would be good because it is present in the Notification settings dashboard...

eeshangarg · 2018-01-31T05:54:55Z

@Rishabh570: Also your build is failing with some pep8 indentation issues! :)

Rishabh570 · 2018-02-02T12:08:32Z

@eeshangarg Done changes here also :)

eeshangarg

@Rishabh570: Thanks so much for working on this! Left one final comment! :)

eeshangarg · 2018-02-04T01:10:10Z

zerver/webhooks/statuspage/doc.md

+{!create-bot-construct-url.md!}
+
+If you're a company and you'd like to enable webhook notifications,
+go to the **Subscriber's Settings** in **Notifications** tab and enable


Replace **Subscriber's Settings** with just **Settings**, your screenshot doesn't say Subscriber's Settings, it says only Settings

Rishabh570 · 2018-02-06T10:34:36Z

@eeshangarg Done changes here...Please take a look :)

eeshangarg

@Rishabh570: Thanks for working on this and bearing with me through so many rounds of feedback! This looks good to me! :)

@timabbott: This LGTM! Could you please take one final look and merge? Thanks! :)

timabbott · 2018-02-07T18:00:23Z

Merged, thanks @Rishabh570 and @eeshangarg! I made a small tweak to the documentation text when merging to make it a bit more clear what the difference was between the two ways to setup notifications on the statuspage.io site (depending whether it's your own product's status page). It may be worth further work to make it even clearer (e.g. using bold or headings or something).

zulipbot added the needs review label Dec 27, 2017

Rishabh570 force-pushed the statuspage-webhook branch from ef57752 to e758c56 Compare December 27, 2017 07:50

Rishabh570 mentioned this pull request Dec 28, 2017

integrations: Add webhook payloads for StatusPage.io. #7923

Merged

Rishabh570 force-pushed the statuspage-webhook branch from e758c56 to 90748a9 Compare December 28, 2017 17:00

Rishabh570 force-pushed the statuspage-webhook branch 4 times, most recently from 5d82ca5 to 0843f20 Compare January 7, 2018 11:03

Rishabh570 force-pushed the statuspage-webhook branch 3 times, most recently from 4571dd5 to a0b1700 Compare January 13, 2018 19:05

Rishabh570 force-pushed the statuspage-webhook branch from a0b1700 to 7e78e93 Compare January 16, 2018 07:15

zulipbot added the size: XL label Jan 16, 2018

eeshangarg reviewed Jan 22, 2018

View reviewed changes

Rishabh570 force-pushed the statuspage-webhook branch 3 times, most recently from 8844cfb to ffea6d3 Compare January 29, 2018 16:06

Rishabh570 force-pushed the statuspage-webhook branch from ffea6d3 to 4bb0df3 Compare January 29, 2018 16:26

eeshangarg reviewed Jan 31, 2018

View reviewed changes

Rishabh570 force-pushed the statuspage-webhook branch 2 times, most recently from e3989b0 to 2d9fc26 Compare January 31, 2018 18:25

eeshangarg reviewed Feb 4, 2018

View reviewed changes

integration: Add statuspage.io webhook integration.

f21f3d2

Rishabh570 force-pushed the statuspage-webhook branch from 2d9fc26 to f21f3d2 Compare February 4, 2018 05:21

eeshangarg reviewed Feb 7, 2018

View reviewed changes

timabbott closed this Feb 7, 2018

Rishabh570 deleted the statuspage-webhook branch March 10, 2018 03:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Statuspage.io webhook integration #7911

Statuspage.io webhook integration #7911

Rishabh570 commented Dec 27, 2017 •

edited

MadElf1337 commented Dec 28, 2017

Rishabh570 commented Dec 28, 2017

showell commented Jan 7, 2018 •

edited

Rishabh570 commented Jan 7, 2018 •

edited

Balaji2198 commented Jan 7, 2018 •

edited

eeshangarg left a comment

eeshangarg Jan 22, 2018

eeshangarg Jan 22, 2018

Rishabh570 Jan 23, 2018

Rishabh570 Jan 23, 2018

eeshangarg Jan 24, 2018 •

edited

Rishabh570 Jan 24, 2018

eeshangarg Jan 22, 2018

eeshangarg Jan 22, 2018

eeshangarg Jan 22, 2018

Rishabh570 commented Jan 29, 2018

eeshangarg left a comment

eeshangarg Jan 31, 2018

eeshangarg Jan 31, 2018

Rishabh570 Jan 31, 2018 •

edited

eeshangarg commented Jan 31, 2018

Rishabh570 commented Feb 2, 2018

eeshangarg left a comment

eeshangarg Feb 4, 2018

Rishabh570 commented Feb 6, 2018

eeshangarg left a comment

timabbott commented Feb 7, 2018


		1. What company needs to do:
		Go to the subscribers settings in notifications tab and enable notifications delivery through webhook.


		What company needs to do:

		Go to the subscribers settings in notifications tab and enable notifications delivery through WEBHOOK.

Statuspage.io webhook integration #7911

Statuspage.io webhook integration #7911

Conversation

Rishabh570 commented Dec 27, 2017 • edited

MadElf1337 commented Dec 28, 2017

Rishabh570 commented Dec 28, 2017

showell commented Jan 7, 2018 • edited

Rishabh570 commented Jan 7, 2018 • edited

Balaji2198 commented Jan 7, 2018 • edited

eeshangarg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eeshangarg Jan 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rishabh570 commented Jan 29, 2018

eeshangarg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rishabh570 Jan 31, 2018 • edited

Choose a reason for hiding this comment

eeshangarg commented Jan 31, 2018

Rishabh570 commented Feb 2, 2018

eeshangarg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rishabh570 commented Feb 6, 2018

eeshangarg left a comment

Choose a reason for hiding this comment

timabbott commented Feb 7, 2018

Rishabh570 commented Dec 27, 2017 •

edited

showell commented Jan 7, 2018 •

edited

Rishabh570 commented Jan 7, 2018 •

edited

Balaji2198 commented Jan 7, 2018 •

edited

eeshangarg Jan 24, 2018 •

edited

Rishabh570 Jan 31, 2018 •

edited