SONAR-7294 Replace Ruby WS api/issues/edit_comment

it/it-tests/src/test/java/it/issue/IssueActionTest.java

@@ -28,6 +28,7 @@ import org.sonarqube.ws.Issues;
 import org.sonarqube.ws.Issues.Issue;
 import org.sonarqube.ws.client.issue.AddCommentRequest;
 import org.sonarqube.ws.client.issue.AssignRequest;
+import org.sonarqube.ws.client.issue.EditCommentRequest;
 import org.sonarqube.ws.client.issue.IssuesService;
 import org.sonarqube.ws.client.issue.SearchWsRequest;
 import org.sonarqube.ws.client.issue.SetSeverityRequest;
@@ -74,7 +75,6 @@ public class IssueActionTest extends AbstractIssueTest {
   @Test
   public void add_comment() throws Exception {
     Issues.Comment comment = issuesService.addComment(new AddCommentRequest(randomIssue.getKey(), "this is my *comment*")).getIssue().getComments().getComments(0);
-
     assertThat(comment.getKey()).isNotNull();
     assertThat(comment.getHtmlText()).isEqualTo("this is my <strong>comment</strong>");
     assertThat(comment.getLogin()).isEqualTo("admin");
@@ -82,7 +82,6 @@ public class IssueActionTest extends AbstractIssueTest {
 
     // reload issue
     Issue reloaded = issueRule.getByKey(randomIssue.getKey());
-
     assertThat(reloaded.getComments().getCommentsList()).hasSize(1);
     assertThat(reloaded.getComments().getComments(0).getKey()).isEqualTo(comment.getKey());
     assertThat(reloaded.getComments().getComments(0).getHtmlText()).isEqualTo("this is my <strong>comment</strong>");
@@ -105,6 +104,18 @@ public class IssueActionTest extends AbstractIssueTest {
     assertThat(reloaded.getComments().getCommentsList()).isEmpty();
   }
 
+  @Test
+  public void edit_comment() throws Exception {
+    Issues.Comment comment = issuesService.addComment(new AddCommentRequest(randomIssue.getKey(), "this is my *comment*")).getIssue().getComments().getComments(0);
+    Issues.Comment editedComment = issuesService.editComment(new EditCommentRequest(comment.getKey(), "new *comment*")).getIssue().getComments().getComments(0);
+    assertThat(editedComment.getHtmlText()).isEqualTo("new <strong>comment</strong>");
+
+    // reload issue
+    Issue reloaded = issueRule.getByKey(randomIssue.getKey());
+    assertThat(reloaded.getComments().getCommentsList()).hasSize(1);
+    assertThat(reloaded.getComments().getComments(0).getHtmlText()).isEqualTo("new <strong>comment</strong>");
+  }
+
   /**
    * SONAR-4352
    */

server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java

@@ -55,16 +55,6 @@ public class InternalRubyIssueService {
     return commentService.deleteComment(commentKey);
   }
 
-  public Result<IssueComment> editComment(String commentKey, String newText) {
-    Result<IssueComment> result = Result.of();
-    try {
-      result.set(commentService.editComment(commentKey, newText));
-    } catch (Exception e) {
-      result.addError(e.getMessage());
-    }
-    return result;
-  }
-
   /**
    * Execute a bulk change
    */

server/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java

@@ -21,13 +21,9 @@ package org.sonar.server.issue;
 
 import com.google.common.base.Strings;
 import java.util.Objects;
-import org.apache.commons.lang.StringUtils;
 import org.sonar.api.issue.IssueComment;
-import org.sonar.api.utils.System2;
 import org.sonar.core.issue.DefaultIssueComment;
 import org.sonar.db.DbClient;
-import org.sonar.db.issue.IssueChangeDto;
-import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.user.UserSession;
@@ -60,26 +56,4 @@ public class IssueCommentService {
     return comment;
   }
 
-  public IssueComment editComment(String commentKey, String text) {
-    DefaultIssueComment comment = dbClient.issueChangeDao().selectDefaultCommentByKey(commentKey);
-    if (StringUtils.isBlank(text)) {
-      throw new BadRequestException("Cannot add empty comments to an issue");
-    }
-    if (comment == null) {
-      throw new NotFoundException("Comment not found: " + commentKey);
-    }
-    if (Strings.isNullOrEmpty(comment.userLogin()) || !Objects.equals(comment.userLogin(), userSession.getLogin())) {
-      throw new ForbiddenException("You can only edit your own comments");
-    }
-
-    // check authorization
-    issueService.getByKey(comment.issueKey());
-
-    IssueChangeDto dto = IssueChangeDto.of(comment);
-    dto.setUpdatedAt(System2.INSTANCE.now());
-    dto.setChangeData(text);
-    dbClient.issueChangeDao().update(dto);
-
-    return comment;
-  }
 }

server/sonar-server/src/main/java/org/sonar/server/issue/ws/AddCommentAction.java

@@ -92,15 +92,15 @@ public class AddCommentAction implements IssuesWsAction {
       IssueDto issueDto = issueFinder.getByKey(dbSession, wsRequest.getIssue());
       IssueChangeContext context = IssueChangeContext.createUser(new Date(system2.now()), userSession.getLogin());
       DefaultIssue defaultIssue = issueDto.toDefaultIssue();
-      issueFieldsSetter.addComment(defaultIssue, wsRequest.getComment(), context);
-      issueUpdater.saveIssue(dbSession, defaultIssue, context, wsRequest.getComment());
+      issueFieldsSetter.addComment(defaultIssue, wsRequest.getText(), context);
+      issueUpdater.saveIssue(dbSession, defaultIssue, context, wsRequest.getText());
       responseWriter.write(defaultIssue.key(), request, response);
     }
   }
 
   private static AddCommentRequest toWsRequest(Request request) {
     AddCommentRequest wsRequest = new AddCommentRequest(request.mandatoryParam(PARAM_ISSUE), request.mandatoryParam(PARAM_TEXT));
-    checkArgument(!isNullOrEmpty(wsRequest.getComment()), "Cannot add empty comment to an issue");
+    checkArgument(!isNullOrEmpty(wsRequest.getText()), "Cannot add empty comment to an issue");
     return wsRequest;
   }
 

server/sonar-server/src/main/java/org/sonar/server/issue/ws/EditCommentAction.java

@@ -0,0 +1,150 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2016 SonarSource SA
+ * mailto:contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+package org.sonar.server.issue.ws;
+
+import java.util.Objects;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.stream.Stream;
+import org.sonar.api.server.ws.Request;
+import org.sonar.api.server.ws.Response;
+import org.sonar.api.server.ws.WebService;
+import org.sonar.api.utils.System2;
+import org.sonar.core.util.stream.Collectors;
+import org.sonar.db.DbClient;
+import org.sonar.db.DbSession;
+import org.sonar.db.issue.IssueChangeDto;
+import org.sonar.db.issue.IssueDto;
+import org.sonar.server.exceptions.NotFoundException;
+import org.sonar.server.issue.IssueFinder;
+import org.sonar.server.user.UserSession;
+import org.sonarqube.ws.client.issue.EditCommentRequest;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.lang.String.format;
+import static org.sonar.core.util.Uuids.UUID_EXAMPLE_01;
+import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_EDIT_COMMENT;
+import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_COMMENT;
+import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_TEXT;
+
+public class EditCommentAction implements IssuesWsAction {
+
+  private final System2 system2;
+  private final UserSession userSession;
+  private final DbClient dbClient;
+  private final IssueFinder issueFinder;
+  private final OperationResponseWriter responseWriter;
+
+  public EditCommentAction(System2 system2, UserSession userSession, DbClient dbClient, IssueFinder issueFinder, OperationResponseWriter responseWriter) {
+    this.system2 = system2;
+    this.userSession = userSession;
+    this.dbClient = dbClient;
+    this.issueFinder = issueFinder;
+    this.responseWriter = responseWriter;
+  }
+
+  @Override
+  public void define(WebService.NewController context) {
+    WebService.NewAction action = context.createAction(ACTION_EDIT_COMMENT)
+      .setDescription("Edit a comment.<br/>" +
+        "Requires authentication and the following permission: 'Browse' on the project of the specified issue.<br/>" +
+        "Since 6.3, the response contains the issue with all details, not only the edited comment.<br/>" +
+        "Since 6.3, 'key' parameter has been renamed %s", PARAM_COMMENT)
+      .setSince("3.6")
+      .setHandler(this)
+      .setPost(true);
+
+    action.createParam(PARAM_COMMENT)
+      .setDescription("Comment key")
+      .setDeprecatedKey("key")
+      .setSince("6.3")
+      .setRequired(true)
+      .setExampleValue(UUID_EXAMPLE_01);
+    action.createParam(PARAM_TEXT)
+      .setDescription("Comment text")
+      .setRequired(true)
+      .setExampleValue("Won't fix because it doesn't apply to the context");
+  }
+
+  @Override
+  public void handle(Request request, Response response) {
+    userSession.checkLoggedIn();
+    try (DbSession dbSession = dbClient.openSession(false)) {
+      IssueDto issueDto = Stream.of(request)
+        .map(toWsRequest())
+        .map(loadCommentData(dbSession))
+        .peek(updateComment(dbSession))
+        .collect(Collectors.toOneElement())
+        .getIssueDto();
+      responseWriter.write(issueDto.getKey(), request, response);
+    }
+  }
+
+  private Function<EditCommentRequest, CommentData> loadCommentData(DbSession dbSession) {
+    return request -> new CommentData(dbSession, request);
+  }
+
+  private Consumer<CommentData> updateComment(DbSession dbSession) {
+    return commentData -> {
+      commentData.getIssueChangeDto().setUpdatedAt(system2.now());
+      commentData.getIssueChangeDto().setChangeData(commentData.getRequest().getText());
+      dbClient.issueChangeDao().update(dbSession, commentData.getIssueChangeDto());
+      dbSession.commit();
+    };
+  }
+
+  private static Function<Request, EditCommentRequest> toWsRequest() {
+    return request -> {
+      EditCommentRequest wsRequest = new EditCommentRequest(request.mandatoryParam(PARAM_COMMENT), request.mandatoryParam(PARAM_TEXT));
+      checkArgument(!isNullOrEmpty(wsRequest.getText()), "Cannot set empty comment to an issue");
+      return wsRequest;
+    };
+  }
+
+  private class CommentData {
+    private final IssueChangeDto issueChangeDto;
+    private final IssueDto issueDto;
+    private final EditCommentRequest request;
+
+    CommentData(DbSession dbSession, EditCommentRequest request) {
+      this.request = request;
+      this.issueChangeDto = dbClient.issueChangeDao().selectCommentByKey(dbSession, request.getComment())
+        .orElseThrow(() -> new NotFoundException(format("Comment with key '%s' does not exist", request.getComment())));
+      // Load issue now to quickly fail if user hasn't permission to see it
+      this.issueDto = issueFinder.getByKey(dbSession, issueChangeDto.getIssueKey());
+      checkArgument(Objects.equals(issueChangeDto.getUserLogin(), userSession.getLogin()), "You can only edit your own comments");
+    }
+
+    IssueChangeDto getIssueChangeDto() {
+      return issueChangeDto;
+    }
+
+    IssueDto getIssueDto() {
+      return issueDto;
+    }
+
+    EditCommentRequest getRequest() {
+      return request;
+    }
+  }
+
+}

server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java

@@ -58,6 +58,7 @@ public class IssueWsModule extends Module {
       OperationResponseWriter.class,
       WsResponseCommonFormat.class,
       AddCommentAction.class,
+      EditCommentAction.class,
       AssignAction.class,
       DoTransitionAction.class,
       SearchAction.class,

server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java

@@ -53,7 +53,6 @@ public class IssuesWs implements WebService {
 
   private static void defineRailsActions(NewController controller) {
     defineDeleteCommentAction(controller);
-    defineEditCommentAction(controller);
     defineBulkChangeAction(controller);
   }
 
@@ -70,23 +69,6 @@ public class IssuesWs implements WebService {
       .setExampleValue("392160d3-a4f2-4c52-a565-e4542cfa2096");
   }
 
-  private static void defineEditCommentAction(NewController controller) {
-    WebService.NewAction action = controller.createAction(EDIT_COMMENT_ACTION)
-      .setDescription("Edit a comment. Requires authentication and User role on project")
-      .setSince("3.6")
-      .setHandler(RailsHandler.INSTANCE)
-      .setPost(true);
-
-    action.createParam("key")
-      .setDescription("Key of the comment")
-      .setRequired(true)
-      .setExampleValue("392160d3-a4f2-4c52-a565-e4542cfa2096");
-    action.createParam("text")
-      .setDescription("New comment")
-      .setExampleValue("blabla2...");
-    RailsHandler.addFormatParam(action);
-  }
-
   private static void defineBulkChangeAction(NewController controller) {
     WebService.NewAction action = controller.createAction(BULK_CHANGE_ACTION)
       .setDescription("Bulk change on issues. Requires authentication and User role on project(s)")

server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceTest.java

@@ -31,13 +31,10 @@ import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.issue.IssueChangeDao;
-import org.sonar.db.issue.IssueChangeDto;
-import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.tester.UserSessionRule;
 
-import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.anyString;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -106,53 +103,4 @@ public class IssueCommentServiceTest {
     verify(changeDao, never()).delete(anyString());
   }
 
-  @Test
-  public void should_update_comment() {
-    when(changeDao.selectDefaultCommentByKey("ABCD")).thenReturn(new DefaultIssueComment().setIssueKey("EFGH").setUserLogin("admin"));
-
-    issueCommentService.editComment("ABCD", "updated comment");
-
-    verify(changeDao).update(any(IssueChangeDto.class));
-    verify(issueService).getByKey("EFGH");
-  }
-
-  @Test
-  public void should_not_update_not_found_comment() {
-    throwable.expect(NotFoundException.class);
-
-    when(changeDao.selectDefaultCommentByKey("ABCD")).thenReturn(null);
-
-    issueCommentService.editComment("ABCD", "updated comment");
-
-    verify(changeDao, never()).update(any(IssueChangeDto.class));
-  }
-
-  @Test
-  public void should_prevent_updating_empty_comment() {
-    throwable.expect(BadRequestException.class);
-
-    issueCommentService.editComment("ABCD", "");
-
-    verify(changeDao, never()).update(any(IssueChangeDto.class));
-  }
-
-  @Test
-  public void should_prevent_updating_null_comment() {
-    throwable.expect(BadRequestException.class);
-
-    issueCommentService.editComment("ABCD", null);
-
-    verify(changeDao, never()).update(any(IssueChangeDto.class));
-  }
-
-  @Test
-  public void should_prevent_updating_others_comment() {
-    throwable.expect(ForbiddenException.class);
-
-    when(changeDao.selectDefaultCommentByKey("ABCD")).thenReturn(new DefaultIssueComment().setUserLogin("julien"));
-
-    issueCommentService.editComment("ABCD", "updated comment");
-
-    verify(changeDao, never()).update(any(IssueChangeDto.class));
-  }
 }

server/sonar-server/src/test/java/org/sonar/server/issue/ws/AddCommentActionTest.java

@@ -32,13 +32,9 @@ import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.System2;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
-import org.sonar.db.component.ComponentDbTester;
-import org.sonar.db.component.ComponentDto;
 import org.sonar.db.issue.IssueChangeDto;
 import org.sonar.db.issue.IssueDbTester;
 import org.sonar.db.issue.IssueDto;
-import org.sonar.db.rule.RuleDbTester;
-import org.sonar.db.rule.RuleDto;
 import org.sonar.server.es.EsTester;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
@@ -63,14 +59,10 @@ import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-import static org.sonar.api.issue.Issue.STATUS_OPEN;
 import static org.sonar.api.web.UserRole.CODEVIEWER;
 import static org.sonar.api.web.UserRole.USER;
 import static org.sonar.core.util.Protobuf.setNullable;
-import static org.sonar.db.component.ComponentTesting.newFileDto;
 import static org.sonar.db.issue.IssueChangeDto.TYPE_COMMENT;
-import static org.sonar.db.issue.IssueTesting.newDto;
-import static org.sonar.db.rule.RuleTesting.newRuleDto;
 
 public class AddCommentActionTest {
 
@@ -80,7 +72,7 @@ public class AddCommentActionTest {
   public ExpectedException expectedException = ExpectedException.none();
 
   @Rule
-  public DbTester dbTester = DbTester.create();
+  public DbTester dbTester = DbTester.create(System2.INSTANCE);
 
   @Rule
   public EsTester esTester = new EsTester(new IssueIndexDefinition(new MapSettings()));
@@ -92,9 +84,7 @@ public class AddCommentActionTest {
 
   private DbClient dbClient = dbTester.getDbClient();
 
-  private RuleDbTester ruleDbTester = new RuleDbTester(dbTester);
   private IssueDbTester issueDbTester = new IssueDbTester(dbTester);
-  private ComponentDbTester componentDbTester = new ComponentDbTester(dbTester);
 
   private IssueUpdater issueUpdater = new IssueUpdater(dbClient,
     new ServerIssueStorage(new DefaultRuleFinder(dbClient), dbClient, new IssueIndexer(system2, dbClient, esTester.client())), mock(NotificationManager.class));
@@ -110,7 +100,7 @@ public class AddCommentActionTest {
 
   @Test
   public void add_comment() throws Exception {
-    IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null));
+    IssueDto issueDto = issueDbTester.insertIssue();
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
 
     call(issueDto.getKey(), "please fix it");
@@ -156,7 +146,7 @@ public class AddCommentActionTest {
 
   @Test
   public void fail_when_empty_comment_text() throws Exception {
-    IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null));
+    IssueDto issueDto = issueDbTester.insertIssue();
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
 
     expectedException.expect(IllegalArgumentException.class);
@@ -171,7 +161,7 @@ public class AddCommentActionTest {
 
   @Test
   public void fail_when_not_enough_permission() throws Exception {
-    IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null));
+    IssueDto issueDto = issueDbTester.insertIssue();
     userSession.login("john").addProjectUuidPermissions(CODEVIEWER, issueDto.getProjectUuid());
 
     expectedException.expect(ForbiddenException.class);
@@ -195,10 +185,4 @@ public class AddCommentActionTest {
     return request.execute();
   }
 
-  private IssueDto newIssue() {
-    RuleDto rule = ruleDbTester.insertRule(newRuleDto());
-    ComponentDto project = componentDbTester.insertProject();
-    ComponentDto file = componentDbTester.insertComponent(newFileDto(project));
-    return newDto(rule, file, project);
-  }
 }

server/sonar-server/src/test/java/org/sonar/server/issue/ws/ChangelogActionTest.java

@@ -74,7 +74,7 @@ public class ChangelogActionTest {
     UserDto user = db.users().insertUser();
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", "MAJOR", "BLOCKER"));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", "MAJOR", "BLOCKER"));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -94,7 +94,7 @@ public class ChangelogActionTest {
     ComponentDto file2 = db.components().insertComponent(newFileDto(project));
     IssueDto issueDto = db.issues().insertIssue(newDto(rule, file2, project));
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setDiff("file", file1.uuid(), file2.uuid()));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setDiff("file", file1.uuid(), file2.uuid()));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -109,7 +109,7 @@ public class ChangelogActionTest {
   public void changelog_of_file_move_is_empty_when_files_does_not_exists() throws Exception {
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setDiff("file", "UNKNOWN_1", "UNKNOWN_2"));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setDiff("file", "UNKNOWN_1", "UNKNOWN_2"));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -123,7 +123,7 @@ public class ChangelogActionTest {
     UserDto user = db.users().insertUser(UserTesting.newUserDto("john", "John", null));
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", "MAJOR", "BLOCKER"));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", "MAJOR", "BLOCKER"));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -137,7 +137,7 @@ public class ChangelogActionTest {
   public void return_changelog_not_having_user() throws Exception {
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setUserLogin(null).setDiff("severity", "MAJOR", "BLOCKER"));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setUserLogin(null).setDiff("severity", "MAJOR", "BLOCKER"));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -152,7 +152,7 @@ public class ChangelogActionTest {
   public void return_changelog_on_none_existing_user() throws Exception {
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setUserLogin("UNKNOWN").setDiff("severity", "MAJOR", "BLOCKER"));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setUserLogin("UNKNOWN").setDiff("severity", "MAJOR", "BLOCKER"));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -168,7 +168,7 @@ public class ChangelogActionTest {
     UserDto user = db.users().insertUser();
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", "MAJOR", "BLOCKER").setDiff("status", "RESOLVED", "CLOSED"));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", "MAJOR", "BLOCKER").setDiff("status", "RESOLVED", "CLOSED"));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -182,7 +182,7 @@ public class ChangelogActionTest {
     UserDto user = db.users().insertUser();
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", null, "BLOCKER"));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", null, "BLOCKER"));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -195,7 +195,7 @@ public class ChangelogActionTest {
     UserDto user = db.users().insertUser();
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", "MAJOR", null));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", "MAJOR", null));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -208,7 +208,7 @@ public class ChangelogActionTest {
     UserDto user = db.users().insertUser();
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto,
+    db.issues().insertFieldDiffs(issueDto,
       new FieldDiffs().setUserLogin(user.getLogin()).setDiff("severity", "MAJOR", "BLOCKER"),
       new FieldDiffs().setDiff("status", "RESOLVED", "CLOSED"));
 
@@ -222,7 +222,7 @@ public class ChangelogActionTest {
     UserDto user = db.users().insertUser();
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("technicalDebt", "10", "20"));
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setUserLogin(user.getLogin()).setDiff("technicalDebt", "10", "20"));
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
@@ -254,7 +254,7 @@ public class ChangelogActionTest {
     UserDto user = db.users().insertUser(newUserDto("john.smith", "John Smith", "john@smith.com"));
     IssueDto issueDto = db.issues().insertIssue(newIssue());
     userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
-    db.issues().insertIssueChanges(issueDto, new FieldDiffs()
+    db.issues().insertFieldDiffs(issueDto, new FieldDiffs()
       .setUserLogin(user.getLogin())
       .setDiff("severity", "MAJOR", "BLOCKER")
       .setCreationDate(DateUtils.parseDateTime("2014-03-04T23:03:44+0100")));

server/sonar-server/src/test/java/org/sonar/server/issue/ws/EditCommentActionTest.java

@@ -0,0 +1,201 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2016 SonarSource SA
+ * mailto:contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+package org.sonar.server.issue.ws;
+
+import javax.annotation.Nullable;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.sonar.api.server.ws.Request;
+import org.sonar.api.server.ws.Response;
+import org.sonar.api.server.ws.WebService;
+import org.sonar.api.utils.System2;
+import org.sonar.db.DbClient;
+import org.sonar.db.DbTester;
+import org.sonar.db.issue.IssueChangeDto;
+import org.sonar.db.issue.IssueDbTester;
+import org.sonar.db.issue.IssueDto;
+import org.sonar.server.exceptions.ForbiddenException;
+import org.sonar.server.exceptions.NotFoundException;
+import org.sonar.server.exceptions.UnauthorizedException;
+import org.sonar.server.issue.IssueFinder;
+import org.sonar.server.tester.UserSessionRule;
+import org.sonar.server.ws.TestRequest;
+import org.sonar.server.ws.TestResponse;
+import org.sonar.server.ws.WsActionTester;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.sonar.api.web.UserRole.CODEVIEWER;
+import static org.sonar.api.web.UserRole.USER;
+import static org.sonar.core.util.Protobuf.setNullable;
+
+public class EditCommentActionTest {
+
+  private static final long NOW = 10_000_000_000L;
+
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
+  @Rule
+  public DbTester dbTester = DbTester.create();
+
+  @Rule
+  public UserSessionRule userSession = UserSessionRule.standalone();
+
+  private System2 system2 = mock(System2.class);
+
+  private DbClient dbClient = dbTester.getDbClient();
+
+  private IssueDbTester issueDbTester = new IssueDbTester(dbTester);
+
+  private OperationResponseWriter responseWriter = mock(OperationResponseWriter.class);
+
+  private WsActionTester tester = new WsActionTester(
+    new EditCommentAction(system2, userSession, dbClient, new IssueFinder(dbClient, userSession), responseWriter));
+
+  @Before
+  public void setUp() throws Exception {
+    when(system2.now()).thenReturn(NOW);
+  }
+
+  @Test
+  public void edit_comment() throws Exception {
+    IssueDto issueDto = issueDbTester.insertIssue();
+    IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, "john", "please fix it");
+    userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
+
+    call(commentDto.getKey(), "please have a look");
+
+    verify(responseWriter).write(eq(issueDto.getKey()), any(Request.class), any(Response.class));
+    IssueChangeDto issueComment = dbClient.issueChangeDao().selectCommentByKey(dbTester.getSession(), commentDto.getKey()).get();
+    assertThat(issueComment.getChangeData()).isEqualTo("please have a look");
+    assertThat(issueComment.getUpdatedAt()).isEqualTo(NOW);
+  }
+
+  @Test
+  public void edit_comment_using_deprecated_key_parameter() throws Exception {
+    IssueDto issueDto = issueDbTester.insertIssue();
+    IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, "john", "please fix it");
+    userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
+
+    tester.newRequest().setParam("key", commentDto.getKey()).setParam("text", "please have a look").execute();
+
+    verify(responseWriter).write(eq(issueDto.getKey()), any(Request.class), any(Response.class));
+    IssueChangeDto issueComment = dbClient.issueChangeDao().selectCommentByKey(dbTester.getSession(), commentDto.getKey()).get();
+    assertThat(issueComment.getChangeData()).isEqualTo("please have a look");
+    assertThat(issueComment.getUpdatedAt()).isEqualTo(NOW);
+  }
+
+  @Test
+  public void fail_when_comment_does_not_belong_to_current_user() throws Exception {
+    IssueDto issueDto = issueDbTester.insertIssue();
+    IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, "john", "please fix it");
+    userSession.login("another").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("You can only edit your own comments");
+    call(commentDto.getKey(), "please have a look");
+  }
+
+  @Test
+  public void fail_when_comment_has_not_user() throws Exception {
+    IssueDto issueDto = issueDbTester.insertIssue();
+    IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, null, "please fix it");
+    userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("You can only edit your own comments");
+    call(commentDto.getKey(), "please have a look");
+  }
+
+  @Test
+  public void fail_when_missing_comment_key() throws Exception {
+    userSession.login("john");
+
+    expectedException.expect(IllegalArgumentException.class);
+    call(null, "please fix it");
+  }
+
+  @Test
+  public void fail_when_comment_does_not_exist() throws Exception {
+    userSession.login("john");
+
+    expectedException.expect(NotFoundException.class);
+    call("ABCD", "please fix it");
+  }
+
+  @Test
+  public void fail_when_missing_comment_text() throws Exception {
+    userSession.login("john");
+
+    expectedException.expect(IllegalArgumentException.class);
+    call("ABCD", null);
+  }
+
+  @Test
+  public void fail_when_empty_comment_text() throws Exception {
+    IssueDto issueDto = issueDbTester.insertIssue();
+    IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, "john", "please fix it");
+    userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid());
+
+    expectedException.expect(IllegalArgumentException.class);
+    call(commentDto.getKey(), "");
+  }
+
+  @Test
+  public void fail_when_not_authenticated() throws Exception {
+    expectedException.expect(UnauthorizedException.class);
+    call("ABCD", "please fix it");
+  }
+
+  @Test
+  public void fail_when_not_enough_permission() throws Exception {
+    IssueDto issueDto = issueDbTester.insertIssue();
+    IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, "john", "please fix it");
+    userSession.login("john").addProjectUuidPermissions(CODEVIEWER, issueDto.getProjectUuid());
+
+    expectedException.expect(ForbiddenException.class);
+    call(commentDto.getKey(), "please have a look");
+  }
+
+  @Test
+  public void test_definition() {
+    WebService.Action action = tester.getDef();
+    assertThat(action.key()).isEqualTo("edit_comment");
+    assertThat(action.isPost()).isTrue();
+    assertThat(action.isInternal()).isFalse();
+    assertThat(action.params()).hasSize(2);
+    assertThat(action.responseExample()).isNull();
+  }
+
+  private TestResponse call(@Nullable String commentKey, @Nullable String commentText) {
+    TestRequest request = tester.newRequest();
+    setNullable(commentKey, comment -> request.setParam("comment", comment));
+    setNullable(commentText, comment -> request.setParam("text", comment));
+    return request.execute();
+  }
+}

