SONAR-4399 Added support for issue filter ownership change (admins only)

16 files changed, 140 insertions, 65 deletions

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 53f55b00162..b7967537165 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
@@ -538,6 +538,7 @@ issue_filter.copy_filter=Copy Filter
 issue_filter.form.name=Name
 issue_filter.form.description=Description
 issue_filter.form.share=Shared with all users
+issue_filter.form.owner=Owner
 issue_filter.favourite_filters=Favourite Filters
 issue_filter.manage.my_filters=My Filters
 issue_filter.no_filters=No filters
diff --git a/sonar-application/src/main/assembly/conf/sonar.properties b/sonar-application/src/main/assembly/conf/sonar.properties
index d4fc1ff02e8..024dc35d67f 100644
--- a/sonar-application/src/main/assembly/conf/sonar.properties
+++ b/sonar-application/src/main/assembly/conf/sonar.properties
@@ -42,12 +42,12 @@ sonar.jdbc.password:                       sonar
 
 #----- Embedded database H2
 # Note : it does not accept connections from remote hosts, so the
-# sonar server and the maven plugin must be executed on the same host.
+# SonarQube server and the maven plugin must be executed on the same host.
 
 # Comment the following line to deactivate the default embedded database.
 sonar.jdbc.url:                            jdbc:h2:tcp://localhost:9092/sonar
 
-# directory containing H2 database files. By default it's the /data directory in the sonar installation.
+# directory containing H2 database files. By default it's the /data directory in the SonarQube installation.
 #sonar.embeddedDatabase.dataDir:
 # H2 embedded database server listening port, defaults to 9092
 #sonar.embeddedDatabase.port:               9092
diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterMapper.xml
index 3e908084468..21a2c87cfee 100644
--- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterMapper.xml
+++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterMapper.xml
@@ -85,6 +85,7 @@
     shared=#{shared},
     description=#{description},
     data=#{data},
