feat(webhook): refactor and additional webhook (#1286)

* feat(webhook): Add/Remove version webhook

https://zanata.atlassian.net/browse/ZNTA-1166

* feat(webhook): Add/Remove version webhook

https://zanata.atlassian.net/browse/ZNTA-1166

* Update functional test

* Add uniqyue constraint on URL

* Make webhook event async

* Add comment on why Async annotation is needed

* Move save webhook to ProjectService

* No check on duplication url when updating webhook

* Check for webhook url duplication other than self

docs/user-guide/projects/project-settings.md

@@ -146,29 +146,57 @@ The access restriction feature is intended for use with special roles that can b
 <figcaption>Project Webhooks Settings tab</figcaption>
 </figure>
 
-The Webhooks feature is HTTP callbacks which are triggered when a document in a language has reached a certain milestone.
-Currently, webhooks events will be triggered when
-
-- A document has reached 100% Translated
-- A document has reached 100% Approved (by reviewer)
-
+The Webhooks feature is HTTP callbacks which are triggered when a selected event happens.
 When an event occurs, Zanata will make a HTTP POST to the provided payload URL in the project.
+Types of events available:
 
-Example webhook response:
-
+#### Translation Milestone
+Trigger when document has reached 100% Translated or Approved
+```
+{
+  "type": "org.zanata.events.DocumentMilestoneEvent",
+  "milestone": "100% Translated",
+  "locale": "de",
+  "docId": "zanata-war/src/main/resources/messages",
+  "version": "master",
+  "project": "zanata-server",
+  "editorDocumentUrl": "https://translate.zanata.org/zanata/webtrans/Application.xhtml?project=zanata-server&iteration=master&localeId=de&locale=en#view:doc;doc:zanata-war/src/main/resources/messages"
+}
+```
+
+#### Translation update
+Trigger when translation is updated
 ```
 {
-    "eventType": "org.zanata.events.DocumentMilestoneEvent",
-    "milestone": "100% Translated",
-    "locale": "de",
-    "docId": "zanata-war/src/main/resources/messages",
-    "version": "master",
-    "project": "zanata-server",
-    "editorDocumentUrl": "https://translate.zanata.org/zanata/webtrans/Application.xhtml?project=zanata-server&iteration=master&localeId=de&locale=en#view:doc;doc:zanata-war/src/main/resources/messages"
+    "username":"aeng",
+    "project":"Zanata",
+    "version":"master",
+    "docId":"doc1id",
+    "locale":"zh-CN",
+    "wordDeltasByState":{"New":-16,"Translated":16},
+    "type":"DocumentStatsEvent"
 }
 ```
 
-If a secret key is provided for that payload URL, Zanata will sign the webhook request with HTTP header `X-Zanata-Webhook`.
+#### Project version
+Trigger when a version is added to removed from a project
+```
+{"project":"zanata","version":"new-version","changeType":"CREATE","type":"VersionChangedEvent"}
+```
+
+#### Project maintainer
+Trigger when a project maintainer is added or removed from project
+```
+{"project":"zanata","username":"aeng","changeType":"REMOVE","role":"Maintainer","type":"ProjectMaintainerChangedEvent"}
+```
+
+#### Document
+Trigger when a source document is added or removed from project version
+```
+{"project":"zanata","version":"new-version"","changeType":"ADD","type":"SourceDocumentChangedEvent"}
+```
+
+If a secret key is provided for that payload URL, Zanata will sign the webhook request with HTTP header `X-Zanata-Webhook`. 
 The header is a double hash of `HMAC-SHA1` in base64 digest. The double hash is generated from the full request body and the payload URL as provided.
 
 Here is some sample pseudocode for checking the validity of a request:
@@ -181,15 +209,24 @@ boolean verifyRequest(request, secret, callbackURL) {
 }
 ```
 
+### Adding a new webhook
+<figure>
+![Project Webhooks New](/images/project-webhooks-new.png)
+</figure>
 
+1. Click on 'New webhook' button.
+2. Enter a valid URL and secret key (optional) into the provided text input. 
+3. Select webhook types for this URL.
+4. Click on 'Add webhook' button to add the URL.
 
-### Adding a webhook
-1. Enter a valid URL and secret key (optional) into the provided text input.
-2. Click on 'Add webhook' button to add the URL.
-3. If secret key is provided, Zanata will include cryptographic signature in HTTP header `X-Zanata-Webhook`.
-
-### Remove a webhook
-- Click on the `X` sign on right side of the webhook to remove the entry.
+### Update a webhook
+<figure>
+![Project Webhooks Edit](/images/project-webhooks-edit.png)
+</figure>
+1. Click on 'Edit' button on the right of the webhook entry
+2. Update any value in the form
+3. Click on 'Update' to save the changes
+4. To remove webhook, click on button 'Delete'
 
 ------------
 

functional-test/src/main/java/org/zanata/page/projects/projectsettings/ProjectWebHooksTab.java

@@ -24,6 +24,7 @@
 import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 import org.openqa.selenium.By;
+import org.openqa.selenium.JavascriptExecutor;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
 import org.zanata.page.projects.ProjectBasePage;
@@ -42,18 +43,33 @@
 public class ProjectWebHooksTab extends ProjectBasePage {
 
     private By webHooksForm = By.id("settings-webhooks-form");
-    private By saveWebhookButton = By.id("add-webhook-button");
-    private By urlInputField = By.id("payloadUrlInput");
-    private By secretInputField = By.id("secretInput");
+    private By newWebHooksForm = By.id("newWebhook");
+    private By deleteBtn = By.name("deleteWebhookBtn");
+    private By editBtn = By.name("editBtn");
+
+    private final JavascriptExecutor jsExecutor =
+        (JavascriptExecutor) getDriver();
 
     public ProjectWebHooksTab(WebDriver driver) {
         super(driver);
     }
 
-    public ProjectWebHooksTab enterUrl(String url, String key) {
-        enterText(readyElement(urlInputField), url);
-        enterText(readyElement(secretInputField), key);
-        readyElement(saveWebhookButton).click();
+    public WebElement getParentElement(WebElement child) {
+        return (WebElement) jsExecutor
+            .executeScript("return arguments[0].parentNode;", child);
+    }
+
+    public ProjectWebHooksTab enterUrl(String url, String key, List<String> types) {
+        enterText(getUrlInputField(newWebHooksForm), url);
+        enterText(getSecretInputField(newWebHooksForm), key);
+
+        for (String type : types) {
+            WebElement checkbox = readyElement(newWebHooksForm)
+                .findElement(By.cssSelector("input[value=" + type + "]"));
+            getParentElement(checkbox).click();
+        }
+
+        getSaveWebhookButton(newWebHooksForm).click();
         return new ProjectWebHooksTab(getDriver());
     }
 
@@ -65,7 +81,14 @@ public List<WebhookItem> getWebHooks() {
 
         return list.stream().map(element -> new WebhookItem(
             element.findElement(By.name("url")).getText(),
-            element.findElement(By.name("type")).getText()))
+            getSelectedTypes(element)))
+            .collect(Collectors.toList());
+    }
+
+    private List<String> getSelectedTypes(WebElement parentForm) {
+        return parentForm.findElement(By.name("types"))
+            .findElements(By.cssSelector("input[checked=checked]")).stream()
+            .map(input -> input.getAttribute("value"))
             .collect(Collectors.toList());
     }
 
@@ -85,7 +108,12 @@ public ProjectWebHooksTab clickRemoveOn(String url) {
         boolean clicked = false;
         for (WebElement listItem : listItems) {
             if (listItem.getText().contains(url)) {
-                listItem.findElement(By.tagName("button")).click();
+                listItem.findElement(editBtn).click();
+                WebElement deleteButton = listItem.findElement(deleteBtn);
+                if (!deleteButton.isDisplayed()) {
+                    listItem.findElement(editBtn).click();
+                }
+                deleteButton.click();
                 clicked = true;
                 break;
             }
@@ -109,6 +137,18 @@ private List<WebElement> getWebhookList() {
     @AllArgsConstructor
     public class WebhookItem {
         private String url;
-        private String type;
+        private List<String> types;
+    }
+
+    private WebElement getSaveWebhookButton(By parentId) {
+        return readyElement(parentId).findElement(By.name("addWebhookBtn"));
+    }
+
+    private WebElement getUrlInputField(By parentId) {
+        return readyElement(parentId).findElement(By.name("payloadUrlInput"));
+    }
+
+    private WebElement getSecretInputField(By parentId) {
+        return readyElement(parentId).findElement(By.name("secretInput"));
     }
 }

functional-test/src/test/java/org/zanata/feature/project/EditWebHooksTest.java

@@ -20,6 +20,7 @@
  */
 package org.zanata.feature.project;
 
+import com.google.common.collect.Lists;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -31,6 +32,8 @@
 import org.zanata.workflow.LoginWorkFlow;
 import org.zanata.workflow.ProjectWorkFlow;
 
+import java.util.List;
+
 import static org.assertj.core.api.Assertions.assertThat;
 /**
  * @author Damian Jansen <a href="mailto:djansen@redhat.com">djansen@redhat.com</a>
@@ -52,11 +55,12 @@ public void before() {
     public void addWebHook() throws Exception {
         String testUrl = "http://www.example.com";
         String key = "secret_key";
+        List<String> types = Lists.newArrayList("DocumentMilestoneEvent");
         ProjectWebHooksTab projectWebHooksTab = new ProjectWorkFlow()
                 .goToProjectByName("about fedora")
                 .gotoSettingsTab()
                 .gotoSettingsWebHooksTab()
-                .enterUrl(testUrl, key);
+                .enterUrl(testUrl, key, types);
 
         assertThat(projectWebHooksTab.getWebHooks())
                 .extracting("url")
@@ -70,11 +74,12 @@ public void addWebHook() throws Exception {
     public void removeWebHook() throws Exception {
         String testUrl = "http://www.example.com";
         String key = "secret_key";
+        List<String> types = Lists.newArrayList("DocumentMilestoneEvent");
         ProjectWebHooksTab projectWebHooksTab = new ProjectWorkFlow()
                 .goToProjectByName("about fedora")
                 .gotoSettingsTab()
                 .gotoSettingsWebHooksTab()
-                .enterUrl(testUrl, key)
+                .enterUrl(testUrl, key, types)
                 .expectWebHooksContains(testUrl)
                 .clickRemoveOn(testUrl);
 

zanata-model/src/main/java/org/zanata/model/HProject.java

@@ -153,7 +153,8 @@ public class HProject extends SlugEntityBase implements Serializable,
     @Column(name = "alias", nullable = false)
     private Map<LocaleId, String> localeAliases = Maps.newHashMap();
 
-    @OneToMany(mappedBy = "project", cascade = CascadeType.ALL)
+    @OneToMany(cascade = CascadeType.ALL, mappedBy = "project",
+        orphanRemoval = true)
     private List<WebHook> webHooks = Lists.newArrayList();
 
     @Enumerated(EnumType.STRING)
@@ -221,7 +222,7 @@ public void addIteration(HProjectIteration iteration) {
      * @see {@link #getMaintainers}
      */
     public void addMaintainer(HPerson maintainer) {
-        getMembers().add(asMember(maintainer, Maintainer));
+        getMembers().add(new HProjectMember(this, maintainer, Maintainer));
     }
 
     /**
@@ -236,87 +237,8 @@ public void removeMaintainer(HPerson maintainer) {
         // there is only one maintainer then removal of any other person would
         // do nothing anyway.
         if (getMaintainers().size() > 1) {
-            getMembers().remove(asMember(maintainer, Maintainer));
-        }
-    }
-
-    /**
-     * Update all security settings for a person.
-     *
-     * The HPerson and HLocale entities in memberships must be attached to avoid
-     * persistence problems with Hibernate.
-     */
-    public void updateProjectPermissions(PersonProjectMemberships memberships) {
-        HPerson person = memberships.getPerson();
-
-        boolean wasMaintainer = getMaintainers().contains(memberships.getPerson());
-        boolean isLastMaintainer = wasMaintainer && getMaintainers().size() <= 1;
-        // business rule: every project must have at least one maintainer
-        boolean isMaintainer = isLastMaintainer || memberships.isMaintainer();
-
-        ensureMembership(isMaintainer, asMember(person, Maintainer));
-
-        // business rule: if someone is a Maintainer, they must also be a TranslationMaintainer
-        boolean isTranslationMaintainer = memberships.isMaintainer() ||
-                memberships.isTranslationMaintainer();
-
-        ensureMembership(isTranslationMaintainer, asMember(person, TranslationMaintainer));
-    }
-
-    public void updateLocalePermissions(PersonProjectMemberships memberships) {
-        HPerson person = memberships.getPerson();
-
-        for (PersonProjectMemberships.LocaleRoles localeRoles
-                : memberships.getLocaleRoles()) {
-            HLocale locale = localeRoles.getLocale();
-            ensureMembership(localeRoles.isTranslator(), asMember(locale, person, Translator));
-            ensureMembership(localeRoles.isReviewer(), asMember(locale, person, Reviewer));
-            ensureMembership(localeRoles.isCoordinator(), asMember(locale, person, Coordinator));
-            ensureMembership(localeRoles.isGlossarist(), asMember(locale, person, Glossarist));
-        }
-    }
-
-    /**
-     * Get a person as a member object in this project for a role.
-     */
-    private HProjectMember asMember(HPerson person, ProjectRole role) {
-        return new HProjectMember(this, person, role);
-    }
-
-    /**
-     * Get a person as a member object in this project for a locale-specific role.
-     */
-    private HProjectLocaleMember asMember(HLocale locale, HPerson person, LocaleRole role) {
-        return new HProjectLocaleMember(this, locale, person, role);
-    }
-
-    /**
-     * Ensure the given membership is present or absent.
-     */
-    private void ensureMembership(boolean shouldBePresent, HProjectMember membership) {
-        final Set<HProjectMember> members = getMembers();
-        final boolean isPresent = members.contains(membership);
-        if (isPresent != shouldBePresent) {
-            if (shouldBePresent) {
-                members.add(membership);
-            } else {
-                members.remove(membership);
-            }
-        }
-    }
-
-    /**
-     * Ensure the given locale membership is present or absent.
-     */
-    private void ensureMembership(boolean shouldBePresent, HProjectLocaleMember membership) {
-        final Set<HProjectLocaleMember> members = getLocaleMembers();
-        final boolean isPresent = members.contains(membership);
-        if (isPresent != shouldBePresent) {
-            if (shouldBePresent) {
-                members.add(membership);
-            } else {
-                members.remove(membership);
-            }
+            getMembers()
+                .remove(new HProjectMember(this, maintainer, Maintainer));
         }
     }