From 2843fe0d6357ef41ca7d4298486ee46538ad58c1 Mon Sep 17 00:00:00 2001 From: Dean Del Ponte Date: Mon, 15 Apr 2019 13:10:51 -0500 Subject: [PATCH 1/2] Issue 106: Removing a Role throws an exception - fixed bug so roles may be deleted without exception - added an integration - added `.java-version` to `.gitignore` so that `jenv` config files are ignored --- .gitignore | 1 + .../groovy/module/RolesTab.groovy | 4 ++ ...{UserSpec.groovy => UserSimpleSpec.groovy} | 54 ++++++++++++++++++- .../ui/SpringSecurityUiService.groovy | 7 ++- 4 files changed, 64 insertions(+), 2 deletions(-) rename examples/simple/src/integration-test/groovy/spec/{UserSpec.groovy => UserSimpleSpec.groovy} (73%) diff --git a/.gitignore b/.gitignore index 30149a0e..3f938b7a 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ classes cobertura.ser kindlegen /plugin/src/main/templates/views +.java-version \ No newline at end of file diff --git a/examples/simple/src/integration-test/groovy/module/RolesTab.groovy b/examples/simple/src/integration-test/groovy/module/RolesTab.groovy index c5b629ba..6d2532d9 100644 --- a/examples/simple/src/integration-test/groovy/module/RolesTab.groovy +++ b/examples/simple/src/integration-test/groovy/module/RolesTab.groovy @@ -33,6 +33,10 @@ class RolesTab extends Module { $("input", type: "checkbox", id: roleName).value(true) } + void disableRole(String roleName) { + $("input", type: "checkbox", id: roleName).value(false) + } + boolean hasEnabledRole(String roleName) { return hasEnabledRoles([roleName]) } diff --git a/examples/simple/src/integration-test/groovy/spec/UserSpec.groovy b/examples/simple/src/integration-test/groovy/spec/UserSimpleSpec.groovy similarity index 73% rename from examples/simple/src/integration-test/groovy/spec/UserSpec.groovy rename to examples/simple/src/integration-test/groovy/spec/UserSimpleSpec.groovy index 4216a940..11fc4322 100644 --- a/examples/simple/src/integration-test/groovy/spec/UserSpec.groovy +++ b/examples/simple/src/integration-test/groovy/spec/UserSimpleSpec.groovy @@ -4,6 +4,7 @@ import page.user.UserCreatePage import page.user.UserEditPage import page.user.UserSearchPage import spock.lang.IgnoreIf +import spock.lang.IgnoreRest import spock.lang.Issue @IgnoreIf({ @@ -18,7 +19,7 @@ import spock.lang.Issue } false }) -class UserSpec extends AbstractSecuritySpec { +class UserSimpleSpec extends AbstractSecuritySpec { void testFindAll() { when: @@ -204,4 +205,55 @@ class UserSpec extends AbstractSecuritySpec { assert rolesTab.hasEnabledRoles(['ROLE_USER', 'ROLE_ADMIN']) assert rolesTab.totalRoles() == 12 } + + @Issue("https://github.com/grails-plugins/grails-spring-security-ui/issues/106") + void testUserRoleAssociationsAreRemoved() { + when: "edit user 1" + go 'user/edit/1' + + then: + at UserEditPage + + when: "select Roles tab" + rolesTab.select() + + then: "12 roles are listed and 1 is enabled" + assert rolesTab.totalRoles() == 12 + assert rolesTab.totalEnabledRoles() == 1 + assert rolesTab.hasEnabledRole('ROLE_USER') + + when: "ROLE_ADMIN is enabled and the changes are saved" + rolesTab.enableRole "ROLE_ADMIN" + submit() + rolesTab.select() + + then: "12 roles are listed and 2 are enabled" + assert rolesTab.totalEnabledRoles() == 2 + assert rolesTab.hasEnabledRoles(['ROLE_USER', 'ROLE_ADMIN']) + assert rolesTab.totalRoles() == 12 + + when: "edit user 1" + go 'user/edit/1' + + then: + at UserEditPage + + when: "select Roles tab" + rolesTab.select() + + then: "12 roles are listed and 2 are enabled" + assert rolesTab.totalRoles() == 12 + assert rolesTab.totalEnabledRoles() == 2 + assert rolesTab.hasEnabledRole('ROLE_USER') + + when: "ROLE_ADMIN is disabled and the changes are saved" + rolesTab.disableRole "ROLE_ADMIN" + submit() + rolesTab.select() + + then: "12 roles are listed and 1 is enabled" + assert rolesTab.totalEnabledRoles() == 1 + assert rolesTab.hasEnabledRoles(['ROLE_USER']) + assert rolesTab.totalRoles() == 12 + } } diff --git a/plugin/grails-app/services/grails/plugin/springsecurity/ui/SpringSecurityUiService.groovy b/plugin/grails-app/services/grails/plugin/springsecurity/ui/SpringSecurityUiService.groovy index 3e408d83..b01c5663 100644 --- a/plugin/grails-app/services/grails/plugin/springsecurity/ui/SpringSecurityUiService.groovy +++ b/plugin/grails-app/services/grails/plugin/springsecurity/ui/SpringSecurityUiService.groovy @@ -440,10 +440,15 @@ class SpringSecurityUiService implements AclStrategy, ErrorsStrategy, Persistent protected void removeUserRoles(user, Set rolesToRemove) { rolesToRemove.each { role -> - removeUserRole(user, role) + removeUserRoleAndReturnBoolean(user, role) } } + @Transactional + boolean removeUserRoleAndReturnBoolean(def user, def role) { + UserRole.remove user, role + } + @Transactional Number removeUserRole(def u, def r) { UserRole.where { user == u && role == r }.deleteAll() From 1abb2e7297cd029be2253ecdf5e849284b2be525 Mon Sep 17 00:00:00 2001 From: Dean Del Ponte Date: Mon, 15 Apr 2019 13:56:58 -0500 Subject: [PATCH 2/2] Issue 106: Removing a Role throws an exception - fixed the new test that was passing when run in isolation with the annotation `@IgnoreRest` but failing when run along with other tests in the `Spec` --- .../groovy/spec/UserSimpleSpec.groovy | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/examples/simple/src/integration-test/groovy/spec/UserSimpleSpec.groovy b/examples/simple/src/integration-test/groovy/spec/UserSimpleSpec.groovy index 11fc4322..678e6487 100644 --- a/examples/simple/src/integration-test/groovy/spec/UserSimpleSpec.groovy +++ b/examples/simple/src/integration-test/groovy/spec/UserSimpleSpec.groovy @@ -208,8 +208,8 @@ class UserSimpleSpec extends AbstractSecuritySpec { @Issue("https://github.com/grails-plugins/grails-spring-security-ui/issues/106") void testUserRoleAssociationsAreRemoved() { - when: "edit user 1" - go 'user/edit/1' + when: "edit user 2" + go 'user/edit/2' then: at UserEditPage @@ -232,8 +232,8 @@ class UserSimpleSpec extends AbstractSecuritySpec { assert rolesTab.hasEnabledRoles(['ROLE_USER', 'ROLE_ADMIN']) assert rolesTab.totalRoles() == 12 - when: "edit user 1" - go 'user/edit/1' + when: "edit user 2" + go 'user/edit/2' then: at UserEditPage @@ -249,6 +249,12 @@ class UserSimpleSpec extends AbstractSecuritySpec { when: "ROLE_ADMIN is disabled and the changes are saved" rolesTab.disableRole "ROLE_ADMIN" submit() + go 'user/edit/2' + + then: + at UserEditPage + + when: "select Roles tab" rolesTab.select() then: "12 roles are listed and 1 is enabled"