[MGNLUI-3062] Deleting not empty folders in groups and roles does not work Created: 15/Jul/14 Updated: 05/Dec/14 Resolved: 11/Aug/14 |
|
| Status: | Closed |
| Project: | Magnolia UI |
| Component/s: | security app |
| Affects Version/s: | 5.3.1 |
| Fix Version/s: | 5.2.9, 5.3.2 |
| Type: | Bug | Priority: | Major |
| Reporter: | Karel de Witte | Assignee: | Evzen Fochr |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||
| Description |
|
Go to Security -> Groups Please see |
| Comments |
| Comment by Mikaël Geljić [ 29/Jul/14 ] |
|
Simply disabling the action makes it hard for the user to find out, when exactly he/she can delete an item. We should rather notify the user, say why we deny the delete. For single groups (resp. roles), if I attempt to delete a group which is still in use, I get a warning alert: Likewise for group folders (resp. role folders). I get a warning alert: If the folder contains only unused items, then we get the confirmation dialog as usual. |
| Comment by Milan Divilek [ 08/Aug/14 ] |
|
Reopen: It looks good, just few thinks to test info.magnolia.security.app.action.DeleteFolderActionTest. 1. #testFolderIsNotDeletedIfGroupInUse - line 164
actionGroups = new DeleteFolderAction(definition, itemRoles, commandsManager, eventBus, uiContext, i18n, securitySupport);
itemGroups should be used instead of itemRoles 2. In #setUp() you are setting two different info.magnolia.cms.security.SecuritySupport instances. securitySupport = mock(SecuritySupport.class); ComponentsTestUtil.setImplementation(SecuritySupport.class, SecuritySupportImpl.class); This then can easily caused problems during tests. For example try run this test. TestFolder will not be removed, but it should.
@Test
public void testFolderIsDeletedWhenContainsGroupAndGroupIsNotUsed() throws Exception {
// GIVEN
MgnlGroupManager mgnlGroupManager = new MgnlGroupManager() {
@Override
protected void validateGroupName(String name) throws AccessDeniedException {
//not needed for test
}
};
mgnlGroupManager.createGroup("/testFolder", "testGroup1");
((SecuritySupportImpl) Security.getSecuritySupport()).setGroupManager(mgnlGroupManager);
((SecuritySupportImpl) Security.getSecuritySupport()).setUserManagers(new HashMap<String, UserManager>());
actionGroups = new DeleteFolderAction(definition, itemGroups, commandsManager, eventBus, uiContext, i18n, securitySupport);
// WHEN
actionGroups.execute();
// THEN
assertTrue(!sessionGroups.itemExists("/testFolder"));
}
Fix this just simply by doing this in #setup() ComponentsTestUtil.setImplementation(SecuritySupport.class, SecuritySupportImpl.class); securitySupport = Components.getComponent(SecuritySupport.class); 3. in tests you are using deprecated Security class for getting SecuritySuppor instance. Use global securitySupport property(which you have already defined private SecuritySupport securitySupport) instead of it. |
| Comment by Evzen Fochr [ 11/Aug/14 ] |
| Comment by Aleksandr Pchelintcev [ 11/Aug/14 ] |
|
| Comment by Evzen Fochr [ 11/Aug/14 ] |