Page MenuHomePhabricator

Replacing a Herald rule with a Doorkeeper extension
Closed, WontfixPublic

Description

We have been using the following Herald custom action to allow us to notify HipChat rooms when a Maniphest task is updated:

<?php

/**
 * A Herald action for sending notifications to HipChat rooms.
 */
final class HeraldHipChatNotificationCustomAction extends HeraldCustomAction {

  public function appliesToAdapter(HeraldAdapter $adapter) {
    return $adapter instanceof HeraldManiphestTaskAdapter;
  }

  public function appliesToRuleType($rule_type) {
    return $rule_type == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL;
  }

  public function getActionKey() {
    return 'hipchat.notification';
  }

  public function getActionName() {
    return pht('Notify a HipChat room');
  }

  public function getActionType() {
    return HeraldAdapter::VALUE_TEXT;
  }

  public function applyEffect(
    HeraldAdapter $adapter,
    $object,
    HeraldEffect $effect) {

    $handle = id(new PhabricatorHandleQuery())
      ->setViewer(PhabricatorUser::getOmnipotentUser())
      ->withPHIDs(array($adapter->getPHID()))
      ->executeOne();

    $author = id(new PhabricatorPeopleQuery())
      ->setViewer(PhabricatorUser::getOmnipotentUser())
      ->withPHIDs(array(
        $adapter->getHeraldField(HeraldAdapter::FIELD_AUTHOR),
      ))
      ->executeOne();

    $assignee = id(new PhabricatorPeopleQuery())
      ->setViewer(PhabricatorUser::getOmnipotentUser())
      ->withPHIDs(array(
        $adapter->getHeraldField(HeraldAdapter::FIELD_ASSIGNEE),
      ))
      ->executeOne();

    $action = $adapter->getHeraldField(HeraldAdapter::FIELD_IS_NEW_OBJECT)
      ? pht('A new task was created')
      : pht('A task was updated');

    try {
      $client = $this->getClient();

      $client->messageRoom(
        $effect->getTarget(),
        PhabricatorEnv::getEnvConfig('hipchat.author'),
        (string)$this->getMessage(
          $action,
          sprintf(
            '%s: %s',
            $object->getMonogram(),
            $adapter->getHeraldField(HeraldAdapter::FIELD_TITLE)),
          $handle),
        false,
        PhabricatorEnv::getEnvConfig('hipchat.color'));

      return new HeraldApplyTranscript(
        $effect,
        true,
        pht('Notified HipChat room.'));
    } catch (Exception $ex) {
      return new HeraldApplyTranscript($effect, false, $ex->getMessage());
    }
  }

  /**
   * Create a new HipChat API object.
   */
  protected function getClient() {
    $server = PhabricatorEnv::getEnvConfig('hipchat.server');
    $token  = PhabricatorEnv::getEnvConfig('hipchat.token');

    if (!$token) {
      throw new Exception('No HipChat API token specified!');
    }

    return new HipChatClient($token, $server);
  }

  /**
   * Create the notification message.
   *
   * @param  string
   * @param  string
   * @param  PhabricatorObjectHandle
   * @return string
   */
  private function getMessage(
    $action,
    $title,
    PhabricatorObjectHandle $handle) {

    $header = phutil_tag(
      'div',
      array(),
      array(
        phutil_tag('b', array(), $action.': '),
        phutil_tag(
          'a',
          array('href' => PhabricatorEnv::getURI($handle->getURI())),
          $title),
      ));

    return (string)phutil_tag('div', array(), $header);
  }

}

We are looking to extend this custom action to work on diffs as well, and it occurred to me that maybe this integration should be in Doorkeeper rather than Herald? Currently, we write Herald rules such as if [project] is [my project] notify HipChat room [my room]. Is if possible to do this in Doorkeeper, or is Doorkeeper an all-or-nothing when it comes to notifications?

Event Timeline

joshuaspence raised the priority of this task from to Needs Triage.
joshuaspence updated the task description. (Show Details)
joshuaspence added projects: Doorkeeper, Herald.
joshuaspence added a subscriber: joshuaspence.
chad added a subscriber: chad.Jul 8 2015, 10:29 PM

What do you need from HipChat that you'd want to see in Conpherence? We've been debating the usefulness of features like Feed inside the Conpherence, curious if you'd use it and what you'd want to see.

chad added a comment.Jul 8 2015, 10:29 PM

(Yes, Doorkeeper is send stuff out, Nuance is pull stuff in).

