From b1c0ec15a796348c80f950e8d9e5f7daeecf9cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 18 Sep 2020 16:02:20 +0200 Subject: [PATCH] Heed peer review remarks --- .../sonia/scm/repository/NamespaceManager.java | 7 +++++++ .../sonia/scm/repository/RepositoryPermission.java | 7 ++++++- scm-ui/ui-types/src/Repositories.ts | 2 +- scm-ui/ui-webapp/public/locales/de/namespaces.json | 5 +++++ scm-ui/ui-webapp/public/locales/en/namespaces.json | 5 +++++ .../repos/components/list/RepositoryGroupEntry.tsx | 9 +++++---- .../src/repos/components/list/groupByNamespace.ts | 2 +- .../repos/namespaces/containers/NamespaceRoot.tsx | 4 +++- .../v2/resources/NamespacePermissionResource.java | 14 +++++++------- .../scm/repository/DefaultNamespaceManager.java | 9 +++++++-- .../src/main/resources/locales/en/plugins.json | 4 ++-- 11 files changed, 49 insertions(+), 19 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/NamespaceManager.java b/scm-core/src/main/java/sonia/scm/repository/NamespaceManager.java index bdd8faac7d..01ad2815d6 100644 --- a/scm-core/src/main/java/sonia/scm/repository/NamespaceManager.java +++ b/scm-core/src/main/java/sonia/scm/repository/NamespaceManager.java @@ -27,6 +27,13 @@ package sonia.scm.repository; import java.util.Collection; import java.util.Optional; +/** + * Manages namespaces. Mind that namespaces do not have a lifecycle on their own, but only do exist through + * repositories. Therefore you cannot create or delete namespaces, but just change related settings like permissions + * associated with them. + * + * @since 2.6.0 + */ public interface NamespaceManager { /** diff --git a/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java b/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java index 8f91bcb8cf..49ddbd4a27 100644 --- a/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java +++ b/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java @@ -119,11 +119,16 @@ public class RepositoryPermission implements PermissionObject, Serializable final RepositoryPermission other = (RepositoryPermission) obj; return Objects.equal(name, other.name) - && (verbs == null && other.verbs == null || verbs != null && other.verbs != null && CollectionUtils.isEqualCollection(verbs, other.verbs)) + && equalVerbs(other) && Objects.equal(role, other.role) && Objects.equal(groupPermission, other.groupPermission); } + public boolean equalVerbs(RepositoryPermission other) { + return verbs == null && other.verbs == null + || verbs != null && other.verbs != null && CollectionUtils.isEqualCollection(verbs, other.verbs); + } + /** * Returns the hash code value for the {@link RepositoryPermission}. * diff --git a/scm-ui/ui-types/src/Repositories.ts b/scm-ui/ui-types/src/Repositories.ts index b585bffb65..02d7085892 100644 --- a/scm-ui/ui-types/src/Repositories.ts +++ b/scm-ui/ui-types/src/Repositories.ts @@ -58,6 +58,6 @@ export type NamespaceCollection = { export type RepositoryGroup = { name: string; - namespace: Namespace; + namespace?: Namespace; repositories: Repository[]; }; diff --git a/scm-ui/ui-webapp/public/locales/de/namespaces.json b/scm-ui/ui-webapp/public/locales/de/namespaces.json index 7a687de4ff..c3cfdcb33b 100644 --- a/scm-ui/ui-webapp/public/locales/de/namespaces.json +++ b/scm-ui/ui-webapp/public/locales/de/namespaces.json @@ -5,5 +5,10 @@ "settingsNavLink": "Einstellungen", "permissionsNavLink": "Berechtigungen" } + }, + "repositoryOverview": { + "settings": { + "tooltip": "Einstellungen für den Namespace" + } } } diff --git a/scm-ui/ui-webapp/public/locales/en/namespaces.json b/scm-ui/ui-webapp/public/locales/en/namespaces.json index a4b99c39ee..a267c03dd7 100644 --- a/scm-ui/ui-webapp/public/locales/en/namespaces.json +++ b/scm-ui/ui-webapp/public/locales/en/namespaces.json @@ -5,5 +5,10 @@ "settingsNavLink": "Settings", "permissionsNavLink": "Permissions" } + }, + "repositoryOverview": { + "settings": { + "tooltip": "Namespace related settings" + } } } diff --git a/scm-ui/ui-webapp/src/repos/components/list/RepositoryGroupEntry.tsx b/scm-ui/ui-webapp/src/repos/components/list/RepositoryGroupEntry.tsx index cdf13dde44..936c5c2ebb 100644 --- a/scm-ui/ui-webapp/src/repos/components/list/RepositoryGroupEntry.tsx +++ b/scm-ui/ui-webapp/src/repos/components/list/RepositoryGroupEntry.tsx @@ -26,17 +26,18 @@ import { Link } from "react-router-dom"; import { CardColumnGroup, RepositoryEntry } from "@scm-manager/ui-components"; import { RepositoryGroup } from "@scm-manager/ui-types"; import { Icon } from "@scm-manager/ui-components"; +import { WithTranslation, withTranslation } from "react-i18next"; -type Props = { +type Props = WithTranslation & { group: RepositoryGroup; }; class RepositoryGroupEntry extends React.Component { render() { - const { group } = this.props; + const { group, t } = this.props; const settingsLink = group.namespace?._links?.permissions && ( - + ); const namespaceHeader = ( @@ -54,4 +55,4 @@ class RepositoryGroupEntry extends React.Component { } } -export default RepositoryGroupEntry; +export default withTranslation("namespaces")(RepositoryGroupEntry); diff --git a/scm-ui/ui-webapp/src/repos/components/list/groupByNamespace.ts b/scm-ui/ui-webapp/src/repos/components/list/groupByNamespace.ts index 7887690b37..f8b04a51c2 100644 --- a/scm-ui/ui-webapp/src/repos/components/list/groupByNamespace.ts +++ b/scm-ui/ui-webapp/src/repos/components/list/groupByNamespace.ts @@ -65,5 +65,5 @@ function sortByName(a, b) { } function findNamespace(namespaces: NamespaceCollection, namespaceToFind: string) { - return namespaces._embedded.namespaces.filter(namespace => namespace.namespace === namespaceToFind)[0]; + return namespaces._embedded.namespaces.find(namespace => namespace.namespace === namespaceToFind); } diff --git a/scm-ui/ui-webapp/src/repos/namespaces/containers/NamespaceRoot.tsx b/scm-ui/ui-webapp/src/repos/namespaces/containers/NamespaceRoot.tsx index a3cbb89610..ae52dc6d34 100644 --- a/scm-ui/ui-webapp/src/repos/namespaces/containers/NamespaceRoot.tsx +++ b/scm-ui/ui-webapp/src/repos/namespaces/containers/NamespaceRoot.tsx @@ -33,7 +33,8 @@ import { CustomQueryFlexWrappedColumns, ErrorPage, Loading, - Page, PrimaryContentColumn, + Page, + PrimaryContentColumn, SecondaryNavigation, SecondaryNavigationColumn, StateMenuContextProvider, @@ -49,6 +50,7 @@ type Props = RouteComponentProps & namespaceName: string; namespacesLink: string; namespace: Namespace; + error: Error; // dispatch functions fetchNamespace: (link: string, namespace: string) => void; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespacePermissionResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespacePermissionResource.java index 4066c32bf0..3fbb2a6a13 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespacePermissionResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespacePermissionResource.java @@ -60,10 +60,10 @@ import static sonia.scm.api.v2.resources.RepositoryPermissionDto.GROUP_PREFIX; @Slf4j public class NamespacePermissionResource { - private RepositoryPermissionDtoToRepositoryPermissionMapper dtoToModelMapper; - private RepositoryPermissionToRepositoryPermissionDtoMapper modelToDtoMapper; - private RepositoryPermissionCollectionToDtoMapper repositoryPermissionCollectionToDtoMapper; - private ResourceLinks resourceLinks; + private final RepositoryPermissionDtoToRepositoryPermissionMapper dtoToModelMapper; + private final RepositoryPermissionToRepositoryPermissionDtoMapper modelToDtoMapper; + private final RepositoryPermissionCollectionToDtoMapper repositoryPermissionCollectionToDtoMapper; + private final ResourceLinks resourceLinks; private final NamespaceManager manager; @Inject @@ -234,7 +234,6 @@ public class NamespacePermissionResource { public void update(@PathParam("namespace") String namespaceName, @PathParam("permission-name") String permissionName, @Valid RepositoryPermissionDto permission) { - log.info("try to update the permission with name: {}. the modified permission is: {}", permissionName, permission); Namespace namespace = load(namespaceName); String extractedPermissionName = getPermissionName(permissionName); if (!isPermissionExist(new RepositoryPermissionDto(extractedPermissionName, isGroupPermission(permissionName)), namespace)) { @@ -256,7 +255,7 @@ public class NamespacePermissionResource { } namespace.addPermission(newPermission); manager.modify(namespace); - log.info("the permission with name: {} is updated.", permissionName); + log.info("the permission with name: {} is updated to {}.", permissionName, permission); } /** @@ -296,7 +295,8 @@ public class NamespacePermissionResource { private Predicate filterPermission(String name) { return permission -> - getPermissionName(name).equals(permission.getName()) && permission.isGroupPermission() == isGroupPermission(name); + getPermissionName(name).equals(permission.getName()) + && permission.isGroupPermission() == isGroupPermission(name); } private String getPermissionName(String permissionName) { diff --git a/scm-webapp/src/main/java/sonia/scm/repository/DefaultNamespaceManager.java b/scm-webapp/src/main/java/sonia/scm/repository/DefaultNamespaceManager.java index 7088fbc28d..c291cfa97e 100644 --- a/scm-webapp/src/main/java/sonia/scm/repository/DefaultNamespaceManager.java +++ b/scm-webapp/src/main/java/sonia/scm/repository/DefaultNamespaceManager.java @@ -80,8 +80,7 @@ public class DefaultNamespaceManager implements NamespaceManager { @Subscribe public void cleanupDeletedNamespaces(RepositoryEvent repositoryEvent) { - HandlerEventType eventType = repositoryEvent.getEventType(); - if (eventType == HandlerEventType.DELETE || eventType == HandlerEventType.MODIFY && !repositoryEvent.getItem().getNamespace().equals(repositoryEvent.getOldItem().getNamespace())) { + if (namespaceRelevantChange(repositoryEvent)) { Collection allNamespaces = repositoryManager.getAllNamespaces(); String oldNamespace = getOldNamespace(repositoryEvent); if (!allNamespaces.contains(oldNamespace)) { @@ -90,6 +89,12 @@ public class DefaultNamespaceManager implements NamespaceManager { } } + public boolean namespaceRelevantChange(RepositoryEvent repositoryEvent) { + HandlerEventType eventType = repositoryEvent.getEventType(); + return eventType == HandlerEventType.DELETE + || eventType == HandlerEventType.MODIFY && !repositoryEvent.getItem().getNamespace().equals(repositoryEvent.getOldItem().getNamespace()); + } + public String getOldNamespace(RepositoryEvent repositoryEvent) { if (repositoryEvent.getEventType() == HandlerEventType.DELETE) { return repositoryEvent.getItem().getNamespace(); diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index 05e70bb299..67e84be257 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -95,11 +95,11 @@ }, "namespace": { "permissionRead": { - "displayName": "read permissions on namespaces", + "displayName": "Read permissions on namespaces", "description": "May see the permissions set for namespaces" }, "permissionWrite": { - "displayName": "modify permissions on namespaces", + "displayName": "Modify permissions on namespaces", "description": "May modify the permissions set for namespaces" } },