From 4b25524c12d74e62d3ad0e02cc6a39a628035491 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 21 Aug 2013 14:19:40 +0200 Subject: [PATCH] SONAR-2986 Revert preventing tracking profile's change on version 1 and display changelog from version 2 in the console --- .../resources/org/sonar/l10n/core.properties | 2 +- .../server/configuration/ProfilesManager.java | 112 ++++++++---------- .../app/controllers/profiles_controller.rb | 19 ++- .../server/configuration/RuleChangeTest.java | 19 --- ..._not_used_first_version_profile-result.xml | 5 - ...tion_on_not_used_first_version_profile.xml | 14 --- ...n_on_used_first_version_profile-result.xml | 5 - ...tivation_on_used_first_version_profile.xml | 14 --- 8 files changed, 65 insertions(+), 125 deletions(-) delete mode 100644 sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_not_track_rule_activation_on_not_used_first_version_profile-result.xml delete mode 100644 sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_not_track_rule_activation_on_not_used_first_version_profile.xml delete mode 100644 sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_track_rule_activation_on_used_first_version_profile-result.xml delete mode 100644 sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_track_rule_activation_on_used_first_version_profile.xml 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 abe9f1aae1e..cd4a4ca5da5 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 @@ -1419,8 +1419,8 @@ quality_profiles.first_use_without_change=No changes have occurred since first u quality_profiles.changelog_from=Changelog from quality_profiles.no_version=no version quality_profiles.last_version_x_with_date=last version {0} ({1}) -quality_profiles.version_1=version 1 quality_profiles.version_x_with_date=version {0} ({1}) +quality_profiles.version_x=version {0} quality_profiles.profile_version=Profile version quality_profiles.severity_changed_from_x_to=Severity changed from {0}{1} to quality_profiles.severity_was_x=Severity was {0}{1} diff --git a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java index 9aa9c6ccbb9..e16bd2e6d67 100644 --- a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java +++ b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java @@ -228,21 +228,15 @@ public class ProfilesManager extends BaseDao { } } - private synchronized boolean shouldTrackChanges(RulesProfile profile) { - return profile.getVersion() > 1 || profile.getVersion() == 1 && profile.getUsed(); - } - /** * Deal with creation of ActiveRuleChange item when a rule param is changed on a profile */ private void ruleParamChanged(RulesProfile profile, Rule rule, String paramKey, String oldValue, String newValue, String userName) { - if (shouldTrackChanges(profile)) { - incrementProfileVersionIfNeeded(profile); - ActiveRuleChange rc = new ActiveRuleChange(userName, profile, rule); - if (!StringUtils.equals(oldValue, newValue)) { - rc.setParameterChange(paramKey, oldValue, newValue); - getSession().saveWithoutFlush(rc); - } + incrementProfileVersionIfNeeded(profile); + ActiveRuleChange rc = new ActiveRuleChange(userName, profile, rule); + if (!StringUtils.equals(oldValue, newValue)) { + rc.setParameterChange(paramKey, oldValue, newValue); + getSession().saveWithoutFlush(rc); } } @@ -250,14 +244,12 @@ public class ProfilesManager extends BaseDao { * Deal with creation of ActiveRuleChange item when a rule severity is changed on a profile */ private void ruleSeverityChanged(RulesProfile profile, Rule rule, RulePriority oldSeverity, RulePriority newSeverity, String userName) { - if (shouldTrackChanges(profile)) { - incrementProfileVersionIfNeeded(profile); - ActiveRuleChange rc = new ActiveRuleChange(userName, profile, rule); - if (!ObjectUtils.equals(oldSeverity, newSeverity)) { - rc.setOldSeverity(oldSeverity); - rc.setNewSeverity(newSeverity); - getSession().saveWithoutFlush(rc); - } + incrementProfileVersionIfNeeded(profile); + ActiveRuleChange rc = new ActiveRuleChange(userName, profile, rule); + if (!ObjectUtils.equals(oldSeverity, newSeverity)) { + rc.setOldSeverity(oldSeverity); + rc.setNewSeverity(newSeverity); + getSession().saveWithoutFlush(rc); } } @@ -265,68 +257,62 @@ public class ProfilesManager extends BaseDao { * Deal with creation of ActiveRuleChange item when a rule is changed (severity and/or param(s)) on a profile */ private void ruleChanged(RulesProfile profile, ActiveRule oldActiveRule, ActiveRule newActiveRule, String userName) { - if (shouldTrackChanges(profile)) { - incrementProfileVersionIfNeeded(profile); - ActiveRuleChange rc = new ActiveRuleChange(userName, profile, newActiveRule.getRule()); + incrementProfileVersionIfNeeded(profile); + ActiveRuleChange rc = new ActiveRuleChange(userName, profile, newActiveRule.getRule()); - if (oldActiveRule.getSeverity() != newActiveRule.getSeverity()) { - rc.setOldSeverity(oldActiveRule.getSeverity()); - rc.setNewSeverity(newActiveRule.getSeverity()); - } - if (oldActiveRule.getRule().getParams() != null) { - for (RuleParam p : oldActiveRule.getRule().getParams()) { - String oldParam = oldActiveRule.getParameter(p.getKey()); - String newParam = newActiveRule.getParameter(p.getKey()); - if (!StringUtils.equals(oldParam, newParam)) { - rc.setParameterChange(p.getKey(), oldParam, newParam); - } + if (oldActiveRule.getSeverity() != newActiveRule.getSeverity()) { + rc.setOldSeverity(oldActiveRule.getSeverity()); + rc.setNewSeverity(newActiveRule.getSeverity()); + } + if (oldActiveRule.getRule().getParams() != null) { + for (RuleParam p : oldActiveRule.getRule().getParams()) { + String oldParam = oldActiveRule.getParameter(p.getKey()); + String newParam = newActiveRule.getParameter(p.getKey()); + if (!StringUtils.equals(oldParam, newParam)) { + rc.setParameterChange(p.getKey(), oldParam, newParam); } } - - getSession().saveWithoutFlush(rc); } + + getSession().saveWithoutFlush(rc); } /** * Deal with creation of ActiveRuleChange item when a rule is enabled on a profile */ private void ruleEnabled(RulesProfile profile, ActiveRule newActiveRule, String userName) { - if (shouldTrackChanges(profile)) { - incrementProfileVersionIfNeeded(profile); - ActiveRuleChange rc = new ActiveRuleChange(userName, profile, newActiveRule.getRule()); - rc.setEnabled(true); - rc.setNewSeverity(newActiveRule.getSeverity()); - if (newActiveRule.getRule().getParams() != null) { - for (RuleParam p : newActiveRule.getRule().getParams()) { - String newParam = newActiveRule.getParameter(p.getKey()); - if (newParam != null) { - rc.setParameterChange(p.getKey(), null, newParam); - } + incrementProfileVersionIfNeeded(profile); + ActiveRuleChange rc = new ActiveRuleChange(userName, profile, newActiveRule.getRule()); + rc.setEnabled(true); + rc.setNewSeverity(newActiveRule.getSeverity()); + if (newActiveRule.getRule().getParams() != null) { + for (RuleParam p : newActiveRule.getRule().getParams()) { + String newParam = newActiveRule.getParameter(p.getKey()); + if (newParam != null) { + rc.setParameterChange(p.getKey(), null, newParam); } } - getSession().saveWithoutFlush(rc); } + getSession().saveWithoutFlush(rc); } /** * Deal with creation of ActiveRuleChange item when a rule is disabled on a profile */ private void ruleDisabled(RulesProfile profile, ActiveRule disabledRule, String userName) { - if (shouldTrackChanges(profile)) { - incrementProfileVersionIfNeeded(profile); - ActiveRuleChange rc = new ActiveRuleChange(userName, profile, disabledRule.getRule()); - rc.setEnabled(false); - rc.setOldSeverity(disabledRule.getSeverity()); - if (disabledRule.getRule().getParams() != null) { - for (RuleParam p : disabledRule.getRule().getParams()) { - String oldParam = disabledRule.getParameter(p.getKey()); - if (oldParam != null) { - rc.setParameterChange(p.getKey(), oldParam, null); - } + incrementProfileVersionIfNeeded(profile); + ActiveRuleChange rc = new ActiveRuleChange(userName, profile, disabledRule.getRule()); + rc.setEnabled(false); + rc.setOldSeverity(disabledRule.getSeverity()); + if (disabledRule.getRule().getParams() != null) { + for (RuleParam p : disabledRule.getRule().getParams()) { + String oldParam = disabledRule.getParameter(p.getKey()); + if (oldParam != null) { + rc.setParameterChange(p.getKey(), oldParam, null); } } - getSession().saveWithoutFlush(rc); } + getSession().saveWithoutFlush(rc); } private void activateOrChange(RulesProfile profile, ActiveRule parentActiveRule, String userName) { @@ -382,8 +368,8 @@ public class ProfilesManager extends BaseDao { private List getChildren(RulesProfile parent) { return getSession().getResults(RulesProfile.class, - "language", parent.getLanguage(), - "parentName", parent.getName()); + "language", parent.getLanguage(), + "parentName", parent.getName()); } private void removeActiveRule(RulesProfile profile, ActiveRule activeRule) { @@ -393,8 +379,8 @@ public class ProfilesManager extends BaseDao { RulesProfile getProfile(String language, String name) { return getSession().getSingleResult(RulesProfile.class, - "language", language, - "name", name); + "language", language, + "name", name); } RulesProfile getParentProfile(RulesProfile profile) { diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb index acbf1540eed..20ccaa80549 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb @@ -199,9 +199,13 @@ class ProfilesController < ApplicationController @profile = Profile.find(params[:id]) versions = ActiveRuleChange.all(:select => 'profile_version, MAX(change_date) AS change_date', :conditions => ['profile_id=?', @profile.id], :group => 'profile_version') + # Add false change version 1 when no change have been made in profile version 1 + versions << ActiveRuleChange.new(:profile_version => 1, :profile_id => @profile.id) unless versions.find {|version| version.profile_version == 1} versions.sort! { |a, b| b.profile_version <=> a.profile_version } - unless versions.empty? + # SONAR-2986 + # Display changelog only from profile version 2 + if @profile.version > 1 last_version = versions[0].profile_version if params[:since].blank? @since_version = last_version - 1 @@ -218,9 +222,16 @@ class ProfilesController < ApplicationController end @changes = ActiveRuleChange.all(:conditions => ['profile_id=? and ? 'id desc') - @select_versions = versions.map { |u| [message(u.profile_version == last_version ? 'quality_profiles.last_version_x_with_date' : 'quality_profiles.version_x_with_date', - :params => [u.profile_version.to_s, l(u.change_date)]), u.profile_version] } | - [[message('quality_profiles.version_1'), 0]] + @select_versions = versions.map do |u| + if u.change_date + message = message(u.profile_version == last_version ? 'quality_profiles.last_version_x_with_date' : 'quality_profiles.version_x_with_date', + :params => [u.profile_version.to_s, l(u.change_date)]) + else + # Specific case when no change have been made in profile version 1 -> no date will be displayed + message = message('quality_profiles.version_x', :params => u.profile_version) + end + [message, u.profile_version] + end end set_profile_breadcrumbs diff --git a/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java b/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java index 710e86eccc1..43f543a7be9 100644 --- a/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java +++ b/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java @@ -21,13 +21,10 @@ package org.sonar.server.configuration; import org.junit.Before; import org.junit.Test; -import org.sonar.api.rules.ActiveRuleChange; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RulePriority; import org.sonar.jpa.test.AbstractDbUnitTestCase; -import static org.fest.assertions.Assertions.assertThat; - public class RuleChangeTest extends AbstractDbUnitTestCase { private ProfilesManager profilesManager; @@ -57,22 +54,6 @@ public class RuleChangeTest extends AbstractDbUnitTestCase { checkTables("ruleActivated", new String[]{"change_date"}, "active_rule_changes"); } - @Test - public void should_not_track_rule_activation_on_not_used_first_version_profile() { - setupData("should_not_track_rule_activation_on_not_used_first_version_profile"); - profilesManager.activated(1, 1, "admin"); - assertThat(getHQLCount(ActiveRuleChange.class)).isEqualTo(0); - checkTables("should_not_track_rule_activation_on_not_used_first_version_profile", "rules_profiles"); - } - - @Test - public void should_track_rule_activation_on_used_first_version_profile() { - setupData("should_track_rule_activation_on_used_first_version_profile"); - profilesManager.activated(1, 1, "admin"); - assertThat(getHQLCount(ActiveRuleChange.class)).isEqualTo(1); - checkTables("should_track_rule_activation_on_used_first_version_profile", "rules_profiles"); - } - @Test public void should_track_rule_deactivation() { setupData("initialData"); diff --git a/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_not_track_rule_activation_on_not_used_first_version_profile-result.xml b/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_not_track_rule_activation_on_not_used_first_version_profile-result.xml deleted file mode 100644 index 9d335e0993e..00000000000 --- a/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_not_track_rule_activation_on_not_used_first_version_profile-result.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - - - diff --git a/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_not_track_rule_activation_on_not_used_first_version_profile.xml b/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_not_track_rule_activation_on_not_used_first_version_profile.xml deleted file mode 100644 index 0493b0f09e6..00000000000 --- a/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_not_track_rule_activation_on_not_used_first_version_profile.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - - - - - - - - - - diff --git a/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_track_rule_activation_on_used_first_version_profile-result.xml b/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_track_rule_activation_on_used_first_version_profile-result.xml deleted file mode 100644 index fd6eacc30ef..00000000000 --- a/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_track_rule_activation_on_used_first_version_profile-result.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - - - diff --git a/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_track_rule_activation_on_used_first_version_profile.xml b/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_track_rule_activation_on_used_first_version_profile.xml deleted file mode 100644 index a1f36c6a350..00000000000 --- a/sonar-server/src/test/resources/org/sonar/server/configuration/RuleChangeTest/should_track_rule_activation_on_used_first_version_profile.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - - - - - - - - - - -- 2.39.5