Merge "Improve assertions in test.rb"
diff --git a/README.md b/README.md
index 34b2d05..372bba1 100644
--- a/README.md
+++ b/README.md
@@ -12,3 +12,14 @@
 when all approvals for all reviews are present, and all reviews are mergeable.
 
 Requires Gerrit 2.14 or later.
+
+Configuration
+-------------
+
+```
+[automerge]
+        botEmail=admin@example.com
+[commentlink "change"]
+        match = "#/c/(\\d+)"
+        html = "<a href=\"/#/c/$1/\">$1</a>"
+```
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java b/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
index e9e548d..98d67b3 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AtomicityHelper.java
@@ -1,77 +1,66 @@
 package com.criteo.gerrit.plugins.automerge;
 
+import static com.google.gerrit.server.permissions.ChangePermission.READ;
+
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
-import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountByEmailCache;
+import com.google.gerrit.server.account.Emails;
 import com.google.gerrit.server.change.ChangesCollection;
 import com.google.gerrit.server.change.GetRelated;
+import com.google.gerrit.server.change.GetRelated.ChangeAndCommit;
 import com.google.gerrit.server.change.GetRelated.RelatedInfo;
-import com.google.gerrit.server.change.PostReview;
 import com.google.gerrit.server.change.RevisionResource;
 import com.google.gerrit.server.change.Submit;
