mirror of
https://github.com/Frooodle/Stirling-PDF.git
synced 2025-10-25 11:17:28 +02:00
feat: Improve team management UX with message-based feedback and internal team protection (#3719)
# Description of Changes - Refactored team management logic to unify and streamline feedback via `messageType` query parameters. - Added backend checks to prevent renaming, deleting, or reassigning users to/from the protected Internal team. - Updated Thymeleaf templates (`teams.html`, `team-details.html`, `adminSettings.html`) to support user-visible success and error messages based on controller redirects. - Ensured `team.cannotMoveInternalUsers`, `team.internalTeamNotAccessible`, and `invalidRoleMessage` are properly internationalized. - Replaced hardcoded `/adminSettings` redirects with `/teams` for more consistent UX. **Why**: To provide admins with immediate, meaningful feedback during team operations and to enforce data integrity around protected teams like "Internal". --- ## Checklist ### General - [x] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [x] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/HowToAddNewLanguage.md) (if applicable) - [x] I have performed a self-review of my own code - [x] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [x] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#6-testing) for more details. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Ludy
GitHub
parent
3ddb370f69
commit
136f16f613
@ -331,6 +331,9 @@ public class AccountWebController {
|
||||
case "userNotFound" -> "userNotFoundMessage";
|
||||
case "downgradeCurrentUser" -> "downgradeCurrentUserMessage";
|
||||
case "disabledCurrentUser" -> "disabledCurrentUserMessage";
|
||||
case "cannotMoveInternalUsers" -> "team.cannotMoveInternalUsers";
|
||||
case "internalTeamNotAccessible" -> "team.internalTeamNotAccessible";
|
||||
case "invalidRole" -> "invalidRoleMessage";
|
||||
default -> messageType;
|
||||
};
|
||||
model.addAttribute("changeMessage", changeMessage);
|
||||
|
||||
@ -31,12 +31,12 @@ public class TeamController {
|
||||
@PostMapping("/create")
|
||||
public RedirectView createTeam(@RequestParam("name") String name) {
|
||||
if (teamRepository.existsByNameIgnoreCase(name)) {
|
||||
return new RedirectView("/adminSettings?messageType=teamExists");
|
||||
return new RedirectView("/teams?messageType=teamExists");
|
||||
}
|
||||
Team team = new Team();
|
||||
team.setName(name);
|
||||
teamRepository.save(team);
|
||||
return new RedirectView("/adminSettings?messageType=teamCreated");
|
||||
return new RedirectView("/teams?messageType=teamCreated");
|
||||
}
|
||||
|
||||
@PreAuthorize("hasRole('ROLE_ADMIN')")
|
||||
@ -45,21 +45,21 @@ public class TeamController {
|
||||
@RequestParam("teamId") Long teamId, @RequestParam("newName") String newName) {
|
||||
Optional<Team> existing = teamRepository.findById(teamId);
|
||||
if (existing.isEmpty()) {
|
||||
return new RedirectView("/adminSettings?messageType=teamNotFound");
|
||||
return new RedirectView("/teams?messageType=teamNotFound");
|
||||
}
|
||||
if (teamRepository.existsByNameIgnoreCase(newName)) {
|
||||
return new RedirectView("/adminSettings?messageType=teamNameExists");
|
||||
return new RedirectView("/teams?messageType=teamNameExists");
|
||||
}
|
||||
Team team = existing.get();
|
||||
|
||||
// Prevent renaming the Internal team
|
||||
if (team.getName().equals(TeamService.INTERNAL_TEAM_NAME)) {
|
||||
return new RedirectView("/adminSettings?messageType=internalTeamNotAccessible");
|
||||
return new RedirectView("/teams?messageType=internalTeamNotAccessible");
|
||||
}
|
||||
|
||||
team.setName(newName);
|
||||
teamRepository.save(team);
|
||||
return new RedirectView("/adminSettings?messageType=teamRenamed");
|
||||
return new RedirectView("/teams?messageType=teamRenamed");
|
||||
}
|
||||
|
||||
@PreAuthorize("hasRole('ROLE_ADMIN')")
|
||||
@ -68,23 +68,23 @@ public class TeamController {
|
||||
public RedirectView deleteTeam(@RequestParam("teamId") Long teamId) {
|
||||
Optional<Team> teamOpt = teamRepository.findById(teamId);
|
||||
if (teamOpt.isEmpty()) {
|
||||
return new RedirectView("/adminSettings?messageType=teamNotFound");
|
||||
return new RedirectView("/teams?messageType=teamNotFound");
|
||||
}
|
||||
|
||||
Team team = teamOpt.get();
|
||||
|
||||
// Prevent deleting the Internal team
|
||||
if (team.getName().equals(TeamService.INTERNAL_TEAM_NAME)) {
|
||||
return new RedirectView("/adminSettings?messageType=internalTeamNotAccessible");
|
||||
return new RedirectView("/teams?messageType=internalTeamNotAccessible");
|
||||
}
|
||||
|
||||
long memberCount = userRepository.countByTeam(team);
|
||||
if (memberCount > 0) {
|
||||
return new RedirectView("/adminSettings?messageType=teamHasUsers");
|
||||
return new RedirectView("/teams?messageType=teamHasUsers");
|
||||
}
|
||||
|
||||
teamRepository.delete(team);
|
||||
return new RedirectView("/adminSettings?messageType=teamDeleted");
|
||||
return new RedirectView("/teams?messageType=teamDeleted");
|
||||
}
|
||||
|
||||
@PreAuthorize("hasRole('ROLE_ADMIN')")
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
package stirling.software.proprietary.security.controller.web;
|
||||
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
import java.util.Date;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
@ -32,7 +33,7 @@ public class TeamWebController {
|
||||
|
||||
@GetMapping
|
||||
@PreAuthorize("hasRole('ROLE_ADMIN')")
|
||||
public String listTeams(Model model) {
|
||||
public String listTeams(HttpServletRequest request, Model model) {
|
||||
// Get teams with user counts using a DTO projection
|
||||
List<TeamWithUserCountDTO> allTeamsWithCounts = teamRepository.findAllTeamsWithUserCount();
|
||||
|
||||
@ -53,6 +54,27 @@ public class TeamWebController {
|
||||
teamLastRequest.put(teamId, lastActivity);
|
||||
}
|
||||
|
||||
String messageType = request.getParameter("messageType");
|
||||
if (messageType != null) {
|
||||
if ("teamCreated".equals(messageType)) {
|
||||
model.addAttribute("addMessage", "teamCreated");
|
||||
} else if ("teamExists".equals(messageType)) {
|
||||
model.addAttribute("errorMessage", "teamExists");
|
||||
} else if ("teamNotFound".equals(messageType)) {
|
||||
model.addAttribute("errorMessage", "teamNotFound");
|
||||
} else if ("teamNameExists".equals(messageType)) {
|
||||
model.addAttribute("errorMessage", "teamNameExists");
|
||||
} else if ("internalTeamNotAccessible".equals(messageType)) {
|
||||
model.addAttribute("errorMessage", "team.internalTeamNotAccessible");
|
||||
} else if ("teamRenamed".equals(messageType)) {
|
||||
model.addAttribute("changeMessage", "teamRenamed");
|
||||
} else if ("teamHasUsers".equals(messageType)) {
|
||||
model.addAttribute("errorMessage", "teamHasUsers");
|
||||
} else if ("teamDeleted".equals(messageType)) {
|
||||
model.addAttribute("deleteMessage", "teamDeleted");
|
||||
}
|
||||
}
|
||||
|
||||
// Add data to the model
|
||||
model.addAttribute("teamsWithCounts", teamsWithCounts);
|
||||
model.addAttribute("teamLastRequest", teamLastRequest);
|
||||
@ -62,7 +84,8 @@ public class TeamWebController {
|
||||
|
||||
@GetMapping("/{id}")
|
||||
@PreAuthorize("hasRole('ROLE_ADMIN')")
|
||||
public String viewTeamDetails(@PathVariable("id") Long id, Model model) {
|
||||
public String viewTeamDetails(
|
||||
HttpServletRequest request, @PathVariable("id") Long id, Model model) {
|
||||
// Get the team
|
||||
Team team =
|
||||
teamRepository
|
||||
@ -105,6 +128,13 @@ public class TeamWebController {
|
||||
userLastRequest.put(username, lastRequest);
|
||||
}
|
||||
|
||||
String errorMessage = request.getParameter("error");
|
||||
if (errorMessage != null) {
|
||||
if ("cannotMoveInternalUsers".equals(errorMessage)) {
|
||||
model.addAttribute("errorMessage", "team.cannotMoveInternalUsers");
|
||||
}
|
||||
}
|
||||
|
||||
model.addAttribute("team", team);
|
||||
model.addAttribute("teamUsers", teamUsers);
|
||||
model.addAttribute("availableUsers", availableUsers);
|
||||
|
||||
@ -32,6 +32,11 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<!-- Alert Messages -->
|
||||
<div th:if="${errorMessage}" class="alert alert-danger data-mb-3">
|
||||
<span th:text="#{${errorMessage}}">Default message if not found</span>
|
||||
</div>
|
||||
|
||||
<div class="data-actions data-actions-start">
|
||||
<a th:href="@{'/teams'}" class="data-btn data-btn-secondary">
|
||||
<span class="material-symbols-rounded">arrow_back</span>
|
||||
|
||||
@ -29,12 +29,29 @@
|
||||
<div class="data-body">
|
||||
<!-- Back Button -->
|
||||
<div class="data-actions data-actions-start">
|
||||
<a href="/adminSettings" class="data-btn data-btn-secondary">
|
||||
<a th:href="@{'/adminSettings'}" class="data-btn data-btn-secondary">
|
||||
<span class="material-symbols-rounded">arrow_back</span>
|
||||
<span th:text="#{back.toSettings}">Back to Settings</span>
|
||||
</a>
|
||||
</div>
|
||||
|
||||
<!-- Alert Messages -->
|
||||
<div th:if="${addMessage}" class="alert alert-success data-mb-3">
|
||||
<span th:text="#{${addMessage}}">Default message if not found</span>
|
||||
</div>
|
||||
|
||||
<div th:if="${changeMessage}" class="alert alert-success data-mb-3">
|
||||
<span th:text="#{${changeMessage}}">Default message if not found</span>
|
||||
</div>
|
||||
|
||||
<div th:if="${deleteMessage}" class="alert alert-danger data-mb-3">
|
||||
<span th:text="#{${deleteMessage}}">Default message if not found</span>
|
||||
</div>
|
||||
|
||||
<div th:if="${errorMessage}" class="alert alert-danger data-mb-3">
|
||||
<span th:text="#{${errorMessage}}">Default message if not found</span>
|
||||
</div>
|
||||
|
||||
<!-- Create New Team Button -->
|
||||
<div class="data-actions">
|
||||
<a href="#" th:data-bs-toggle="${@runningProOrHigher} ? 'modal' : null"
|
||||
|
||||
@ -200,6 +200,7 @@ disabledCurrentUserMessage=The current user cannot be disabled
|
||||
downgradeCurrentUserLongMessage=Cannot downgrade current user's role. Hence, current user will not be shown.
|
||||
userAlreadyExistsOAuthMessage=The user already exists as an OAuth2 user.
|
||||
userAlreadyExistsWebMessage=The user already exists as an web user.
|
||||
invalidRoleMessage=Invalid role.
|
||||
error=Error
|
||||
oops=Oops!
|
||||
help=Help
|
||||
|
||||
@ -93,8 +93,8 @@
|
||||
<span class="material-symbols-rounded">person_add</span>
|
||||
<span th:text="#{adminUserSettings.addUser}">Add New User</span>
|
||||
</button>
|
||||
|
||||
<a href="/teams" class="data-btn data-btn-secondary" th:title="#{adminUserSettings.teams}">
|
||||
|
||||
<a th:href="@{'/teams'}" class="data-btn data-btn-secondary" th:title="#{adminUserSettings.teams}">
|
||||
<span class="material-symbols-rounded">group</span>
|
||||
<span th:text="#{adminUserSettings.teams}">Manage Teams</span>
|
||||
</a>
|
||||
@ -108,7 +108,7 @@
|
||||
<span th:text="#{adminUserSettings.changeUserRole}">Change User's Role</span>
|
||||
</button>
|
||||
|
||||
<a href="/usage" th:if="${@runningEE}" class="data-btn data-btn-secondary" th:title="#{adminUserSettings.usage}">
|
||||
<a th:href="@{'/usage'}" th:if="${@runningEE}" class="data-btn data-btn-secondary" th:title="#{adminUserSettings.usage}">
|
||||
<span class="material-symbols-rounded">analytics</span>
|
||||
<span th:text="#{adminUserSettings.usage}">Usage Statistics</span>
|
||||
</a>
|
||||
@ -120,27 +120,27 @@
|
||||
<thead>
|
||||
<tr>
|
||||
<th scope="col">#</th>
|
||||
<th scope="col" th:title="#{username}" th:text="#{username}">Username</th>
|
||||
<th scope="col" th:title="#{adminUserSettings.team}" th:text="#{adminUserSettings.team}">Team</th>
|
||||
<th scope="col" th:title="#{adminUserSettings.role}" th:text="#{adminUserSettings.role}">Roles</th>
|
||||
<th scope="col" th:title="#{username}" class="text-overflow" th:text="#{username}">Username</th>
|
||||
<th scope="col" th:title="#{adminUserSettings.team}" class="text-overflow" th:text="#{adminUserSettings.team}">Team</th>
|
||||
<th scope="col" th:title="#{adminUserSettings.role}" class="text-overflow" th:text="#{adminUserSettings.role}">Roles</th>
|
||||
<th scope="col" th:title="#{adminUserSettings.authenticated}" class="text-overflow" th:text="#{adminUserSettings.authenticated}">Authenticated</th>
|
||||
<th scope="col" th:title="#{adminUserSettings.lastRequest}" class="text-overflow" th:text="#{adminUserSettings.lastRequest}">Last Request</th>
|
||||
<th scope="col" th:title="#{adminUserSettings.actions}" th:text="#{adminUserSettings.actions}">Actions</th>
|
||||
<th scope="col" th:title="#{adminUserSettings.actions}" class="text-overflow" th:text="#{adminUserSettings.actions}">Actions</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
<tr th:each="user : ${users}">
|
||||
<td th:text="${user.id}"></td>
|
||||
<td th:text="${user.username}" th:classappend="${userSessions[user.username] ? 'active-user' : ''}"></td>
|
||||
<td th:text="${user.team != null ? user.team.name : '—'}"></td>
|
||||
<td th:text="${user.id}" th:title="${user.id}" class="text-overflow"></td>
|
||||
<td th:text="${user.username}" th:title="${user.username}" class="text-overflow" th:classappend="${userSessions[user.username] ? 'active-user' : ''}"></td>
|
||||
<td th:text="${user.team != null ? user.team.name : '—'}" th:title="${user.team != null ? user.team.name : '—'}" class="text-overflow"></td>
|
||||
<td>
|
||||
<span class="data-badge" style="background-color: var(--md-sys-color-secondary-container); color: var(--md-sys-color-secondary); padding: 0.25rem 0.5rem; border-radius: 1rem; font-size: 0.875rem; display: inline-flex; align-items: center; gap: 0.25rem;">
|
||||
<span class="material-symbols-rounded" style="font-size: 1rem;">shield</span>
|
||||
<span th:text="#{${user.roleName}}">Role</span>
|
||||
<span th:text="#{${user.roleName}}" th:title="#{${user.roleName}}" class="text-overflow">Role</span>
|
||||
</span>
|
||||
</td>
|
||||
<td th:text="${user.authenticationType}"></td>
|
||||
<td th:text="${userLastRequest[user.username] != null ? #dates.format(userLastRequest[user.username], 'yyyy-MM-dd HH:mm:ss') : 'N/A'}"></td>
|
||||
<td th:text="${user.authenticationType}" th:title="${user.authenticationType}"></td>
|
||||
<td th:text="${userLastRequest[user.username] != null ? #dates.format(userLastRequest[user.username], 'yyyy-MM-dd HH:mm:ss') : 'N/A'}" th:title="${userLastRequest[user.username] != null ? #dates.format(userLastRequest[user.username], 'yyyy-MM-dd HH:mm:ss') : 'N/A'}"></td>
|
||||
<td>
|
||||
<div class="data-action-cell">
|
||||
<form th:if="${user.username != currentUsername}" th:action="@{'/api/v1/user/admin/deleteUser/' + ${user.username}}" method="post" onsubmit="return confirmDeleteUser()" style="display: inline;">
|
||||
|
||||
Loading…
Reference in New Issue
Block a user