Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Kibela WebHook #75

Merged
merged 7 commits into from Apr 4, 2017
Merged

Add Kibela WebHook #75

merged 7 commits into from Apr 4, 2017

Conversation

yoshiori
Copy link
Contributor

@yoshiori yoshiori commented Mar 24, 2017

Add support for Kibela WebHook

idobata_2017-03-24_19-24-19

@hibariya
Copy link
Member

hibariya commented Mar 27, 2017

@yoshiori Thank you for the Pull Request! I'll confirm the details as soon as possible.

module Idobata::Hook
class Kibela < Base
screen_name 'kibe.la'
icon_url hook_image_url('icon.png')
Copy link
Member

@hibariya hibariya Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there is not the icon.png here but icon.jpg is. I'd like to use png format rather than jpg as far as possible. How about replacing icon.jpg with this ? (It can be found in this article).

Copy link
Contributor Author

@yoshiori yoshiori Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏 Sorry. Fix it. db277f0

Copy link
Member

@hibariya hibariya Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 😸

Copy link
Member

@hibariya hibariya left a comment

I've tried this branch on my development environment. Could you please check the comments?

</span>
kibe
deleted
<a href='https://docs.kibe.la/@kibe/1#comment_1'></a>
Copy link
Member

@hibariya hibariya Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we will want to know the link of the comment when the notification comes. Although a comment doesn't have its title, we can use the title of it's entry (blog entry or wiki page) like this:

on created:
selection_614

on deleted:
selection_615

💭 I guess that a payload of comment event includes information about either Blog or Wiki, and both of them has title.

Copy link
Member

@hibariya hibariya Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, leading <i> and 【comment】 may unnecessary. Even if without them, (in my opinion) the notifications look informative enough.

Copy link
Contributor Author

@yoshiori yoshiori Mar 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified notification message. c171eb0

Copy link
Member

@hibariya hibariya Apr 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks very good!

class Kibela < Base
screen_name 'kibe.la'
icon_url hook_image_url('icon.png')
template_name { "#{action}.html.haml" }
Copy link
Member

@hibariya hibariya Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kibela sends a test notification. Notifying user either the hook setting succeeded or not seems important. Could you support that?

📝 For your information: When I clicked the "Send Test" on Webhook page on Kibela,

selection_593

Then I got a payload like this:

{
  "team": {
    "name": "idobata",
    "url": "https://idobata.kibe.la/"
  },
  "resource_type": "test",
  "action": "send",
  "action_user": {
    "id": 2,
    "account": "hibariya",
    "real_name": "",
    "url": "https://idobata.kibe.la/@hibariya",
    "avatar_photo": {
      "url": "https://cdn.kibe.la/media/public/1600/W1siZiIsInB1YmxpYy9kZWZhdWx0X2F2YXRhcnMvYXZhdGFyMkAyeC5wbmciXSxbInAiLCJlbmNvZGUiLCJwbmciXSxbInAiLCJjb252ZXJ0IiwiLWFscGhhIHNldCAtYmFja2dyb3VuZCBub25lIC12aWduZXR0ZSAweDArMCswIl0sWyJwIiwidGh1bWIiLCI0MHg0MCJdXQ/6a6e944cbdd1681b/file.png"
    }
  }
}

Copy link
Contributor Author

@yoshiori yoshiori Mar 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix 0901b39

Copy link
Member

@hibariya hibariya Apr 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

payload.action
end

def hide_body?
Copy link
Member

@hibariya hibariya Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks nice option 👍

@hibariya hibariya merged commit ce5d0ae into idobata:master Apr 4, 2017
1 check passed
@yoshiori
Copy link
Contributor Author

yoshiori commented Apr 4, 2017

🎉

@hibariya
Copy link
Member

hibariya commented Apr 4, 2017

I've deployed this to https://idobata.io 🎉 @yoshiori Thank you again!

@yoshiori yoshiori deleted the add_support_kibela branch Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants