Merge 7b76949 into 028bd1f

src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ManagedGroupService.scala

@@ -77,9 +77,12 @@ class ManagedGroupService(private val resourceService: ResourceService, private
 
   def deleteManagedGroup(groupId: ResourceId): Future[Unit] = {
     for {
-      _ <- directoryDAO.deleteGroup(WorkbenchGroupName(groupId.value))
+      // order is important here, we want to make sure we do all the cloudExtensions calls before we touch ldap
+      // so failures there do not leave ldap in a bad state
+      // resourceService.deleteResource also does cloudExtensions.onGroupDelete first thing
       _ <- cloudExtensions.onGroupDelete(WorkbenchEmail(constructEmail(groupId.value)))
       _ <- resourceService.deleteResource(Resource(managedGroupType.name, groupId))
+      _ <- directoryDAO.deleteGroup(WorkbenchGroupName(groupId.value))
     } yield ()
   }
 

src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceService.scala

@@ -140,9 +140,10 @@ class ResourceService(private val resourceTypes: Map[ResourceTypeName, ResourceT
   def deleteResource(resource: Resource): Future[Unit] = {
     for {
       policiesToDelete <- accessPolicyDAO.listAccessPolicies(resource)
+      // remove from cloud extensions first so a failure there does not leave ldap in a bad state
+      _ <- Future.traverse(policiesToDelete) { policy => cloudExtensions.onGroupDelete(policy.email) }
       _ <- Future.traverse(policiesToDelete) {accessPolicyDAO.deletePolicy}
       _ <- maybeDeleteResource(resource)
-      _ <- Future.traverse(policiesToDelete) { policy => cloudExtensions.onGroupDelete(policy.email) }
     } yield ()
   }