Add code that will allow build triggers to be deactivated or deleted from their services.

data/model.py

@@ -1508,16 +1508,6 @@ def list_build_triggers(namespace_name, repository_name):
            Repository.name == repository_name))
 
 
-def delete_build_trigger(namespace_name, repository_name, trigger_uuid):
-  trigger = get_build_trigger(namespace_name, repository_name, trigger_uuid)
-
-  # Delete the access token created for this trigger, and the trigger itself
-  if trigger.write_token and trigger.write_token.code:
-    trigger.write_token.delete_instance()
-
-  trigger.delete_instance()
-
-
 def list_trigger_builds(namespace_name, repository_name, trigger_uuid,
                         limit=None):
   query = (list_repository_builds(namespace_name, repository_name)

endpoints/api.py

@@ -29,7 +29,8 @@ from auth.permissions import (ReadRepositoryPermission,
                               ViewTeamPermission,
                               UserPermission)
 from endpoints.common import common_login, get_route_data, truthy_param
-from endpoints.trigger import BuildTrigger, TriggerActivationException
+from endpoints.trigger import (BuildTrigger, TriggerActivationException,
+                               TriggerDeactivationException)
 from util.cache import cache_control
 from datetime import datetime, timedelta
 
@@ -1413,15 +1414,15 @@ def activate_build_trigger(namespace, repository, trigger_uuid):
                                           token.code, app.config['URL_HOST'],
                                           path)
 
-        handler.activate(trigger.uuid, authed_url, trigger.auth_token,
+        final_config = handler.activate(trigger.uuid, authed_url,
-                         new_config_dict)
+                                        trigger.auth_token, new_config_dict)
       except TriggerActivationException as e:
         token.delete_instance()
         abort(400, message = e.msg)
         return
 
       # Save the updated config.
-      trigger.config = json.dumps(new_config_dict)
+      trigger.config = json.dumps(final_config)
       trigger.write_token = token
       trigger.save()
 
@@ -1492,7 +1493,22 @@ def list_build_triggers(namespace, repository):
 def delete_build_trigger(namespace, repository, trigger_uuid):
   permission = AdministerRepositoryPermission(namespace, repository)
   if permission.can():
-    model.delete_build_trigger(namespace, repository, trigger_uuid)
+    try:
+      trigger = model.get_build_trigger(namespace, repository, trigger_uuid)
+    except model.InvalidBuildTriggerException:
+      abort(404)
+      return
+
+    handler = BuildTrigger.get_trigger_for_service(trigger.service.name)
+    config_dict = json.loads(trigger.config)
+    if handler.is_active(config_dict):
+      try:
+        handler.deactivate(trigger.auth_token, config_dict)
+      except TriggerDeactivationException as ex:
+        # We are just going to eat this error
+        logger.warning('Trigger deactivation problem.', ex)
+
+    trigger.delete_instance()
     log_action('delete_repo_trigger', namespace, 
                {'repo': repository, 'trigger_id': trigger_uuid},
                repo=model.get_repository(namespace, repository))

endpoints/trigger.py

@@ -28,6 +28,9 @@ class InvalidServiceException(Exception):
 class TriggerActivationException(Exception):
   pass
 
+class TriggerDeactivationException(Exception):
+  pass
+
 class ValidationRequestException(Exception):
   pass
 
@@ -70,6 +73,15 @@ class BuildTrigger(object):
   def activate(self, trigger_uuid, standard_webhook_url, auth_token, config):
     """ 
     Activates the trigger for the service, with the given new configuration.
+    Returns new configuration that should be stored if successful.
+    """
+    raise NotImplementedError
+
+  def deactivate(self, auth_token, config):
+    """
+    Deactivates the trigger for the service, removing any hooks installed in
+    the remote service. Returns the new config that should be stored if this
+    trigger is going to be re-activated.
     """
     raise NotImplementedError
 
@@ -104,7 +116,7 @@ class GithubBuildTrigger(BuildTrigger):
     return 'github'
 
   def is_active(self, config):
-    return 'build_source' in config and len(config['build_source']) > 0
+    return 'hook_id' in config
 
   def activate(self, trigger_uuid, standard_webhook_url, auth_token, config):
     new_build_source = config['build_source']
@@ -122,11 +134,30 @@ class GithubBuildTrigger(BuildTrigger):
     }
 
     try:
-      to_add_webhook.create_hook('web', webhook_config)
+      hook = to_add_webhook.create_hook('web', webhook_config)
+      config['hook_id'] = hook.id
     except GithubException:
       msg = 'Unable to create webhook on repository: %s'
       raise TriggerActivationException(msg % new_build_source)
 
+    return config
+
+  def deactivate(self, auth_token, config):
+    gh_client = self._get_client(auth_token)
+
+    try:
+      repo = gh_client.get_repo(config['build_source'])
+      to_delete = repo.get_hook(config['hook_id'])
+      to_delete.delete()
+    except GithubException:
+      msg = 'Unable to remove hook: %s' % config['hook_id']
+      raise TriggerDeactivationException(msg)
+
+    config.pop('hook_id', None)
+
+    return config
+
+
   def list_build_sources(self, auth_token):
     gh_client = self._get_client(auth_token)
     usr = gh_client.get_user()