From 63deafdb8bac4e0f907397094f4855167d3dd1e5 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 18 Jun 2013 18:54:40 +0200 Subject: [PATCH] SONAR-4394 Provide ability to share issue filters --- .../resources/org/sonar/l10n/core.properties | 6 +- .../sonar/core/issue/db/IssueFilterDao.java | 9 ++ .../sonar/core/issue/db/IssueFilterDto.java | 2 +- .../issue/db/IssueFilterFavouriteDao.java | 14 +- .../issue/db/IssueFilterFavouriteMapper.java | 4 +- .../core/issue/db/IssueFilterMapper.java | 2 + .../org/sonar/core/user/AuthorizationDao.java | 16 ++- .../issue/db/IssueFilterFavouriteMapper.xml | 4 + .../sonar/core/issue/db/IssueFilterMapper.xml | 9 ++ .../sonar/core/user/AuthorizationMapper.xml | 11 ++ .../core/issue/db/IssueFilterDaoTest.java | 8 ++ .../issue/db/IssueFilterFavouriteDaoTest.java | 9 ++ .../sonar/core/user/AuthorizationDaoTest.java | 10 ++ ...hould_delete_by_issue_filter_id-result.xml | 9 ++ .../should_return_global_permissions.xml | 13 ++ .../issue/InternalRubyIssueService.java | 4 + .../server/issue/IssueFilterService.java | 102 +++++++++----- .../app/controllers/issues_controller.rb | 1 + .../app/views/issues/_action_links.html.erb | 17 --- .../app/views/issues/_shared_form.html.erb | 3 +- .../WEB-INF/app/views/issues/manage.html.erb | 64 ++++++++- .../WEB-INF/app/views/issues/search.html.erb | 38 ++++- .../issue/InternalRubyIssueServiceTest.java | 14 +- .../server/issue/IssueFilterServiceTest.java | 132 +++++++++++++++++- 24 files changed, 421 insertions(+), 80 deletions(-) create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest/should_delete_by_issue_filter_id-result.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/should_return_global_permissions.xml delete mode 100644 sonar-server/src/main/webapp/WEB-INF/app/views/issues/_action_links.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 bc3dc9e4f67..fc6c3b895ad 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 @@ -541,8 +541,10 @@ issue_filter.manage.my_filters=My Filters issue_filter.no_filters=No filters 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}"? - - +issue_filter.private=Private +issue_filter.shared_with_all_users=Shared with all users +issue_filter.sharing=Sharing +issue_filter.manage.shared_filters=Shared Filters #------------------------------------------------------------------------------ # diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDao.java index d74fafd02ae..b4ffcaeb880 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDao.java @@ -80,6 +80,15 @@ public class IssueFilterDao implements BatchComponent, ServerComponent { } } + public List selectSharedForUser(String user) { + SqlSession session = mybatis.openSession(); + try { + return getMapper(session).selectSharedForUser(user); + } finally { + MyBatis.closeQuietly(session); + } + } + public void insert(IssueFilterDto filter) { SqlSession session = mybatis.openSession(); try { diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDto.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDto.java index ac366e66a1f..5a831c285fa 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDto.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterDto.java @@ -33,7 +33,7 @@ public class IssueFilterDto { private Long id; private String name; private String userLogin; - private Boolean shared; + private boolean shared; private String description; private String data; private Date createdAt; diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteDao.java index 2f692c1b967..17e2b17b1de 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteDao.java @@ -64,10 +64,20 @@ public class IssueFilterFavouriteDao implements BatchComponent, ServerComponent } } - public void delete(Long id) { + public void delete(Long issueFilterFavouriteId) { SqlSession session = mybatis.openSession(); try { - getMapper(session).delete(id); + getMapper(session).delete(issueFilterFavouriteId); + session.commit(); + } finally { + MyBatis.closeQuietly(session); + } + } + + public void deleteByIssueFilterId(Long issueFilterId) { + SqlSession session = mybatis.openSession(); + try { + getMapper(session).deleteByIssueFilterId(issueFilterId); session.commit(); } finally { MyBatis.closeQuietly(session); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteMapper.java index 03986f82118..d11fc9d1ca1 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterFavouriteMapper.java @@ -36,5 +36,7 @@ public interface IssueFilterFavouriteMapper { void insert(IssueFilterFavouriteDto filterFavourite); - void delete(Long id); + void delete(Long issueFilterFavouriteId); + + void deleteByIssueFilterId(Long issueFilterId); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterMapper.java index f07316f07a1..53cf2b936eb 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueFilterMapper.java @@ -41,6 +41,8 @@ public interface IssueFilterMapper { List selectByUserWithOnlyFavoriteFilters(String user); + List selectSharedForUser(String user); + void insert(IssueFilterDto filter); void update(IssueFilterDto filter); diff --git a/sonar-core/src/main/java/org/sonar/core/user/AuthorizationDao.java b/sonar-core/src/main/java/org/sonar/core/user/AuthorizationDao.java index 90f0fcefb69..7a826417df0 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/AuthorizationDao.java +++ b/sonar-core/src/main/java/org/sonar/core/user/AuthorizationDao.java @@ -27,10 +27,7 @@ import org.sonar.core.persistence.MyBatis; import javax.annotation.Nullable; -import java.util.Collection; -import java.util.Collections; -import java.util.Map; -import java.util.Set; +import java.util.*; import static com.google.common.collect.Maps.newHashMap; @@ -92,4 +89,15 @@ public class AuthorizationDao implements ServerComponent { return session.selectList(sql, params); } + + public List selectGlobalPermissions(String userLogin){ + SqlSession session = mybatis.openSession(); + try { + Map params = newHashMap(); + params.put("userLogin", userLogin); + return session.selectList("selectGlobalPermissions", params); + } finally { + MyBatis.closeQuietly(session); + } + } } diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterFavouriteMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterFavouriteMapper.xml index 3c8d2db4718..80c36ec8c09 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterFavouriteMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueFilterFavouriteMapper.xml @@ -46,4 +46,8 @@ delete from issue_filter_favourites where id=#{id} + + delete from issue_filter_favourites where issue_filter_id=#{issueFilterId} + + 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 2d62e89af3d..a31b00273c5 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 @@ -52,6 +52,15 @@ + + INSERT INTO issue_filters (name, user_login, shared, description, data, created_at, updated_at) VALUES (#{name}, #{userLogin}, #{shared}, #{description}, #{data}, #{createdAt}, #{updatedAt}) diff --git a/sonar-core/src/main/resources/org/sonar/core/user/AuthorizationMapper.xml b/sonar-core/src/main/resources/org/sonar/core/user/AuthorizationMapper.xml index 88677e1dd94..c872b7274ac 100644 --- a/sonar-core/src/main/resources/org/sonar/core/user/AuthorizationMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/user/AuthorizationMapper.xml @@ -67,4 +67,15 @@ + + 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 514a7b29d7c..dd832ecf550 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 @@ -81,6 +81,14 @@ public class IssueFilterDaoTest extends AbstractDaoTestCase { assertThat(issueFilterDto.getId()).isEqualTo(2L); } + @Test + public void should_select_shared() { + setupData("shared"); + + assertThat(dao.selectSharedForUser("michael")).hasSize(1); + assertThat(dao.selectSharedForUser("stephane")).isEmpty(); + } + @Test public void should_insert() { setupData("shared"); diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest.java index 4cdaa9de69e..c41e1ab52a9 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest.java @@ -83,4 +83,13 @@ public class IssueFilterFavouriteDaoTest extends AbstractDaoTestCase { checkTables("should_delete", new String[]{"created_at"}, "issue_filter_favourites"); } + @Test + public void should_delete_by_issue_filter_id() { + setupData("shared"); + + dao.deleteByIssueFilterId(10l); + + checkTables("should_delete_by_issue_filter_id", new String[]{"created_at"}, "issue_filter_favourites"); + } + } diff --git a/sonar-core/src/test/java/org/sonar/core/user/AuthorizationDaoTest.java b/sonar-core/src/test/java/org/sonar/core/user/AuthorizationDaoTest.java index 42746971ac1..df198a60280 100644 --- a/sonar-core/src/test/java/org/sonar/core/user/AuthorizationDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/user/AuthorizationDaoTest.java @@ -150,4 +150,14 @@ public class AuthorizationDaoTest extends AbstractDaoTestCase { rootProjectIds = authorization.selectAuthorizedRootProjectsIds(null, "admin"); assertThat(rootProjectIds).isEmpty(); } + + @Test + public void should_return_global_permissions() { + setupData("should_return_global_permissions"); + + AuthorizationDao authorization = new AuthorizationDao(getMyBatis()); + assertThat(authorization.selectGlobalPermissions("john")).containsOnly("user", "admin"); + assertThat(authorization.selectGlobalPermissions("arthur")).containsOnly("user"); + assertThat(authorization.selectGlobalPermissions("none")).isEmpty(); + } } diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest/should_delete_by_issue_filter_id-result.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest/should_delete_by_issue_filter_id-result.xml new file mode 100644 index 00000000000..e1df29f80cb --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueFilterFavouriteDaoTest/should_delete_by_issue_filter_id-result.xml @@ -0,0 +1,9 @@ + + + + + diff --git a/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/should_return_global_permissions.xml b/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/should_return_global_permissions.xml new file mode 100644 index 00000000000..863d395048f --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/user/AuthorizationDaoTest/should_return_global_permissions.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + 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 e44e0bffd6c..50611a9dd39 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 @@ -502,6 +502,10 @@ public class InternalRubyIssueService implements ServerComponent { return result; } + public List findSharedFiltersForCurrentUser() { + return issueFilterService.findSharedFilters(UserSession.get()); + } + public List findFavouriteIssueFiltersForCurrentUser() { return issueFilterService.findFavoriteFilters(UserSession.get()); } 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 8de1a6da836..8be218777e8 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 @@ -26,11 +26,13 @@ import org.sonar.api.ServerComponent; import org.sonar.api.issue.IssueFinder; import org.sonar.api.issue.IssueQuery; import org.sonar.api.issue.IssueQueryResult; +import org.sonar.api.web.UserRole; import org.sonar.core.issue.DefaultIssueFilter; import org.sonar.core.issue.db.IssueFilterDao; import org.sonar.core.issue.db.IssueFilterDto; import org.sonar.core.issue.db.IssueFilterFavouriteDao; import org.sonar.core.issue.db.IssueFilterFavouriteDto; +import org.sonar.core.user.AuthorizationDao; import org.sonar.server.user.UserSession; import javax.annotation.CheckForNull; @@ -46,11 +48,13 @@ public class IssueFilterService implements ServerComponent { private final IssueFilterDao issueFilterDao; private final IssueFilterFavouriteDao issueFilterFavouriteDao; private final IssueFinder issueFinder; + private final AuthorizationDao authorizationDao; - public IssueFilterService(IssueFilterDao issueFilterDao, IssueFilterFavouriteDao issueFilterFavouriteDao, IssueFinder issueFinder) { + public IssueFilterService(IssueFilterDao issueFilterDao, IssueFilterFavouriteDao issueFilterFavouriteDao, IssueFinder issueFinder, AuthorizationDao authorizationDao) { this.issueFilterDao = issueFilterDao; this.issueFilterFavouriteDao = issueFilterFavouriteDao; this.issueFinder = issueFinder; + this.authorizationDao = authorizationDao; } public IssueQueryResult execute(IssueQuery issueQuery) { @@ -73,13 +77,7 @@ public class IssueFilterService implements ServerComponent { public List findByUser(UserSession userSession) { if (userSession.isLoggedIn() && userSession.login() != null) { - List issueFilterDtoList = issueFilterDao.selectByUser(userSession.login()); - return newArrayList(Iterables.transform(issueFilterDtoList, new Function() { - @Override - public DefaultIssueFilter apply(IssueFilterDto issueFilterDto) { - return issueFilterDto.toIssueFilter(); - } - })); + return toIssueFilters(issueFilterDao.selectByUser(userSession.login())); } return Collections.emptyList(); } @@ -88,15 +86,12 @@ public class IssueFilterService implements ServerComponent { verifyLoggedIn(userSession); issueFilter.setUser(userSession.login()); verifyNameIsNotAlreadyUsed(issueFilter, userSession); - - IssueFilterDto issueFilterDto = IssueFilterDto.toIssueFilter(issueFilter); - issueFilterDao.insert(issueFilterDto); - addFavouriteIssueFilter(issueFilterDto.getId(), userSession.login()); - return issueFilterDto.toIssueFilter(); + return insertIssueFilter(issueFilter, userSession.login()); } public DefaultIssueFilter update(DefaultIssueFilter issueFilter, UserSession userSession) { - findIssueFilter(issueFilter.id(), userSession); + IssueFilterDto issueFilterDto = findIssueFilter(issueFilter.id(), userSession); + verifyCurrentUserCanModifyFilter(issueFilterDto, userSession); verifyNameIsNotAlreadyUsed(issueFilter, userSession); issueFilterDao.update(IssueFilterDto.toIssueFilter(issueFilter)); @@ -105,6 +100,8 @@ public class IssueFilterService implements ServerComponent { public DefaultIssueFilter updateData(Long issueFilterId, Map mapData, UserSession userSession) { IssueFilterDto issueFilterDto = findIssueFilter(issueFilterId, userSession); + verifyCurrentUserCanModifyFilter(issueFilterDto, userSession); + DefaultIssueFilter issueFilter = issueFilterDto.toIssueFilter(); issueFilter.setData(mapData); issueFilterDao.update(IssueFilterDto.toIssueFilter(issueFilter)); @@ -112,11 +109,10 @@ public class IssueFilterService implements ServerComponent { } public void delete(Long issueFilterId, UserSession userSession) { - findIssueFilter(issueFilterId, userSession); - IssueFilterFavouriteDto issueFilterFavouriteDto = findFavouriteIssueFilter(userSession.login(), issueFilterId); - if (issueFilterFavouriteDto != null) { - deleteFavouriteIssueFilter(issueFilterFavouriteDto.getId()); - } + IssueFilterDto issueFilterDto = findIssueFilter(issueFilterId, userSession); + verifyCurrentUserCanModifyFilter(issueFilterDto, userSession); + + deleteFavouriteIssueFilters(issueFilterDto); issueFilterDao.delete(issueFilterId); } @@ -126,20 +122,19 @@ public class IssueFilterService implements ServerComponent { issueFilter.setData(issueFilterDtoToCopy.getData()); verifyNameIsNotAlreadyUsed(issueFilter, userSession); - IssueFilterDto issueFilterDto = IssueFilterDto.toIssueFilter(issueFilter); - issueFilterDao.insert(issueFilterDto); - return issueFilterDto.toIssueFilter(); + return insertIssueFilter(issueFilter, userSession.login()); + } + + public List findSharedFilters(UserSession userSession) { + if (userSession.isLoggedIn() && userSession.login() != null) { + return toIssueFilters(issueFilterDao.selectSharedForUser(userSession.login())); + } + return Collections.emptyList(); } public List findFavoriteFilters(UserSession userSession) { if (userSession.isLoggedIn() && userSession.login() != null) { - List issueFilterDtoList = issueFilterDao.selectByUserWithOnlyFavoriteFilters(userSession.login()); - return newArrayList(Iterables.transform(issueFilterDtoList, new Function() { - @Override - public DefaultIssueFilter apply(IssueFilterDto issueFilterDto) { - return issueFilterDto.toIssueFilter(); - } - })); + return toIssueFilters(issueFilterDao.selectByUserWithOnlyFavoriteFilters(userSession.login())); } return Collections.emptyList(); } @@ -150,18 +145,18 @@ public class IssueFilterService implements ServerComponent { if (issueFilterFavouriteDto == null) { addFavouriteIssueFilter(issueFilterId, userSession.login()); } else { - deleteFavouriteIssueFilter(issueFilterFavouriteDto.getId()); + deleteFavouriteIssueFilter(issueFilterFavouriteDto); } } - public IssueFilterDto findIssueFilter(Long id, UserSession userSession){ + public IssueFilterDto findIssueFilter(Long id, UserSession userSession) { verifyLoggedIn(userSession); IssueFilterDto issueFilterDto = issueFilterDao.selectById(id); if (issueFilterDto == null) { // TODO throw 404 throw new IllegalArgumentException("Filter not found: " + id); } - verifyCurrentUserIsOwnerOfFilter(issueFilterDto, userSession); + verifyCurrentUserCanReadFilter(issueFilterDto, userSession); return issueFilterDto; } @@ -171,14 +166,21 @@ public class IssueFilterService implements ServerComponent { } } - private void verifyCurrentUserIsOwnerOfFilter(IssueFilterDto issueFilterDto, UserSession userSession){ - if (!issueFilterDto.getUserLogin().equals(userSession.login())) { + private void verifyCurrentUserCanReadFilter(IssueFilterDto issueFilterDto, UserSession userSession) { + if (!issueFilterDto.getUserLogin().equals(userSession.login()) && !issueFilterDto.isShared()) { + // TODO throw unauthorized + throw new IllegalStateException("User is not authorized to read this filter"); + } + } + + private void verifyCurrentUserCanModifyFilter(IssueFilterDto issueFilterDto, UserSession userSession) { + if (!issueFilterDto.getUserLogin().equals(userSession.login()) && (!issueFilterDto.isShared() || !isAdmin(userSession.login()))) { // TODO throw unauthorized - throw new IllegalStateException("User is not authorized to get this filter"); + throw new IllegalStateException("User is not authorized to modify this filter"); } } - 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"); } @@ -195,8 +197,32 @@ public class IssueFilterService implements ServerComponent { issueFilterFavouriteDao.insert(issueFilterFavouriteDto); } - private void deleteFavouriteIssueFilter(Long issueFilterFavoriteId) { - issueFilterFavouriteDao.delete(issueFilterFavoriteId); + private void deleteFavouriteIssueFilter(IssueFilterFavouriteDto issueFilterFavouriteDto) { + issueFilterFavouriteDao.delete(issueFilterFavouriteDto.getId()); + } + + private void deleteFavouriteIssueFilters(IssueFilterDto issueFilterDto) { + issueFilterFavouriteDao.deleteByIssueFilterId(issueFilterDto.getId()); + } + + private DefaultIssueFilter insertIssueFilter(DefaultIssueFilter issueFilter, String user) { + IssueFilterDto issueFilterDto = IssueFilterDto.toIssueFilter(issueFilter); + issueFilterDao.insert(issueFilterDto); + addFavouriteIssueFilter(issueFilterDto.getId(), user); + return issueFilterDto.toIssueFilter(); + } + + private List toIssueFilters(List issueFilterDtoList) { + return newArrayList(Iterables.transform(issueFilterDtoList, new Function() { + @Override + public DefaultIssueFilter apply(IssueFilterDto issueFilterDto) { + return issueFilterDto.toIssueFilter(); + } + })); + } + + private boolean isAdmin(String user) { + return authorizationDao.selectGlobalPermissions(user).contains(UserRole.ADMIN); } } 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 d3d4500aed8..2d05a10d4d9 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 @@ -57,6 +57,7 @@ class IssuesController < ApplicationController def manage @issue_query = Internal.issues.toQuery({}) @filters = Internal.issues.findIssueFiltersForCurrentUser() + @shared_filters = Internal.issues.findSharedFiltersForCurrentUser() @favourite_filter_ids = @favourite_filters.map { |filter| filter.id } end 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 deleted file mode 100644 index 127732b939a..00000000000 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_action_links.html.erb +++ /dev/null @@ -1,17 +0,0 @@ -<% if logged_in? && !@criteria_params.empty? %> -
    - <% if @filter.id %> -
  • <%= message('copy') -%>
    • - <% end %> - <% if !defined?(@unchanged) && @filter.id && @filter.user == current_user.login %> -
  • - <%= link_to message('save'), params.merge({:action => 'save', :id => @filter.id}), :class => 'link-action', :id => 'save', :method => :post -%> -
    • - <% end %> - <% unless @filter.id %> -
  • - <%= message('save_as') -%> -
    • - <% end %> -