Personally, I'd much rather use Conpherence because I absolutely love mentions and find them very useful. Having said that, convincing 200+ others to use Conpherence is much more difficult.

I think that the big features that I would miss from HipChat are:

  1. Notifications: HipChat sends notifications to my phone when I receive a message. This is made more useful because it only occurs when I am "away" on HipChat (T7575 is the blocking task for this to happen, I guess).
  2. Integrations: We have HipChat integrated with a bunch of other tools:
    • Phabricator
    • Jenkins
    • PagerDuty

To chime in on the hijacked direction of this thread. I had my users switch from XMPP/pidgin to conpherence for a week to see how viable it was. Here were the main complaints. My comments are added at the end of some in italics.

  1. can't see when other person is typing
  2. can't create new message from the conpherence side pane (This has since been fixed)
  3. doesn't give screen alert about new im like pidgin (T7568)
  4. no formatting toolbar in conpherence side pane (new user, wasn't as comfortable with remarkup)
  5. can't see who's online/how long they've been inactive (T7575)
  6. doesn't play sounds when I get a message (T7567)
  7. no remarkup preview (there's a full screen window for conpherence which seems just shows a lot of whitespace but seems like it should show a preview)
  8. refreshing page seems to put focus on first conversation in conpherence side pane, not last active one (T8220)
  9. should be able to save setting for notifications instead of emails for all new convos (not sure exatcly what is wanted here)
  10. right now i can search for a word, and it'll show me threads that contain that word but doesn't look like there's a way to go to the spot in the thread that had the word (T3165)
  11. incredible amounts of spam plus no way to notice if there's been a new message short of checking my email. to leave the entire browser open sacrifices vast amounts of real estate. if they implement desktop notifications then i think we'll have something workable (I think this is the same as #3)

Not sure how these match up against what other people are looking for though

chad added a comment.Jul 8 2015, 11:31 PM

I'm mostly just curious in learning about real integration use cases.

I know you don't provide support for writing third party extensions, but is it possible to do what I am trying to do with Doorkeeper currently? Basically I guess I need to define conditions which define what is published and where it is published to.

avivey added a subscriber: avivey.Jul 10 2015, 8:22 PM

AFAIK, DoorKeeper is a way to represent foreign objects and to interact with them; Here the remote object-like thing would be the chat channel?
This might be better served by:

  • Fixing/improving chat-bot, or
  • Use hipchat's write-only api with the feed.http-hooks (Might require extra steps), or
  • A feed worker, like DoorkeeperFeedWorker, sans the Doorkeeper part.
eadler added a project: Restricted Project.Jan 8 2016, 11:22 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.
eadler edited projects, added Restricted Project; removed Restricted Project.Feb 26 2016, 7:58 PM
eadler edited projects, added Restricted Project; removed Restricted Project.Mar 11 2016, 6:24 AM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jul 4 2016, 9:05 PM
epriestley closed this task as Wontfix.May 27 2019, 2:30 PM
epriestley claimed this task.
epriestley added a subscriber: epriestley.

I think the modern answer here is "use Webhooks". They may not do everything you want if you're writing a chat bot (notably, they intentionally do not currently provide a human-readable text representation of transactions) but there generally suitable for publishing changes to Phabricator objects into a remote system and will produce a program with generally reasonable behaviors and no weird demons lurking under the surface.

Doorkeeper is more focused on synchronizing object state between systems than publishing an event feed. If you wanted to literally bridge a Conpherence room to a HipChat room so that all the chat in either one showed up in the other, Doorkeeper might be appropriate, but real-time synchronization isn't a primary goal so it might be unacceptably slow. It's fine if a tweet or a GitHub issue take a minute to make it into Phabricator, but it's not fine if chat is delayed by even a few seconds.

Basically, if your use case is:

  • publish a feed of changes into a remote system (e.g., a chatbot who broadcasts certain feed stories to a chat channel); and
  • no part of the system cares much about state (e.g., the chatbot isn't interactive and just parrots feed stories).

...then Webhooks are the right tool.

If one side of the system is maintaining objects representing objects on the other side of the system and needs to synchronize state, Webhooks alone may not cut it, and Doorkeeper or some other approach may be better. However, we rarely see this sort of thing and I don't think this use case was a complex one.

(This task also had a bunch of Conpherence feedback but I think most of it is known / filed elsewhere / self-evident so I'm just going to let it fade into memory here and we can revisit it the next time Conpherence gets updates if anything was missed.)