-import com.google.gerrit.server.data.ChangeAttribute;
-import com.google.gerrit.server.git.MergeUtil;
-import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import java.io.IOException;
 import java.util.List;
 import java.util.Set;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class AtomicityHelper {
 
-  private final static Logger log = LoggerFactory.getLogger(AtomicityHelper.class);
+  private static final Logger log = LoggerFactory.getLogger(AtomicityHelper.class);
 
-  @Inject
-  private AccountByEmailCache byEmailCache;
+  @Inject ChangeData.Factory changeDataFactory;
 
-  @Inject
-  ChangeData.Factory changeDataFactory;
+  @Inject private ChangesCollection collection;
 
-  @Inject
-  private ChangeControl.GenericFactory changeFactory;
+  @Inject AutomergeConfig config;
 
-  @Inject
-  private ChangesCollection collection;
+  @Inject Provider<ReviewDb> db;
 
-  @Inject
-  AutomergeConfig config;
+  @Inject private IdentifiedUser.GenericFactory factory;
 
-  @Inject
-  Provider<ReviewDb> db;
+  @Inject GetRelated getRelated;
 
-  @Inject
-  private IdentifiedUser.GenericFactory factory;
+  @Inject Submit submitter;
 
-  @Inject
-  GetRelated getRelated;
+  @Inject Emails emails;
 
-  @Inject
-  MergeUtil.Factory mergeUtilFactory;
+  @Inject ChangeNotes.Factory changeNotesFactory;
 
-  @Inject
-  Provider<PostReview> reviewer;
+  @Inject PermissionBackend permissionBackend;
 
-  @Inject
-  Submit submitter;
+  @Inject SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
 
   /**
-   * Check if the current patchset of the specified change has dependent
-   * unmerged changes.
+   * Check if the current patchset of the specified change has dependent unmerged changes.
    *
    * @param project
    * @param number
@@ -80,23 +69,52 @@
    * @throws NoSuchChangeException
    * @throws OrmException
    */
-  public boolean hasDependentReview(String project, int number) throws IOException, NoSuchChangeException, OrmException {
-      RevisionResource r = getRevisionResource(project, number);
+  public boolean hasDependentReview(String project, int number)
+      throws IOException, NoSuchChangeException, OrmException {
+    RevisionResource r = getRevisionResource(project, number);
     RelatedInfo related = getRelated.apply(r);
     log.debug(String.format("Checking for related changes on review %d", number));
-    return related.changes.size() > 0;
+
+    String checkedCommitSha1 = r.getPatchSet().getRevision().get();
+    int firstParentIndex = 0;
+    int i = 0;
+    for (ChangeAndCommit c : related.changes) {
+      if (checkedCommitSha1.equals(c.commit.commit)) {
+        firstParentIndex = i + 1;
+        log.debug(
+            String.format(
+                "First parent index on review %d is %d on commit %s",
+                number, firstParentIndex, c.commit.commit));
+        break;
+      }
+      i++;
+    }
+
+    boolean hasNonMergedParent = false;
+    for (ChangeAndCommit c : related.changes.subList(firstParentIndex, related.changes.size())) {
+      if (!ChangeStatus.MERGED.toString().equals(c.status)) {
+        log.info(
+            String.format(
+                "Found non merged parent commit on review %d: %s", number, c.commit.commit));
+        hasNonMergedParent = true;
+        break;
+      }
+    }
+
+    return hasNonMergedParent;
   }
 
   /**
-   * Check if a change is an atomic change or not. A change is atomic if it has
-   * the atomic topic prefix.
+   * Check if a change is an atomic change or not. A change is atomic if it has the atomic topic
+   * prefix.
    *
-   * @param change a ChangeAttribute instance
+   * @param change a Change instance
    * @return true or false
    */
-  public boolean isAtomicReview(final ChangeAttribute change) {
+  public boolean isAtomicReview(final Change change) {
     final boolean atomic = change.topic != null && change.topic.startsWith(config.getTopicPrefix());
-    log.debug(String.format("Checking if change %s is an atomic change: %b", change.number, atomic));
+    log.debug(
+        String.format("Checking if change %s is an atomic change: %b", change.number, atomic));
     return atomic;
   }
 
@@ -109,9 +127,17 @@
    * @throws OrmException
    */
   public boolean isSubmittable(String project, int change) throws OrmException {
-    ChangeData changeData = changeDataFactory.create(db.get(), new Project.NameKey(project), new Change.Id(change));
+    ChangeData changeData =
+        changeDataFactory.create(
+            db.get(),
+            new Project.NameKey(project),
+            new com.google.gerrit.reviewdb.client.Change.Id(change));
     // For draft reviews, the patchSet must be set to avoid an NPE.
-    final List<SubmitRecord> cansubmit = new SubmitRuleEvaluator(changeData).setPatchSet(changeData.currentPatchSet()).evaluate();
+    final List<SubmitRecord> cansubmit =
+        submitRuleEvaluatorFactory
+            .create(changeData)
+            .setPatchSet(changeData.currentPatchSet())
+            .evaluate();
     log.debug(String.format("Checking if change %d is submitable.", change));
     for (SubmitRecord submit : cansubmit) {
       if (submit.status != SubmitRecord.Status.OK) {
@@ -123,29 +149,37 @@
     return true;
   }
 
-  /**
-   * Merge a review.
-   *
-   * @param info
-   */
-  public void mergeReview(ChangeInfo info) throws Exception {
-    submitter.apply(getRevisionResource(info.project, info._number),
-        new SubmitInput());
+  /** Merge a review. */
+  public void mergeReview(String project, int number) throws Exception {
+    submitter.apply(getRevisionResource(project, number), new SubmitInput());
   }
 
-  public RevisionResource getRevisionResource(String project, int changeNumber) throws NoSuchChangeException, OrmException {
-    ChangeControl ctl = changeFactory.validateFor(db.get(), new Change.Id(changeNumber), getBotUser());
-    ChangeData changeData = changeDataFactory.create(db.get(), new Project.NameKey(project), new Change.Id(changeNumber));
-    RevisionResource r = new RevisionResource(collection.parse(ctl), changeData.currentPatchSet());
-    return r;
+  public RevisionResource getRevisionResource(String project, int changeNumber)
+      throws OrmException {
+    com.google.gerrit.reviewdb.client.Change.Id changeId =
+        new com.google.gerrit.reviewdb.client.Change.Id(changeNumber);
+    ChangeNotes notes = changeNotesFactory.createChecked(changeId);
+    try {
+      permissionBackend.user(getBotUser()).change(notes).database(db).check(READ);
+      ChangeData changeData =
+          changeDataFactory.create(db.get(), new Project.NameKey(project), changeId);
+      RevisionResource r =
+          new RevisionResource(collection.parse(changeId), changeData.currentPatchSet());
+      return r;
+    } catch (ResourceNotFoundException | AuthException | PermissionBackendException e) {
+      throw new NoSuchChangeException(changeId);
+    }
   }
 
   private IdentifiedUser getBotUser() {
-    final Set<Account.Id> ids = byEmailCache.get(config.getBotEmail());
-    if (ids.isEmpty()) {
-      throw new RuntimeException("No user found with email: " + config.getBotEmail());
+    try {
+      Set<Account.Id> ids = emails.getAccountFor(config.getBotEmail());
+      if (ids.isEmpty()) {
+        throw new RuntimeException("No user found with email: " + config.getBotEmail());
+      }
+      return factory.create(ids.iterator().next());
+    } catch (IOException | OrmException e) {
+      throw new RuntimeException("Unable to get account with email: " + config.getBotEmail(), e);
     }
-    final IdentifiedUser bot = factory.create(ids.iterator().next());
-    return bot;
   }
 }
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
index e83f47b..864de7d 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomaticMerger.java
@@ -18,8 +18,8 @@
 import com.google.gerrit.common.EventListener;
 import com.google.gerrit.extensions.api.GerritApi;
 import com.google.gerrit.extensions.api.changes.ChangeApi;
-import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.events.LifecycleListener;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -27,88 +27,72 @@
 import com.google.gerrit.server.change.PostReview;
 import com.google.gerrit.server.change.Submit;
 import com.google.gerrit.server.data.AccountAttribute;
-import com.google.gerrit.server.data.ApprovalAttribute;
 import com.google.gerrit.server.data.ChangeAttribute;
+import com.google.gerrit.server.events.ChangeEvent;
 import com.google.gerrit.server.events.CommentAddedEvent;
+import com.google.gerrit.server.events.DraftPublishedEvent;
 import com.google.gerrit.server.events.Event;
 import com.google.gerrit.server.events.PatchSetCreatedEvent;
+import com.google.gerrit.server.events.RefUpdatedEvent;
+import com.google.gerrit.server.events.ReviewerDeletedEvent;
 import com.google.gerrit.server.events.TopicChangedEvent;
 import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
-
+import java.util.EnumSet;
+import java.util.List;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.EnumSet;
-import java.util.List;
-
-/**
- * Starts at the same time as the gerrit server, and sets up our change hook
- * listener.
- */
+/** Starts at the same time as the gerrit server, and sets up our change hook listener. */
 public class AutomaticMerger implements EventListener, LifecycleListener {
 
-  private final static Logger log = LoggerFactory.getLogger(AutomaticMerger.class);
+  private static final Logger log = LoggerFactory.getLogger(AutomaticMerger.class);
 
-  @Inject
-  private GerritApi api;
+  @Inject private GerritApi api;
 
-  @Inject
-  private AtomicityHelper atomicityHelper;
+  @Inject private AtomicityHelper atomicityHelper;
 
-  @Inject
-  ChangeData.Factory changeDataFactory;
+  @Inject ChangeData.Factory changeDataFactory;
 
-  @Inject
-  private AutomergeConfig config;
+  @Inject private AutomergeConfig config;
 
-  @Inject
-  Provider<ReviewDb> db;
+  @Inject Provider<ReviewDb> db;
 
-  @Inject
-  GetRelated getRelated;
+  @Inject GetRelated getRelated;
 
-  @Inject
-  MergeUtil.Factory mergeUtilFactory;
+  @Inject MergeUtil.Factory mergeUtilFactory;
 
-  @Inject
-  Provider<PostReview> reviewer;
+  @Inject Provider<PostReview> reviewer;
 
-  @Inject
-  private ReviewUpdater reviewUpdater;
+  @Inject private ReviewUpdater reviewUpdater;
 
-  @Inject
-  Submit submitter;
+  @Inject Submit submitter;
 
   @Override
-  synchronized public void onEvent(final Event event) {
-    if (event instanceof TopicChangedEvent) {
-      onTopicChanged((TopicChangedEvent)event);
+  public synchronized void onEvent(final Event event) {
+    if (event instanceof TopicChangedEvent
+        || event instanceof DraftPublishedEvent
+        || event instanceof ReviewerDeletedEvent
+        || // A blocking score might be removed when a reviewer is deleted.
+        event instanceof PatchSetCreatedEvent) {
+      Change change = Change.from(((ChangeEvent) event).change.get());
+      onNewOrChangedPatchSet(change);
+    } else if (event instanceof CommentAddedEvent) {
+      onCommentAdded((CommentAddedEvent) event);
     }
-    else if (event instanceof PatchSetCreatedEvent) {
-      onPatchSetCreated((PatchSetCreatedEvent)event);
-    }
-    else if (event instanceof CommentAddedEvent) {
-      onCommentAdded((CommentAddedEvent)event);
+    // it is not an else since the previous automatic submit(s) can potentially
+    // trigger others on the whole project/branch
+    if (event instanceof RefUpdatedEvent) {
+      onRefUpdatedEvent((RefUpdatedEvent) event);
     }
   }
 
-  private void onTopicChanged(final TopicChangedEvent event) {
-    ChangeAttribute change = event.change.get();
-    if (!atomicityHelper.isAtomicReview(change)) {
-      return;
-    }
-    processNewAtomicPatchSet(change);
-  }
-
-  private void onPatchSetCreated(final PatchSetCreatedEvent event) {
-    ChangeAttribute change = event.change.get();
+  private void onNewOrChangedPatchSet(Change change) {
     if (atomicityHelper.isAtomicReview(change)) {
       processNewAtomicPatchSet(change);
     }
-
     try {
       autoSubmitIfMergeable(change);
     } catch (Exception e) {
@@ -124,23 +108,63 @@
     ChangeAttribute change = newComment.change.get();
     try {
       checkReviewExists(change.number);
-      autoSubmitIfMergeable(change);
+      autoSubmitIfMergeable(Change.from(change));
     } catch (Exception e) {
       log.error("An exception occured while trying to atomic merge a change.", e);
       throw new RuntimeException(e);
     }
   }
 
-  private void autoSubmitIfMergeable(ChangeAttribute change) throws Exception {
+  private void onRefUpdatedEvent(final RefUpdatedEvent event) {
+    String refName = event.getRefName();
+    String projectName = event.getProjectNameKey().get();
+    try {
+      api.changes()
+          .query("branch:" + refName + " project:" + projectName + " is:submittable")
+          .get()
+          .forEach(
+              submittable -> {
+                try {
+                  log.info(
+                      "Found another submittable change #"
+                          + submittable._number
+                          + " on project "
+                          + projectName
+                          + " during update of ref "
+                          + refName
+                          + ": Submitting ...");
+                  autoSubmitIfMergeable(Change.from(submittable));
+                } catch (Exception e) {
+                  log.error(
+                      "Cannot autosubmit change "
+                          + submittable._number
+                          + " on project "
+                          + projectName
+                          + " to ref "
+                          + refName,
+                      e);
+                }
+              });
+    } catch (RestApiException e) {
+      log.error(
+          "Cannot query submittable changes on project " + projectName + " for ref " + refName);
+    }
+  }
+
+  private void autoSubmitIfMergeable(Change change) throws Exception {
     if (atomicityHelper.isSubmittable(change.project, change.number)) {
-      log.info(String.format("Change %d is submittable. Will try to merge all related changes.", change.number));
-      attemptToMerge(change);
+      if (atomicityHelper.isAtomicReview(change)) {
+        attemptToMergeAtomic(change);
+      } else {
+        log.info("Submitting non-atomic change {}...", change.number);
+        atomicityHelper.mergeReview(change.project, change.number);
+      }
     }
   }
 
   /**
-   * Returns true if the plugin must handle this comment, i.e. if we are sure it does not come
-   * from this plugin (to avoid infinite loop).
+   * Returns true if the plugin must handle this comment, i.e. if we are sure it does not come from
+   * this plugin (to avoid infinite loop).
    *
    * @param comment
    * @return a boolean
@@ -150,60 +174,71 @@
     if (!config.getBotEmail().equals(account.email)) {
       return true;
     }
-    ApprovalAttribute[] approvals = comment.approvals.get();
-    if (approvals != null) {
-      for (ApprovalAttribute approval : approvals) {
-        // See ReviewUpdate#setMinusOne
-        if (!("Code-Review".equals(approval.type) && "-1".equals(approval.value))) {
-          return true;
-        }
-      }
+    if (!comment.comment.contains(ReviewUpdater.commentsPrefix)) {
+      return true;
     }
     return false;
   }
 
-  private void attemptToMerge(ChangeAttribute change) throws Exception {
+  private void attemptToMergeAtomic(Change change) throws Exception {
     final List<ChangeInfo> related = Lists.newArrayList();
     if (atomicityHelper.isAtomicReview(change)) {
-      related.addAll(api.changes().query("status: open AND topic: " + change.topic)
-          .withOption(ListChangesOption.CURRENT_REVISION).get());
+      related.addAll(
+          api.changes()
+              .query("status: open AND topic: " + change.topic)
+              .withOption(ListChangesOption.CURRENT_REVISION)
+              .get());
     } else {
       ChangeApi changeApi = api.changes().id(change.project, change.branch, change.id);
       related.add(changeApi.get(EnumSet.of(ListChangesOption.CURRENT_REVISION)));
     }
-    boolean submittable = true;
-    boolean mergeable = true;
+
     for (final ChangeInfo info : related) {
-      if (!info.mergeable) {
-        mergeable = false;
-      }
       if (!atomicityHelper.isSubmittable(info.project, info._number)) {
-        submittable = false;
+        log.info(
+            "Change {} is not submittable because same topic change {} has not all approvals.",
+            change.number,
+            info._number);
+        return;
       }
     }
 
-    if (submittable) {
-      if (mergeable) {
-        log.debug(String.format("Change %d is mergeable", change.number));
-        for (final ChangeInfo info : related) {
-          atomicityHelper.mergeReview(info);
-        }
-      } else {
-	  reviewUpdater.commentOnReview(change.project, change.number, AutomergeConfig.CANT_MERGE_COMMENT_FILE);
+    for (final ChangeInfo info : related) {
+      boolean dependsOnNonMergedCommit =
+          atomicityHelper.hasDependentReview(info.project, info._number);
+      if (!info.mergeable || dependsOnNonMergedCommit) {
+        log.info(
+            "Change {} is not mergeable because same topic change {} {}",
+            change.number,
+            info._number,
+            !info.mergeable ? "is non mergeable" : "depends on a non merged commit.");
+        PluginComment comment =
+            !info.mergeable ? config.cantMergeGitConflict : config.cantMergeDependsOnNonMerged;
+        reviewUpdater.commentOnReview(
+            change.project, change.number, String.format(comment.getContent(), info._number));
+        return;
       }
     }
+
+    log.info("Submitting atomic change {}...", change.number);
+    for (final ChangeInfo info : related) {
+      atomicityHelper.mergeReview(info.project, info._number);
+    }
   }
 
-  private void processNewAtomicPatchSet(ChangeAttribute change) {
+  private void processNewAtomicPatchSet(Change change) {
     try {
       checkReviewExists(change.number);
+      log.info(String.format("Detected atomic review on change %d.", change.number));
+      reviewUpdater.commentOnReview(
+          change.project, change.number, config.atomicReviewDetected.getContent());
       if (atomicityHelper.hasDependentReview(change.project, change.number)) {
-        log.info(String.format("Warn the user by setting -1 on change %d, as other atomic changes exists on the same repository.",
-            change.number));
-        reviewUpdater.setMinusOne(change.project, change.number, AutomergeConfig.ATOMIC_REVIEWS_SAME_REPO_FILE);
-      } else {
-        log.info(String.format("Detected atomic review on change %d.", change.number));
-        reviewUpdater.commentOnReview(change.project, change.number, AutomergeConfig.ATOMIC_REVIEW_DETECTED_FILE);
+        log.info(
+            String.format(
+                "Warn the user on change %d, as other atomic changes exists on the same repository.",
+                change.number));
+        reviewUpdater.commentOnReview(
+            change.project, change.number, config.atomicReviewsSameRepo.getContent());
       }
     } catch (Exception e) {
       throw new RuntimeException(e);
@@ -220,6 +255,5 @@
   }
 
   @Override
-  public void stop() {
-  }
+  public void stop() {}
 }
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
index bd1bbca..8eb1010 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeConfig.java
@@ -3,22 +3,21 @@
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.inject.Inject;
-
+import java.io.File;
 import org.eclipse.jgit.lib.Config;
 
-import java.io.File;
-
 public class AutomergeConfig {
+  public static final String AUTOMERGE_SECTION = "automerge";
+  public static final String BOT_EMAIL_KEY = "botEmail";
 
-  public final static String ATOMIC_REVIEW_DETECTED_FILE = "atomic_review_detected.txt";
-  public final static String ATOMIC_REVIEWS_SAME_REPO_FILE = "atomic_review_same_repo.txt";
-  public final static String AUTOMERGE_SECTION = "automerge";
-  public final static String BOT_EMAIL_KEY = "botEmail";
-  public final static String CANT_MERGE_COMMENT_FILE = "cantmerge.txt";
+  public final PluginComment atomicReviewDetected;
+  public final PluginComment atomicReviewsSameRepo;
+  public final PluginComment cantMergeGitConflict;
+  public final PluginComment cantMergeDependsOnNonMerged;
 
-  private final static String defaultBotEmail = "qabot@criteo.com";
-  private final static String defaultTopicPrefix = "crossrepo/";
-  public final static String TOPIC_PREFIX_KEY = "topicPrefix";
+  private static final String defaultBotEmail = "qabot@criteo.com";
+  private static final String defaultTopicPrefix = "crossrepo/";
+  public static final String TOPIC_PREFIX_KEY = "topicPrefix";
 
   public static final String getDefaultBotEmail() {
     return defaultBotEmail;
@@ -44,19 +43,32 @@
     }
 
     templatesPath = paths.etc_dir.toFile();
+
+    atomicReviewDetected =
+        new PluginComment(
+            getCommentPath("atomic_review_detected.txt"),
+            "This review is part of a cross-repository change.\n"
+                + "It will be submitted once all related reviews are submittable.");
+    atomicReviewsSameRepo =
+        new PluginComment(
+            getCommentPath("atomic_review_same_repo.txt"),
+            "This cross-repo review depends on a not merged commit that must be merged first.");
+    cantMergeGitConflict =
+        new PluginComment(
+            getCommentPath("cantmerge_git_conflict.txt"),
+            "This cross-repo review is blocked by a git conflict on change #/c/%d.");
+    cantMergeDependsOnNonMerged =
+        new PluginComment(
+            getCommentPath("cantmerge_depends_on_non_merged.txt"),
+            "This cross-repo review is blocked by a non merged commit below #/c/%d.");
   }
 
   public final String getBotEmail() {
     return botEmail;
   }
 
-  /**
-   * Return a comment from a file located in the gerrit etc_dir
-   *
-   * @return a string containing a comment.
-   */
-  public final String getTemplatesPath() {
-    return templatesPath.getPath();
+  public final File getCommentPath(String fileName) {
+    return new File(templatesPath.getPath(), fileName);
   }
 
   public final String getTopicPrefix() {
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeModule.java b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeModule.java
index 744bb2d..dda0aa5 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeModule.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/AutomergeModule.java
@@ -21,7 +21,7 @@
 /**
  * Main automerge Guice module.
  *
- * Configures how all classes in the plugin are instantiated via Guice.
+ * <p>Configures how all classes in the plugin are instantiated via Guice.
  */
 public class AutomergeModule extends AbstractModule {
 
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/Change.java b/src/main/java/com/criteo/gerrit/plugins/automerge/Change.java
new file mode 100644
index 0000000..8728bed
--- /dev/null
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/Change.java
@@ -0,0 +1,40 @@
+package com.criteo.gerrit.plugins.automerge;
+
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.server.data.ChangeAttribute;
+
+/**
+ * A change represented only with the fields that this plugin requires.
+ *
+ * <p>As a change can be a {@link ChangeAttribute} or a {@link ChangeInfo}, this intermediate class
+ * is needed.
+ */
+public class Change {
+  public final String project;
+  public final int number;
+  public final String id;
+  public final String topic;
+  public final String branch;
+
+  private Change(String project, int number, String id, String topic, String branch) {
+    this.project = project;
+    this.number = number;
+    this.id = id;
+    this.topic = topic;
+    this.branch = branch;
+  }
+
+  public static Change from(ChangeAttribute changeAttribute) {
+    return new Change(
+        changeAttribute.project,
+        changeAttribute.number,
+        changeAttribute.id,
+        changeAttribute.topic,
+        changeAttribute.branch);
+  }
+
+  public static Change from(ChangeInfo changeInfo) {
+    return new Change(
+        changeInfo.project, changeInfo._number, changeInfo.id, changeInfo.topic, changeInfo.branch);
+  }
+}
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/PluginComment.java b/src/main/java/com/criteo/gerrit/plugins/automerge/PluginComment.java
new file mode 100644
index 0000000..4cacdd6
--- /dev/null
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/PluginComment.java
@@ -0,0 +1,38 @@
+package com.criteo.gerrit.plugins.automerge;
+
+import com.google.common.base.Charsets;
+import com.google.common.io.Files;
+import java.io.File;
+import java.io.IOException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** A comment pushed by the plugin to a Gerrit patchset. */
+public class PluginComment {
+
+  private static final Logger log = LoggerFactory.getLogger(PluginComment.class);
+
+  private final File templatePath;
+  private final String defaultMessage;
+
+  PluginComment(File templatePath, String defaultMessage) {
+    this.templatePath = templatePath;
+    this.defaultMessage = defaultMessage;
+  }
+
+  /**
+   * Returns the comment message, possibly with interpolation placeholders.
+   *
+   * @return a string
+   */
+  String getContent() {
+    if (templatePath.exists()) {
+      try {
+        return Files.toString(templatePath, Charsets.UTF_8);
+      } catch (final IOException exc) {
+        log.error("Not able to read " + templatePath, exc);
+      }
+    }
+    return defaultMessage;
+  }
+}
diff --git a/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java b/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
index 9c1285f..690ebc0 100644
--- a/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
+++ b/src/main/java/com/criteo/gerrit/plugins/automerge/ReviewUpdater.java
@@ -1,64 +1,32 @@
 package com.criteo.gerrit.plugins.automerge;
 
-import com.google.common.base.Charsets;
-import com.google.common.io.Files;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.change.PostReview;
 import com.google.gerrit.server.change.RevisionResource;
-import com.google.gerrit.server.query.change.ChangeData;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.io.File;
-import java.io.IOException;
-
 public class ReviewUpdater {
-  private final static Logger log = LoggerFactory.getLogger(AutomaticMerger.class);
+  /**
+   * Prefix used in front of messages pushed to Gerrit by this plugin. This prefix is used to
+   * discriminate the messages emitted by the plugin from the other messages and avoid infinite
+   * loops.
+   */
+  public static final String commentsPrefix = "[Autosubmitter] ";
 
-  @Inject
-  ChangeData.Factory changeDataFactory;
+  @Inject Provider<PostReview> reviewer;
 
-  @Inject
-  AutomergeConfig config;
+  @Inject private AtomicityHelper atomicityHelper;
 
-  @Inject
-  Provider<ReviewDb> db;
-
-  @Inject
-  Provider<PostReview> reviewer;
-
-  @Inject
-  private AtomicityHelper atomicityHelper;
-
-  public void commentOnReview(String project, int number, String commentTemplate) throws Exception {
-    ReviewInput comment = createComment(commentTemplate);
-    applyComment(project, number, comment);
+  public void commentOnReview(String project, int number, String comment) throws Exception {
+    ReviewInput reviewInput = createComment(comment);
+    applyComment(project, number, reviewInput);
   }
 
-    public void setMinusOne(String project, int number, String commentTemplate) throws Exception {
-    ReviewInput message = createComment(commentTemplate).label("Code-Review", -1);
-    applyComment(project, number, message);
+  private ReviewInput createComment(String comment) {
+    return new ReviewInput().message(commentsPrefix + comment);
   }
 
-  private ReviewInput createComment(final String commentTemplate) {
-    return new ReviewInput().message(getCommentFromFile(commentTemplate));
-  }
-
-  private String getCommentFromFile(final String filename) {
-     try {
-       return Files.toString(new File(config.getTemplatesPath(), filename), Charsets.UTF_8);
-     } catch (final IOException exc) {
-       final String errmsg =
-           String.format("Cannot find %s file in gerrit etc dir. Please check your gerrit configuration", filename);
-       log.error(errmsg);
-       return errmsg;
-     }
-   }
-
   private void applyComment(String project, int number, ReviewInput comment) throws Exception {
     RevisionResource r = atomicityHelper.getRevisionResource(project, number);
     reviewer.get().apply(r, comment);
diff --git a/src/test/java/com/criteo/gerrit/plugins/automerge/AutomergeConfigTest.java b/src/test/java/com/criteo/gerrit/plugins/automerge/AutomergeConfigTest.java
index 49535db..69e4ef9 100644
--- a/src/test/java/com/criteo/gerrit/plugins/automerge/AutomergeConfigTest.java
+++ b/src/test/java/com/criteo/gerrit/plugins/automerge/AutomergeConfigTest.java
@@ -4,13 +4,11 @@
 import static org.junit.Assert.fail;
 
 import com.google.gerrit.server.config.SitePaths;
-
-import org.eclipse.jgit.lib.Config;
-import org.junit.Test;
-
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.file.Paths;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
 
 public class AutomergeConfigTest {
 
@@ -35,8 +33,13 @@
     try {
       final SitePaths paths = new SitePaths(Paths.get("."));
 
-      conf.setString(AutomergeConfig.AUTOMERGE_SECTION, null, AutomergeConfig.BOT_EMAIL_KEY, "Foo@bar.com");
-      conf.setString(AutomergeConfig.AUTOMERGE_SECTION, null, AutomergeConfig.TOPIC_PREFIX_KEY, "fake_topic_prefix");
+      conf.setString(
+          AutomergeConfig.AUTOMERGE_SECTION, null, AutomergeConfig.BOT_EMAIL_KEY, "Foo@bar.com");
+      conf.setString(
+          AutomergeConfig.AUTOMERGE_SECTION,
+          null,
+          AutomergeConfig.TOPIC_PREFIX_KEY,
+          "fake_topic_prefix");
 
       final AutomergeConfig amconf = new AutomergeConfig(conf, paths);
       assertEquals(amconf.getBotEmail(), "Foo@bar.com");
diff --git a/test.rb b/test.rb
index dd3523f..92ccfab 100755
--- a/test.rb
+++ b/test.rb
@@ -14,11 +14,12 @@
 PROJECTS_DIR="~/"
 PROJECT1="project1"
 PROJECT2="project2"
+USER="admin"
 
 class TestAutomerge < MiniTest::Test
   HOST = "0.0.0.0"
   PORT = 29418
-  GERRIT_SSH = "ssh -p #{PORT} #{HOST}"
+  GERRIT_SSH = "ssh -l #{USER} -p #{PORT} #{HOST}"
 
   def setup
     clean_local_repo(PROJECT1)
@@ -48,8 +49,8 @@
   def test_crossrepo_topic_1_repo_over_not_merged_commit
     commit0 = create_review(PROJECT1, "review0 on #{PROJECT1}")
     commit0b = create_review(PROJECT1, "review0b on #{PROJECT1}", "crossrepo/topic1")
-    check_label(commit0b, "Code-Review", "-1")
-    check_last_message_contains(commit0b, "atomic_review_same_repo.txt")
+
+    check_last_message_contains(commit0b, "This cross-repo review depends on a not merged commit")
   end
 
   def test_normal_topic_2_repos
@@ -68,11 +69,90 @@
     approve_review(commit1)
     check_status(commit1, 'NEW')
     check_status(commit2, 'NEW')
+
     approve_review(commit2)
+
     check_status(commit1, 'MERGED')
     check_status(commit2, 'MERGED')
   end
 
+  def test_crossrepo_topic_2_repos_above_non_mergeable_commit
+    commit1a = create_review(PROJECT1, "review1a on #{PROJECT1}")
+    commit1b = create_review(PROJECT1, "review1b on #{PROJECT1}", "crossrepo/topic2")
+    commit2 = create_review(PROJECT2, "review2 on #{PROJECT2}", "crossrepo/topic2")
+    approve_review(commit1b)
+    check_status(commit1a, 'NEW')
+    check_status(commit1b, 'NEW')
+    check_status(commit2, 'NEW')
+
+    approve_review(commit2)
+
+    check_last_message_contains(commit2, "blocked by a non merged commit below")
+    check_status(commit1a, 'NEW')
+    check_status(commit1b, 'NEW')
+    check_status(commit2, 'NEW')
+  end
+
+ def test_crossrepo_topic_2_repos_below_not_merged_commit
+    commit1 = create_review(PROJECT1, "review1 on #{PROJECT1}", "crossrepo/topic2")
+    commit1b = create_review(PROJECT1, "review1b on #{PROJECT1}")
+    commit2 = create_review(PROJECT2, "review2 on #{PROJECT2}", "crossrepo/topic2")
+    approve_review(commit1)
+    check_status(commit1, 'NEW')
+    check_status(commit2, 'NEW')
+
+    approve_review(commit2)
+
+    check_status(commit1, 'MERGED')
+    check_status(commit2, 'MERGED')
+  end
+
+  def test_refupdatedevent_merge_upper_commit
+    commit1a = create_review(PROJECT1, "review1a on #{PROJECT1}")
+    commit1b = create_review(PROJECT1, "review1b on #{PROJECT1}")
+    approve_review(commit1b)
+    check_status(commit1a, 'NEW')
+    check_status(commit1b, 'NEW')
+
+    approve_review(commit1a)
+
+    check_status(commit1a, 'MERGED')
+    check_status(commit1b, 'MERGED')
+  end
+
+ def test_refupdatedevent_merge_upper_crossrepo
+    commit1a = create_review(PROJECT1, "review1a on #{PROJECT1}")
+    commit1b = create_review(PROJECT1, "review1b on #{PROJECT1}", "crossrepo/topic2")
+    commit2 = create_review(PROJECT2, "review2 on #{PROJECT2}", "crossrepo/topic2")
+    approve_review(commit1b)
+    approve_review(commit2)
+    check_status(commit1a, 'NEW')
+    check_status(commit1b, 'NEW')
+    check_status(commit2, 'NEW')
+
+    approve_review(commit1a)
+
+    check_status(commit1a, 'MERGED')
+    check_status(commit1b, 'MERGED')
+    check_status(commit2, 'MERGED')
+  end
+
+  def test_refupdatedevent_does_not_merge_non_mergeable_upper_crossrepo
+    commit1a = create_review(PROJECT1, "review1a on #{PROJECT1}")
+    commit1b = create_review(PROJECT1, "review1b on #{PROJECT1}", "crossrepo/topic2")
+    commit2 = create_review(PROJECT2, "review2 on #{PROJECT2}", "crossrepo/topic2")
+    approve_review(commit1b)
+    check_status(commit1a, 'NEW')
+    check_status(commit1b, 'NEW')
+    check_status(commit2, 'NEW')
+
+    approve_review(commit1a)
+
+    check_status(commit1a, 'MERGED')
+    check_status(commit1b, 'NEW')
+    check_status(commit2, 'NEW')
+  end
+
   def test_two_reviews_with_same_changed_id
     commit1 = create_review(PROJECT1, "review1 on #{PROJECT1}")
     change_id = read_change_id(PROJECT1)
@@ -85,6 +165,16 @@
     check_status(commit2, 'MERGED')
   end
 
+  def test_published_draft_is_autosubmitted
+    commit_id = create_review(PROJECT1, "review0 on #{PROJECT1}", draft: true)
+    approve_review(commit_id)
+    check_status(commit_id, 'DRAFT')
+
+    publish_draft(commit_id)
+
+    check_status(commit_id, 'MERGED')
+  end
+
   private
 
   def project_dir(project_name)
@@ -101,7 +191,11 @@
     reviews = gerrit_query(query)
     reviews.each do |review|
       review_number = review['number']
-      execute("#{GERRIT_SSH} gerrit review --abandon #{review_number},1")
+      if review['status'] == 'DRAFT'
+        execute("#{GERRIT_SSH} gerrit review --delete #{review_number},1")
+      else
+        execute("#{GERRIT_SSH} gerrit review --abandon #{review_number},1")
+      end
     end
   end
 
@@ -113,14 +207,14 @@
     change_id
   end
 
-  def create_review(project_name, message, topic = nil, change_id = nil)
+  def create_review(project_name, message, topic = nil, change_id = nil, draft: false)
     topic_suffix = "/#{topic}" if topic
     message = "#{message}\n\nChange-Id: #{change_id}" if change_id
     execute(["cd #{project_dir(project_name)}",
              "echo 0 >> a",
              "git add .",
              %Q(git commit -m "#{message}"),
-             "git push origin HEAD:refs/for/master#{topic_suffix}"
+             "git push origin HEAD:refs/#{draft ? 'drafts' : 'for'}/master#{topic_suffix}"
             ].join(" && "))
     commit_id = execute("cd #{project_dir(project_name)} && git rev-parse HEAD")
     refute(commit_id.empty?, "missing commit-id")
@@ -131,6 +225,10 @@
     execute("#{GERRIT_SSH} gerrit review --strict-labels --verified 1 --code-review 2 #{commit_id}")
   end
 
+  def publish_draft(commit_id)
+    execute("#{GERRIT_SSH} gerrit review --publish #{commit_id}")
+  end
+
   def abandon_review(commit_id)
     execute("#{GERRIT_SSH} gerrit review --abandon #{commit_id}")
   end