-<% end %> \ No newline at end of file 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 45f1bb7599b..695f50a8ff7 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 @@ -8,8 +8,7 @@ - - \ No newline at end of file diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/search.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/search.html.erb index 48f0fbb4f42..4e7b9f1652c 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/search.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/search.html.erb @@ -8,7 +8,43 @@ <%= render :partial => 'display_errors' %>
- <%= render :partial => 'action_links' -%> + <% if logged_in? && !@criteria_params.empty? %> +
    + <% if @filter.id %> +
  • <%= message('copy') -%>
    • + <% end %> + <% if !defined?(@unchanged) && @filter.id && @filter.user == current_user.login %> +
  • + <%= link_to message('save'), params.merge({:action => 'save', :id => @filter.id}), :class => 'link-action', :id => 'save', :method => :post -%> +
    • + <% end %> + <% unless @filter.id %> +
  • + <%= message('save_as') -%> +
    • + <% end %> +
+ +
+ <% if @filter.id && @filter.name.present? %> +

+ <%= h @filter.name -%> + + <% if !@filter.shared %> + [<%= message 'issue_filter.private' -%>] + <% elsif @filter.user==current_user.login %> + [<%= message 'issue_filter.shared_with_all_users' -%>] + <% elsif @filter.user %> + [<%= message 'shared_by' -%> <%= Api.users.findByLogin(@filter.user).name -%>] + <% end %> + + <% if @filter.user == current_user.login %> +  <%= image_tag 'pencil-small.png', :alt => message('edit') -%> + <% end %> +