server/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueWsModuleTest.java

@@ -29,6 +29,6 @@ public class IssueWsModuleTest {
   public void verify_count_of_added_components() {
     ComponentContainer container = new ComponentContainer();
     new IssueWsModule().configure(container);
-    assertThat(container.size()).isEqualTo(2 + 29);
+    assertThat(container.size()).isEqualTo(2 + 30);
   }
 }

server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb

@@ -38,32 +38,6 @@ class Api::IssuesController < Api::ApiController
     render :json => jsonp({:comment => Issue.comment_to_hash(comment)})
   end
 
-  #
-  # POST /api/issues/edit_comment?key=<key>&text=<new text>
-  #
-  # -- Mandatory parameters
-  # 'key' is the comment key
-  # 'text' is the new value
-  #
-  # -- Example
-  # curl -X POST -v -u admin:admin 'http://localhost:9000/api/issues/edit_comment?key=392160d3-a4f2-4c52-a565-e4542cfa2096&text=foo'
-  #
-  def edit_comment
-    verify_post_request
-    require_parameters :key, :text
-
-    text = Api::Utils.read_post_request_param(params[:text])
-    result = Internal.issues.editComment(params[:key], text)
-    hash = result_to_hash(result)
-    hash[:comment] = Issue.comment_to_hash(result.get) if result.get
-
-    respond_to do |format|
-      # if the request header "Accept" is "*/*", then the default format is the first one (json)
-      format.json { render :json => jsonp(hash), :status => result.httpStatus }
-      format.xml { render :xml => hash.to_xml(:skip_types => true, :root => 'sonar', :status => result.httpStatus) }
-    end
-  end
-
   #
   # Execute a bulk change on a list of issues
   #

sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDao.java

@@ -106,17 +106,9 @@ public class IssueChangeDao implements Dao {
     }
   }
 
-  public boolean update(IssueChangeDto change) {
-    DbSession session = mybatis.openSession(false);
-    try {
-      IssueChangeMapper mapper = mapper(session);
-      int count = mapper.update(change);
-      session.commit();
-      return count == 1;
-
-    } finally {
-      MyBatis.closeQuietly(session);
-    }
+  public boolean update(DbSession dbSession, IssueChangeDto change) {
+    int count = mapper(dbSession).update(change);
+    return count == 1;
   }
 
   private static IssueChangeMapper mapper(DbSession session) {

sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDto.java

@@ -110,6 +110,7 @@ public final class IssueChangeDto implements Serializable {
     return this;
   }
 
+  @CheckForNull
   public String getUserLogin() {
     return userLogin;
   }

sonar-db/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java

@@ -178,7 +178,8 @@ public class IssueChangeDaoTest {
     change.setChangeData("new comment");
     change.setUpdatedAt(1_500_000_000_000L);
 
-    assertThat(underTest.update(change)).isTrue();
+    assertThat(underTest.update(db.getSession(), change)).isTrue();
+    db.commit();
 
     db.assertDbUnit(getClass(), "update-result.xml", "issue_changes");
   }
@@ -189,12 +190,10 @@ public class IssueChangeDaoTest {
 
     IssueChangeDto change = new IssueChangeDto();
     change.setKey("UNKNOWN");
-
-    // Only the following fields can be updated:
     change.setChangeData("new comment");
     change.setUpdatedAt(DateUtils.parseDate("2013-06-30").getTime());
 
-    assertThat(underTest.update(change)).isFalse();
+    assertThat(underTest.update(db.getSession(), change)).isFalse();
   }
 
 }

sonar-ws/src/main/java/org/sonarqube/ws/client/issue/AddCommentRequest.java

@@ -25,18 +25,18 @@ import static java.util.Objects.requireNonNull;
 public class AddCommentRequest {
 
   private final String issue;
-  private final String comment;
+  private final String text;
 
-  public AddCommentRequest(String issue, String comment) {
+  public AddCommentRequest(String issue, String text) {
     this.issue = requireNonNull(issue, "Issue key cannot be null");
-    this.comment = requireNonNull(comment, "Comment cannot be null");
+    this.text = requireNonNull(text, "Text cannot be null");
   }
 
   public String getIssue() {
     return issue;
   }
 
-  public String getComment() {
-    return comment;
+  public String getText() {
+    return text;
   }
 }

sonar-ws/src/main/java/org/sonarqube/ws/client/issue/EditCommentRequest.java

@@ -0,0 +1,42 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2016 SonarSource SA
+ * mailto:contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+package org.sonarqube.ws.client.issue;
+
+import static java.util.Objects.requireNonNull;
+
+public class EditCommentRequest {
+
+  private final String comment;
+  private final String text;
+
+  public EditCommentRequest(String comment, String text) {
+    this.comment = requireNonNull(comment, "Comment key cannot be null");
+    this.text = requireNonNull(text, "Text cannot be null");
+  }
+
+  public String getComment() {
+    return comment;
+  }
+
+  public String getText() {
+    return text;
+  }
+}

sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesService.java

@@ -35,6 +35,7 @@ import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_ADD_COMMEN
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_ASSIGN;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_CHANGELOG;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_DO_TRANSITION;
+import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_EDIT_COMMENT;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_SEARCH;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_SET_SEVERITY;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_SET_TYPE;
@@ -47,6 +48,7 @@ import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ASSIGNED;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ASSIGNEE;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ASSIGNEES;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_AUTHORS;
+import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_COMMENT;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_COMPONENTS;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_COMPONENT_KEYS;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_COMPONENT_ROOTS;
@@ -88,7 +90,7 @@ public class IssuesService extends BaseService {
   public Issues.Operation addComment(AddCommentRequest request) {
     return call(new PostRequest(path(ACTION_ADD_COMMENT))
       .setParam(PARAM_ISSUE, request.getIssue())
-      .setParam(PARAM_TEXT, request.getComment()),
+      .setParam(PARAM_TEXT, request.getText()),
       Issues.Operation.parser());
   }
 
@@ -112,6 +114,13 @@ public class IssuesService extends BaseService {
       Issues.Operation.parser());
   }
 
+  public Issues.Operation editComment(EditCommentRequest request) {
+    return call(new PostRequest(path(ACTION_EDIT_COMMENT))
+        .setParam(PARAM_COMMENT, request.getComment())
+        .setParam(PARAM_TEXT, request.getText()),
+      Issues.Operation.parser());
+  }
+
   public SearchWsResponse search(SearchWsRequest request) {
     return call(
       new GetRequest(path(ACTION_SEARCH))

sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesWsParameters.java

@@ -32,6 +32,7 @@ public class IssuesWsParameters {
   public static final String ACTION_SEARCH = "search";
   public static final String ACTION_CHANGELOG = "changelog";
   public static final String ACTION_ADD_COMMENT = "add_comment";
+  public static final String ACTION_EDIT_COMMENT = "edit_comment";
   public static final String ACTION_ASSIGN = "assign";
   public static final String ACTION_AUTHORS = "authors";
   public static final String ACTION_DO_TRANSITION = "do_transition";
@@ -41,6 +42,7 @@ public class IssuesWsParameters {
   public static final String ACTION_SET_TYPE = "set_type";
 
   public static final String PARAM_ISSUE = "issue";
+  public static final String PARAM_COMMENT = "comment";
   public static final String PARAM_TEXT = "text";
   public static final String PARAM_ASSIGNEE = "assignee";
   public static final String PARAM_TRANSITION = "transition";

sonar-ws/src/test/java/org/sonarqube/ws/client/issue/IssuesServiceTest.java

@@ -85,6 +85,18 @@ public class IssuesServiceTest {
       .andNoOtherParam();
   }
 
+  @Test
+  public void edit_comment() {
+    underTest.editComment(new EditCommentRequest("ABCD", "Please help me to fix this issue"));
+    PostRequest request = serviceTester.getPostRequest();
+
+    assertThat(serviceTester.getPostParser()).isSameAs(Issues.Operation.parser());
+    serviceTester.assertThat(request)
+      .hasParam("comment", "ABCD")
+      .hasParam("text", "Please help me to fix this issue")
+      .andNoOtherParam();
+  }
+
   @Test
   public void set_severity() {
     underTest.setSeverity(new SetSeverityRequest("ABCD", "confirm"));