From d72ceed07f959888e5868c16d22845d97e768a5b Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 18 Jun 2013 11:03:03 +0200 Subject: [PATCH] SONAR-4392 Add copy action on issue filters --- .../resources/org/sonar/l10n/core.properties | 2 + .../issue/InternalRubyIssueService.java | 50 +++++++++++++++---- .../server/issue/IssueFilterService.java | 20 ++++++-- .../app/controllers/issues_controller.rb | 50 +++++++++++++------ .../app/views/issues/_action_links.html.erb | 3 +- .../app/views/issues/_copy_form.html.erb | 18 +++++++ .../app/views/issues/_edit_form.html.erb | 23 ++------- .../app/views/issues/_save_as_form.html.erb | 25 ++-------- .../app/views/issues/_shared_form.html.erb | 16 ++++++ .../issue/InternalRubyIssueServiceTest.java | 22 ++++++++ .../server/issue/IssueFilterServiceTest.java | 13 +++++ 11 files changed, 171 insertions(+), 71 deletions(-) create mode 100644 sonar-server/src/main/webapp/WEB-INF/app/views/issues/_copy_form.html.erb create mode 100644 sonar-server/src/main/webapp/WEB-INF/app/views/issues/_shared_form.html.erb 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 4c36dc7c676..bc3dc9e4f67 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 @@ -532,6 +532,7 @@ issue_filter.max_results_reached=Only the first {0} issues matching the search c issue_filter.no_result=No matching issues found. issue_filter.save_filter=Save Filter issue_filter.edit_filter=Edit Filter +issue_filter.copy_filter=Copy Filter issue_filter.form.name=Name issue_filter.form.description=Description issue_filter.form.share=Shared with all users @@ -542,6 +543,7 @@ issue_filter.delete_confirm_title=Delete Filter issue_filter.are_you_sure_want_delete_filter_x=Are you sure that you want to delete the filter "{0}"? + #------------------------------------------------------------------------------ # # ALL PROJECTS PAGE 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 533566f8ade..8cdbd1166e8 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 @@ -280,7 +280,7 @@ public class InternalRubyIssueService implements ServerComponent { String projectParam = parameters.get("project"); checkMandatorySizeParameter(name, "name", 200, result); - checkOptionnalSizeParameter(description, "description", 1000, result); + checkOptionnalSizeParameter(description, "description", 1000, result); // Can only set project on creation if (existingActionPlan == null) { @@ -311,7 +311,7 @@ public class InternalRubyIssueService implements ServerComponent { return result; } - private void checkProject(String projectParam, Result result){ + private void checkProject(String projectParam, Result result) { if (Strings.isNullOrEmpty(projectParam)) { result.addError(Result.Message.ofL10n("errors.cant_be_empty", "project")); } else { @@ -322,7 +322,7 @@ public class InternalRubyIssueService implements ServerComponent { } } - private Date checkAndReturnDeadline(String deadLineParam, Result result){ + private Date checkAndReturnDeadline(String deadLineParam, Result result) { Date deadLine = null; if (!Strings.isNullOrEmpty(deadLineParam)) { try { @@ -404,8 +404,8 @@ public class InternalRubyIssueService implements ServerComponent { /** * Create issue filter */ - public Result createIssueFilter(Map params) { - Result result = createIssueFilterResult(params, false); + public Result createIssueFilter(Map parameters) { + Result result = createIssueFilterResult(parameters, false); if (result.ok()) { try { result.set(issueFilterService.save(result.get(), UserSession.get())); @@ -444,6 +444,34 @@ public class InternalRubyIssueService implements ServerComponent { return result; } + /** + * Delete issue filter + */ + public Result deleteIssueFilter(Long issueFilterId) { + Result result = Result.of(); + try { + issueFilterService.delete(issueFilterId, UserSession.get()); + } catch (Exception e) { + result.addError(e.getMessage()); + } + return result; + } + + /** + * Copy issue filter + */ + public Result copyIssueFilter(Long issueFilterIdToCopy, Map parameters) { + Result result = createIssueFilterResult(parameters, false); + if (result.ok()) { + try { + result.set(issueFilterService.copy(issueFilterIdToCopy, result.get(), UserSession.get())); + } catch (Exception e) { + result.addError(e.getMessage()); + } + } + return result; + } + @VisibleForTesting Result createIssueFilterResult(Map params, boolean isUpdate) { Result result = Result.of(); @@ -457,8 +485,8 @@ public class InternalRubyIssueService implements ServerComponent { if (isUpdate) { checkMandatoryParameter(id, "id", result); } - checkMandatorySizeParameter(name, "name", 100, result); - checkOptionnalSizeParameter(description, "description", 4000, result); + checkMandatorySizeParameter(name, "name", 100, result); + checkOptionnalSizeParameter(description, "description", 4000, result); if (result.ok()) { DefaultIssueFilter defaultIssueFilter = DefaultIssueFilter.create(name) @@ -474,20 +502,20 @@ public class InternalRubyIssueService implements ServerComponent { return result; } - private void checkMandatoryParameter(String value, String paramName, Result result){ + private void checkMandatoryParameter(String value, String paramName, Result result) { if (Strings.isNullOrEmpty(value)) { result.addError(Result.Message.ofL10n("errors.cant_be_empty", paramName)); } } - private void checkMandatorySizeParameter(String value, String paramName, Integer size, Result result){ + private void checkMandatorySizeParameter(String value, String paramName, Integer size, Result result) { checkMandatoryParameter(value, paramName, result); if (!Strings.isNullOrEmpty(value) && value.length() > size) { - result.addError(Result.Message.ofL10n("errors.is_too_long", paramName, size)); + result.addError(Result.Message.ofL10n("errors.is_too_long", paramName, size)); } } - private void checkOptionnalSizeParameter(String value, String paramName, Integer size, Result result){ + private void checkOptionnalSizeParameter(String value, String paramName, Integer size, Result result) { if (!Strings.isNullOrEmpty(value) && value.length() > size) { result.addError(Result.Message.ofL10n("errors.is_too_long", paramName, size)); } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java index 772c0db0c8e..02c0ca17ed2 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueFilterService.java @@ -73,7 +73,7 @@ public class IssueFilterService implements ServerComponent { public DefaultIssueFilter save(DefaultIssueFilter issueFilter, UserSession userSession) { verifyLoggedIn(userSession); issueFilter.setUser(userSession.login()); - VerifyNameIsNotAlreadyUsed(issueFilter, userSession); + verifyNameIsNotAlreadyUsed(issueFilter, userSession); IssueFilterDto issueFilterDto = IssueFilterDto.toIssueFilter(issueFilter); issueFilterDao.insert(issueFilterDto); @@ -84,7 +84,7 @@ public class IssueFilterService implements ServerComponent { verifyLoggedIn(userSession); IssueFilterDto issueFilterDto = findIssueFilter(issueFilter.id()); verifyCurrentUserIsOwnerOfFilter(issueFilterDto, userSession); - VerifyNameIsNotAlreadyUsed(issueFilter, userSession); + verifyNameIsNotAlreadyUsed(issueFilter, userSession); issueFilterDao.update(IssueFilterDto.toIssueFilter(issueFilter)); return issueFilter; @@ -107,6 +107,18 @@ public class IssueFilterService implements ServerComponent { issueFilterDao.delete(issueFilterId); } + public DefaultIssueFilter copy(Long issueFilterIdToCopy, DefaultIssueFilter issueFilter, UserSession userSession) { + verifyLoggedIn(userSession); + IssueFilterDto issueFilterDtoToCopy = findIssueFilter(issueFilterIdToCopy); + issueFilter.setUser(userSession.login()); + issueFilter.setData(issueFilterDtoToCopy.getData()); + verifyNameIsNotAlreadyUsed(issueFilter, userSession); + + IssueFilterDto issueFilterDto = IssueFilterDto.toIssueFilter(issueFilter); + issueFilterDao.insert(issueFilterDto); + return issueFilterDto.toIssueFilter(); + } + public IssueQueryResult execute(IssueQuery issueQuery) { return issueFinder.find(issueQuery); } @@ -130,7 +142,7 @@ public class IssueFilterService implements ServerComponent { } private void verifyLoggedIn(UserSession userSession) { - if (!userSession.isLoggedIn()) { + if (!userSession.isLoggedIn() || userSession.login() == null) { throw new IllegalStateException("User is not logged in"); } } @@ -142,7 +154,7 @@ public class IssueFilterService implements ServerComponent { } } - private void VerifyNameIsNotAlreadyUsed(DefaultIssueFilter issueFilter, UserSession userSession){ + private void verifyNameIsNotAlreadyUsed(DefaultIssueFilter issueFilter, UserSession userSession){ if (issueFilterDao.selectByNameAndUser(issueFilter.name(), userSession.login(), issueFilter.id()) != null) { throw new IllegalArgumentException("Name already exists"); } diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb index eef280470d1..2ecf08ad93a 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb @@ -62,25 +62,17 @@ class IssuesController < ApplicationController @issue_query = Internal.issues.toQuery({}) end - # GET /issues/save_as_form?[id=][&criteria] + # GET /issues/save_as_form?[&criteria] def save_as_form - if params[:id].present? - @filter = find_filter(params[:id]) - else - @filter = Internal.issues.createFilterFromMap(criteria_params) - end + @filter = Internal.issues.createFilterFromMap(criteria_params) render :partial => 'issues/save_as_form' end - # POST /issues/save_as?[id=]&name=[¶meters] + # POST /issues/save_as?name=[¶meters] def save_as verify_post_request - options = {'id' => params[:id], 'name' => params[:name], 'description' => params[:description], 'data' => URI.unescape(params[:data]), 'shared' => params[:shared]=='true' } - if params[:id].present? - @filter = Internal.issues.updateIssueFilter(options) - else - filter_result = Internal.issues.createIssueFilter(options) - end + options = {'name' => params[:name], 'description' => params[:description], 'data' => URI.unescape(params[:data]), 'shared' => params[:shared]=='true' } + filter_result = Internal.issues.createIssueFilter(options) if filter_result.ok @filter = filter_result.get() @@ -131,11 +123,41 @@ class IssuesController < ApplicationController render :text => @filter.id.to_s, :status => 200 else @errors = filter_result.errors - @filter = find_filter(params[:id].to_i) render :partial => 'issues/edit_form', :status => 400 end end + # GET /issues/copy_form/ + def copy_form + require_parameters :id + @filter = find_filter(params[:id].to_i) + render :partial => 'issues/copy_form' + end + + # POST /issues/copy/?name=&description= + def copy + verify_post_request + + options = {'name' => params[:name], 'description' => params[:description], 'shared' => params[:shared]=='true' } + filter_result = Internal.issues.copyIssueFilter(params[:id].to_i, options) + + if filter_result.ok + @filter = filter_result.get() + render :text => @filter.id.to_s, :status => 200 + else + @errors = filter_result.errors + render :partial => 'issues/copy_form', :status => 400 + end + end + + # POST /issues/delete/ + def delete + verify_post_request + require_parameters :id + Internal.issues.deleteIssueFilter(params[:id].to_i) + redirect_to :action => 'manage' + end + private diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_action_links.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_action_links.html.erb index 32cfd27c28b..a3cf7eaecac 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_action_links.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_action_links.html.erb @@ -1,8 +1,7 @@
    <% if logged_in? %> <% if @filter.id %> - - +
  • <%= message('copy') -%>
    • <% end %> <% if !defined?(@unchanged) && @filter.id && @filter.user == current_user.login %>
  • diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_copy_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_copy_form.html.erb new file mode 100644 index 00000000000..6e4f8cc5a14 --- /dev/null +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_copy_form.html.erb @@ -0,0 +1,18 @@ +
    + +
    +
    +

    <%= message('issue_filter.copy_filter') -%>

    +
    + <%= render :partial => 'shared_form' %> +
    + + <%= h message('cancel') -%> +
    +
    +
    + \ No newline at end of file diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_edit_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_edit_form.html.erb index 809dab66674..42ea290e3ae 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_edit_form.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_edit_form.html.erb @@ -1,28 +1,13 @@
    - +

    <%= message('issue_filter.edit_filter') -%>

    -
    - <%= render :partial => 'display_errors' %> -
    - - -
    -
    - - -
    - - -
    + <%= render :partial => 'shared_form' %>
    - - <%= h message('cancel') -%> + + <%= message('cancel') -%>
    diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_save_as_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_save_as_form.html.erb index 77797e3616a..4407f216bd7 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_save_as_form.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_save_as_form.html.erb @@ -1,30 +1,13 @@
    - - +

    <%= message('issue_filter.save_filter') -%>

    - -
    - <%= render :partial => 'display_errors' %> -
    - - -
    -
    - - -
    - - -
    + <%= render :partial => 'shared_form' %>
    - - <%= h message('cancel') -%> + + <%= message('cancel') -%>
    diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_shared_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_shared_form.html.erb new file mode 100644 index 00000000000..45f1bb7599b --- /dev/null +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_shared_form.html.erb @@ -0,0 +1,16 @@ +
    + <%= render :partial => 'display_errors' %> +
    + + +
    +
    + + +
    + + +
    \ No newline at end of file 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 495bcf5ea4b..a8113367b0c 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 @@ -322,6 +322,28 @@ public class InternalRubyIssueServiceTest { assertThat(issueFilter.description()).isEqualTo("Long term issues"); } + @Test + public void should_delete_issue_filter() { + Result result = service.deleteIssueFilter(1L); + assertThat(result.ok()).isTrue(); + } + + @Test + public void should_copy_issue_filter() { + Map parameters = newHashMap(); + parameters.put("name", "Copy of Long term"); + parameters.put("description", "Copy of Long term issues"); + + Result result = service.copyIssueFilter(1L, parameters); + assertThat(result.ok()).isTrue(); + + ArgumentCaptor issueFilterCaptor = ArgumentCaptor.forClass(DefaultIssueFilter.class); + verify(issueFilterService).copy(eq(1L), issueFilterCaptor.capture(), any(UserSession.class)); + DefaultIssueFilter issueFilter = issueFilterCaptor.getValue(); + assertThat(issueFilter.name()).isEqualTo("Copy of Long term"); + assertThat(issueFilter.description()).isEqualTo("Copy of Long term issues"); + } + @Test public void should_get_error_on_issue_filter_result_when_no_name() { Map parameters = newHashMap(); diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java index 2bb870c6b26..429322c1fa8 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueFilterServiceTest.java @@ -233,6 +233,19 @@ public class IssueFilterServiceTest { verify(issueFilterDao, never()).delete(anyLong()); } + @Test + public void should_copy() { + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("perceval").setData("componentRoots=struts")); + DefaultIssueFilter issueFilter = new DefaultIssueFilter().setName("Copy Of My Issue"); + + DefaultIssueFilter result = service.copy(1L, issueFilter, userSession); + assertThat(result.name()).isEqualTo("Copy Of My Issue"); + assertThat(result.user()).isEqualTo("john"); + assertThat(result.data()).isEqualTo("componentRoots=struts"); + + verify(issueFilterDao).insert(any(IssueFilterDto.class)); + } + @Test public void should_execute_from_issue_query() { IssueQuery issueQuery = IssueQuery.builder().build(); -- 2.39.5