[MGNLUI-318] Deletion of assigned roles and/or groups should be prevented Created: 03/Dec/12 Updated: 16/Aug/13 Resolved: 16/Jul/13 |
|
| Status: | Closed |
| Project: | Magnolia UI |
| Component/s: | security app |
| Affects Version/s: | 5.0 |
| Fix Version/s: | 5.1 |
| Type: | Task | Priority: | Critical |
| Reporter: | Federico Grilli | Assignee: | Jozef Chocholacek |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Template: |
|
| Acceptance criteria: |
Empty
|
| Task DoR: |
Empty
|
| Date of First Response: |
| Description |
|
at the very least a message should warn beforehand that a given role/group is assigned to an existing (AND enabled?) user. |
| Comments |
| Comment by Jozef Chocholacek [ 12/Jul/13 ] |
|
In contrary to the |
| Comment by Eric Hechinger [ 16/Jul/13 ] |
|
In DeleteRoleAction
Same remarks for DeleteGroupAction. Would it not be better to create an Abstract Action that expose getErrorMessage instead of duplicate them. |
| Comment by Daniel Lipp [ 16/Jul/13 ] |
|
I think there's more potential to avoid DRY - the two actions are almost identical. Two proposals to simplify: a) introduced common supertype e.g. AbstractDeleteGroupOrRoleAction (sry - can't think of a better name now) or even just have one DeleteGroupOrRoleAction that e.g. has a flag telling whether it's currently used for deleting a Group or a Role. |
| Comment by Daniel Lipp [ 16/Jul/13 ] |
|
a) there's more potential to be DRY
only getGroupsOrRolesFor(User) and getGroupsOrRolesFor(Group) have to be implemented in the subclasses b) we don't have i18n for error messages - so executeAfterConfirmation could just have the line
log.error("Cannot get the users/groups the group or role is assigned to.", e);
and one log on the abstract supertype would be enough c) StringBuilder would be even better than StringBuffer - the builder isn't synchronized so it's much faster d) looks like there's no unit-tests for the new types - pls add |