SONAR-4394 Fixed issue filters problem when saving existing filter

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

@@ -21,6 +21,7 @@ package org.sonar.server.issue;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
+import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.collect.Maps;
 import org.apache.commons.lang.StringUtils;
@@ -409,6 +410,19 @@ public class InternalRubyIssueService implements ServerComponent {
     return issueFilterService.serializeFilterQuery(filterQuery);
   }
 
+  public Map<String, Object> deserializeFilterQuery(DefaultIssueFilter issueFilter) {
+    return issueFilterService.deserializeIssueFilterQuery(issueFilter);
+  }
+
+  public Map<String, Object> sanitizeFilterQuery(Map<String, Object> filterQuery) {
+    return Maps.filterEntries(filterQuery, new Predicate<Map.Entry<String, Object>>() {
+      @Override
+      public boolean apply(Map.Entry<String, Object> input) {
+        return IssueFilterParameters.ALL.contains(input.getKey());
+      }
+    });
+  }
+
   /**
    * Execute issue filter from parameters
    */
@@ -429,13 +443,8 @@ public class InternalRubyIssueService implements ServerComponent {
   }
 
   private void overrideProps(Map<String, Object> props, Map<String, Object> overrideProps) {
-    overrideProp(props, overrideProps, "pageSize");
-    overrideProp(props, overrideProps, "pageIndex");
-  }
-
-  private void overrideProp(Map<String, Object> props, Map<String, Object> overrideProps, String key) {
-    if (overrideProps.containsKey(key)) {
-      props.put(key, overrideProps.get(key));
+    for (Map.Entry<String, Object> entry : overrideProps.entrySet()) {
+      props.put(entry.getKey(), entry.getValue());
     }
   }
 

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

@@ -64,19 +64,19 @@ public class IssueBulkChangeService {
     IssueBulkChangeResult result = new IssueBulkChangeResult();
     IssueQueryResult issueQueryResult = issueFinder.find(IssueQuery.builder().issueKeys(issueBulkChangeQuery.issues()).requiredRole(UserRole.USER).build());
     List<Issue> issues = issueQueryResult.issues();
-    List<Action> actions = newArrayList();
+    List<Action> bulkActions = newArrayList();
     for (String actionName : issueBulkChangeQuery.actions()) {
       Action action = getAction(actionName);
       if (action == null) {
         throw new IllegalArgumentException("The action : '"+ actionName + "' is unknown");
       }
       action.verify(issueBulkChangeQuery.properties(actionName), issues, userSession);
-      actions.add(action);
+      bulkActions.add(action);
     }
 
     IssueChangeContext issueChangeContext = IssueChangeContext.createUser(new Date(), userSession.login());
     for (Issue issue : issues) {
-      for (Action action : actions) {
+      for (Action action : bulkActions) {
         try {
           ActionContext actionContext = new ActionContext(issue, issueChangeContext);
           if (action.supports(issue) && action.execute(issueBulkChangeQuery.properties(action.key()), actionContext)) {

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

@@ -0,0 +1,66 @@
+/*
+ * 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.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+
+import java.util.List;
+
+import static com.google.common.collect.Lists.newArrayList;
+
+/**
+ * @since 3.7
+ */
+public class IssueFilterParameters {
+
+  public static final String ISSUES = "issues";
+  public static final String SEVERITIES = "severities";
+  public static final String STATUSES = "statuses";
+  public static final String RESOLUTIONS = "resolutions";
+  public static final String RESOLVED = "resolved";
+  public static final String COMPONENTS = "components";
+  public static final String COMPONENT_ROOTS = "componentRoots";
+  public static final String RULES = "rules";
+  public static final String ACTION_PLANS = "actionPlans";
+  public static final String REPORTERS = "reporters";
+  public static final String ASSIGNEES = "assignees";
+  public static final String ASSIGNED = "assigned";
+  public static final String PLANNED = "planned";
+  public static final String CREATED_AFTER = "createdAfter";
+  public static final String CREATED_BEFORE = "createdBefore";
+  public static final String PAGE_SIZE = "pageSize";
+  public static final String PAGE_INDEX = "pageIndex";
+  public static final String SORT = "sort";
+  public static final String ASC = "asc";
+
+  public static final List<String> ALL = ImmutableList.of(ISSUES, SEVERITIES, STATUSES, RESOLUTIONS, RESOLVED, COMPONENTS, COMPONENT_ROOTS, RULES, ACTION_PLANS, REPORTERS,
+    ASSIGNEES, ASSIGNED, PLANNED, CREATED_AFTER, CREATED_BEFORE, PAGE_SIZE, PAGE_INDEX, SORT, ASC);
+
+  public static final List<String> ALL_WITHOUT_PAGINATION = newArrayList(Iterables.filter(ALL, new Predicate<String>() {
+    @Override
+    public boolean apply(String input) {
+      return !PAGE_INDEX.equals(input) && !PAGE_SIZE.equals(input);
+    }
+  }));
+
+}

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

@@ -21,7 +21,9 @@
 package org.sonar.server.issue;
 
 import com.google.common.base.Function;
+import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.issue.IssueFinder;
 import org.sonar.api.issue.IssueQuery;
@@ -154,7 +156,13 @@ public class IssueFilterService implements ServerComponent {
   }
 
   public String serializeFilterQuery(Map<String, Object> filterQuery) {
-    return issueFilterSerializer.serialize(filterQuery);
+    Map<String, Object> filterQueryFiltered = Maps.filterEntries(filterQuery, new Predicate<Map.Entry<String, Object>>() {
+      @Override
+      public boolean apply(Map.Entry<String, Object> input) {
+        return IssueFilterParameters.ALL_WITHOUT_PAGINATION.contains(input.getKey());
+      }
+    });
+    return issueFilterSerializer.serialize(filterQueryFiltered);
   }
 
   public Map<String, Object> deserializeIssueFilterQuery(DefaultIssueFilter issueFilter) {

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

@@ -75,27 +75,27 @@ public class PublicRubyIssueService implements RubyIssueService {
   static IssueQuery toQuery(Map<String, Object> props) {
     IssueQuery.Builder builder = IssueQuery.builder()
       .requiredRole(UserRole.USER)
-      .issueKeys(RubyUtils.toStrings(props.get("issues")))
-      .severities(RubyUtils.toStrings(props.get("severities")))
-      .statuses(RubyUtils.toStrings(props.get("statuses")))
-      .resolutions(RubyUtils.toStrings(props.get("resolutions")))
-      .resolved(RubyUtils.toBoolean(props.get("resolved")))
-      .components(RubyUtils.toStrings(props.get("components")))
-      .componentRoots(RubyUtils.toStrings(props.get("componentRoots")))
-      .rules(toRules(props.get("rules")))
-      .actionPlans(RubyUtils.toStrings(props.get("actionPlans")))
-      .reporters(RubyUtils.toStrings(props.get("reporters")))
-      .assignees(RubyUtils.toStrings(props.get("assignees")))
-      .assigned(RubyUtils.toBoolean(props.get("assigned")))
-      .planned(RubyUtils.toBoolean(props.get("planned")))
-      .createdAfter(RubyUtils.toDate(props.get("createdAfter")))
-      .createdBefore(RubyUtils.toDate(props.get("createdBefore")))
-      .pageSize(RubyUtils.toInteger(props.get("pageSize")))
-      .pageIndex(RubyUtils.toInteger(props.get("pageIndex")));
-    String sort = (String) props.get("sort");
+      .issueKeys(RubyUtils.toStrings(props.get(IssueFilterParameters.ISSUES)))
+      .severities(RubyUtils.toStrings(props.get(IssueFilterParameters.SEVERITIES)))
+      .statuses(RubyUtils.toStrings(props.get(IssueFilterParameters.STATUSES)))
+      .resolutions(RubyUtils.toStrings(props.get(IssueFilterParameters.RESOLUTIONS)))
+      .resolved(RubyUtils.toBoolean(props.get(IssueFilterParameters.RESOLVED)))
+      .components(RubyUtils.toStrings(props.get(IssueFilterParameters.COMPONENTS)))
+      .componentRoots(RubyUtils.toStrings(props.get(IssueFilterParameters.COMPONENT_ROOTS)))
+      .rules(toRules(props.get(IssueFilterParameters.RULES)))
+      .actionPlans(RubyUtils.toStrings(props.get(IssueFilterParameters.ACTION_PLANS)))
+      .reporters(RubyUtils.toStrings(props.get(IssueFilterParameters.REPORTERS)))
+      .assignees(RubyUtils.toStrings(props.get(IssueFilterParameters.ASSIGNEES)))
+      .assigned(RubyUtils.toBoolean(props.get(IssueFilterParameters.ASSIGNED)))
+      .planned(RubyUtils.toBoolean(props.get(IssueFilterParameters.PLANNED)))
+      .createdAfter(RubyUtils.toDate(props.get(IssueFilterParameters.CREATED_AFTER)))
+      .createdBefore(RubyUtils.toDate(props.get(IssueFilterParameters.CREATED_BEFORE)))
+      .pageSize(RubyUtils.toInteger(props.get(IssueFilterParameters.PAGE_SIZE)))
+      .pageIndex(RubyUtils.toInteger(props.get(IssueFilterParameters.PAGE_INDEX)));
+    String sort = (String) props.get(IssueFilterParameters.SORT);
     if (!Strings.isNullOrEmpty(sort)) {
       builder.sort(sort);
-      builder.asc(RubyUtils.toBoolean(props.get("asc")));
+      builder.asc(RubyUtils.toBoolean(props.get(IssueFilterParameters.ASC)));
     }
     return builder.build();
   }

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

@@ -17,6 +17,8 @@
 # along with this program; if not, write to the Free Software Foundation,
 # Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #
+require 'set'
+
 class IssuesController < ApplicationController
 
   before_filter :init_options
@@ -34,10 +36,8 @@ class IssuesController < ApplicationController
     if params[:id]
       @filter = find_filter(params[:id].to_i)
     end
-
-    @first_search = criteria_params.empty?
+    @first_search = Internal.issues.sanitizeFilterQuery(params).to_hash.empty?
     @criteria_params = criteria_params
-    @criteria_params['pageSize'] = PAGE_SIZE
     issue_filter_result = Internal.issues.execute(@criteria_params)
     @issue_query = issue_filter_result.query
     @issues_result = issue_filter_result.result
@@ -51,10 +51,10 @@ class IssuesController < ApplicationController
     @first_search = false
     @unchanged = true
 
-    @criteria_params = criteria_params
-    @criteria_params['pageSize'] = PAGE_SIZE
+    issue_filter_result = Internal.issues.execute(params[:id].to_i, params)
     @filter = find_filter(params[:id].to_i)
-    issue_filter_result = Internal.issues.execute(params[:id].to_i, @criteria_params)
+    @criteria_params = Internal.issues.deserializeFilterQuery(@filter).to_hash
+    @criteria_params[:id] = @filter.id
     @issue_query = issue_filter_result.query
     @issues_result = issue_filter_result.result
 
@@ -71,7 +71,7 @@ class IssuesController < ApplicationController
 
   # GET /issues/save_as_form?[&criteria]
   def save_as_form
-    @filter_query_serialized = Internal.issues.serializeFilterQuery(criteria_params_to_save)
+    @filter_query_serialized = Internal.issues.serializeFilterQuery(params)
     render :partial => 'issues/filter_save_as_form'
   end
 
@@ -95,7 +95,7 @@ class IssuesController < ApplicationController
     verify_post_request
     require_parameters :id
 
-    filter_result = Internal.issues.updateIssueFilterQuery(params[:id].to_i, criteria_params_to_save)
+    filter_result = Internal.issues.updateIssueFilterQuery(params[:id].to_i, params)
     if filter_result.ok
       @filter = filter_result.get()
       redirect_to :action => 'filter', :id => @filter.id.to_s
@@ -103,7 +103,7 @@ class IssuesController < ApplicationController
       @unchanged = true
       @errors = filter_result.errors
 
-      issue_filter_result = Internal.issues.execute(@filter.id, criteria_params)
+      issue_filter_result = Internal.issues.execute(@filter.id, params)
       @issue_query = issue_filter_result.query
       @issues_result = issue_filter_result.result
 
@@ -183,12 +183,21 @@ class IssuesController < ApplicationController
   def bulk_change_form
 
     # Load maximum number of issues
-    @criteria_params = criteria_params_to_save
+    @criteria_params = params
     @criteria_params['pageSize'] = -1
     issue_filter_result = Internal.issues.execute(@criteria_params)
     issue_query = issue_filter_result.query
     issues_result = issue_filter_result.result
 
+    @transitions_by_issues = {}
+    issues_result.issues.each do |issue|
+      transitions = Internal.issues.listTransitions(issue)
+      transitions.each do |transition|
+        issues_for_transition = @transitions_by_issues[transition.key] || 0
+        issues_for_transition += 1
+        @transitions_by_issues[transition.key] = issues_for_transition
+      end
+    end
     @issues = issues_result.issues.map {|issue| issue.key()}
     @project = issue_query.componentRoots.to_a.first if issue_query.componentRoots and issue_query.componentRoots.size == 1
 
@@ -224,20 +233,8 @@ class IssuesController < ApplicationController
   end
 
   def criteria_params
-    criteria = params
-    criteria.delete('controller')
-    criteria.delete('action')
-    criteria.delete('search')
-    criteria.delete('edit')
-    criteria.delete('pageSize')
-    criteria
-  end
-
-  def criteria_params_to_save
-    criteria = criteria_params
-    criteria.delete('id')
-    criteria.delete('pageIndex')
-    criteria
+    params['pageSize'] = PAGE_SIZE
+    params
   end
 
 end

sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb

@@ -6,6 +6,7 @@
 <form id="bulk-change-form" method="post" action="<%= ApplicationController.root_context -%>/issues/bulk_change">
   <input type="hidden" name="issues" value="<%= params[:issues] || @issues.join(',') -%>">
   <input type="hidden" name="criteria_params" value="<%= params[:criteria_params] || @criteria_params.to_query -%>">
+  <input type="hidden" name="issues_by_transitions" value="<%= @transitions_by_issues.to_query -%>">
   <input type="hidden" name="project" value="<%= project -%>">
   <fieldset>
     <div class="modal-head">
@@ -58,6 +59,13 @@
           <% end %>
         </select>
       </div>
+      <div class="modal-field">
+        <%
+           @transitions_by_issues.keys.each do |transition|
+        %>
+          <input type="radio" name="transition" value="<%= transition -%>"><%= message("issue.transition.#{transition}") -%> (<%= @transitions_by_issues[transition].to_s %>) <br>
+        <% end %>
+      </div>
     </div>
     <div class="modal-foot">
       <input type="submit" value="<%= message('submit') -%>" id="bulk-change-submit"/>

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

@@ -476,9 +476,12 @@ public class InternalRubyIssueServiceTest {
   public void should_execute_issue_filter_from_existing_filter() {
     Map<String, Object> props = newHashMap();
     props.put("componentRoots", "struts");
+    props.put("statuses", "OPEN");
     when(issueFilterService.deserializeIssueFilterQuery(any(DefaultIssueFilter.class))).thenReturn(props);
 
     Map<String, Object> overrideProps = newHashMap();
+    overrideProps.put("statuses", "CLOSED");
+    overrideProps.put("resolved", true);
     overrideProps.put("pageSize", 20);
     overrideProps.put("pageIndex", 2);
     service.execute(10L, overrideProps);
@@ -488,10 +491,37 @@ public class InternalRubyIssueServiceTest {
 
     IssueQuery issueQuery = captor.getValue();
     assertThat(issueQuery.componentRoots()).contains("struts");
+    assertThat(issueQuery.statuses()).contains("CLOSED");
+    assertThat(issueQuery.resolved()).isTrue();
     assertThat(issueQuery.pageSize()).isEqualTo(20);
     assertThat(issueQuery.pageIndex()).isEqualTo(2);
   }
 
+  @Test
+  public void should_serialize_filter_query() {
+    Map<String, Object> props = newHashMap();
+    props.put("componentRoots", "struts");
+    service.serializeFilterQuery(props);
+    verify(issueFilterService).serializeFilterQuery(props);
+  }
+
+  @Test
+  public void should_deserialize_filter_query() {
+    DefaultIssueFilter issueFilter = new DefaultIssueFilter();
+    service.deserializeFilterQuery(issueFilter);
+    verify(issueFilterService).deserializeIssueFilterQuery(issueFilter);
+  }
+
+  @Test
+  public void should_sanitize_filter_query(){
+    Map<String, Object> query = newHashMap();
+    query.put("statuses", "CLOSED");
+    query.put("resolved", true);
+    query.put("unknown", "john");
+    Map<String, Object> result = service.sanitizeFilterQuery(query);
+    assertThat(result.keySet()).containsOnly("statuses", "resolved");
+  }
+
   @Test
   public void should_find_user_issue_filters() {
     service.findIssueFiltersForCurrentUser();
@@ -561,6 +591,7 @@ public class InternalRubyIssueServiceTest {
     assertThat(((Result.Message) result.errors().get(0)).text()).contains("Error");
   }
 
+
   private String createLongString(int size) {
     String result = "";
     for (int i = 0; i < size; i++) {

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

@@ -526,6 +526,27 @@ public class IssueFilterServiceTest {
     verify(issueFilterFavouriteDao, never()).delete(anyLong());
   }
 
+  @Test
+  public void should_serialize_filter_query_ignore_unknown_parameter() {
+    Map<String, Object> props = newHashMap();
+    props.put("componentRoots", "struts");
+    props.put("statuses", "OPEN");
+    props.put("unkwown", "JOHN");
+    service.serializeFilterQuery(props);
+
+    Map<String, Object> expected = newHashMap();
+    expected.put("componentRoots", "struts");
+    expected.put("statuses", "OPEN");
+    verify(issueFilterSerializer).serialize(expected);
+  }
+
+  @Test
+  public void should_deserialize_filter_query() {
+    DefaultIssueFilter issueFilter = new DefaultIssueFilter().setData("componentRoots=struts");
+    service.deserializeIssueFilterQuery(issueFilter);
+    verify(issueFilterSerializer).deserialize("componentRoots=struts");
+  }
+
   private static class Matches extends BaseMatcher<IssueFilterDto> {
 
     private final IssueFilterDto referenceFilter;