+ <% end %> +
+ <% end %>
<% if @issues_result && @issues_result.maxResultsReached() %> 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 8f82db7347a..30a501842f4 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,13 @@ public class InternalRubyIssueServiceTest { assertThat(issueFilter.description()).isEqualTo("Long term issues"); } + @Test + public void should_update_data() { + Map data = newHashMap(); + service.updateIssueFilterData(10L, data); + verify(issueFilterService).updateData(eq(10L), eq(data), any(UserSession.class)); + } + @Test public void should_delete_issue_filter() { Result result = service.deleteIssueFilter(1L); @@ -408,10 +415,9 @@ public class InternalRubyIssueServiceTest { } @Test - public void should_update_data() { - Map data = newHashMap(); - service.updateIssueFilterData(10L, data); - verify(issueFilterService).updateData(eq(10L), eq(data), any(UserSession.class)); + public void should_find_shared_issue_filters() { + service.findSharedFiltersForCurrentUser(); + verify(issueFilterService).findSharedFilters(any(UserSession.class)); } @Test 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 a1e1aa0e179..5a2aad576cd 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 @@ -25,11 +25,13 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.sonar.api.issue.IssueFinder; import org.sonar.api.issue.IssueQuery; +import org.sonar.api.web.UserRole; import org.sonar.core.issue.DefaultIssueFilter; import org.sonar.core.issue.db.IssueFilterDao; import org.sonar.core.issue.db.IssueFilterDto; import org.sonar.core.issue.db.IssueFilterFavouriteDao; import org.sonar.core.issue.db.IssueFilterFavouriteDto; +import org.sonar.core.user.AuthorizationDao; import org.sonar.server.user.UserSession; import java.util.List; @@ -49,6 +51,7 @@ public class IssueFilterServiceTest { private IssueFilterDao issueFilterDao; private IssueFilterFavouriteDao issueFilterFavouriteDao; private IssueFinder issueFinder; + private AuthorizationDao authorizationDao; private UserSession userSession; @@ -62,8 +65,9 @@ public class IssueFilterServiceTest { issueFilterDao = mock(IssueFilterDao.class); issueFilterFavouriteDao = mock(IssueFilterFavouriteDao.class); issueFinder = mock(IssueFinder.class); + authorizationDao = mock(AuthorizationDao.class); - service = new IssueFilterService(issueFilterDao, issueFilterFavouriteDao, issueFinder); + service = new IssueFilterService(issueFilterDao, issueFilterFavouriteDao, issueFinder, authorizationDao); } @Test @@ -76,6 +80,16 @@ public class IssueFilterServiceTest { assertThat(issueFilter.id()).isEqualTo(1L); } + @Test + public void should_find_shared_filter_by_id() { + IssueFilterDto issueFilterDto = new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("arthur").setShared(true); + when(issueFilterDao.selectById(1L)).thenReturn(issueFilterDto); + + DefaultIssueFilter issueFilter = service.findById(1L, userSession); + assertThat(issueFilter).isNotNull(); + assertThat(issueFilter.id()).isEqualTo(1L); + } + @Test public void should_not_find_by_id_on_not_existing_issue() { when(issueFilterDao.selectById(1L)).thenReturn(null); @@ -101,15 +115,15 @@ public class IssueFilterServiceTest { } @Test - public void should_not_find_if_user_is_not_the_owner_of_filter() { - IssueFilterDto issueFilterDto = new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("eric"); + public void should_not_find_if_not_shared_and_user_is_not_the_owner_of_filter() { + IssueFilterDto issueFilterDto = new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("eric").setShared(false); when(issueFilterDao.selectById(1L)).thenReturn(issueFilterDto); try { // John is not authorized to see eric filters service.findById(1L, userSession); fail(); } catch (Exception e) { - assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("User is not authorized to get this filter"); + assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("User is not authorized to read this filter"); } } @@ -184,6 +198,17 @@ public class IssueFilterServiceTest { verify(issueFilterDao).update(any(IssueFilterDto.class)); } + @Test + public void should_update_shared_filter_if_admin() { + when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(UserRole.ADMIN)); + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("arthur").setShared(true)); + + DefaultIssueFilter result = service.update(new DefaultIssueFilter().setId(1L).setName("My New Filter"), userSession); + assertThat(result.name()).isEqualTo("My New Filter"); + + verify(issueFilterDao).update(any(IssueFilterDto.class)); + } + @Test public void should_not_update_if_filter_not_found() { when(issueFilterDao.selectById(1L)).thenReturn(null); @@ -197,6 +222,20 @@ public class IssueFilterServiceTest { verify(issueFilterDao, never()).update(any(IssueFilterDto.class)); } + @Test + public void should_not_update_if_shared_and_not_admin() { + when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(UserRole.USER)); + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Old Filter").setUserLogin("arthur").setShared(true)); + + try { + service.update(new DefaultIssueFilter().setId(1L).setName("My New Filter"), userSession); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("User is not authorized to modify this filter"); + } + verify(issueFilterDao, never()).update(any(IssueFilterDto.class)); + } + @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")); @@ -241,7 +280,7 @@ public class IssueFilterServiceTest { service.delete(1L, userSession); verify(issueFilterDao).delete(1L); - verify(issueFilterFavouriteDao).delete(10L); + verify(issueFilterFavouriteDao).deleteByIssueFilterId(1L); } @Test @@ -257,6 +296,44 @@ public class IssueFilterServiceTest { verify(issueFilterDao, never()).delete(anyLong()); } + @Test + public void should_delete_shared_filter_if_user_is_admin() { + when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(UserRole.ADMIN)); + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("arthur").setShared(true)); + + service.delete(1L, userSession); + + verify(issueFilterDao).delete(1L); + } + + @Test + public void should_not_delete_not_shared_filter_if_user_is_admin() { + when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(UserRole.ADMIN)); + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("arthur").setShared(false)); + + try { + service.delete(1L, userSession); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("User is not authorized to read this filter"); + } + verify(issueFilterDao, never()).delete(anyLong()); + } + + @Test + public void should_not_delete_shared_filter_if_not_admin() { + when(authorizationDao.selectGlobalPermissions("john")).thenReturn(newArrayList(UserRole.USER)); + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("arthur").setShared(true)); + + try { + service.delete(1L, userSession); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("User is not authorized to modify this filter"); + } + verify(issueFilterDao, never()).delete(anyLong()); + } + @Test public void should_copy() { when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("john").setData("componentRoots=struts")); @@ -270,6 +347,28 @@ public class IssueFilterServiceTest { verify(issueFilterDao).insert(any(IssueFilterDto.class)); } + @Test + public void should_copy_shared_filter() { + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("arthur").setShared(true)); + 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"); + + verify(issueFilterDao).insert(any(IssueFilterDto.class)); + } + + @Test + public void should_add_favorite_on_copy() { + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("john").setData("componentRoots=struts")); + DefaultIssueFilter issueFilter = new DefaultIssueFilter().setName("Copy Of My Issue"); + service.copy(1L, issueFilter, userSession); + + verify(issueFilterDao).insert(any(IssueFilterDto.class)); + verify(issueFilterFavouriteDao).insert(any(IssueFilterFavouriteDto.class)); + } + @Test public void should_execute_from_issue_query() { IssueQuery issueQuery = IssueQuery.builder().build(); @@ -292,6 +391,14 @@ public class IssueFilterServiceTest { assertThat(issueQuery.componentRoots()).contains("struts"); } + @Test + public void should_find_shared_issue_filter() { + when(issueFilterDao.selectSharedForUser("john")).thenReturn(newArrayList(new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("john").setShared(true))); + + List results = service.findSharedFilters(userSession); + assertThat(results).hasSize(1); + } + @Test public void should_find_favourite_issue_filter() { when(issueFilterDao.selectByUserWithOnlyFavoriteFilters("john")).thenReturn(newArrayList(new IssueFilterDto().setId(1L).setName("My Issue").setUserLogin("john"))); @@ -323,6 +430,21 @@ public class IssueFilterServiceTest { assertThat(issueFilterFavouriteDto.getUserLogin()).isEqualTo("john"); } + @Test + public void should_add_favourite_on_shared_filter() { + when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("arthur").setShared(true)); + // The filter is not in the favorite list --> add to favorite + when(issueFilterFavouriteDao.selectByUserAndIssueFilterId("john", 1L)).thenReturn(null); + + ArgumentCaptor issueFilterFavouriteDtoCaptor = ArgumentCaptor.forClass(IssueFilterFavouriteDto.class); + service.toggleFavouriteIssueFilter(1L, userSession); + verify(issueFilterFavouriteDao).insert(issueFilterFavouriteDtoCaptor.capture()); + + IssueFilterFavouriteDto issueFilterFavouriteDto = issueFilterFavouriteDtoCaptor.getValue(); + assertThat(issueFilterFavouriteDto.getIssueFilterId()).isEqualTo(1L); + assertThat(issueFilterFavouriteDto.getUserLogin()).isEqualTo("john"); + } + @Test public void should_delete_favourite_issue_filter_id() { when(issueFilterDao.selectById(1L)).thenReturn(new IssueFilterDto().setId(1L).setName("My Issues").setUserLogin("john").setData("componentRoots=struts")); -- 2.39.5