+    user_login=#{userLogin},
     updated_at=current_timestamp
     where id=#{id}
   </update>
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterDaoTest.java
index 5a2a714ec95..fd3eb8e7e93 100644
--- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterDaoTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterDaoTest.java
@@ -130,6 +130,7 @@ public class IssueFilterDaoTest extends AbstractDaoTestCase {
     filterDto.setShared(false);
     filterDto.setDescription("All closed issues");
     filterDto.setData("statuses=CLOSED");
+    filterDto.setUserLogin("bernard");
 
     dao.update(filterDto);
 
diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_update-result.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_update-result.xml
index daf43474984..134411beaf4 100644
--- a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_update-result.xml
+++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterDaoTest/should_update-result.xml
@@ -13,7 +13,7 @@
   <issue_filters
       id="2"
       name="Closed issues"
-      user_login="michael"
+      user_login="bernard"
       shared="[false]"
       description="All closed issues"
       data="statuses=CLOSED"
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 858c061f1cc..0491ce1f029 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
@@ -525,6 +525,7 @@ public class InternalRubyIssueService implements ServerComponent {
     String name = params.get("name");
     String description = params.get("description");
     String data = params.get("data");
+    String user = params.get("user");
     Boolean sharedParam = RubyUtils.toBoolean(params.get("shared"));
     boolean shared = sharedParam != null ? sharedParam : false;
 
@@ -538,6 +539,7 @@ public class InternalRubyIssueService implements ServerComponent {
       DefaultIssueFilter defaultIssueFilter = DefaultIssueFilter.create(name)
         .setDescription(description)
         .setShared(shared)
+        .setUser(user)
         .setData(data);
       if (isUpdate) {
         defaultIssueFilter.setId(Long.valueOf(id));
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 7eaae9e62ba..eddbe7c0245 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
@@ -95,6 +95,9 @@ public class IssueFilterService implements ServerComponent {
     String user = getNotNullLogin(userSession);
     IssueFilterDto issueFilterDto = findIssueFilterDto(issueFilter.id(), user);
     verifyCurrentUserCanModifyFilter(issueFilterDto.toIssueFilter(), user);
+    if(issueFilterDto.getUserLogin() != issueFilter.user()) {
+      verifyCurrentUserCanChangeFilterOwnership(user);
+    }
     validateFilter(issueFilter, user);
 
     issueFilterDao.update(IssueFilterDto.toIssueFilter(issueFilter));
@@ -190,6 +193,13 @@ public class IssueFilterService implements ServerComponent {
     }
   }
 
+  private void verifyCurrentUserCanChangeFilterOwnership(String user) {
+    if(!isAdmin(user)) {
+      // TODO throw unauthorized
+      throw new IllegalStateException("User is not authorized to change the owner of this filter");
+    }
+  }
+
   private void validateFilter(DefaultIssueFilter issueFilter, String user) {
     if (issueFilterDao.selectByNameAndUser(issueFilter.name(), user, 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 a40844c6583..4ff635695fb 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
@@ -123,7 +123,8 @@ class IssuesController < ApplicationController
     verify_post_request
 
     existing_filter = find_filter(params[:id].to_i)
-    options = {'id' => params[:id].to_s, 'name' => params[:name], 'description' => params[:description], 'data' => existing_filter.data, 'shared' => params[:shared]=='true' }
+    options = {'id' => params[:id].to_s, 'name' => params[:name], 'description' => params[:description],
+               'data' => existing_filter.data, 'shared' => params[:shared]=='true', 'user' => params[:user]}
     filter_result = Internal.issues.updateIssueFilter(options)
     if filter_result.ok
       @filter = filter_result.get()
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/helpers/issues_helper.rb b/sonar-server/src/main/webapp/WEB-INF/app/helpers/issues_helper.rb
index 62abbc88488..911d658f87d 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/helpers/issues_helper.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/helpers/issues_helper.rb
@@ -44,4 +44,8 @@ module IssuesHelper
     "<a href='#' class='issue-filter-star #{style}' filter-id='#{filter.id.to_s}' title='#{title}'></a>"
   end
 
+  def can_be_reassigned_by(user, filter)
+    user.has_role?(:admin) && filter.shared
+  end
+
 end
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 42ea290e3ae..dbd158d01ce 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
@@ -4,7 +4,9 @@
     <div class="modal-head">
       <h2><%= message('issue_filter.edit_filter') -%></h2>
     </div>
-    <%= render :partial => 'shared_form' %>
+
+    <%= render :partial => 'shared_form', :locals => {:display_owner => true} %>
+
     <div class="modal-foot">
       <input type="submit" value="<%= message('save') -%>" id="save-submit"/>
       <a href="#" onclick="return closeModalWindow()" id="save-cancel"><%= message('cancel') -%></a>
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
index af3d073e3c0..f9574560297 100644
--- 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
@@ -1,3 +1,6 @@
+<% if !local_assigns.has_key? :display_owner
+     display_owner = false
+   end %>
 <div class="modal-body">
   <%= render :partial => 'display_errors' %>
   <div class="modal-field">
@@ -8,6 +11,13 @@
     <label for="description"><%= message('issue_filter.form.description') -%></label>
     <input id="description" name="description" type="text" size="50" maxlength="4000" value="<%= params[:description] || (h(@filter.description) if @filter)  -%>"/>
   </div>
+  <% if display_owner && can_be_reassigned_by(current_user, @filter) %>
+    <% filter_owner = Api.users.findByLogin(@filter.user) %>
+    <div class="modal-field">
+      <label for="user"><%= h message('issue_filter.form.owner') -%></label>
+      <%= user_select_tag('user', :html_id => 'select-filter-owner', :selected_user => filter_owner) -%>
+    </div>
+  <% end %>
   <div class="modal-field">
     <label for="shared"><%= message('issue_filter.form.share') -%></label>
     <input id="shared" name="shared" type="checkbox" value="true" <%= 'checked' if (params[:shared] || (@filter && @filter.shared)) -%>/>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_copy_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_copy_form.html.erb
index dab5991e4e7..4aac628b14c 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_copy_form.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_copy_form.html.erb
@@ -4,23 +4,9 @@
     <div class="modal-head">
       <h2>Copy Filter</h2>
     </div>
-    <div class="modal-body">
-      <% @filter.errors.each do |attr, msg| %>
-        <p class="error"><%= h msg -%></p>
-      <% end %>
-      <div class="modal-field">
-        <label for="name"><%= h message('name') -%> <em class="mandatory">*</em></label>
-        <input id="name" name="name" type="text" size="50" maxlength="100" value="" autofocus="autofocus"/>
-      </div>
-      <div class="modal-field">
-        <label for="description"><%= h message('description') -%></label>
-        <input id="description" name="description" type="text" size="50" maxlength="4000" value="<%= h @filter.description -%>" />
-      </div>
-      <div class="modal-field">
-        <label for="shared"><%= h message('measure_filter.shared_with_all_users') -%></label>
-        <input id="shared" name="shared" type="checkbox" value="true" <%= 'checked' if @filter.shared -%>/>
-      </div>
-    </div>
+
+    <%= render :partial => 'shared_form' %>
+
     <div class="modal-foot">
       <input type="submit" value="<%= h message('copy') -%>" id="copy-submit"/>
       <a href="#" onclick="return closeModalWindow()" id="copy-cancel"><%= h message('cancel') -%></a>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_edit_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_edit_form.html.erb
index 5b9f6b43984..9a26c12a220 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_edit_form.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_edit_form.html.erb
@@ -4,29 +4,9 @@
     <div class="modal-head">
       <h2>Edit Filter</h2>
     </div>
-    <div class="modal-body">
-      <% @filter.errors.each do |attr, msg| %>
-        <p class="error"><%= h "#{attr} #{msg}" -%></p>
-      <% end %>
-      <div class="modal-field">
-        <label for="name"><%= h message('name') -%> <em class="mandatory">*</em></label>
-        <input id="name" name="name" type="text" size="50" maxlength="100" value="<%= h @filter.name -%>" autofocus="autofocus"/>
-      </div>
-      <div class="modal-field">
-        <label for="description"><%= h message('description') -%></label>
-        <input id="description" name="description" type="text" size="50" maxlength="4000" value="<%= h @filter.description -%>"/>
-      </div>
-      <% if @filter.can_be_reassigned_by(current_user) %>
-        <div class="modal-field">
-          <label for="owner"><%= h message('owner') -%></label>
-          <%= user_select_tag('owner', :html_id => 'select-filter-owner', :selected_user => @filter.user) -%>
-        </div>
-      <% end %>
-      <div class="modal-field">
-        <label for="shared"><%= h message('measure_filter.shared_with_all_users') -%></label>
-        <input id="shared" name="shared" type="checkbox" value="true" <%= 'checked' if @filter.shared -%>/>
-      </div>
-    </div>
+
+    <%= render :partial => 'shared_form', :locals => {:display_owner => true} %>
+
     <div class="modal-foot">
       <input type="submit" value="<%= h message('save') -%>" id="save-submit"/>
       <a href="#" onclick="return closeModalWindow()" id="save-cancel"><%= h message('cancel') -%></a>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_save_as_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_save_as_form.html.erb
index 19a6be0ef97..6bfdc3ae1c1 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_save_as_form.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_save_as_form.html.erb
@@ -6,23 +6,8 @@
       <h2>Save Filter</h2>
     </div>
 
-    <div class="modal-body">
-      <% @filter.errors.each do |attr, msg| %>
-        <p class="error"><%= h msg -%></p>
-      <% end %>
-      <div class="modal-field">
-        <label for="name">Name <em class="mandatory">*</em></label>
-        <input id="name" name="name" type="text" size="50" maxlength="100" value="<%= h @filter.name -%>" autofocus="autofocus"/>
-      </div>
-      <div class="modal-field">
-        <label for="description">Description</label>
-        <input id="description" name="description" type="text" size="50" maxlength="4000" value="<%= h @filter.description -%>"/>
-      </div>
-      <div class="modal-field">
-        <label for="shared">Shared with all users</label>
-        <input id="shared" name="shared" type="checkbox" value="true" <%= 'checked' if @filter.shared -%>/>
-      </div>
-    </div>
+    <%= render :partial => 'shared_form' %>
+
     <div class="modal-foot">
       <input type="submit" value="<%= h message('save') -%>" id="save-as-submit"/>
       <a href="#" onclick="return closeModalWindow()" id="save-as-cancel"><%= h message('cancel') -%></a>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_shared_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_shared_form.html.erb
new file mode 100644
index 00000000000..0bd280781b0
--- /dev/null
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/measures/_shared_form.html.erb
@@ -0,0 +1,26 @@
+<% if !local_assigns.has_key? :display_owner
+  display_owner = false
+   end %>
+<div class="modal-body">
+  <% @filter.errors.full_messages.each do |msg| %>
+    <p class="error"><%= h msg -%></p>
+  <% end %>
+  <div class="modal-field">
+    <label for="name"><%= h message('name') -%> <em class="mandatory">*</em></label>
+    <input id="name" name="name" type="text" size="50" maxlength="100" value="<%= h @filter.name -%>" autofocus="autofocus"/>
+  </div>
+  <div class="modal-field">
+    <label for="description"><%= h message('description') -%></label>
+    <input id="description" name="description" type="text" size="50" maxlength="4000" value="<%= h @filter.description -%>"/>
+  </div>
+  <% if display_owner && @filter.can_be_reassigned_by(current_user) %>
+    <div class="modal-field">
+      <label for="owner"><%= h message('owner') -%></label>
+      <%= user_select_tag('owner', :html_id => 'select-filter-owner', :selected_user => @filter.user) -%>
+    </div>
+  <% end %>
+  <div class="modal-field">
+    <label for="shared"><%= h message('measure_filter.shared_with_all_users') -%></label>
+    <input id="shared" name="shared" type="checkbox" value="true" <%= 'checked' if @filter.shared -%>/>
+  </div>
+</div>
\ No newline at end of file
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 cd6c87a504c..2eaee024cdb 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
@@ -20,6 +20,8 @@
 
 package org.sonar.server.issue;
 
+import org.hamcrest.BaseMatcher;
+import org.hamcrest.Description;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -222,7 +224,7 @@ public class IssueFilterServiceTest {
   public void should_update() {
     when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("john"));
 
-    DefaultIssueFilter result = service.update(new DefaultIssueFilter().setId(1L).setName("My New Filter"), userSession);
+    DefaultIssueFilter result = service.update(new DefaultIssueFilter().setId(1L).setName("My New Filter").setUser("john"), userSession);
     assertThat(result.name()).isEqualTo("My New Filter");
 
     verify(issueFilterDao).update(any(IssueFilterDto.class));
@@ -270,10 +272,10 @@ public class IssueFilterServiceTest {
   @Test
   public void should_not_update_if_name_already_used() {
     when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("john"));
-    when(issueFilterDao.selectByNameAndUser(eq("My Issue"), eq("john"), eq(1L))).thenReturn(new IssueFilterDto());
+    when(issueFilterDao.selectByNameAndUser("My Issue", "john", 1L)).thenReturn(new IssueFilterDto());
 
     try {
-      service.update(new DefaultIssueFilter().setId(1L).setName("My Issue"), userSession);
+      service.update(new DefaultIssueFilter().setId(1L).setUser("john").setName("My Issue"), userSession);
       fail();
     } catch (Exception e) {
       assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Name already exists");
@@ -294,6 +296,40 @@ public class IssueFilterServiceTest {
 
     verify(issueFilterDao).update(any(IssueFilterDto.class));
   }
+  
+  @Test
+  public void should_change_shared_filter_ownership_when_admin() throws Exception {
+    String currentUser = "dave.loper";
+    IssueFilterDto sharedFilter = new IssueFilterDto().setId(1L).setName("My filter").setUserLogin("former.owner").setShared(true);
+    IssueFilterDto expectedDto = new IssueFilterDto().setName("My filter").setUserLogin("new.owner").setShared(true);
+
+    when(authorizationDao.selectGlobalPermissions(currentUser)).thenReturn(newArrayList(UserRole.ADMIN));
+    when(issueFilterDao.selectById(1L)).thenReturn(sharedFilter);
+
+    DefaultIssueFilter issueFilter = new DefaultIssueFilter().setId(1L).setName("My filter").setShared(true).setUser("new.owner");
+    service.update(issueFilter, new UserSession(1, currentUser, null));
+
+    verify(issueFilterDao).update(argThat(Matches.filter(expectedDto)));
+  }
+
+  @Test
+  public void should_not_change_own_filter_ownership() throws Exception {
+    String currentUser = "dave.loper";
+    IssueFilterDto sharedFilter = new IssueFilterDto().setId(1L).setName("My filter").setUserLogin(currentUser).setShared(true);
+
+    when(authorizationDao.selectGlobalPermissions(currentUser)).thenReturn(newArrayList(UserRole.USER));
+    when(issueFilterDao.selectById(1L)).thenReturn(sharedFilter);
+
+    try {
+      DefaultIssueFilter issueFilter = new DefaultIssueFilter().setId(1L).setName("My filter").setShared(true).setUser("new.owner");
+      service.update(issueFilter, new UserSession(1, currentUser, null));
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("User is not authorized to change the owner of this filter");
+    }
+
+    verify(issueFilterDao, never()).update(any(IssueFilterDto.class));
+  }
 
   @Test
   public void should_delete() {
@@ -490,4 +526,34 @@ public class IssueFilterServiceTest {
     verify(issueFilterFavouriteDao, never()).delete(anyLong());
   }
 
+  private static class Matches extends BaseMatcher<IssueFilterDto> {
+
+    private final IssueFilterDto referenceFilter;
+
+    private Matches(IssueFilterDto reference) {
+      referenceFilter = reference;
+    }
+
+    static Matches filter(IssueFilterDto filterDto) {
+      return new Matches(filterDto);
+    }
+
+    @Override
+    public boolean matches(Object o) {
+      if(o != null && o instanceof IssueFilterDto) {
+        IssueFilterDto otherFilter = (IssueFilterDto) o;
+        return referenceFilter.isShared() == otherFilter.isShared()
+          && referenceFilter.getUserLogin() == otherFilter.getUserLogin()
+          && referenceFilter.getDescription() == otherFilter.getDescription()
+          && referenceFilter.getName() == otherFilter.getName()
+          && referenceFilter.getData() == otherFilter.getData();
+      }
+      return false;
+    }
+
+    @Override
+    public void describeTo(Description description) {
+    }
+  }
+
 }