From 2b90c6d9deb4fafe99ae7c88cdc67d02cd1d2d76 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 4 Jun 2013 15:54:15 +0200 Subject: [PATCH] SONAR-4284 Display a changelog in the Issue detail page --- .../resources/org/sonar/l10n/core.properties | 16 +---- .../SendIssueNotificationsPostJobTest.java | 4 +- .../sonar/core/issue/IssueNotifications.java | 4 +- .../org/sonar/core/issue/IssueUpdater.java | 16 ++--- .../sonar/core/issue/db/IssueChangeDao.java | 34 +++++----- .../org/sonar/core/issue/db/IssueStorage.java | 2 +- .../sonar/core/user/DefaultUserFinder.java | 3 +- .../java/org/sonar/core/user/UserDao.java | 1 + .../core/issue/IssueNotificationsTest.java | 4 +- .../sonar/core/issue/IssueUpdaterTest.java | 54 +++++++-------- .../core/issue/db/IssueChangeDaoTest.java | 27 ++++++-- .../sonar/core/issue/db/IssueStorageTest.java | 2 +- .../java/org/sonar/core/user/UserDaoTest.java | 5 +- .../api/issue/internal/DefaultIssue.java | 23 ++++--- .../sonar/api/issue/internal/FieldDiffs.java | 4 +- .../java/org/sonar/api/user/UserFinder.java | 1 + .../issue/InternalRubyIssueService.java | 23 +++---- .../sonar/server/issue/IssueChangelog.java | 58 +++++++++++++++++ .../server/issue/IssueChangelogService.java | 59 +++++++++++++++++ ...eService.java => IssueCommentService.java} | 12 +--- .../org/sonar/server/platform/Platform.java | 3 +- .../app/controllers/issue_controller.rb | 2 +- .../app/views/issue/_changelog.html.erb | 48 ++++++++++---- .../issue/InternalRubyIssueServiceTest.java | 65 ++++++++++++------- .../issue/IssueChangelogServiceTest.java | 57 ++++++++++++++++ 25 files changed, 373 insertions(+), 154 deletions(-) create mode 100644 sonar-server/src/main/java/org/sonar/server/issue/IssueChangelog.java create mode 100644 sonar-server/src/main/java/org/sonar/server/issue/IssueChangelogService.java rename sonar-server/src/main/java/org/sonar/server/issue/{IssueChangeService.java => IssueCommentService.java} (91%) create mode 100644 sonar-server/src/test/java/org/sonar/server/issue/IssueChangelogServiceTest.java diff --git a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties index e3f1199d772..4355c95aad8 100644 --- a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties +++ b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties @@ -106,6 +106,7 @@ refresh=Refresh remove=Remove rename=Rename reset_verb=Reset +resolution=Resolution result=Result results=Results x_results={0} results @@ -501,7 +502,6 @@ issue.planned_for=Planned for issue.manual.missing_rule=Missing rule issue.manual.no_rules=No rules. issue.reported_by=Reported by -issue.component_deleted=Removed #------------------------------------------------------------------------------ @@ -989,7 +989,7 @@ widget.my_reviews.no_issue=No issue. widget.my_reviews.property.numberOfLines.name=Number of lines widget.my_reviews.property.numberOfLines.desc=Maximum number of issues displayed at the same time. -widget.false_positive_reviews.name=False Positives +widget.false_positive_reviews.name=False Positives Issues widget.false_positive_reviews.description=Shows all the false positives found on the project. widget.false_positive_reviews.property.numberOfLines.name=Number of lines widget.false_positive_reviews.property.numberOfLines.desc=Maximum number of issues displayed at the same time. @@ -1004,7 +1004,7 @@ widget.unresolved_issues_statuses.description=Displays the number of unresolved widget.action_plans.name=Action Plans widget.action_plans.description=Shows all the open action plans of the project. widget.action_plans.property.showResolvedIssues.name=Show Resolved Issues -widget.action_plans.title=Open Action Plans +widget.action_plans.title=Open action plans widget.action_plans.no_action_plan=No action plan widget.action_plans.x_unresolved_issues={0} unresolved issues @@ -2220,16 +2220,6 @@ global_role.profileadmin=Quality Profile Administrators global_role.profileadmin.desc=Ability to perform any action on the quality profiles. -#------------------------------------------------------------------------------ -# -# PROJECTS ROLES -# -#------------------------------------------------------------------------------ -projects_role.role=Role Membership For New {0} -projects_role.users=Users -projects_role.groups=Groups - - #------------------------------------------------------------------------------ # # ERRORS HANDLING diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/SendIssueNotificationsPostJobTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/SendIssueNotificationsPostJobTest.java index 055af97946e..fd9af8ea8ec 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/SendIssueNotificationsPostJobTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/SendIssueNotificationsPostJobTest.java @@ -91,7 +91,7 @@ public class SendIssueNotificationsPostJobTest { Rule rule = new Rule("squid", "AvoidCycles"); DefaultIssue issue = new DefaultIssue() .setChanged(true) - .setFieldDiff(mock(IssueChangeContext.class), "severity", "MINOR", "BLOCKER") + .setFieldChange(mock(IssueChangeContext.class), "severity", "MINOR", "BLOCKER") .setRuleKey(ruleKey); when(issueCache.all()).thenReturn(Arrays.asList(issue)); when(ruleFinder.findByKey(ruleKey)).thenReturn(rule); @@ -110,7 +110,7 @@ public class SendIssueNotificationsPostJobTest { RuleKey ruleKey = RuleKey.of("squid", "AvoidCycles"); DefaultIssue issue = new DefaultIssue() .setChanged(true) - .setFieldDiff(changeContext, "severity", "MINOR", "BLOCKER") + .setFieldChange(changeContext, "severity", "MINOR", "BLOCKER") .setRuleKey(ruleKey); when(issueCache.all()).thenReturn(Arrays.asList(issue)); when(ruleFinder.findByKey(ruleKey)).thenReturn(null); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java index f10e4da4ff6..3d2013e6a41 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java @@ -87,7 +87,7 @@ public class IssueNotifications implements BatchComponent, ServerComponent { @CheckForNull private Notification createChangeNotification(DefaultIssue issue, IssueChangeContext context, Rule rule, Component project, @Nullable Component component, @Nullable String comment) { - if (comment == null && (issue.diffs() == null || issue.diffs().diffs().isEmpty())) { + if (comment == null && (issue.currentChange() == null || issue.currentChange().diffs().isEmpty())) { return null; } Notification notification = newNotification(project, "issue-changes"); @@ -105,7 +105,7 @@ public class IssueNotifications implements BatchComponent, ServerComponent { notification.setFieldValue("comment", comment); } - FieldDiffs diffs = issue.diffs(); + FieldDiffs diffs = issue.currentChange(); if (diffs != null) { for (Map.Entry entry : diffs.diffs().entrySet()) { String type = entry.getKey(); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java index 66bda0c6e28..a91d401e0df 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java @@ -41,7 +41,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { throw new IllegalStateException("Severity can't be changed"); } if (!Objects.equal(severity, issue.severity())) { - issue.setFieldDiff(context, "severity", issue.severity(), severity); + issue.setFieldChange(context, "severity", issue.severity(), severity); issue.setSeverity(severity); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -58,7 +58,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean setManualSeverity(DefaultIssue issue, String severity, IssueChangeContext context) { if (!issue.manualSeverity() || !Objects.equal(severity, issue.severity())) { - issue.setFieldDiff(context, "severity", issue.severity(), severity); + issue.setFieldChange(context, "severity", issue.severity(), severity); issue.setSeverity(severity); issue.setManualSeverity(true); issue.setUpdateDate(context.date()); @@ -71,7 +71,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean assign(DefaultIssue issue, @Nullable String assignee, IssueChangeContext context) { String sanitizedAssignee = StringUtils.defaultIfBlank(assignee, null); if (!Objects.equal(sanitizedAssignee, issue.assignee())) { - issue.setFieldDiff(context, "assignee", issue.assignee(), sanitizedAssignee); + issue.setFieldChange(context, "assignee", issue.assignee(), sanitizedAssignee); issue.setAssignee(sanitizedAssignee); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -97,7 +97,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean setResolution(DefaultIssue issue, @Nullable String resolution, IssueChangeContext context) { if (!Objects.equal(resolution, issue.resolution())) { - issue.setFieldDiff(context, "resolution", issue.resolution(), resolution); + issue.setFieldChange(context, "resolution", issue.resolution(), resolution); issue.setResolution(resolution); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -108,7 +108,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean setStatus(DefaultIssue issue, String status, IssueChangeContext context) { if (!Objects.equal(status, issue.status())) { - issue.setFieldDiff(context, "status", issue.status(), status); + issue.setFieldChange(context, "status", issue.status(), status); issue.setStatus(status); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -119,7 +119,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean setAuthorLogin(DefaultIssue issue, @Nullable String authorLogin, IssueChangeContext context) { if (!Objects.equal(authorLogin, issue.authorLogin())) { - issue.setFieldDiff(context, "author", issue.authorLogin(), authorLogin); + issue.setFieldChange(context, "author", issue.authorLogin(), authorLogin); issue.setAuthorLogin(authorLogin); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -177,7 +177,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean setAttribute(DefaultIssue issue, String key, @Nullable String value, IssueChangeContext context) { String oldValue = issue.attribute(key); if (!Objects.equal(oldValue, value)) { - issue.setFieldDiff(context, key, oldValue, value); + issue.setFieldChange(context, key, oldValue, value); issue.setAttribute(key, value); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -189,7 +189,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean plan(DefaultIssue issue, @Nullable String actionPlanKey, IssueChangeContext context) { String sanitizedActionPlanKey = StringUtils.defaultIfBlank(actionPlanKey, null); if (!Objects.equal(sanitizedActionPlanKey, issue.actionPlanKey())) { - issue.setFieldDiff(context, "actionPlanKey", issue.actionPlanKey(), sanitizedActionPlanKey); + issue.setFieldChange(context, "actionPlanKey", issue.actionPlanKey(), sanitizedActionPlanKey); issue.setActionPlanKey(sanitizedActionPlanKey); issue.setUpdateDate(context.date()); issue.setChanged(true); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java index c06f5d61b8e..3061586f253 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java @@ -26,11 +26,13 @@ import org.apache.ibatis.session.SqlSession; import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; import org.sonar.api.issue.internal.DefaultIssueComment; +import org.sonar.api.issue.internal.FieldDiffs; import org.sonar.core.persistence.MyBatis; import javax.annotation.CheckForNull; - +import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; /** @@ -45,15 +47,21 @@ public class IssueChangeDao implements BatchComponent, ServerComponent { } public List selectCommentsByIssues(SqlSession session, Collection issueKeys) { - return selectByIssuesAndType(session, issueKeys, IssueChangeDto.TYPE_COMMENT); + List comments = Lists.newArrayList(); + for (IssueChangeDto dto : selectByIssuesAndType(session, issueKeys, IssueChangeDto.TYPE_COMMENT)) { + comments.add(dto.toComment()); + } + return comments; } - public List selectIssueChangelog(String issueKey) { + public List selectChangelogByIssue(String issueKey) { SqlSession session = mybatis.openSession(); try { - IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); - return mapper.selectByIssue(issueKey); - + List result = Lists.newArrayList(); + for (IssueChangeDto dto : selectByIssuesAndType(session, Arrays.asList(issueKey), IssueChangeDto.TYPE_FIELD_CHANGE)) { + result.add(dto.toFieldDiffs()); + } + return result; } finally { MyBatis.closeQuietly(session); } @@ -72,17 +80,13 @@ public class IssueChangeDao implements BatchComponent, ServerComponent { } } - List selectByIssuesAndType(SqlSession session, Collection issueKeys, String changeType) { + List selectByIssuesAndType(SqlSession session, Collection issueKeys, String changeType) { Preconditions.checkArgument(issueKeys.size() < 1000, "Number of issue keys is greater than or equal 1000"); - List result = Lists.newArrayList(); - if (!issueKeys.isEmpty()) { - IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); - List dtos = mapper.selectByIssuesAndType(issueKeys, changeType); - for (IssueChangeDto dto : dtos) { - result.add(dto.toComment()); - } + if (issueKeys.isEmpty()) { + return Collections.emptyList(); } - return result; + IssueChangeMapper mapper = session.getMapper(IssueChangeMapper.class); + return mapper.selectByIssuesAndType(issueKeys, changeType); } public boolean delete(String key) { diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java index eddc19c23f7..b8287a83655 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java @@ -88,7 +88,7 @@ public abstract class IssueStorage { mapper.insert(changeDto); } } - FieldDiffs diffs = issue.diffs(); + FieldDiffs diffs = issue.currentChange(); if (diffs != null) { IssueChangeDto changeDto = IssueChangeDto.of(issue.key(), diffs); mapper.insert(changeDto); diff --git a/sonar-core/src/main/java/org/sonar/core/user/DefaultUserFinder.java b/sonar-core/src/main/java/org/sonar/core/user/DefaultUserFinder.java index 9773d17f26e..2ddfb1a76c9 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/DefaultUserFinder.java +++ b/sonar-core/src/main/java/org/sonar/core/user/DefaultUserFinder.java @@ -25,6 +25,7 @@ import org.sonar.api.user.UserFinder; import org.sonar.api.user.UserQuery; import javax.annotation.CheckForNull; +import java.util.Collection; import java.util.List; /** @@ -57,7 +58,7 @@ public class DefaultUserFinder implements UserFinder { return toUsers(dtos); } - private List toUsers(List dtos) { + private List toUsers(Collection dtos) { List users = Lists.newArrayList(); for (UserDto dto : dtos) { users.add(dto.toUser()); diff --git a/sonar-core/src/main/java/org/sonar/core/user/UserDao.java b/sonar-core/src/main/java/org/sonar/core/user/UserDao.java index 01bc6aae388..32b3978e6b0 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/UserDao.java +++ b/sonar-core/src/main/java/org/sonar/core/user/UserDao.java @@ -25,6 +25,7 @@ import org.sonar.api.user.UserQuery; import org.sonar.core.persistence.MyBatis; import javax.annotation.CheckForNull; +import java.util.Collection; import java.util.List; /** diff --git a/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java index eae906010a9..7f14933a985 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java @@ -74,8 +74,8 @@ public class IssueNotificationsTest { .setMessage("the message") .setKey("ABCDE") .setAssignee("freddy") - .setFieldDiff(context, "resolution", null, "FIXED") - .setFieldDiff(context, "status", "OPEN", "RESOLVED") + .setFieldChange(context, "resolution", null, "FIXED") + .setFieldChange(context, "status", "OPEN", "RESOLVED") .setComponentKey("struts:Action") .setProjectKey("struts"); DefaultIssueQueryResult queryResult = new DefaultIssueQueryResult(Arrays.asList(issue)); diff --git a/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java index 6a87cdc8674..ade4e76cbd2 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java @@ -39,7 +39,7 @@ public class IssueUpdaterTest { boolean updated = updater.assign(issue, "emmerik", context); assertThat(updated).isTrue(); assertThat(issue.assignee()).isEqualTo("emmerik"); - FieldDiffs.Diff diff = issue.diffs().get("assignee"); + FieldDiffs.Diff diff = issue.currentChange().get("assignee"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("emmerik"); } @@ -50,7 +50,7 @@ public class IssueUpdaterTest { boolean updated = updater.assign(issue, null, context); assertThat(updated).isTrue(); assertThat(issue.assignee()).isNull(); - FieldDiffs.Diff diff = issue.diffs().get("assignee"); + FieldDiffs.Diff diff = issue.currentChange().get("assignee"); assertThat(diff.oldValue()).isEqualTo("morgan"); assertThat(diff.newValue()).isNull(); } @@ -61,7 +61,7 @@ public class IssueUpdaterTest { boolean updated = updater.assign(issue, "emmerik", context); assertThat(updated).isTrue(); assertThat(issue.assignee()).isEqualTo("emmerik"); - FieldDiffs.Diff diff = issue.diffs().get("assignee"); + FieldDiffs.Diff diff = issue.currentChange().get("assignee"); assertThat(diff.oldValue()).isEqualTo("morgan"); assertThat(diff.newValue()).isEqualTo("emmerik"); } @@ -71,7 +71,7 @@ public class IssueUpdaterTest { issue.setAssignee("morgan"); boolean updated = updater.assign(issue, "morgan", context); assertThat(updated).isFalse(); - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @@ -82,7 +82,7 @@ public class IssueUpdaterTest { assertThat(issue.severity()).isEqualTo("BLOCKER"); assertThat(issue.manualSeverity()).isFalse(); - FieldDiffs.Diff diff = issue.diffs().get("severity"); + FieldDiffs.Diff diff = issue.currentChange().get("severity"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("BLOCKER"); } @@ -94,7 +94,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.severity()).isEqualTo("BLOCKER"); - FieldDiffs.Diff diff = issue.diffs().get("severity"); + FieldDiffs.Diff diff = issue.currentChange().get("severity"); assertThat(diff.oldValue()).isEqualTo("INFO"); assertThat(diff.newValue()).isEqualTo("BLOCKER"); } @@ -106,7 +106,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.severity()).isEqualTo("MINOR"); - FieldDiffs.Diff diff = issue.diffs().get("severity"); + FieldDiffs.Diff diff = issue.currentChange().get("severity"); assertThat(diff.oldValue()).isEqualTo("BLOCKER"); assertThat(diff.newValue()).isEqualTo("MINOR"); } @@ -116,7 +116,7 @@ public class IssueUpdaterTest { issue.setSeverity("MINOR"); boolean updated = updater.setSeverity(issue, "MINOR", context); assertThat(updated).isFalse(); - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @Test @@ -137,7 +137,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.severity()).isEqualTo("MINOR"); assertThat(issue.manualSeverity()).isTrue(); - FieldDiffs.Diff diff = issue.diffs().get("severity"); + FieldDiffs.Diff diff = issue.currentChange().get("severity"); assertThat(diff.oldValue()).isEqualTo("BLOCKER"); assertThat(diff.newValue()).isEqualTo("MINOR"); } @@ -147,7 +147,7 @@ public class IssueUpdaterTest { issue.setSeverity("MINOR").setManualSeverity(true); boolean updated = updater.setManualSeverity(issue, "MINOR", context); assertThat(updated).isFalse(); - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @Test @@ -157,7 +157,7 @@ public class IssueUpdaterTest { assertThat(issue.line()).isEqualTo(123); // do not save change - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @Test @@ -168,7 +168,7 @@ public class IssueUpdaterTest { assertThat(issue.line()).isEqualTo(42); // do not save change - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @Test @@ -177,7 +177,7 @@ public class IssueUpdaterTest { boolean updated = updater.setLine(issue, 123); assertThat(updated).isFalse(); assertThat(issue.line()).isEqualTo(123); - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @Test @@ -186,7 +186,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.resolution()).isEqualTo("OPEN"); - FieldDiffs.Diff diff = issue.diffs().get("resolution"); + FieldDiffs.Diff diff = issue.currentChange().get("resolution"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("OPEN"); } @@ -197,7 +197,7 @@ public class IssueUpdaterTest { boolean updated = updater.setResolution(issue, "FIXED", context); assertThat(updated).isFalse(); assertThat(issue.resolution()).isEqualTo("FIXED"); - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @Test @@ -206,7 +206,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.status()).isEqualTo("OPEN"); - FieldDiffs.Diff diff = issue.diffs().get("status"); + FieldDiffs.Diff diff = issue.currentChange().get("status"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("OPEN"); } @@ -217,7 +217,7 @@ public class IssueUpdaterTest { boolean updated = updater.setStatus(issue, "CLOSED", context); assertThat(updated).isFalse(); assertThat(issue.status()).isEqualTo("CLOSED"); - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @Test @@ -225,9 +225,9 @@ public class IssueUpdaterTest { boolean updated = updater.setAttribute(issue, "JIRA", "FOO-123", context); assertThat(updated).isTrue(); assertThat(issue.attribute("JIRA")).isEqualTo("FOO-123"); - assertThat(issue.diffs().diffs()).hasSize(1); - assertThat(issue.diffs().get("JIRA").oldValue()).isNull(); - assertThat(issue.diffs().get("JIRA").newValue()).isEqualTo("FOO-123"); + assertThat(issue.currentChange().diffs()).hasSize(1); + assertThat(issue.currentChange().get("JIRA").oldValue()).isNull(); + assertThat(issue.currentChange().get("JIRA").newValue()).isEqualTo("FOO-123"); } @Test @@ -236,9 +236,9 @@ public class IssueUpdaterTest { boolean updated = updater.setAttribute(issue, "JIRA", null, context); assertThat(updated).isTrue(); assertThat(issue.attribute("JIRA")).isNull(); - assertThat(issue.diffs().diffs()).hasSize(1); - assertThat(issue.diffs().get("JIRA").oldValue()).isEqualTo("FOO-123"); - assertThat(issue.diffs().get("JIRA").newValue()).isNull(); + assertThat(issue.currentChange().diffs()).hasSize(1); + assertThat(issue.currentChange().get("JIRA").oldValue()).isEqualTo("FOO-123"); + assertThat(issue.currentChange().get("JIRA").newValue()).isNull(); } @Test @@ -254,7 +254,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.actionPlanKey()).isEqualTo("ABCD"); - FieldDiffs.Diff diff = issue.diffs().get("actionPlanKey"); + FieldDiffs.Diff diff = issue.currentChange().get("actionPlanKey"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("ABCD"); } @@ -284,7 +284,7 @@ public class IssueUpdaterTest { assertThat(issue.effortToFix()).isEqualTo(3.14); // do not save change - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @Test @@ -303,7 +303,7 @@ public class IssueUpdaterTest { assertThat(issue.message()).isEqualTo("new message"); // do not save change - assertThat(issue.diffs()).isNull(); + assertThat(issue.currentChange()).isNull(); } @Test @@ -312,7 +312,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.authorLogin()).isEqualTo("eric"); - FieldDiffs.Diff diff = issue.diffs().get("author"); + FieldDiffs.Diff diff = issue.currentChange().get("author"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("eric"); } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java index 21a3f64bd34..9c3bf875728 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java @@ -23,6 +23,7 @@ import org.apache.ibatis.session.SqlSession; import org.junit.Before; import org.junit.Test; import org.sonar.api.issue.internal.DefaultIssueComment; +import org.sonar.api.issue.internal.FieldDiffs; import org.sonar.api.utils.DateUtils; import org.sonar.core.persistence.AbstractDaoTestCase; @@ -63,16 +64,28 @@ public class IssueChangeDaoTest extends AbstractDaoTestCase { } @Test - public void selectIssueChangelog() { + public void selectCommentByKey() { setupData("shared"); - List changes = dao.selectIssueChangelog("1000"); - assertThat(changes).hasSize(3); + DefaultIssueComment comment = dao.selectCommentByKey("FGHIJ"); + assertThat(comment).isNotNull(); + assertThat(comment.key()).isEqualTo("FGHIJ"); + assertThat(comment.key()).isEqualTo("FGHIJ"); + assertThat(comment.userLogin()).isEqualTo("arthur"); - // chronological order - assertThat(changes.get(0).getId()).isEqualTo(100); - assertThat(changes.get(1).getId()).isEqualTo(101); - assertThat(changes.get(2).getId()).isEqualTo(102); + assertThat(dao.selectCommentByKey("UNKNOWN")).isNull(); + } + + + @Test + public void selectIssueChangelog() { + setupData("shared"); + + List changelog = dao.selectChangelogByIssue("1000"); + assertThat(changelog).hasSize(1); + assertThat(changelog.get(0).diffs()).hasSize(1); + assertThat(changelog.get(0).diffs().get("severity").newValue()).isEqualTo("BLOCKER"); + assertThat(changelog.get(0).diffs().get("severity").oldValue()).isEqualTo("MAJOR"); } @Test diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java index b65d623e911..b8744185375 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java @@ -89,7 +89,7 @@ public class IssueStorageTest extends AbstractDaoTestCase { .setChecksum("FFFFF") .setAuthorLogin("simon") .setAssignee("loic") - .setFieldDiff(context, "severity", "INFO", "BLOCKER") + .setFieldChange(context, "severity", "INFO", "BLOCKER") .setReporter("emmerik") .setResolution("FIXED") .setStatus("RESOLVED") diff --git a/sonar-core/src/test/java/org/sonar/core/user/UserDaoTest.java b/sonar-core/src/test/java/org/sonar/core/user/UserDaoTest.java index 186b20eaf56..3df4e1d875e 100644 --- a/sonar-core/src/test/java/org/sonar/core/user/UserDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/user/UserDaoTest.java @@ -25,6 +25,7 @@ import org.sonar.api.user.UserQuery; import org.sonar.core.persistence.AbstractDaoTestCase; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.List; @@ -60,7 +61,7 @@ public class UserDaoTest extends AbstractDaoTestCase { public void selectUsersByLogins() throws Exception { setupData("selectUsersByLogins"); - List users = dao.selectUsersByLogins(Arrays.asList("marius", "inactive_user", "other")); + Collection users = dao.selectUsersByLogins(Arrays.asList("marius", "inactive_user", "other")); assertThat(users).hasSize(2); assertThat(users).onProperty("login").containsOnly("marius", "inactive_user"); } @@ -68,7 +69,7 @@ public class UserDaoTest extends AbstractDaoTestCase { @Test public void selectUsersByLogins_empty_logins() throws Exception { // no need to access db - List users = dao.selectUsersByLogins(Collections.emptyList()); + Collection users = dao.selectUsersByLogins(Collections.emptyList()); assertThat(users).isEmpty(); } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/DefaultIssue.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/DefaultIssue.java index 11a9e67dcb7..e594b754145 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/DefaultIssue.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/DefaultIssue.java @@ -63,16 +63,19 @@ public class DefaultIssue implements Issue { private String checksum; private Map attributes = null; private String authorLogin = null; - private FieldDiffs diffs = null; private String actionPlanKey; private List comments = null; - // functional dates + // FUNCTIONAL DATES private Date creationDate; private Date updateDate; private Date closeDate; - // The following states are used only during scan. + + // FOLLOWING FIELDS ARE AVAILABLE ONLY DURING SCAN + + // Current changes + private FieldDiffs currentChange = null; // true if the the issue did not exist in the previous scan. private boolean isNew = true; @@ -348,20 +351,20 @@ public class DefaultIssue implements Issue { return this; } - public DefaultIssue setFieldDiff(IssueChangeContext context, String field, @Nullable Serializable oldValue, @Nullable Serializable newValue) { + public DefaultIssue setFieldChange(IssueChangeContext context, String field, @Nullable Serializable oldValue, @Nullable Serializable newValue) { if (!Objects.equal(oldValue, newValue)) { - if (diffs == null) { - diffs = new FieldDiffs(); - diffs.setUserLogin(context.login()); + if (currentChange == null) { + currentChange = new FieldDiffs(); + currentChange.setUserLogin(context.login()); } - diffs.setDiff(field, oldValue, newValue); + currentChange.setDiff(field, oldValue, newValue); } return this; } @CheckForNull - public FieldDiffs diffs() { - return diffs; + public FieldDiffs currentChange() { + return currentChange; } public DefaultIssue addComment(DefaultIssueComment comment) { diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/FieldDiffs.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/FieldDiffs.java index 5479bfb4392..b0d44ee6bf9 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/FieldDiffs.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/FieldDiffs.java @@ -25,7 +25,6 @@ import com.google.common.collect.Maps; import javax.annotation.CheckForNull; import javax.annotation.Nullable; - import java.io.Serializable; import java.util.Date; import java.util.Map; @@ -79,7 +78,7 @@ public class FieldDiffs implements Serializable { @SuppressWarnings("unchecked") - public void setDiff(String field, @Nullable Serializable oldValue, @Nullable Serializable newValue) { + public FieldDiffs setDiff(String field, @Nullable Serializable oldValue, @Nullable Serializable newValue) { Diff diff = diffs.get(field); if (diff == null) { diff = new Diff(oldValue, newValue); @@ -87,6 +86,7 @@ public class FieldDiffs implements Serializable { } else { diff.setNewValue(newValue); } + return this; } @Override diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/user/UserFinder.java b/sonar-plugin-api/src/main/java/org/sonar/api/user/UserFinder.java index 66234bb177d..6bf2c76017c 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/user/UserFinder.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/user/UserFinder.java @@ -22,6 +22,7 @@ package org.sonar.api.user; import org.sonar.api.ServerComponent; import javax.annotation.CheckForNull; +import java.util.Collection; import java.util.List; /** diff --git a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index f8ad8068b62..87533a774ab 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -36,7 +36,6 @@ import org.sonar.api.utils.SonarException; import org.sonar.core.issue.ActionPlanStats; import org.sonar.core.issue.DefaultActionPlan; import org.sonar.core.issue.DefaultIssueBuilder; -import org.sonar.core.issue.db.IssueChangeDto; import org.sonar.core.issue.workflow.Transition; import org.sonar.core.resource.ResourceDao; import org.sonar.core.resource.ResourceDto; @@ -57,18 +56,20 @@ import java.util.Map; public class InternalRubyIssueService implements ServerComponent { private final IssueService issueService; - private final IssueChangeService changeService; + private final IssueCommentService commentService; + private final IssueChangelogService changelogService; private final ActionPlanService actionPlanService; private final IssueStatsFinder issueStatsFinder; private final ResourceDao resourceDao; private final ActionService actionService; public InternalRubyIssueService(IssueService issueService, - IssueChangeService changeService, - ActionPlanService actionPlanService, + IssueCommentService commentService, + IssueChangelogService changelogService, ActionPlanService actionPlanService, IssueStatsFinder issueStatsFinder, ResourceDao resourceDao, ActionService actionService) { this.issueService = issueService; - this.changeService = changeService; + this.commentService = commentService; + this.changelogService = changelogService; this.actionPlanService = actionPlanService; this.issueStatsFinder = issueStatsFinder; this.resourceDao = resourceDao; @@ -95,9 +96,9 @@ public class InternalRubyIssueService implements ServerComponent { return Issue.RESOLUTIONS; } - public List changelog(String issueKey) { + public IssueChangelog changelog(String issueKey) { // TODO verify security - return changeService.changelog(issueKey); + return changelogService.changelog(issueKey, UserSession.get()); } public Result doTransition(String issueKey, String transitionKey) { @@ -143,7 +144,7 @@ public class InternalRubyIssueService implements ServerComponent { public Result addComment(String issueKey, String text) { Result result = Result.of(); try { - result.set(changeService.addComment(issueKey, text, UserSession.get())); + result.set(commentService.addComment(issueKey, text, UserSession.get())); } catch (Exception e) { result.addError(e.getMessage()); } @@ -151,15 +152,15 @@ public class InternalRubyIssueService implements ServerComponent { } public IssueComment deleteComment(String commentKey) { - return changeService.deleteComment(commentKey, UserSession.get()); + return commentService.deleteComment(commentKey, UserSession.get()); } public IssueComment editComment(String commentKey, String newText) { - return changeService.editComment(commentKey, newText, UserSession.get()); + return commentService.editComment(commentKey, newText, UserSession.get()); } public IssueComment findComment(String commentKey) { - return changeService.findComment(commentKey); + return commentService.findComment(commentKey); } /** diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelog.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelog.java new file mode 100644 index 00000000000..5eb1e1aeb9b --- /dev/null +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelog.java @@ -0,0 +1,58 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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; + +import com.google.common.collect.Maps; +import org.sonar.api.issue.internal.FieldDiffs; +import org.sonar.api.user.User; + +import javax.annotation.CheckForNull; +import java.util.Collection; +import java.util.List; +import java.util.Map; + +/** + * @since 3.6 + */ +public class IssueChangelog { + + private final List changes; + private final Map usersByLogin; + + public IssueChangelog(List changes, Collection users) { + this.changes = changes; + this.usersByLogin = Maps.newHashMap(); + for (User user : users) { + usersByLogin.put(user.login(), user); + } + } + + public List changes() { + return changes; + } + + @CheckForNull + public User user(FieldDiffs change) { + if (change.userLogin() == null) { + return null; + } + return usersByLogin.get(change.userLogin()); + } +} diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelogService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelogService.java new file mode 100644 index 00000000000..3f9101e226c --- /dev/null +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelogService.java @@ -0,0 +1,59 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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; + +import com.google.common.collect.Lists; +import org.sonar.api.ServerComponent; +import org.sonar.api.issue.internal.FieldDiffs; +import org.sonar.api.user.User; +import org.sonar.api.user.UserFinder; +import org.sonar.core.issue.db.IssueChangeDao; +import org.sonar.server.user.UserSession; + +import java.util.Collection; +import java.util.List; + +public class IssueChangelogService implements ServerComponent { + + private final IssueChangeDao changeDao; + private final UserFinder userFinder; + + public IssueChangelogService(IssueChangeDao changeDao, UserFinder userFinder) { + this.changeDao = changeDao; + this.userFinder = userFinder; + } + + public IssueChangelog changelog(String issueKey, UserSession userSession) { + // TODO verify security + List changes = changeDao.selectChangelogByIssue(issueKey); + + // Load users + List logins = Lists.newArrayList(); + for (FieldDiffs change : changes) { + if (change.userLogin() != null) { + logins.add(change.userLogin()); + } + } + Collection users = userFinder.findByLogins(logins); + + return new IssueChangelog(changes, users); + + } +} diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueChangeService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java similarity index 91% rename from sonar-server/src/main/java/org/sonar/server/issue/IssueChangeService.java rename to sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java index 3d73cbdea8c..ba8962c757f 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/IssueChangeService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java @@ -38,9 +38,8 @@ import org.sonar.server.user.UserSession; import java.util.Arrays; import java.util.Date; -import java.util.List; -public class IssueChangeService implements ServerComponent { +public class IssueCommentService implements ServerComponent { private final IssueUpdater updater; private final IssueChangeDao changeDao; @@ -48,7 +47,7 @@ public class IssueChangeService implements ServerComponent { private final DefaultIssueFinder finder; private final IssueNotifications issueNotifications; - public IssueChangeService(IssueUpdater updater, IssueChangeDao changeDao, IssueStorage storage, DefaultIssueFinder finder, IssueNotifications issueNotifications) { + public IssueCommentService(IssueUpdater updater, IssueChangeDao changeDao, IssueStorage storage, DefaultIssueFinder finder, IssueNotifications issueNotifications) { this.updater = updater; this.changeDao = changeDao; this.storage = storage; @@ -56,11 +55,6 @@ public class IssueChangeService implements ServerComponent { this.issueNotifications = issueNotifications; } - public List changelog(String issueKey) { - // TODO verify security - return changeDao.selectIssueChangelog(issueKey); - } - public IssueComment findComment(String commentKey) { return changeDao.selectCommentByKey(commentKey); } @@ -129,7 +123,7 @@ public class IssueChangeService implements ServerComponent { public IssueQueryResult loadIssue(String issueKey) { IssueQuery query = IssueQuery.builder().issueKeys(Arrays.asList(issueKey)).requiredRole(UserRole.USER).build(); IssueQueryResult result = finder.find(query); - if (result.issues().size()!=1) { + if (result.issues().size() != 1) { throw new IllegalStateException("Issue not found: " + issueKey); } return result; diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java index aaf8182ead1..8a52b6f8124 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java @@ -263,12 +263,13 @@ public final class Platform { servicesContainer.addSingleton(FunctionExecutor.class); servicesContainer.addSingleton(IssueWorkflow.class); servicesContainer.addSingleton(IssueService.class); - servicesContainer.addSingleton(IssueChangeService.class); + servicesContainer.addSingleton(IssueCommentService.class); servicesContainer.addSingleton(DefaultIssueFinder.class); servicesContainer.addSingleton(IssueStatsFinder.class); servicesContainer.addSingleton(PublicRubyIssueService.class); servicesContainer.addSingleton(InternalRubyIssueService.class); servicesContainer.addSingleton(ActionPlanService.class); + servicesContainer.addSingleton(IssueChangelogService.class); servicesContainer.addSingleton(IssueNotifications.class); servicesContainer.addSingleton(ActionService.class); servicesContainer.addSingleton(Actions.class); diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb index 980c1bee0cd..bc5ae43b029 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb @@ -193,7 +193,7 @@ class IssueController < ApplicationController require_parameters :id @issue_results = Api.issues.find(params[:id]) @issue = @issue_results.first() - @changes = Internal.issues.changelog(params[:id]) + @changelog = Internal.issues.changelog(params[:id]) render :partial => 'issue/changelog' end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_changelog.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_changelog.html.erb index 421d203caad..26c12118c09 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_changelog.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_changelog.html.erb @@ -1,15 +1,35 @@ -
    -
  • - <%= format_datetime(@issue.creationDate()) -%> - <% if @issue.reporter %> - - <%= message('issue.reported_by') -%> <%= @issue_results.user(@issue.reporter).name -%> - <% end %> - - <%= message('created') -%> -
    • - - <% @changes.each do |change| %> -
  • - <%= format_datetime(change.createdAt()) -%> - <%= change.userLogin -%> - <%= change.changeData -%> -
    • + + + + + + + <% + @changelog.changes.each do |change| + user = @changelog.user(change) + %> + + + + + <% end %> - \ No newline at end of file +
    <%= format_datetime(@issue.creationDate()) -%><%= @issue_results.user(@issue.reporter).name if @issue.reporter -%><%= message('created') -%>
    <%= format_datetime(change.createdAt()) -%><%= h(user.name()) if user -%> + <% + change.diffs.entrySet().each_with_index do |entry, index| + key = entry.getKey() + diff = entry.getValue() + %> + <% if index>0 %>
    <% end %> + <% if diff.newValue.present? %> + <%= message('issue.changelog.changed_to', :params => [message(key), diff.newValue()]) -%> + <% else %> + <%= message('issue.changelog.removed', :params => [message(key)]) -%> + <% end %> + <% if diff.oldValue.present? %> + (<%= message('issue.changelog.was', :params => [diff.oldValue]) -%>) + <% end %> + <% end %> +
    + + diff --git a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java index 7c3b38d610e..93cf0d33881 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java @@ -24,12 +24,15 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.sonar.api.issue.ActionPlan; +import org.sonar.api.issue.internal.FieldDiffs; +import org.sonar.api.user.User; import org.sonar.core.issue.DefaultActionPlan; import org.sonar.core.resource.ResourceDao; import org.sonar.core.resource.ResourceDto; import org.sonar.core.resource.ResourceQuery; import org.sonar.server.user.UserSession; +import java.util.Collections; import java.util.Map; import static com.google.common.collect.Maps.newHashMap; @@ -39,19 +42,20 @@ import static org.mockito.Mockito.*; public class InternalRubyIssueServiceTest { - private InternalRubyIssueService internalRubyIssueService; - private IssueService issueService = mock(IssueService.class); - private IssueChangeService commentService = mock(IssueChangeService.class); - private ActionPlanService actionPlanService = mock(ActionPlanService.class); - private ResourceDao resourceDao = mock(ResourceDao.class); - private IssueStatsFinder issueStatsFinder = mock(IssueStatsFinder.class); - private ActionService actionService = mock(ActionService.class); + InternalRubyIssueService service; + IssueService issueService = mock(IssueService.class); + IssueCommentService commentService = mock(IssueCommentService.class); + IssueChangelogService changelogService = mock(IssueChangelogService.class); + ActionPlanService actionPlanService = mock(ActionPlanService.class); + ResourceDao resourceDao = mock(ResourceDao.class); + IssueStatsFinder issueStatsFinder = mock(IssueStatsFinder.class); + ActionService actionService = mock(ActionService.class); @Before - public void before() { + public void setUp() { ResourceDto project = new ResourceDto().setKey("org.sonar.Sample"); when(resourceDao.getResource(any(ResourceQuery.class))).thenReturn(project); - internalRubyIssueService = new InternalRubyIssueService(issueService, commentService, actionPlanService, issueStatsFinder, resourceDao, actionService); + service = new InternalRubyIssueService(issueService, commentService, changelogService, actionPlanService, issueStatsFinder, resourceDao, actionService); } @Test @@ -63,7 +67,7 @@ public class InternalRubyIssueServiceTest { parameters.put("deadLine", "2113-05-13"); ArgumentCaptor actionPlanCaptor = ArgumentCaptor.forClass(ActionPlan.class); - Result result = internalRubyIssueService.createActionPlan(parameters); + Result result = service.createActionPlan(parameters); assertThat(result.ok()).isTrue(); verify(actionPlanService).create(actionPlanCaptor.capture(), any(UserSession.class)); @@ -87,7 +91,7 @@ public class InternalRubyIssueServiceTest { parameters.put("project", "org.sonar.MultiSample"); ArgumentCaptor actionPlanCaptor = ArgumentCaptor.forClass(ActionPlan.class); - Result result = internalRubyIssueService.updateActionPlan("ABCD", parameters); + Result result = service.updateActionPlan("ABCD", parameters); assertThat(result.ok()).isTrue(); verify(actionPlanService).update(actionPlanCaptor.capture(), any(UserSession.class)); @@ -110,7 +114,7 @@ public class InternalRubyIssueServiceTest { parameters.put("deadLine", "2113-05-13"); ArgumentCaptor actionPlanCaptor = ArgumentCaptor.forClass(ActionPlan.class); - Result result = internalRubyIssueService.updateActionPlan("ABCD", parameters); + Result result = service.updateActionPlan("ABCD", parameters); assertThat(result.ok()).isTrue(); verify(actionPlanService).update(actionPlanCaptor.capture(), any(UserSession.class)); @@ -128,7 +132,7 @@ public class InternalRubyIssueServiceTest { public void should_not_update_action_plan_when_action_plan_is_not_found() { when(actionPlanService.findByKey(eq("ABCD"), any(UserSession.class))).thenReturn(null); - Result result = internalRubyIssueService.updateActionPlan("ABCD", null); + Result result = service.updateActionPlan("ABCD", null); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("action_plans.errors.action_plan_does_not_exist", "ABCD")); } @@ -137,7 +141,7 @@ public class InternalRubyIssueServiceTest { public void should_delete_action_plan() { when(actionPlanService.findByKey(eq("ABCD"), any(UserSession.class))).thenReturn(DefaultActionPlan.create("Long term")); - Result result = internalRubyIssueService.deleteActionPlan("ABCD"); + Result result = service.deleteActionPlan("ABCD"); verify(actionPlanService).delete(eq("ABCD"), any(UserSession.class)); assertThat(result.ok()).isTrue(); } @@ -146,7 +150,7 @@ public class InternalRubyIssueServiceTest { public void should_not_delete_action_plan_if_action_plan_not_found() { when(actionPlanService.findByKey(eq("ABCD"), any(UserSession.class))).thenReturn(null); - Result result = internalRubyIssueService.deleteActionPlan("ABCD"); + Result result = service.deleteActionPlan("ABCD"); verify(actionPlanService, never()).delete(eq("ABCD"), any(UserSession.class)); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("action_plans.errors.action_plan_does_not_exist", "ABCD")); @@ -156,7 +160,7 @@ public class InternalRubyIssueServiceTest { public void should_close_action_plan() { when(actionPlanService.findByKey(eq("ABCD"), any(UserSession.class))).thenReturn(DefaultActionPlan.create("Long term")); - Result result = internalRubyIssueService.closeActionPlan("ABCD"); + Result result = service.closeActionPlan("ABCD"); verify(actionPlanService).setStatus(eq("ABCD"), eq("CLOSED"), any(UserSession.class)); assertThat(result.ok()).isTrue(); } @@ -165,7 +169,7 @@ public class InternalRubyIssueServiceTest { public void should_open_action_plan() { when(actionPlanService.findByKey(eq("ABCD"), any(UserSession.class))).thenReturn(DefaultActionPlan.create("Long term")); - Result result = internalRubyIssueService.openActionPlan("ABCD"); + Result result = service.openActionPlan("ABCD"); verify(actionPlanService).setStatus(eq("ABCD"), eq("OPEN"), any(UserSession.class)); assertThat(result.ok()).isTrue(); } @@ -176,7 +180,7 @@ public class InternalRubyIssueServiceTest { parameters.put("name", "Long term"); parameters.put("description", "Long term issues"); - Result result = internalRubyIssueService.createActionPlanResult(parameters); + Result result = service.createActionPlanResult(parameters); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("errors.cant_be_empty", "project")); } @@ -188,7 +192,7 @@ public class InternalRubyIssueServiceTest { parameters.put("description", "Long term issues"); parameters.put("project", "org.sonar.Sample"); - Result result = internalRubyIssueService.createActionPlanResult(parameters); + Result result = service.createActionPlanResult(parameters); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("errors.cant_be_empty", "name")); } @@ -200,7 +204,7 @@ public class InternalRubyIssueServiceTest { parameters.put("description", "Long term issues"); parameters.put("project", "org.sonar.Sample"); - Result result = internalRubyIssueService.createActionPlanResult(parameters); + Result result = service.createActionPlanResult(parameters); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("errors.is_too_long", "name", 200)); } @@ -212,7 +216,7 @@ public class InternalRubyIssueServiceTest { parameters.put("description", createLongString(1001)); parameters.put("project", "org.sonar.Sample"); - Result result = internalRubyIssueService.createActionPlanResult(parameters); + Result result = service.createActionPlanResult(parameters); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("errors.is_too_long", "description", 1000)); } @@ -225,7 +229,7 @@ public class InternalRubyIssueServiceTest { parameters.put("project", "org.sonar.Sample"); parameters.put("deadLine", "18/05/2013"); - Result result = internalRubyIssueService.createActionPlanResult(parameters); + Result result = service.createActionPlanResult(parameters); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("errors.is_not_valid", "date")); } @@ -238,7 +242,7 @@ public class InternalRubyIssueServiceTest { parameters.put("project", "org.sonar.Sample"); parameters.put("deadLine", "2000-01-01"); - Result result = internalRubyIssueService.createActionPlanResult(parameters); + Result result = service.createActionPlanResult(parameters); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("action_plans.date_cant_be_in_past")); } @@ -252,7 +256,7 @@ public class InternalRubyIssueServiceTest { when(actionPlanService.isNameAlreadyUsedForProject(anyString(), anyString())).thenReturn(true); - Result result = internalRubyIssueService.createActionPlanResult(parameters, DefaultActionPlan.create("Short term")); + Result result = service.createActionPlanResult(parameters, DefaultActionPlan.create("Short term")); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("action_plans.same_name_in_same_project")); } @@ -266,10 +270,11 @@ public class InternalRubyIssueServiceTest { when(resourceDao.getResource(any(ResourceQuery.class))).thenReturn(null); - Result result = internalRubyIssueService.createActionPlanResult(parameters); + Result result = service.createActionPlanResult(parameters); assertThat(result.ok()).isFalse(); assertThat(result.errors()).contains(Result.Message.ofL10n("action_plans.errors.project_does_not_exist", "org.sonar.Sample")); } + public String createLongString(int size) { String result = ""; for (int i = 0; i < size; i++) { @@ -277,4 +282,14 @@ public class InternalRubyIssueServiceTest { } return result; } + + @Test + public void test_changelog() throws Exception { + IssueChangelog changelog = new IssueChangelog(Collections.emptyList(), Collections.emptyList()); + when(changelogService.changelog(eq("ABCDE"), any(UserSession.class))).thenReturn(changelog); + + IssueChangelog result = service.changelog("ABCDE"); + + assertThat(result).isSameAs(changelog); + } } diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueChangelogServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueChangelogServiceTest.java new file mode 100644 index 00000000000..0e517c8ea0f --- /dev/null +++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueChangelogServiceTest.java @@ -0,0 +1,57 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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; + +import org.junit.Test; +import org.sonar.api.issue.internal.FieldDiffs; +import org.sonar.api.user.User; +import org.sonar.api.user.UserFinder; +import org.sonar.core.issue.db.IssueChangeDao; +import org.sonar.core.user.DefaultUser; +import org.sonar.server.user.UserSession; + +import java.util.Arrays; + +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class IssueChangelogServiceTest { + + IssueChangeDao changeDao = mock(IssueChangeDao.class); + UserFinder userFinder = mock(UserFinder.class); + IssueChangelogService service = new IssueChangelogService(changeDao, userFinder); + + @Test + public void should_load_changelog_and_related_users() throws Exception { + FieldDiffs userChange = new FieldDiffs().setUserLogin("arthur").setDiff("severity", "MAJOR", "BLOCKER"); + FieldDiffs scanChange = new FieldDiffs().setDiff("status", "RESOLVED", "CLOSED"); + when(changeDao.selectChangelogByIssue("ABCDE")).thenReturn(Arrays.asList(userChange, scanChange)); + User arthur = new DefaultUser().setLogin("arthur").setName("Arthur"); + when(userFinder.findByLogins(Arrays.asList("arthur"))).thenReturn(Arrays.asList(arthur)); + + IssueChangelog changelog = service.changelog("ABCDE", mock(UserSession.class)); + + assertThat(changelog).isNotNull(); + assertThat(changelog.changes()).containsOnly(userChange, scanChange); + assertThat(changelog.user(scanChange)).isNull(); + assertThat(changelog.user(userChange)).isSameAs(arthur); + } +} -- 2.39.5