diff --git a/backend/src/main/java/com/roleimpact/workspace/editor/api/RoleRequest.java b/backend/src/main/java/com/roleimpact/workspace/editor/api/RoleRequest.java index d70f11f..3d3142f 100644 --- a/backend/src/main/java/com/roleimpact/workspace/editor/api/RoleRequest.java +++ b/backend/src/main/java/com/roleimpact/workspace/editor/api/RoleRequest.java @@ -1,5 +1,6 @@ package com.roleimpact.workspace.editor.api; +import java.util.Set; import java.util.UUID; import com.roleimpact.shared.model.Sensitivity; @@ -12,5 +13,9 @@ public record RoleRequest( @NotBlank @Size(max = 120) String name, @NotBlank @Size(max = 2000) String description, @NotNull Sensitivity sensitivity, - UUID ownerMemberId) { + UUID ownerMemberId, + Set holderMemberIds) { + public RoleRequest { + if (holderMemberIds != null) holderMemberIds = Set.copyOf(holderMemberIds); + } } diff --git a/backend/src/main/java/com/roleimpact/workspace/editor/application/DraftCatalogService.java b/backend/src/main/java/com/roleimpact/workspace/editor/application/DraftCatalogService.java index c70a4dc..a1d01e1 100644 --- a/backend/src/main/java/com/roleimpact/workspace/editor/application/DraftCatalogService.java +++ b/backend/src/main/java/com/roleimpact/workspace/editor/application/DraftCatalogService.java @@ -97,7 +97,9 @@ public DraftCatalogResource updateMember(UUID workspaceId, UUID memberId, Member public DraftCatalogResource createRole(UUID workspaceId, RoleRequest request) { requireDraft(workspaceId); if (request.ownerMemberId() != null) requireMember(workspaceId, request.ownerMemberId()); - mutate(() -> repository.insertRole(workspaceId, request), "A role with this name already exists"); + requireHolders(workspaceId, request.holderMemberIds()); + UUID roleId = mutate(() -> repository.insertRole(workspaceId, request), "A role with this name already exists"); + if (request.holderMemberIds() != null) repository.replaceRoleHolders(roleId, request.holderMemberIds()); touch(workspaceId); return repository.findCatalog(workspaceId); } @@ -119,8 +121,10 @@ public DraftCatalogResource updateRole(UUID workspaceId, UUID roleId, RoleReques requireDraft(workspaceId); requireRole(workspaceId, roleId); if (request.ownerMemberId() != null) requireMember(workspaceId, request.ownerMemberId()); + requireHolders(workspaceId, request.holderMemberIds()); mutate(() -> { repository.updateRole(workspaceId, roleId, request); return roleId; }, "A role with this name already exists"); + if (request.holderMemberIds() != null) repository.replaceRoleHolders(roleId, request.holderMemberIds()); touch(workspaceId); return repository.findCatalog(workspaceId); } @@ -214,15 +218,24 @@ private void requireRole(UUID workspaceId, UUID roleId) { } } + private void requireHolders(UUID workspaceId, Set holderMemberIds) { + if (holderMemberIds == null || repository.countMatchingMembers(workspaceId, holderMemberIds) == holderMemberIds.size()) return; + UUID unknownMemberId = holderMemberIds.stream() + .filter(memberId -> !repository.memberExists(workspaceId, memberId)) + .findFirst() + .orElseThrow(); + throw new DraftCatalogNotFoundException("Member", unknownMemberId); + } + private void requireCoverage(int minimumActors, int resilienceTarget) { if (resilienceTarget < minimumActors) { throw new DraftCatalogConflictException("Healthy coverage cannot be lower than the minimum people required"); } } - private void mutate(Supplier mutation, String conflictMessage) { + private UUID mutate(Supplier mutation, String conflictMessage) { try { - mutation.get(); + return mutation.get(); } catch (DataIntegrityViolationException exception) { throw new DraftCatalogConflictException(conflictMessage); diff --git a/backend/src/main/java/com/roleimpact/workspace/editor/persistence/DraftCatalogRepository.java b/backend/src/main/java/com/roleimpact/workspace/editor/persistence/DraftCatalogRepository.java index 69f915e..9d77afb 100644 --- a/backend/src/main/java/com/roleimpact/workspace/editor/persistence/DraftCatalogRepository.java +++ b/backend/src/main/java/com/roleimpact/workspace/editor/persistence/DraftCatalogRepository.java @@ -280,6 +280,23 @@ public int countMatchingRoles(UUID workspaceId, Set roleIds) { .param("workspaceId", workspaceId).param("roleIds", roleIds).query(Integer.class).single(); } + public int countMatchingMembers(UUID workspaceId, Set memberIds) { + if (memberIds.isEmpty()) return 0; + return jdbcClient.sql("SELECT COUNT(*) FROM employees WHERE organization_id = :workspaceId AND id IN (:memberIds)") + .param("workspaceId", workspaceId).param("memberIds", memberIds).query(Integer.class).single(); + } + + public void replaceRoleHolders(UUID roleId, Set memberIds) { + jdbcClient.sql("DELETE FROM employee_roles WHERE role_id = :roleId") + .param("roleId", roleId).update(); + for (UUID memberId : memberIds) { + jdbcClient.sql(""" + INSERT INTO employee_roles (employee_id, role_id, assigned_at, assigned_by) + VALUES (:memberId, :roleId, CURRENT_TIMESTAMP, 'workspace editor') + """).param("memberId", memberId).param("roleId", roleId).update(); + } + } + public void replaceAssignments(UUID memberId, Set roleIds) { jdbcClient.sql("DELETE FROM employee_roles WHERE employee_id = :memberId").param("memberId", memberId).update(); for (UUID roleId : roleIds) { diff --git a/backend/src/main/java/com/roleimpact/workspace/preview/api/DraftContinuityRiskResource.java b/backend/src/main/java/com/roleimpact/workspace/preview/api/DraftContinuityRiskResource.java new file mode 100644 index 0000000..654b5c1 --- /dev/null +++ b/backend/src/main/java/com/roleimpact/workspace/preview/api/DraftContinuityRiskResource.java @@ -0,0 +1,39 @@ +package com.roleimpact.workspace.preview.api; + +import java.util.List; +import java.util.UUID; + +import com.roleimpact.impactengine.ImpactResult.WorkflowStatus; +import com.roleimpact.shared.model.WorkflowCriticality; + +public record DraftContinuityRiskResource( + String key, + UUID workflowId, + String workflowName, + WorkflowCriticality criticality, + UUID requirementId, + String requirementName, + int minimumActors, + int resilienceTarget, + UUID roleId, + String roleName, + List eligibleMembers, + List members) { + + public DraftContinuityRiskResource { + eligibleMembers = List.copyOf(eligibleMembers); + members = List.copyOf(members); + } + + public record EligibleMember(UUID id, String name) { + } + + public record MemberScenario( + UUID id, + String name, + boolean eligible, + boolean losesCoverage, + int remainingEligibleActorCount, + WorkflowStatus scenarioStatus) { + } +} diff --git a/backend/src/main/java/com/roleimpact/workspace/preview/api/DraftImpactPreviewController.java b/backend/src/main/java/com/roleimpact/workspace/preview/api/DraftImpactPreviewController.java index e54685c..96e2b13 100644 --- a/backend/src/main/java/com/roleimpact/workspace/preview/api/DraftImpactPreviewController.java +++ b/backend/src/main/java/com/roleimpact/workspace/preview/api/DraftImpactPreviewController.java @@ -1,13 +1,16 @@ package com.roleimpact.workspace.preview.api; import java.util.UUID; +import java.util.List; import com.roleimpact.impactengine.ImpactResult; +import com.roleimpact.workspace.preview.application.DraftContinuityProjectionService; import com.roleimpact.workspace.preview.application.DraftImpactPreviewService; import jakarta.validation.Valid; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -18,9 +21,18 @@ public class DraftImpactPreviewController { private final DraftImpactPreviewService service; + private final DraftContinuityProjectionService continuityProjectionService; - public DraftImpactPreviewController(DraftImpactPreviewService service) { + public DraftImpactPreviewController( + DraftImpactPreviewService service, + DraftContinuityProjectionService continuityProjectionService) { this.service = service; + this.continuityProjectionService = continuityProjectionService; + } + + @GetMapping("/continuity") + public List continuity(@PathVariable UUID workspaceId) { + return continuityProjectionService.project(workspaceId); } @PostMapping diff --git a/backend/src/main/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionService.java b/backend/src/main/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionService.java new file mode 100644 index 0000000..267b4f1 --- /dev/null +++ b/backend/src/main/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionService.java @@ -0,0 +1,153 @@ +package com.roleimpact.workspace.preview.application; + +import java.util.Comparator; +import java.util.List; +import java.util.UUID; + +import com.roleimpact.catalog.snapshot.OrganizationSnapshot; +import com.roleimpact.catalog.snapshot.OrganizationSnapshotAssembler; +import com.roleimpact.impactengine.ImpactEngine; +import com.roleimpact.impactengine.ImpactResult; +import com.roleimpact.impactengine.RevokeEmployeeRole; +import com.roleimpact.shared.model.WorkflowCriticality; +import com.roleimpact.workspace.application.WorkspaceNotFoundException; +import com.roleimpact.workspace.editor.api.DraftCatalogResource; +import com.roleimpact.workspace.editor.application.PublishedWorkspaceMutationException; +import com.roleimpact.workspace.editor.persistence.DraftCatalogRepository; +import com.roleimpact.workspace.preview.api.DraftContinuityRiskResource; + +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Isolation; +import org.springframework.transaction.annotation.Transactional; + +@Service +public class DraftContinuityProjectionService { + + private static final Comparator TEXT_ORDER = Comparator.nullsFirst(String::compareTo); + private static final Comparator ID_ORDER = Comparator.nullsFirst(UUID::compareTo); + + private final DraftCatalogRepository catalogRepository; + private final OrganizationSnapshotAssembler snapshotAssembler; + private final ImpactEngine impactEngine; + + public DraftContinuityProjectionService( + DraftCatalogRepository catalogRepository, + OrganizationSnapshotAssembler snapshotAssembler, + ImpactEngine impactEngine) { + this.catalogRepository = catalogRepository; + this.snapshotAssembler = snapshotAssembler; + this.impactEngine = impactEngine; + } + + @Transactional(readOnly = true, isolation = Isolation.REPEATABLE_READ) + public List project(UUID workspaceId) { + validateDraftWorkspace(workspaceId); + DraftCatalogResource catalog = catalogRepository.findCatalog(workspaceId); + OrganizationSnapshot snapshot = snapshotAssembler.assemble(workspaceId); + return catalog.workflows().stream() + .sorted(Comparator.comparing(DraftCatalogResource.WorkflowItem::name, TEXT_ORDER) + .thenComparing(DraftCatalogResource.WorkflowItem::id, ID_ORDER)) + .flatMap(workflow -> projectWorkflow(catalog, snapshot, workflow).stream()) + .toList(); + } + + private List projectWorkflow( + DraftCatalogResource catalog, + OrganizationSnapshot snapshot, + DraftCatalogResource.WorkflowItem workflow) { + return workflow.requirements().stream() + .sorted(Comparator.comparing(DraftCatalogResource.WorkflowRequirementItem::name, TEXT_ORDER) + .thenComparing(DraftCatalogResource.WorkflowRequirementItem::id, ID_ORDER)) + .flatMap(requirement -> requirement.roleIds().stream() + .map(roleId -> new RoleReference(roleId, findRole(catalog, roleId))) + .filter(role -> role.item() != null) + .sorted(Comparator.comparing((RoleReference role) -> role.item().name(), TEXT_ORDER) + .thenComparing(RoleReference::id, ID_ORDER)) + .map(role -> projectRole(catalog, snapshot, workflow, requirement, role))) + .filter(java.util.Optional::isPresent) + .map(java.util.Optional::get) + .toList(); + } + + private java.util.Optional projectRole( + DraftCatalogResource catalog, + OrganizationSnapshot snapshot, + DraftCatalogResource.WorkflowItem workflow, + DraftCatalogResource.WorkflowRequirementItem requirement, + RoleReference role) { + List holders = catalog.members().stream() + .filter(member -> member.roleIds().contains(role.id())) + .sorted(Comparator.comparing(DraftCatalogResource.MemberItem::name, TEXT_ORDER) + .thenComparing(DraftCatalogResource.MemberItem::id, ID_ORDER)) + .toList(); + if (holders.isEmpty()) return java.util.Optional.empty(); + + List scenarios = holders.stream() + .map(holder -> scenarioFor(snapshot, workflow.id(), requirement.id(), holder, role.id())) + .toList(); + ImpactResult.StepImpact baseline = scenarios.getFirst().step(); + return java.util.Optional.of(new DraftContinuityRiskResource( + workflow.id() + ":" + requirement.id() + ":" + role.id(), + workflow.id(), + workflow.name(), + WorkflowCriticality.valueOf(workflow.criticality()), + requirement.id(), + requirement.name(), + requirement.minimumActors(), + requirement.resilienceTarget(), + role.id(), + role.item().name(), + baseline.baselineEligibleActors().stream() + .sorted(Comparator.comparing(ImpactResult.ActorRef::name, TEXT_ORDER) + .thenComparing(ImpactResult.ActorRef::id, ID_ORDER)) + .map(actor -> new DraftContinuityRiskResource.EligibleMember(actor.id(), actor.name())) + .toList(), + scenarios.stream().map(Scenario::toMemberScenario).toList())); + } + + private Scenario scenarioFor( + OrganizationSnapshot snapshot, + UUID workflowId, + UUID stepId, + DraftCatalogResource.MemberItem holder, + UUID roleId) { + ImpactResult result = impactEngine.analyze(snapshot, new RevokeEmployeeRole(holder.id(), roleId)); + ImpactResult.WorkflowImpact workflow = result.workflowImpacts().stream() + .filter(candidate -> candidate.workflowId().equals(workflowId)) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Impact result did not include workflow " + workflowId)); + ImpactResult.StepImpact step = workflow.steps().stream() + .filter(candidate -> candidate.stepId().equals(stepId)) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Impact result did not include workflow step " + stepId)); + return new Scenario(holder, workflow.scenarioStatus(), step); + } + + private DraftCatalogResource.RoleItem findRole(DraftCatalogResource catalog, UUID roleId) { + return catalog.roles().stream().filter(role -> role.id().equals(roleId)).findFirst().orElse(null); + } + + private void validateDraftWorkspace(UUID workspaceId) { + String status = catalogRepository.findWorkspaceStatus(workspaceId) + .orElseThrow(() -> new WorkspaceNotFoundException(workspaceId)); + if (!"DRAFT".equals(status)) throw new PublishedWorkspaceMutationException(); + } + + private record RoleReference(UUID id, DraftCatalogResource.RoleItem item) { + } + + private record Scenario( + DraftCatalogResource.MemberItem holder, + ImpactResult.WorkflowStatus workflowStatus, + ImpactResult.StepImpact step) { + + DraftContinuityRiskResource.MemberScenario toMemberScenario() { + boolean eligible = step.baselineEligibleActors().stream().anyMatch(actor -> actor.id().equals(holder.id())); + boolean losesCoverage = eligible && step.scenarioEligibleActors().stream() + .noneMatch(actor -> actor.id().equals(holder.id())); + return new DraftContinuityRiskResource.MemberScenario( + holder.id(), holder.name(), eligible, losesCoverage, + step.scenarioEligibleActors().size(), workflowStatus); + } + } +} diff --git a/backend/src/test/java/com/roleimpact/workspace/WorkspaceIntegrationTest.java b/backend/src/test/java/com/roleimpact/workspace/WorkspaceIntegrationTest.java index ab65842..b2927ef 100644 --- a/backend/src/test/java/com/roleimpact/workspace/WorkspaceIntegrationTest.java +++ b/backend/src/test/java/com/roleimpact/workspace/WorkspaceIntegrationTest.java @@ -210,23 +210,18 @@ void buildsTeamsMembersAndRolesInsideADraftOnly() throws Exception { .content(""" { "name":"Release Manager", "description":"Approves production releases", - "sensitivity":"HIGH", "ownerMemberId":"%s" + "sensitivity":"HIGH", "ownerMemberId":"%s", + "holderMemberIds":["%s"] } - """.formatted(memberId))) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.roles[0].name").value("Release Manager")) - .andReturn(); + """.formatted(memberId, memberId))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.roles[0].name").value("Release Manager")) + .andExpect(jsonPath("$.roles[0].memberCount").value(1)) + .andReturn(); UUID roleId = UUID.fromString(objectMapper.readTree(roleResponse.getResponse().getContentAsString()) .path("roles").get(0).path("id").asText()); - - var assignmentResponse = mockMvc.perform(put("/api/v1/workspaces/{workspaceId}/catalog/members/{memberId}/roles", workspaceId, memberId) - .contentType(MediaType.APPLICATION_JSON) - .content("{\"roleIds\":[\"" + roleId + "\"]}")) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.roles[0].memberCount").value(1)) - .andReturn(); assertThat(findById( - objectMapper.readTree(assignmentResponse.getResponse().getContentAsString()), + objectMapper.readTree(roleResponse.getResponse().getContentAsString()), "members", memberId).path("roleIds").get(0).asText()).isEqualTo(roleId.toString()); @@ -255,12 +250,46 @@ void buildsTeamsMembersAndRolesInsideADraftOnly() throws Exception { .content(""" { "name":"Production Release Manager", "description":"Approves production releases", - "sensitivity":"CRITICAL", "ownerMemberId":"%s" + "sensitivity":"CRITICAL", "ownerMemberId":"%s", + "holderMemberIds":["%s","%s"] } - """.formatted(memberId))) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.roles[0].name").value("Production Release Manager")) - .andExpect(jsonPath("$.roles[0].sensitivity").value("CRITICAL")); + """.formatted(memberId, memberId, candidateId))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.roles[0].name").value("Production Release Manager")) + .andExpect(jsonPath("$.roles[0].sensitivity").value("CRITICAL")) + .andExpect(jsonPath("$.roles[0].memberCount").value(2)); + + UUID unknownHolderId = UUID.fromString("00000000-0000-0000-0000-000000000999"); + mockMvc.perform(put("/api/v1/workspaces/{workspaceId}/catalog/roles/{roleId}", workspaceId, roleId) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "name":"Should Not Persist", "description":"Should not persist", + "sensitivity":"LOW", "ownerMemberId":null, + "holderMemberIds":["%s","%s"] + } + """.formatted(memberId, unknownHolderId))) + .andExpect(status().isNotFound()); + var catalogAfterRejectedRoleUpdate = objectMapper.readTree(mockMvc.perform(get("/api/v1/workspaces/{workspaceId}/catalog", workspaceId)) + .andExpect(status().isOk()).andReturn().getResponse().getContentAsString()); + assertThat(findById(catalogAfterRejectedRoleUpdate, "roles", roleId).path("name").asText()) + .isEqualTo("Production Release Manager"); + assertThat(findById(catalogAfterRejectedRoleUpdate, "members", memberId).path("roleIds")) + .extracting(com.fasterxml.jackson.databind.JsonNode::asText).containsExactly(roleId.toString()); + assertThat(findById(catalogAfterRejectedRoleUpdate, "members", candidateId).path("roleIds")) + .extracting(com.fasterxml.jackson.databind.JsonNode::asText).containsExactly(roleId.toString()); + + mockMvc.perform(put("/api/v1/workspaces/{workspaceId}/catalog/roles/{roleId}", workspaceId, roleId) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "name":"Production Release Manager", "description":"Approves production releases", + "sensitivity":"CRITICAL", "ownerMemberId":"%s", + "holderMemberIds":["%s"] + } + """.formatted(memberId, memberId))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.roles[0].memberCount").value(1)); var workflowResponse = mockMvc.perform(post("/api/v1/workspaces/{workspaceId}/catalog/workflows/quick", workspaceId) .contentType(MediaType.APPLICATION_JSON) @@ -344,6 +373,13 @@ void buildsTeamsMembersAndRolesInsideADraftOnly() throws Exception { .andExpect(jsonPath("$.mitigation.overallSeverity").value("CRITICAL")) .andExpect(jsonPath("$.mitigation.executiveSummary.workflowsBlocked").value(1)); + mockMvc.perform(get("/api/v1/workspaces/{workspaceId}/impact-previews/continuity", workspaceId)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$[?(@.workflowName == 'Production Deployment' && @.requirementName == 'Approve production release')].eligibleMembers[0].id") + .value(memberId.toString())) + .andExpect(jsonPath("$[?(@.workflowName == 'Production Deployment' && @.requirementName == 'Approve production release')].members[0].scenarioStatus") + .value("BLOCKED")); + var catalogAfterMitigation = mockMvc.perform(get("/api/v1/workspaces/{workspaceId}/catalog", workspaceId)) .andExpect(status().isOk()) .andReturn(); diff --git a/backend/src/test/java/com/roleimpact/workspace/editor/application/DraftCatalogServiceTest.java b/backend/src/test/java/com/roleimpact/workspace/editor/application/DraftCatalogServiceTest.java new file mode 100644 index 0000000..9893b03 --- /dev/null +++ b/backend/src/test/java/com/roleimpact/workspace/editor/application/DraftCatalogServiceTest.java @@ -0,0 +1,124 @@ +package com.roleimpact.workspace.editor.application; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.LinkedHashSet; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; + +import com.roleimpact.shared.model.Sensitivity; +import com.roleimpact.workspace.editor.api.RoleRequest; +import com.roleimpact.workspace.editor.persistence.DraftCatalogRepository; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InOrder; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class DraftCatalogServiceTest { + + @Mock + private DraftCatalogRepository repository; + + @Test + void createsRoleAfterValidatingEveryHolderAndReplacesTheCompleteHolderSetOnce() { + UUID workspaceId = UUID.fromString("00000000-0000-0000-0000-000000000101"); + UUID roleId = UUID.fromString("00000000-0000-0000-0000-000000000102"); + Set holderIds = Set.of( + UUID.fromString("00000000-0000-0000-0000-000000000103"), + UUID.fromString("00000000-0000-0000-0000-000000000104")); + RoleRequest request = roleRequest(holderIds); + DraftCatalogService service = serviceForDraft(workspaceId); + when(repository.countMatchingMembers(workspaceId, holderIds)).thenReturn(2); + when(repository.insertRole(workspaceId, request)).thenReturn(roleId); + + service.createRole(workspaceId, request); + + InOrder order = inOrder(repository); + order.verify(repository).countMatchingMembers(workspaceId, holderIds); + order.verify(repository).insertRole(workspaceId, request); + order.verify(repository).replaceRoleHolders(roleId, holderIds); + order.verify(repository).touchWorkspace(workspaceId); + } + + @Test + void updatesRoleWithAnExplicitEmptyHolderSetByClearingAllHolders() { + UUID workspaceId = UUID.fromString("00000000-0000-0000-0000-000000000111"); + UUID roleId = UUID.fromString("00000000-0000-0000-0000-000000000112"); + Set holderIds = Set.of(); + RoleRequest request = roleRequest(holderIds); + DraftCatalogService service = serviceForDraft(workspaceId); + when(repository.roleExists(workspaceId, roleId)).thenReturn(true); + when(repository.countMatchingMembers(workspaceId, holderIds)).thenReturn(0); + + service.updateRole(workspaceId, roleId, request); + + verify(repository).updateRole(workspaceId, roleId, request); + verify(repository).replaceRoleHolders(roleId, holderIds); + } + + @Test + void updatesRoleWithoutHolderIdsWithoutChangingCurrentHolders() { + UUID workspaceId = UUID.fromString("00000000-0000-0000-0000-000000000121"); + UUID roleId = UUID.fromString("00000000-0000-0000-0000-000000000122"); + RoleRequest request = roleRequest(null); + DraftCatalogService service = serviceForDraft(workspaceId); + when(repository.roleExists(workspaceId, roleId)).thenReturn(true); + + service.updateRole(workspaceId, roleId, request); + + verify(repository).updateRole(workspaceId, roleId, request); + verify(repository, never()).countMatchingMembers(eq(workspaceId), any()); + verify(repository, never()).replaceRoleHolders(any(), any()); + } + + @Test + void rejectsUnknownHolderBeforeChangingRoleMetadataOrAssignments() { + UUID workspaceId = UUID.fromString("00000000-0000-0000-0000-000000000131"); + UUID roleId = UUID.fromString("00000000-0000-0000-0000-000000000132"); + Set holderIds = Set.of( + UUID.fromString("00000000-0000-0000-0000-000000000133"), + UUID.fromString("00000000-0000-0000-0000-000000000134")); + RoleRequest request = roleRequest(holderIds); + DraftCatalogService service = serviceForDraft(workspaceId); + when(repository.roleExists(workspaceId, roleId)).thenReturn(true); + when(repository.countMatchingMembers(workspaceId, holderIds)).thenReturn(1); + + assertThatThrownBy(() -> service.updateRole(workspaceId, roleId, request)) + .isInstanceOf(DraftCatalogNotFoundException.class); + + verify(repository, never()).updateRole(workspaceId, roleId, request); + verify(repository, never()).replaceRoleHolders(any(), any()); + verify(repository, never()).touchWorkspace(workspaceId); + } + + @Test + void copiesSuppliedHolderIdsSoCallersCannotMutateTheRequestAfterConstruction() { + Set suppliedIds = new LinkedHashSet<>(); + suppliedIds.add(UUID.fromString("00000000-0000-0000-0000-000000000141")); + RoleRequest request = roleRequest(suppliedIds); + + suppliedIds.add(UUID.fromString("00000000-0000-0000-0000-000000000142")); + + assertThat(request.holderMemberIds()).containsExactly(UUID.fromString("00000000-0000-0000-0000-000000000141")); + } + + private DraftCatalogService serviceForDraft(UUID workspaceId) { + when(repository.findWorkspaceStatus(workspaceId)).thenReturn(Optional.of("DRAFT")); + return new DraftCatalogService(repository); + } + + private RoleRequest roleRequest(Set holderMemberIds) { + return new RoleRequest("Release Manager", "Approves production releases", Sensitivity.HIGH, null, holderMemberIds); + } +} diff --git a/backend/src/test/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionConcurrencyIntegrationTest.java b/backend/src/test/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionConcurrencyIntegrationTest.java new file mode 100644 index 0000000..ceb505d --- /dev/null +++ b/backend/src/test/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionConcurrencyIntegrationTest.java @@ -0,0 +1,188 @@ +package com.roleimpact.workspace.preview.application; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.doAnswer; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicBoolean; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.roleimpact.workspace.editor.api.DraftCatalogResource; +import com.roleimpact.workspace.editor.persistence.DraftCatalogRepository; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.testcontainers.service.connection.ServiceConnection; +import org.springframework.http.MediaType; +import org.springframework.test.context.bean.override.mockito.MockitoSpyBean; +import org.springframework.test.web.servlet.MockMvc; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +@SpringBootTest +@Testcontainers +@AutoConfigureMockMvc +class DraftContinuityProjectionConcurrencyIntegrationTest { + + @Container + @ServiceConnection + static final PostgreSQLContainer postgres = new PostgreSQLContainer<>("postgres:17-alpine") + .withDatabaseName("roleimpact") + .withUsername("roleimpact") + .withPassword("roleimpact"); + + @Autowired + private MockMvc mockMvc; + + @Autowired + private ObjectMapper objectMapper; + + @Autowired + private DraftContinuityProjectionService projectionService; + + @MockitoSpyBean + private DraftCatalogRepository catalogRepository; + + @Test + void projectsOneCommittedCatalogStateWhenRoleHoldersChangeBetweenItsReads() throws Exception { + UUID workspaceId = createWorkspace("Concurrent Projection Workspace"); + UUID teamId = createTeam(workspaceId); + UUID originalHolderId = createMember(workspaceId, teamId, "Original Holder"); + UUID replacementHolderId = createMember(workspaceId, teamId, "Replacement Holder"); + UUID roleId = createRole(workspaceId, originalHolderId); + createWorkflow(workspaceId, roleId); + + var pauseProjection = new AtomicBoolean(true); + var catalogRead = new CountDownLatch(1); + var continueProjection = new CountDownLatch(1); + doAnswer(invocation -> { + DraftCatalogResource catalog = (DraftCatalogResource) invocation.callRealMethod(); + if (pauseProjection.compareAndSet(true, false)) { + catalogRead.countDown(); + if (!continueProjection.await(10, SECONDS)) { + throw new AssertionError("Timed out waiting for the concurrent holder replacement"); + } + } + return catalog; + }).when(catalogRepository).findCatalog(workspaceId); + + var executor = Executors.newSingleThreadExecutor(); + try { + CompletableFuture> projection = + CompletableFuture.supplyAsync(() -> projectionService.project(workspaceId), executor); + assertThat(catalogRead.await(10, SECONDS)).as("projection reached the catalog/snapshot boundary").isTrue(); + + mockMvc.perform(put("/api/v1/workspaces/{workspaceId}/catalog/roles/{roleId}", workspaceId, roleId) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "name":"Release Manager", "description":"Approves production releases", + "sensitivity":"HIGH", "ownerMemberId":"%s", + "holderMemberIds":["%s"] + } + """.formatted(originalHolderId, replacementHolderId))) + .andExpect(status().isOk()); + continueProjection.countDown(); + + var risks = projection.get(10, SECONDS); + + assertThat(risks).hasSize(1); + assertThat(risks.getFirst().members()) + .extracting(member -> member.id(), member -> member.name()) + .containsExactly(org.assertj.core.groups.Tuple.tuple(originalHolderId, "Original Holder")); + assertThat(risks.getFirst().eligibleMembers()) + .extracting(member -> member.id(), member -> member.name()) + .containsExactly(org.assertj.core.groups.Tuple.tuple(originalHolderId, "Original Holder")); + assertThat(catalogRepository.findCatalog(workspaceId).members().stream() + .filter(member -> member.roleIds().contains(roleId)) + .map(DraftCatalogResource.MemberItem::id)) + .containsExactly(replacementHolderId); + } + finally { + continueProjection.countDown(); + executor.shutdownNow(); + } + } + + private UUID createWorkspace(String name) throws Exception { + var response = mockMvc.perform(post("/api/v1/workspaces") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"name\":\"%s\"}".formatted(name))) + .andExpect(status().isCreated()) + .andReturn(); + return UUID.fromString(objectMapper.readTree(response.getResponse().getContentAsString()).path("id").asText()); + } + + private UUID createTeam(UUID workspaceId) throws Exception { + JsonNode catalog = responseJson(mockMvc.perform(post("/api/v1/workspaces/{workspaceId}/catalog/teams", workspaceId) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"name\":\"Platform\",\"department\":\"Technology\"}")) + .andExpect(status().isOk()) + .andReturn()); + return findId(catalog, "teams", "Platform"); + } + + private UUID createMember(UUID workspaceId, UUID teamId, String name) throws Exception { + JsonNode catalog = responseJson(mockMvc.perform(post("/api/v1/workspaces/{workspaceId}/catalog/members", workspaceId) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "teamId":"%s", "name":"%s", "status":"ACTIVE", + "region":"NORTH_AMERICA", "shift":"DAY" + } + """.formatted(teamId, name))) + .andExpect(status().isOk()) + .andReturn()); + return findId(catalog, "members", name); + } + + private UUID createRole(UUID workspaceId, UUID originalHolderId) throws Exception { + JsonNode catalog = responseJson(mockMvc.perform(post("/api/v1/workspaces/{workspaceId}/catalog/roles", workspaceId) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "name":"Release Manager", "description":"Approves production releases", + "sensitivity":"HIGH", "ownerMemberId":"%s", + "holderMemberIds":["%s"] + } + """.formatted(originalHolderId, originalHolderId))) + .andExpect(status().isOk()) + .andReturn()); + return findId(catalog, "roles", "Release Manager"); + } + + private void createWorkflow(UUID workspaceId, UUID roleId) throws Exception { + mockMvc.perform(post("/api/v1/workspaces/{workspaceId}/catalog/workflows/quick", workspaceId) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "name":"Production Deployment", "criticality":"CRITICAL", + "requirementName":"Approve production release", "roleId":"%s", + "minimumActors":1, "resilienceTarget":1 + } + """.formatted(roleId))) + .andExpect(status().isOk()); + } + + private JsonNode responseJson(org.springframework.test.web.servlet.MvcResult response) throws Exception { + return objectMapper.readTree(response.getResponse().getContentAsString()); + } + + private UUID findId(JsonNode catalog, String collection, String name) { + for (JsonNode item : catalog.path(collection)) { + if (name.equals(item.path("name").asText())) return UUID.fromString(item.path("id").asText()); + } + throw new AssertionError("Missing " + name + " in " + collection); + } +} diff --git a/backend/src/test/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionServiceTest.java b/backend/src/test/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionServiceTest.java new file mode 100644 index 0000000..f3c1420 --- /dev/null +++ b/backend/src/test/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionServiceTest.java @@ -0,0 +1,207 @@ +package com.roleimpact.workspace.preview.application; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Set; +import java.util.UUID; + +import com.roleimpact.catalog.snapshot.OrganizationSnapshot; +import com.roleimpact.catalog.snapshot.OrganizationSnapshotAssembler; +import com.roleimpact.impactengine.ImpactEngine; +import com.roleimpact.impactengine.ImpactResult; +import com.roleimpact.impactengine.ImpactResult.ActorRef; +import com.roleimpact.impactengine.ImpactResult.StepImpact; +import com.roleimpact.impactengine.ImpactResult.WorkflowImpact; +import com.roleimpact.impactengine.RevokeEmployeeRole; +import com.roleimpact.shared.model.WorkflowCriticality; +import com.roleimpact.workspace.application.WorkspaceNotFoundException; +import com.roleimpact.workspace.editor.api.DraftCatalogResource; +import com.roleimpact.workspace.editor.application.PublishedWorkspaceMutationException; +import com.roleimpact.workspace.editor.persistence.DraftCatalogRepository; + +import org.junit.jupiter.api.Test; + +import static org.mockito.Mockito.mock; + +class DraftContinuityProjectionServiceTest { + + private static final UUID WORKSPACE_ID = UUID.fromString("00000000-0000-0000-0000-000000000001"); + private static final UUID ALPHA_WORKFLOW_ID = UUID.fromString("00000000-0000-0000-0000-000000000010"); + private static final UUID ZULU_WORKFLOW_ID = UUID.fromString("00000000-0000-0000-0000-000000000020"); + private static final UUID ALPHA_STEP_ID = UUID.fromString("00000000-0000-0000-0000-000000000011"); + private static final UUID ZULU_STEP_ID = UUID.fromString("00000000-0000-0000-0000-000000000021"); + private static final UUID ALPHA_ROLE_ID = UUID.fromString("00000000-0000-0000-0000-000000000012"); + private static final UUID ZULU_ROLE_ID = UUID.fromString("00000000-0000-0000-0000-000000000022"); + private static final UUID AARON_ID = UUID.fromString("00000000-0000-0000-0000-000000000101"); + private static final UUID ZOE_ID = UUID.fromString("00000000-0000-0000-0000-000000000102"); + private static final UUID ENGINE_BACKUP_ID = UUID.fromString("00000000-0000-0000-0000-000000000103"); + private static final UUID ALPHA_HOLDER_ID = UUID.fromString("00000000-0000-0000-0000-000000000104"); + + @Test + void projectsEngineEligibilityAndWorkflowVerdictsInDeterministicOrder() { + var repository = mock(DraftCatalogRepository.class); + var snapshots = mock(OrganizationSnapshotAssembler.class); + var snapshot = mock(OrganizationSnapshot.class); + when(repository.findWorkspaceStatus(WORKSPACE_ID)).thenReturn(java.util.Optional.of("DRAFT")); + when(repository.findCatalog(WORKSPACE_ID)).thenReturn(catalog()); + when(snapshots.assemble(WORKSPACE_ID)).thenReturn(snapshot); + ImpactEngine engine = (baseline, change) -> resultFor((RevokeEmployeeRole) change); + + var service = new DraftContinuityProjectionService(repository, snapshots, engine); + + var risks = service.project(WORKSPACE_ID); + + assertThat(risks).extracting(risk -> risk.workflowName() + ":" + risk.roleName()) + .containsExactly("Alpha workflow:Alpha role", "Zulu workflow:Zulu role"); + var zuluRisk = risks.get(1); + assertThat(zuluRisk.eligibleMembers()) + .extracting(member -> member.id(), member -> member.name()) + .containsExactly( + tuple(ENGINE_BACKUP_ID, "Engine Backup"), + tuple(ZOE_ID, "Zoe Holder")); + assertThat(zuluRisk.members()).extracting(member -> member.name()) + .containsExactly("Aaron Holder", "Zoe Holder"); + assertThat(zuluRisk.members()).extracting( + member -> member.eligible(), + member -> member.losesCoverage(), + member -> member.remainingEligibleActorCount(), + member -> member.scenarioStatus()) + .containsExactly( + tuple(false, false, 2, ImpactResult.WorkflowStatus.OPERATIONAL), + tuple(true, true, 1, ImpactResult.WorkflowStatus.BLOCKED)); + assertThat(zuluRisk.members().get(1).scenarioStatus()).isEqualTo(ImpactResult.WorkflowStatus.BLOCKED); + } + + @Test + void ordersEveryRequirementAndRoleWithinAWorkflow() { + var repository = mock(DraftCatalogRepository.class); + var snapshots = mock(OrganizationSnapshotAssembler.class); + var snapshot = mock(OrganizationSnapshot.class); + when(repository.findWorkspaceStatus(WORKSPACE_ID)).thenReturn(java.util.Optional.of("DRAFT")); + when(repository.findCatalog(WORKSPACE_ID)).thenReturn(new DraftCatalogResource( + WORKSPACE_ID, + List.of(), + List.of( + member(ZOE_ID, "Zoe Holder", ZULU_ROLE_ID), + member(ALPHA_HOLDER_ID, "Alpha Holder", ALPHA_ROLE_ID)), + List.of( + role(ZULU_ROLE_ID, "Zulu role"), + role(ALPHA_ROLE_ID, "Alpha role")), + List.of(new DraftCatalogResource.WorkflowItem( + ALPHA_WORKFLOW_ID, + "Alpha workflow", + "HIGH", + List.of( + new DraftCatalogResource.WorkflowRequirementItem( + ZULU_STEP_ID, "Zulu requirement", 2, 1, 2, + "Finance", "EUROPE", "NIGHT", Set.of(ZULU_ROLE_ID, ALPHA_ROLE_ID)), + new DraftCatalogResource.WorkflowRequirementItem( + ALPHA_STEP_ID, "Alpha requirement", 1, 1, 2, + "Finance", "EUROPE", "NIGHT", Set.of(ZULU_ROLE_ID, ALPHA_ROLE_ID))), + true)))); + when(snapshots.assemble(WORKSPACE_ID)).thenReturn(snapshot); + ImpactEngine engine = (baseline, change) -> result(new WorkflowImpact( + ALPHA_WORKFLOW_ID, "Alpha workflow", WorkflowCriticality.HIGH, + ImpactResult.WorkflowStatus.OPERATIONAL, ImpactResult.WorkflowStatus.OPERATIONAL, + List.of( + step(ZULU_STEP_ID, "Zulu requirement", List.of(), List.of()), + step(ALPHA_STEP_ID, "Alpha requirement", List.of(), List.of())), + List.of())); + + var risks = new DraftContinuityProjectionService(repository, snapshots, engine).project(WORKSPACE_ID); + + assertThat(risks).extracting(risk -> risk.requirementName() + ":" + risk.roleName()) + .containsExactly( + "Alpha requirement:Alpha role", + "Alpha requirement:Zulu role", + "Zulu requirement:Alpha role", + "Zulu requirement:Zulu role"); + } + + @Test + void rejectsPublishedAndUnknownWorkspacesBeforeProjecting() { + var repository = mock(DraftCatalogRepository.class); + var snapshots = mock(OrganizationSnapshotAssembler.class); + ImpactEngine engine = (baseline, change) -> null; + var service = new DraftContinuityProjectionService(repository, snapshots, engine); + + when(repository.findWorkspaceStatus(WORKSPACE_ID)).thenReturn(java.util.Optional.of("PUBLISHED")); + assertThatThrownBy(() -> service.project(WORKSPACE_ID)) + .isInstanceOf(PublishedWorkspaceMutationException.class); + + UUID unknownWorkspaceId = UUID.fromString("00000000-0000-0000-0000-000000000099"); + when(repository.findWorkspaceStatus(unknownWorkspaceId)).thenReturn(java.util.Optional.empty()); + assertThatThrownBy(() -> service.project(unknownWorkspaceId)) + .isInstanceOf(WorkspaceNotFoundException.class); + } + + private static DraftCatalogResource catalog() { + return new DraftCatalogResource( + WORKSPACE_ID, + List.of(), + List.of( + member(ZOE_ID, "Zoe Holder", ZULU_ROLE_ID), + member(AARON_ID, "Aaron Holder", ZULU_ROLE_ID), + member(ALPHA_HOLDER_ID, "Alpha Holder", ALPHA_ROLE_ID)), + List.of( + role(ZULU_ROLE_ID, "Zulu role"), + role(ALPHA_ROLE_ID, "Alpha role")), + List.of( + workflow(ZULU_WORKFLOW_ID, "Zulu workflow", ZULU_STEP_ID, "Zulu requirement", ZULU_ROLE_ID), + workflow(ALPHA_WORKFLOW_ID, "Alpha workflow", ALPHA_STEP_ID, "Alpha requirement", ALPHA_ROLE_ID))); + } + + private static DraftCatalogResource.MemberItem member(UUID id, String name, UUID roleId) { + return new DraftCatalogResource.MemberItem(id, null, null, name, null, "INACTIVE", "EUROPE", "NIGHT", Set.of(roleId)); + } + + private static DraftCatalogResource.RoleItem role(UUID id, String name) { + return new DraftCatalogResource.RoleItem(id, name, "fixture", "HIGH", null, 2); + } + + private static DraftCatalogResource.WorkflowItem workflow( + UUID workflowId, String workflowName, UUID requirementId, String requirementName, UUID roleId) { + return new DraftCatalogResource.WorkflowItem( + workflowId, + workflowName, + "HIGH", + List.of(new DraftCatalogResource.WorkflowRequirementItem( + requirementId, requirementName, 1, 1, 2, "Finance", "EUROPE", "NIGHT", Set.of(roleId))), + true); + } + + private static ImpactResult resultFor(RevokeEmployeeRole change) { + if (change.roleId().equals(ALPHA_ROLE_ID)) { + return result(new WorkflowImpact( + ALPHA_WORKFLOW_ID, "Alpha workflow", WorkflowCriticality.HIGH, + ImpactResult.WorkflowStatus.OPERATIONAL, ImpactResult.WorkflowStatus.OPERATIONAL, + List.of(step(ALPHA_STEP_ID, "Alpha requirement", List.of(), List.of())), List.of())); + } + var baselineActors = List.of(new ActorRef(ZOE_ID, "Zoe Holder"), new ActorRef(ENGINE_BACKUP_ID, "Engine Backup")); + var scenarioActors = change.employeeId().equals(ZOE_ID) + ? List.of(new ActorRef(ENGINE_BACKUP_ID, "Engine Backup")) + : baselineActors; + return result(new WorkflowImpact( + ZULU_WORKFLOW_ID, "Zulu workflow", WorkflowCriticality.HIGH, + ImpactResult.WorkflowStatus.OPERATIONAL, + change.employeeId().equals(ZOE_ID) ? ImpactResult.WorkflowStatus.BLOCKED : ImpactResult.WorkflowStatus.OPERATIONAL, + List.of(step(ZULU_STEP_ID, "Zulu requirement", baselineActors, scenarioActors)), List.of())); + } + + private static StepImpact step(UUID id, String name, List baseline, List scenario) { + return new StepImpact(id, name.toUpperCase().replace(' ', '_'), name, null, 1, 2, + ImpactResult.StepStatus.OPERATIONAL, ImpactResult.StepStatus.OPERATIONAL, baseline, scenario, "fixture"); + } + + private static ImpactResult result(WorkflowImpact workflow) { + return new ImpactResult("1.0", WORKSPACE_ID, 0, ImpactResult.ResultStatus.COMPLETE, + ImpactResult.Severity.NONE, null, null, null, List.of(workflow), List.of(), List.of(), List.of(), null); + } + + private static org.assertj.core.groups.Tuple tuple(Object... values) { + return org.assertj.core.groups.Tuple.tuple(values); + } +} diff --git a/docs/superpowers/plans/2026-08-20-mvp-architecture-hardening.md b/docs/superpowers/plans/2026-08-20-mvp-architecture-hardening.md new file mode 100644 index 0000000..a41ce5e --- /dev/null +++ b/docs/superpowers/plans/2026-08-20-mvp-architecture-hardening.md @@ -0,0 +1,147 @@ +# MVP Architecture Hardening Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make draft continuity verdicts authoritative and role-holder changes atomic before the MVP is finalized. + +**Architecture:** Preserve the deep `ImpactEngine.analyze` interface. A draft continuity projection module maps engine-produced `ImpactResult` values into a compact frontend read model, while role mutations move exact holder-set synchronization into the existing backend transaction. + +**Tech Stack:** Java 21, Spring Boot 3.5, Spring JDBC, PostgreSQL/Testcontainers, React 19, TypeScript, TanStack Query, Vitest/Testing Library. + +**Spec:** `docs/superpowers/specs/2026-08-20-mvp-architecture-hardening.md` + +## Global Constraints + +- `ImpactEngine.analyze(OrganizationSnapshot, SimulationChange)` remains the only public impact-engine interface. +- The browser must not calculate employee eligibility or continuity status from catalog fields. +- Existing impact and mitigation preview response contracts remain unchanged. +- Role metadata and a supplied exact holder set commit in one Spring transaction. +- Omitted `holderMemberIds` preserves existing update assignments; an explicitly empty set clears assignments. +- No database migration or new frontend dependency. +- Strict RED-GREEN-REFACTOR evidence is required in every task report. +- Do not restructure the graph projection or split `DeterministicImpactEngine`. + +--- + +### Task 1: Authoritative Draft Continuity Projection + +**Files:** +- Create: `backend/src/main/java/com/roleimpact/workspace/preview/api/DraftContinuityRiskResource.java` +- Create: `backend/src/main/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionService.java` +- Create: `backend/src/test/java/com/roleimpact/workspace/preview/application/DraftContinuityProjectionServiceTest.java` +- Modify: `backend/src/main/java/com/roleimpact/workspace/preview/api/DraftImpactPreviewController.java` +- Modify: `backend/src/test/java/com/roleimpact/workspace/WorkspaceIntegrationTest.java` +- Modify: `frontend/src/api/draftImpact.ts` +- Create: `frontend/src/components/workspaces/editor/continuityRiskPresentation.ts` +- Create: `frontend/src/components/workspaces/editor/continuityRiskPresentation.test.ts` +- Modify: `frontend/src/components/workspaces/editor/DraftEditor.tsx` +- Modify: `frontend/src/components/workspaces/editor/DraftImpactTesting.tsx` +- Modify: `frontend/src/components/workspaces/editor/OrganizationCanvas.tsx` +- Modify: `frontend/src/App.test.tsx` +- Delete: `frontend/src/components/workspaces/editor/workflowRisks.ts` + +**Interfaces:** +- Consumes: `ImpactEngine.analyze(snapshot, new RevokeEmployeeRole(memberId, roleId))` and matching `ImpactResult.WorkflowImpact` / `ImpactResult.StepImpact` records. +- Produces: `GET /api/v1/workspaces/{workspaceId}/impact-previews/continuity` returning `List`. +- Produces frontend `DraftContinuityRisk` with `key`, workflow/requirement/role facts, `eligibleMembers`, and `members`; each member carries `eligible`, `losesCoverage`, `remainingEligibleActorCount`, and `scenarioStatus` (`OPERATIONAL | DEGRADED | BLOCKED`). + +- [ ] **Step 1: Write failing backend projection tests** + +Create `DraftContinuityProjectionServiceTest` with hand-built catalog and engine-result fixtures. Prove that the projection: + +1. uses `StepImpact.baselineEligibleActors` rather than team/region/shift catalog guesses; +2. exposes the engine's workflow `scenarioStatus`, including `BLOCKED` when the step itself remains operational (the `DIFFERENT_ACTORS` case); +3. marks `losesCoverage` by comparing the selected holder against `scenarioEligibleActors`; +4. sorts workflows, requirements, roles, and holders deterministically; +5. rejects a non-draft or unknown workspace consistently with existing preview behavior. + +- [ ] **Step 2: Run the focused backend test and verify RED** + +Run the Maven unit test selector for `DraftContinuityProjectionServiceTest`. Expected: compilation/test failure because the projection records and module do not exist. + +- [ ] **Step 3: Implement the minimal backend projection** + +Add immutable records under `DraftContinuityRiskResource` and a read-only transactional `DraftContinuityProjectionService`. Load one catalog and one immutable snapshot. For every workflow requirement/role/current-holder tuple, call the existing impact engine with `RevokeEmployeeRole`, locate the matching workflow and step, and map only engine-produced eligibility and outcome facts. Add `GET /continuity` to `DraftImpactPreviewController`. Do not copy `DeterministicImpactEngine.isEligible`, `workflowStatus`, or constraint logic. + +- [ ] **Step 4: Verify backend GREEN and add endpoint integration coverage** + +Run `DraftContinuityProjectionServiceTest` to PASS. Add `WorkspaceIntegrationTest` coverage asserting the continuity endpoint returns engine-derived baseline actors and scenario status for the custom release workflow. Compile integration tests; run them when Docker is available. + +- [ ] **Step 5: Write failing frontend presentation and UI tests** + +Create literal server-projection fixtures where catalog demographics suggest safety but the supplied status is `BLOCKED`. Prove display-only ranking and labels follow `scenarioStatus`. Extend the app/editor test routing for the continuity GET and assert the risk callout and impact controls use the supplied verdict. Add an assertion that a successful catalog mutation causes the continuity query to be fetched again. + +- [ ] **Step 6: Run focused frontend tests and verify RED** + +Run the new presentation test and focused editor/App test. Expected: failure because the continuity client/query does not exist and the UI still imports `findWorkflowRisks`. + +- [ ] **Step 7: Implement the frontend projection consumer** + +Add `fetchDraftContinuityRisks` and exact TypeScript response types to `draftImpact.ts`. Query it from `DraftEditor`, pass authoritative risks to `DraftImpactTesting`, and keep only sorting/wording helpers in `continuityRiskPresentation.ts`. Make loading/error states explicit. Invalidate `['draft-continuity', workspaceId]` after every successful catalog mutation in both editor and map mutation paths. Delete `workflowRisks.ts` and all catalog-based eligibility calculations. + +- [ ] **Step 8: Verify frontend GREEN and refactor** + +Run focused tests, the full frontend suite, lint, and build. The already-recorded unrelated `Focused impact graph` baseline assertion may be updated only if the current production heading intentionally changed; record that separately in the task report. + +- [ ] **Step 9: Commit Task 1** + +Commit with subject `refactor: make continuity projection authoritative`. + +### Task 2: Atomic Role-Holder Synchronization + +**Files:** +- Modify: `backend/src/main/java/com/roleimpact/workspace/editor/api/RoleRequest.java` +- Modify: `backend/src/main/java/com/roleimpact/workspace/editor/application/DraftCatalogService.java` +- Modify: `backend/src/main/java/com/roleimpact/workspace/editor/persistence/DraftCatalogRepository.java` +- Create: `backend/src/test/java/com/roleimpact/workspace/editor/application/DraftCatalogServiceTest.java` +- Modify: `backend/src/test/java/com/roleimpact/workspace/WorkspaceIntegrationTest.java` +- Modify: `frontend/src/api/draftCatalog.ts` +- Modify: `frontend/src/components/workspaces/editor/DraftEditor.tsx` +- Modify: `frontend/src/components/workspaces/editor/OrganizationCanvas.tsx` +- Modify: `frontend/src/App.test.tsx` + +**Interfaces:** +- Consumes: existing `POST /catalog/roles` and `PUT /catalog/roles/{roleId}`. +- Extends request: optional `holderMemberIds: Set | null`; omitted update preserves holders, present set replaces holders exactly. +- Produces: `DraftCatalogRepository.replaceRoleHolders(UUID roleId, Set memberIds)` invoked once inside the role mutation transaction. + +- [ ] **Step 1: Write failing backend service tests** + +Create `DraftCatalogServiceTest` with controlled repository behavior. Prove: + +1. create with holder IDs validates the complete member set and performs one role-centric replacement; +2. update with an explicit empty set clears holders; +3. update with omitted holder IDs does not touch holders; +4. an unknown holder is rejected before role metadata or assignments are mutated. + +- [ ] **Step 2: Run the focused backend test and verify RED** + +Run the Maven unit test selector for `DraftCatalogServiceTest`. Expected: compilation/test failure because `RoleRequest.holderMemberIds`, member-set validation, and `replaceRoleHolders` do not exist. + +- [ ] **Step 3: Implement the atomic backend mutation** + +Extend `RoleRequest` with nullable `holderMemberIds` and defensively copy non-null sets. Add repository member-count validation and a role-centric replace method that deletes assignments for the role and inserts the exact desired members. In `createRole` and `updateRole`, validate all holder IDs first, then mutate metadata and holders inside the existing `@Transactional` method. Generalize the existing mutation helper only as needed to return the created role ID. + +- [ ] **Step 4: Verify backend GREEN and transactional integration behavior** + +Run `DraftCatalogServiceTest` to PASS. Extend `WorkspaceIntegrationTest` to assert create/update exact holder sets and that a mixed valid/unknown holder request leaves prior role metadata and holders unchanged. Compile integration tests; run them when Docker is available. + +- [ ] **Step 5: Write failing frontend role-flow tests** + +Use request-body assertions against the real frontend mutation functions and role flows. Prove inventory create/edit and organization-map quick-create/reuse send exactly one role request with the complete `holderMemberIds` set and do not issue sequential `/members/{id}/roles` calls. + +- [ ] **Step 6: Run focused frontend tests and verify RED** + +Run the relevant App tests. Expected: failure because role input has no holder set and both role flows still synchronize members sequentially. + +- [ ] **Step 7: Implement the single role mutation in the frontend** + +Extend `RoleInput` with optional `holderMemberIds`. Inventory create/edit/reuse and map create/reuse must call `createDraftRole` or `updateDraftRole` once with the exact selected holders. Delete `syncRoleHolders` and `assignRoleToMembers`. Leave member editing and one-off graph connections on `replaceMemberRoles`. + +- [ ] **Step 8: Verify frontend GREEN and all available checks** + +Run focused tests, full frontend tests, lint, build, backend unit tests, and backend test compilation. Run Testcontainers integration tests only if Docker is available. + +- [ ] **Step 9: Commit Task 2** + +Commit with subject `refactor: synchronize role holders atomically`. diff --git a/docs/superpowers/specs/2026-08-20-mvp-architecture-hardening.md b/docs/superpowers/specs/2026-08-20-mvp-architecture-hardening.md new file mode 100644 index 0000000..bcb9aa8 --- /dev/null +++ b/docs/superpowers/specs/2026-08-20-mvp-architecture-hardening.md @@ -0,0 +1,56 @@ +# MVP Architecture Hardening Specification + +## Objective + +Remove the two correctness risks approved before MVP completion: + +1. Browser-generated workflow continuity verdicts can disagree with the deterministic impact engine. +2. Role-holder edits are applied as a sequence of member mutations and can leave a partial result. + +## Authoritative continuity projection + +- `ImpactEngine.analyze(OrganizationSnapshot, SimulationChange)` remains the only public impact-engine interface. +- The backend exposes a read-only continuity projection for draft workspaces at + `GET /api/v1/workspaces/{workspaceId}/impact-previews/continuity`. +- The projection is derived from `ImpactResult` values produced by the existing engine. It must not reimplement employee eligibility, effective-permission, capability, application-access, coverage, or `DIFFERENT_ACTORS` rules. +- Each projection item identifies one workflow requirement and one supporting role and contains: + - workflow and requirement identity and display names; + - workflow criticality; + - minimum actors and resilience target; + - role identity and display name; + - baseline eligible members from the engine's matching `StepImpact`; + - one scenario per current holder of that role, including whether that holder was baseline-eligible, whether the holder loses coverage after revocation, the remaining eligible-actor count, and the engine's workflow scenario status. +- Items and nested members use deterministic ordering. +- The frontend uses this projection for workflow ordering, status labels, continuity callouts, member eligibility text, and post-run explanatory copy. +- The frontend may keep display-only sorting and wording. It must not recompute domain eligibility or coverage from catalog fields. +- If the projection is loading or unavailable, the organization editor remains usable but does not invent a verdict. The impact view presents an explicit loading or retry state. +- Any successful catalog mutation invalidates the continuity projection so it cannot remain stale. +- Delete `frontend/src/components/workspaces/editor/workflowRisks.ts` after all callers consume the server projection. + +## Atomic role-holder synchronization + +- Role create and role update requests accept optional `holderMemberIds`. +- Omission preserves compatibility: create means no holders; update leaves the current holders unchanged. +- When `holderMemberIds` is present, it is the exact desired holder set, including an explicitly empty set. +- The backend validates that every requested holder belongs to the draft workspace before mutating anything. +- Role metadata and the supplied holder set are committed in the same Spring transaction. +- Persistence replaces the role's complete holder set with one role-centric repository operation. It must not call the member-centric replacement operation in a loop. +- The inventory role form and organization-map quick-create/reuse paths send one role mutation containing the exact holder set. +- Member editing and one-off graph connections may retain the member-centric role-assignment interface because those user intents are member-centric and individually atomic. +- A failed holder validation or persistence operation must leave role metadata and existing assignments unchanged. + +## Compatibility and scope + +- No database migration is required. +- Existing published-workspace mutation protection remains in force. +- Existing impact and mitigation preview response contracts remain unchanged. +- No new frontend state-management or graph libraries are introduced. +- Do not restructure the graph projection or split `DeterministicImpactEngine` in this work. + +## Verification + +- Follow strict RED-GREEN-REFACTOR for every production change. +- Backend unit tests must remain runnable without Docker. +- Add PostgreSQL integration coverage for the endpoint and transactional holder replacement; compilation must succeed even if Docker is unavailable locally. +- Frontend tests must prove that server-supplied continuity statuses drive labels and that catalog mutations invalidate the projection. +- Run frontend tests, lint, and build; run backend unit tests and test compilation. Run PostgreSQL integration tests when Docker is available. diff --git a/frontend/src/App.css b/frontend/src/App.css index 5aea09d..31cad50 100644 --- a/frontend/src/App.css +++ b/frontend/src/App.css @@ -1691,8 +1691,10 @@ } .continuity-signal.critical { border-color: #704352; background: #27171e; } +.continuity-signal.warning { border-color: #7b6434; background: #2b2414; } .continuity-signal strong { color: #9cddc1; font-size: 0.75rem; } .continuity-signal.critical strong { color: #f6aebb; } +.continuity-signal.warning strong { color: #efc46a; } .continuity-signal span { color: #89958f; font-size: 0.67rem; } .impact-test-result { diff --git a/frontend/src/App.test.tsx b/frontend/src/App.test.tsx index 7e030f3..4785b37 100644 --- a/frontend/src/App.test.tsx +++ b/frontend/src/App.test.tsx @@ -4,7 +4,7 @@ import userEvent from '@testing-library/user-event' import { afterEach, describe, expect, it, vi } from 'vitest' import App from './App' import type { DraftCatalog } from './api/draftCatalog' -import type { DraftImpactResult, DraftMitigationPreview } from './api/draftImpact' +import type { DraftContinuityRisk, DraftImpactResult, DraftMitigationPreview } from './api/draftImpact' import type { Simulation } from './api/simulations' afterEach(() => { @@ -155,14 +155,186 @@ describe('App', () => { expect(screen.getByRole('button', { name: /Teams/ })).toHaveAttribute('aria-current', 'step') }) + it('creates an inventory role with its complete holder set in one role request', async () => { + const user = userEvent.setup() + const requests = mockEditableClone(clonedCatalogFixture) + + renderApp() + await user.click(await screen.findByRole('button', { name: 'Clone and customize' })) + await user.click(await screen.findByRole('button', { name: 'Detailed inventory' })) + await user.click(screen.getByRole('button', { name: /Roles/ })) + await user.type(screen.getByLabelText('Role name'), 'Release Manager') + await user.type(screen.getByLabelText('Description'), 'Approves production releases') + await user.click(screen.getByRole('checkbox', { name: 'Priya Sharma' })) + await user.click(screen.getByRole('button', { name: 'Add shared role & continue' })) + + expect((await screen.findAllByText(/Release Manager/)).length).toBeGreaterThan(0) + expect(requests.roleRequests.filter((request) => request.method === 'POST')).toHaveLength(1) + expect(requests.roleRequests.filter((request) => request.method === 'POST')[0].body.holderMemberIds).toEqual(['member-priya']) + expect(requests.memberRoleRequests).toEqual([]) + }) + + it('updates an inventory role with its complete holder set in one role request', async () => { + const user = userEvent.setup() + const requests = mockEditableClone(clonedCatalogFixture) + + renderApp() + await user.click(await screen.findByRole('button', { name: 'Clone and customize' })) + await user.click(await screen.findByRole('button', { name: 'Detailed inventory' })) + await user.click(screen.getByRole('button', { name: /Roles/ })) + const financeRole = screen.getByText('Finance Approver', { selector: 'strong' }).closest('article') + expect(financeRole).not.toBeNull() + await user.click(within(financeRole!).getByRole('button', { name: 'Edit' })) + await user.selectOptions(screen.getByLabelText('Sensitivity'), 'HIGH') + await user.click(screen.getByRole('button', { name: 'Save role and holders' })) + + expect(await screen.findByText(/HIGH · 1 holders/)).toBeInTheDocument() + expect(requests.roleRequests.filter((request) => request.method === 'PUT')).toHaveLength(1) + expect(requests.roleRequests.filter((request) => request.method === 'PUT')[0].body.holderMemberIds).toEqual(['member-priya']) + expect(requests.memberRoleRequests).toEqual([]) + }) + + it('quick-creates a map role with its complete holder set in one role request', async () => { + const user = userEvent.setup() + const requests = mockEditableClone(clonedCatalogFixture) + + renderApp() + await user.click(await screen.findByRole('button', { name: 'Clone and customize' })) + await user.click(await screen.findByRole('button', { name: 'Add Role' })) + await user.type(screen.getByLabelText('Role name'), 'Release Manager') + await user.click(screen.getByRole('checkbox', { name: 'Priya Sharma' })) + await user.click(screen.getByRole('button', { name: 'Create role' })) + + expect(await screen.findByText('Role created')).toBeInTheDocument() + expect(requests.roleRequests.filter((request) => request.method === 'POST')).toHaveLength(1) + expect(requests.roleRequests.filter((request) => request.method === 'POST')[0].body.holderMemberIds).toEqual(['member-priya']) + expect(requests.memberRoleRequests).toEqual([]) + }) + + it('reuses a map role with its complete holder set in one role request', async () => { + const user = userEvent.setup() + const requests = mockEditableClone(clonedCatalogFixture) + + renderApp() + await user.click(await screen.findByRole('button', { name: 'Clone and customize' })) + await user.click(await screen.findByRole('button', { name: 'Add Role' })) + await user.type(screen.getByLabelText('Role name'), 'Finance Approver') + expect(screen.getByRole('checkbox', { name: 'Priya Sharma' })).toBeChecked() + await user.click(screen.getByRole('button', { name: 'Assign existing role' })) + + expect(await screen.findByText('Role assigned')).toBeInTheDocument() + expect(requests.roleRequests.filter((request) => request.method === 'PUT')).toHaveLength(1) + expect(requests.roleRequests.filter((request) => request.method === 'PUT')[0].body.holderMemberIds).toEqual(['member-priya']) + expect(requests.memberRoleRequests).toEqual([]) + }) + + it('preselects current holders before reusing a map role', async () => { + const user = userEvent.setup() + mockEditableClone(clonedCatalogFixture) + + renderApp() + await user.click(await screen.findByRole('button', { name: 'Clone and customize' })) + await user.click(await screen.findByRole('button', { name: 'Add Role' })) + await user.type(screen.getByLabelText('Role name'), 'Finance Approver') + + expect(screen.getByRole('checkbox', { name: 'Priya Sharma' })).toBeChecked() + }) + + it('opens impact testing after a deferred continuity projection resolves', async () => { + const user = userEvent.setup() + const projection = deferred() + vi.spyOn(globalThis, 'fetch').mockImplementation(async (input, init) => { + const url = input.toString() + if (url.endsWith('/api/v1/workspaces') && !init?.method) return jsonResponse(workspaceFixture) + if (url.endsWith('/clones') && init?.method === 'POST') return jsonResponse(clonedWorkspaceFixture, 201) + if (url.endsWith('/catalog')) return jsonResponse(clonedCatalogFixture) + if (url.endsWith('/impact-previews/continuity') && !init?.method) return projection.promise + return new Response(null, { status: 404 }) + }) + + renderApp() + await user.click(await screen.findByRole('button', { name: 'Clone and customize' })) + await user.click(await screen.findByRole('button', { name: 'Test impact' })) + expect(await screen.findByRole('heading', { name: 'Loading continuity analysis' })).toBeInTheDocument() + + projection.resolve(jsonResponse(clonedContinuityFixture)) + + expect(await screen.findByRole('heading', { name: 'Test impact map' })).toBeInTheDocument() + expect(screen.getByRole('button', { name: /Vendor Payment.*would block/ })).toBeInTheDocument() + }) + + it('opens impact testing after retrying a failed continuity projection', async () => { + const user = userEvent.setup() + let continuityAttempts = 0 + vi.spyOn(globalThis, 'fetch').mockImplementation(async (input, init) => { + const url = input.toString() + if (url.endsWith('/api/v1/workspaces') && !init?.method) return jsonResponse(workspaceFixture) + if (url.endsWith('/clones') && init?.method === 'POST') return jsonResponse(clonedWorkspaceFixture, 201) + if (url.endsWith('/catalog')) return jsonResponse(clonedCatalogFixture) + if (url.endsWith('/impact-previews/continuity') && !init?.method) { + continuityAttempts += 1 + return continuityAttempts === 1 + ? jsonResponse({ message: 'Continuity service is unavailable' }, 503) + : jsonResponse(clonedContinuityFixture) + } + return new Response(null, { status: 404 }) + }) + + renderApp() + await user.click(await screen.findByRole('button', { name: 'Clone and customize' })) + await user.click(await screen.findByRole('button', { name: 'Test impact' })) + expect(await screen.findByRole('heading', { name: 'Continuity analysis is unavailable' })).toBeInTheDocument() + + await user.click(screen.getByRole('button', { name: 'Retry continuity analysis' })) + + expect(await screen.findByRole('heading', { name: 'Test impact map' })).toBeInTheDocument() + expect(continuityAttempts).toBe(2) + }) + + it('renders a degraded engine verdict with the warning continuity treatment', async () => { + const user = userEvent.setup() + const degradedContinuity = clonedContinuityFixture.map((risk) => ({ + ...risk, + members: risk.members.map((member) => ({ + ...member, + remainingEligibleActorCount: 1, + scenarioStatus: 'DEGRADED' as const, + })), + })) + vi.spyOn(globalThis, 'fetch').mockImplementation(async (input, init) => { + const url = input.toString() + if (url.endsWith('/api/v1/workspaces') && !init?.method) return jsonResponse(workspaceFixture) + if (url.endsWith('/clones') && init?.method === 'POST') return jsonResponse(clonedWorkspaceFixture, 201) + if (url.endsWith('/catalog')) return jsonResponse(clonedCatalogFixture) + if (url.endsWith('/impact-previews/continuity') && !init?.method) return jsonResponse(degradedContinuity) + return new Response(null, { status: 404 }) + }) + + renderApp() + await user.click(await screen.findByRole('button', { name: 'Clone and customize' })) + await user.click(await screen.findByRole('button', { name: 'Test impact' })) + + const heading = await screen.findByText('This workflow would lose resilience') + const signal = heading.closest('.continuity-signal') + expect(signal).toHaveClass('warning') + expect(getComputedStyle(signal!).backgroundColor).toBe('rgb(43, 36, 20)') + }) + it('builds a blank draft through teams, members, workflows, impact testing, and shared roles', async () => { const user = userEvent.setup() let catalog: DraftCatalog = structuredClone(blankCatalogFixture) + let continuityRequests = 0 + const roleRequests: Array<{ method: string; body: { holderMemberIds?: string[] } }> = [] + const memberRoleRequests: string[] = [] vi.spyOn(globalThis, 'fetch').mockImplementation(async (input, init) => { const url = input.toString() if (url.endsWith('/api/v1/workspaces') && !init?.method) return jsonResponse(workspaceFixture) if (url.endsWith('/api/v1/workspaces') && init?.method === 'POST') return jsonResponse(blankWorkspaceFixture, 201) if (url.endsWith('/catalog') && !init?.method) return jsonResponse(catalog) + if (url.endsWith('/impact-previews/continuity') && !init?.method) { + continuityRequests += 1 + return jsonResponse(catalog.workflows.length === 0 ? [] : customDraftContinuityFixture) + } if (url.endsWith('/catalog/teams') && init?.method === 'POST') { const input = JSON.parse(String(init.body)) as { name: string; department: string } catalog = { ...catalog, teams: [{ id: 'team-1', name: input.name, department: input.department, memberCount: 0 }] } @@ -176,8 +348,9 @@ describe('App', () => { return jsonResponse(catalog) } if (url.endsWith('/catalog/roles') && init?.method === 'POST') { - const input = JSON.parse(String(init.body)) as { name: string; description: string; sensitivity: DraftCatalog['roles'][number]['sensitivity'] } - catalog = { ...catalog, roles: [{ id: 'role-1', name: input.name, description: input.description, sensitivity: input.sensitivity, ownerMemberId: null, memberCount: 0 }] } + const input = JSON.parse(String(init.body)) as { name: string; description: string; sensitivity: DraftCatalog['roles'][number]['sensitivity']; ownerMemberId: string | null; holderMemberIds?: string[] } + roleRequests.push({ method: 'POST', body: input }) + catalog = applyRoleHolders({ ...catalog, roles: [{ id: 'role-1', name: input.name, description: input.description, sensitivity: input.sensitivity, ownerMemberId: input.ownerMemberId, memberCount: 0 }] }, 'role-1', input.holderMemberIds) return jsonResponse(catalog) } if (url.endsWith('/catalog/members/member-1') && init?.method === 'PUT') { @@ -186,12 +359,14 @@ describe('App', () => { return jsonResponse(catalog) } if (url.endsWith('/catalog/roles/role-1') && init?.method === 'PUT') { - const input = JSON.parse(String(init.body)) as { name: string; description: string; sensitivity: DraftCatalog['roles'][number]['sensitivity']; ownerMemberId: string | null } - catalog = { ...catalog, roles: [{ ...catalog.roles[0], ...input }] } + const input = JSON.parse(String(init.body)) as { name: string; description: string; sensitivity: DraftCatalog['roles'][number]['sensitivity']; ownerMemberId: string | null; holderMemberIds?: string[] } + roleRequests.push({ method: 'PUT', body: input }) + catalog = applyRoleHolders({ ...catalog, roles: [{ ...catalog.roles[0], ...input }] }, 'role-1', input.holderMemberIds) return jsonResponse(catalog) } if (url.match(/\/members\/[^/]+\/roles$/) && init?.method === 'PUT') { const memberId = url.match(/\/members\/([^/]+)\/roles$/)![1] + memberRoleRequests.push(memberId) const input = JSON.parse(String(init.body)) as { roleIds: string[] } const members = catalog.members.map((member) => member.id === memberId ? { ...member, roleIds: input.roleIds } : member) const roles = catalog.roles.map((role) => ({ ...role, memberCount: members.filter((member) => member.roleIds.includes(role.id)).length })) @@ -216,7 +391,7 @@ describe('App', () => { await user.type(await screen.findByLabelText('Organization name'), 'Atlas Systems') await user.click(screen.getByRole('button', { name: 'Start blank' })) expect(await screen.findByRole('heading', { name: 'Map how your organization works' })).toBeInTheDocument() - expect(screen.getByRole('button', { name: 'Add Member' })).toBeDisabled() + expect(await screen.findByRole('button', { name: 'Add Member' })).toBeDisabled() await user.click(screen.getByRole('button', { name: 'Add Team' })) await user.type(screen.getByLabelText('Team name'), 'Platform') @@ -244,6 +419,9 @@ describe('App', () => { await user.click(screen.getByRole('checkbox', { name: 'Maya Singh' })) await user.click(screen.getByRole('button', { name: 'Create role' })) expect(await screen.findByText('Role created')).toBeInTheDocument() + expect(roleRequests.filter((request) => request.method === 'POST')).toHaveLength(1) + expect(roleRequests.filter((request) => request.method === 'POST')[0].body.holderMemberIds).toEqual(['member-1']) + expect(memberRoleRequests).toHaveLength(0) expect(within(screen.getByLabelText('Organization inventory')).queryByRole('button', { name: 'Delete object' })).not.toBeInTheDocument() @@ -252,8 +430,10 @@ describe('App', () => { await user.type(screen.getByLabelText('Workflow name'), 'Production Deployment') await user.selectOptions(screen.getByLabelText('Business criticality'), 'CRITICAL') await user.type(screen.getByLabelText('Responsibility or step'), 'Release Manager responsibility') + const continuityRequestsBeforeWorkflow = continuityRequests await user.click(screen.getByRole('button', { name: 'Create workflow' })) expect(await screen.findByText(/CRITICAL · 1 role responsibilities/)).toBeInTheDocument() + expect(continuityRequests).toBeGreaterThan(continuityRequestsBeforeWorkflow) await user.click(screen.getByRole('button', { name: 'Organization map' })) expect(screen.getByLabelText('Organization inventory')).toHaveTextContent('Production Deployment') expect(screen.getByLabelText('Organization relationship canvas')).toBeInTheDocument() @@ -264,7 +444,10 @@ describe('App', () => { expect(screen.getByLabelText('Role Release Manager')).toBeInTheDocument() expect(screen.getByLabelText('Responsibility Release Manager responsibility')).toBeInTheDocument() expect(screen.getByLabelText('Workflow Production Deployment')).toBeInTheDocument() - expect(screen.getByLabelText('Continuity risks found')).toHaveTextContent('critical coverage gap') + const continuityCallout = screen.getByLabelText('Continuity risks found') + expect(continuityCallout).toHaveTextContent('critical coverage gap') + expect(continuityCallout).toHaveTextContent('According to the impact engine, removing Release Manager from Maya Singh would block Production Deployment.') + expect(continuityCallout).not.toHaveTextContent('provide the minimum coverage') const graphInventory = screen.getByLabelText('Organization inventory') await user.click(within(graphInventory).getByRole('button', { name: 'Role' })) @@ -307,10 +490,10 @@ describe('App', () => { const selectedMaya = screen.getByLabelText('Selected Member Maya Singh') expect(selectedMaya).toHaveTextContent('Connected workflows') await user.click(within(selectedMaya).getByRole('button', { name: 'Test losing Release Manager' })) - expect(await screen.findByText('This responsibility is now blocked')).toBeInTheDocument() - expect(screen.getByText(/Maya Singh was the only eligible holder/)).toBeInTheDocument() + expect(await screen.findAllByText('This workflow would be blocked')).not.toHaveLength(0) + expect(screen.getByText(/the impact engine finds 2 eligible actors/)).toBeInTheDocument() await user.click(screen.getByRole('button', { name: /Production Deployment.*would block/ })) - expect(screen.getByText('This responsibility is now blocked')).toBeInTheDocument() + expect(screen.getAllByText('This workflow would be blocked')).not.toHaveLength(0) expect(await screen.findByRole('heading', { name: 'A workflow would be blocked' })).toBeInTheDocument() expect(screen.getByLabelText('Complete organization impact map')).toBeInTheDocument() expect(screen.getByLabelText(/member Arjun Mehta.*Recommended #1/i)).toBeInTheDocument() @@ -339,6 +522,9 @@ describe('App', () => { await user.click(screen.getByRole('checkbox', { name: 'Arjun Mehta' })) await user.click(screen.getByRole('button', { name: 'Assign existing role' })) expect(await screen.findByText('Role assigned')).toBeInTheDocument() + expect(roleRequests.filter((request) => request.method === 'PUT')).toHaveLength(1) + expect(roleRequests.filter((request) => request.method === 'PUT')[0].body.holderMemberIds).toEqual(['member-1', 'member-2']) + expect(memberRoleRequests).toHaveLength(0) await user.click(screen.getByRole('button', { name: 'Detailed inventory' })) await user.click(screen.getByRole('button', { name: /Members/ })) @@ -352,6 +538,7 @@ describe('App', () => { const mayaNode = screen.getByLabelText('Member Maya Singh').closest('.react-flow__node') const arjunNode = screen.getByLabelText('Member Arjun Mehta').closest('.react-flow__node') expect(mayaNode?.getAttribute('style')).not.toEqual(arjunNode?.getAttribute('style')) + const memberRoleRequestsBeforeRoleEdit = memberRoleRequests.length await user.click(screen.getByRole('button', { name: 'Detailed inventory' })) await user.click(screen.getByRole('button', { name: /Roles/ })) expect(await screen.findByText(/2 holders/)).toBeInTheDocument() @@ -360,6 +547,9 @@ describe('App', () => { await user.selectOptions(screen.getByLabelText('Sensitivity'), 'CRITICAL') await user.click(screen.getByRole('button', { name: 'Save role and holders' })) expect(await screen.findByText(/CRITICAL · 2 holders/)).toBeInTheDocument() + expect(roleRequests.filter((request) => request.method === 'PUT')).toHaveLength(2) + expect(roleRequests.at(-1)).toEqual({ method: 'PUT', body: expect.objectContaining({ holderMemberIds: ['member-1', 'member-2'] }) }) + expect(memberRoleRequests).toHaveLength(memberRoleRequestsBeforeRoleEdit) }, 15000) }) @@ -370,6 +560,78 @@ function jsonResponse(value: unknown, status = 200) { }) } +function applyRoleHolders(catalog: DraftCatalog, roleId: string, holderMemberIds?: string[]) { + if (holderMemberIds === undefined) return catalog + const holderIds = new Set(holderMemberIds) + const members = catalog.members.map((member) => ({ + ...member, + roleIds: holderIds.has(member.id) + ? [...new Set([...member.roleIds, roleId])] + : member.roleIds.filter((id) => id !== roleId), + })) + return { + ...catalog, + members, + roles: catalog.roles.map((role) => ({ ...role, memberCount: members.filter((member) => member.roleIds.includes(role.id)).length })), + } +} + +type RoleMutationRequest = { + method: 'POST' | 'PUT' + body: { + name: string + description: string + sensitivity: DraftCatalog['roles'][number]['sensitivity'] + ownerMemberId: string | null + holderMemberIds?: string[] + } +} + +function mockEditableClone(initialCatalog: DraftCatalog) { + let catalog = structuredClone(initialCatalog) + const roleRequests: RoleMutationRequest[] = [] + const memberRoleRequests: string[] = [] + vi.spyOn(globalThis, 'fetch').mockImplementation(async (input, init) => { + const url = input.toString() + if (url.endsWith('/api/v1/workspaces') && !init?.method) return jsonResponse(workspaceFixture) + if (url.endsWith('/clones') && init?.method === 'POST') return jsonResponse(clonedWorkspaceFixture, 201) + if (url.endsWith('/catalog') && !init?.method) return jsonResponse(catalog) + if (url.endsWith('/impact-previews/continuity') && !init?.method) return jsonResponse([]) + if (url.endsWith('/catalog/roles') && init?.method === 'POST') { + const body = JSON.parse(String(init.body)) as RoleMutationRequest['body'] + roleRequests.push({ method: 'POST', body }) + catalog = applyRoleHolders({ + ...catalog, + roles: [...catalog.roles, { id: 'role-release', name: body.name, description: body.description, sensitivity: body.sensitivity, ownerMemberId: body.ownerMemberId, memberCount: 0 }], + }, 'role-release', body.holderMemberIds) + return jsonResponse(catalog) + } + const roleMatch = url.match(/\/catalog\/roles\/([^/]+)$/) + if (roleMatch && init?.method === 'PUT') { + const body = JSON.parse(String(init.body)) as RoleMutationRequest['body'] + const roleId = roleMatch[1] + roleRequests.push({ method: 'PUT', body }) + catalog = applyRoleHolders({ + ...catalog, + roles: catalog.roles.map((role) => role.id === roleId ? { + ...role, + name: body.name, + description: body.description, + sensitivity: body.sensitivity, + ownerMemberId: body.ownerMemberId, + } : role), + }, roleId, body.holderMemberIds) + return jsonResponse(catalog) + } + if (url.match(/\/members\/[^/]+\/roles$/) && init?.method === 'PUT') { + memberRoleRequests.push(url) + return jsonResponse(catalog) + } + return new Response(null, { status: 404 }) + }) + return { roleRequests, memberRoleRequests } +} + const workspaceFixture = [ { id: '00000000-0000-0000-0000-000000000001', @@ -429,6 +691,30 @@ const clonedCatalogFixture: DraftCatalog = { ], } +const clonedContinuityFixture: DraftContinuityRisk[] = [ + { + key: 'workflow-payment:requirement-payment:role-approver', + workflowId: 'workflow-payment', + workflowName: 'Vendor Payment', + criticality: 'CRITICAL', + requirementId: 'requirement-payment', + requirementName: 'Approve payment', + minimumActors: 1, + resilienceTarget: 1, + roleId: 'role-approver', + roleName: 'Finance Approver', + eligibleMembers: [{ id: 'member-priya', name: 'Priya Sharma' }], + members: [{ + id: 'member-priya', + name: 'Priya Sharma', + eligible: true, + losesCoverage: true, + remainingEligibleActorCount: 0, + scenarioStatus: 'BLOCKED', + }], + }, +] + const dashboardFixture = { organization: { id: '00000000-0000-0000-0000-000000000001', @@ -733,6 +1019,42 @@ const customDraftImpactFixture: DraftImpactResult = { excludedCandidateReasons: [], } +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise((resolvePromise) => { resolve = resolvePromise }) + return { promise, resolve } +} + +const customDraftContinuityFixture: DraftContinuityRisk[] = [ + { + key: 'workflow-1:requirement-1:role-1', + workflowId: 'workflow-1', + workflowName: 'Production Deployment', + criticality: 'CRITICAL', + requirementId: 'requirement-1', + requirementName: 'Release Manager responsibility', + minimumActors: 1, + resilienceTarget: 1, + roleId: 'role-1', + roleName: 'Release Manager', + // The supplied BLOCKED verdict is authoritative even with two baseline actors. + eligibleMembers: [ + { id: 'member-1', name: 'Maya Singh' }, + { id: 'member-2', name: 'Arjun Mehta' }, + ], + members: [ + { + id: 'member-1', + name: 'Maya Singh', + eligible: true, + losesCoverage: false, + remainingEligibleActorCount: 2, + scenarioStatus: 'BLOCKED', + }, + ], + }, +] + const customDraftMitigationPreviewFixture: DraftMitigationPreview = { original: customDraftImpactFixture, mitigation: { diff --git a/frontend/src/api/draftCatalog.ts b/frontend/src/api/draftCatalog.ts index 0501a8f..87865a3 100644 --- a/frontend/src/api/draftCatalog.ts +++ b/frontend/src/api/draftCatalog.ts @@ -56,7 +56,7 @@ export type DraftCatalog = { export type TeamInput = { name: string; department: string } export type MemberInput = Omit -export type RoleInput = Omit +export type RoleInput = Omit & { holderMemberIds?: string[] } export type WorkflowRequirementInput = { name: string roleId: string diff --git a/frontend/src/api/draftImpact.ts b/frontend/src/api/draftImpact.ts index 2ee6d0f..63fbd27 100644 --- a/frontend/src/api/draftImpact.ts +++ b/frontend/src/api/draftImpact.ts @@ -6,8 +6,39 @@ export type DraftMitigationPreview = { mitigation: DraftImpactResult } +export type DraftContinuityEligibleMember = { id: string; name: string } +export type DraftContinuityRiskMember = DraftContinuityEligibleMember & { + eligible: boolean + losesCoverage: boolean + remainingEligibleActorCount: number + scenarioStatus: 'OPERATIONAL' | 'DEGRADED' | 'BLOCKED' +} +export type DraftContinuityRisk = { + key: string + workflowId: string + workflowName: string + criticality: 'LOW' | 'MEDIUM' | 'HIGH' | 'CRITICAL' + requirementId: string + requirementName: string + minimumActors: number + resilienceTarget: number + roleId: string + roleName: string + eligibleMembers: DraftContinuityEligibleMember[] + members: DraftContinuityRiskMember[] +} + const apiBaseUrl = import.meta.env.VITE_API_BASE_URL ?? '' +export async function fetchDraftContinuityRisks(workspaceId: string, signal?: AbortSignal): Promise { + const response = await fetch(`${apiBaseUrl}/api/v1/workspaces/${workspaceId}/impact-previews/continuity`, { signal }) + if (!response.ok) { + const error = await response.json().catch(() => null) as { message?: string } | null + throw new Error(error?.message ?? 'The continuity projection could not be loaded') + } + return response.json() as Promise +} + export async function runDraftImpactPreview( workspaceId: string, memberId: string, diff --git a/frontend/src/components/workspaces/editor/DraftEditor.tsx b/frontend/src/components/workspaces/editor/DraftEditor.tsx index 1535569..99d1413 100644 --- a/frontend/src/components/workspaces/editor/DraftEditor.tsx +++ b/frontend/src/components/workspaces/editor/DraftEditor.tsx @@ -18,7 +18,7 @@ import { type DraftCatalog, type DraftMember, } from '../../../api/draftCatalog' -import { findWorkflowRisks } from './workflowRisks' +import { fetchDraftContinuityRisks } from '../../../api/draftImpact' const OrganizationCanvas = lazy(() => import('./OrganizationCanvas')) const DraftImpactTesting = lazy(() => import('./DraftImpactTesting')) @@ -33,6 +33,10 @@ export default function DraftEditor({ workspaceId, isTemplateClone }: { workspac queryKey: ['draft-catalog', workspaceId], queryFn: ({ signal }) => fetchDraftCatalog(workspaceId, signal), }) + const continuityQuery = useQuery({ + queryKey: ['draft-continuity', workspaceId], + queryFn: ({ signal }) => fetchDraftContinuityRisks(workspaceId, signal), + }) if (catalogQuery.isPending) return

Loading the draft catalog…

if (catalogQuery.isError) { @@ -51,9 +55,10 @@ export default function DraftEditor({ workspaceId, isTemplateClone }: { workspac { label: 'roles', value: catalog.roles.length }, { label: 'workflows', value: catalog.workflows.length }, ] - const singlePointRisks = findWorkflowRisks(catalog).filter((risk) => - risk.eligibleMembers.length > 0 && risk.eligibleMembers.length === risk.minimumActors, - ) + const continuityRisks = continuityQuery.data ?? [] + const singlePointRisks = continuityRisks.filter((risk) => risk.members.some((member) => member.scenarioStatus === 'BLOCKED')) + const firstBlockedRisk = singlePointRisks[0] + const firstBlockedMember = firstBlockedRisk?.members.find((member) => member.scenarioStatus === 'BLOCKED') function moveToStage(nextStage: EditorStage) { setStage(nextStage) @@ -86,10 +91,10 @@ export default function DraftEditor({ workspaceId, isTemplateClone }: { workspac ))} - {view === 'map' && singlePointRisks.length > 0 ? ( + {view === 'map' && !continuityQuery.isFetching && !continuityQuery.isError && firstBlockedRisk && firstBlockedMember ? (
-
{singlePointRisks.length} critical coverage gap{singlePointRisks.length === 1 ? '' : 's'} found

{singlePointRisks[0].eligibleMembers.map((member) => member.name).join(', ')} provide the minimum coverage for {singlePointRisks[0].requirementName} in {singlePointRisks[0].workflowName}.

+
{singlePointRisks.length} critical coverage gap{singlePointRisks.length === 1 ? '' : 's'} found

According to the impact engine, removing {firstBlockedRisk.roleName} from {firstBlockedMember.name} would block {firstBlockedRisk.workflowName}.

) : null} @@ -97,7 +102,7 @@ export default function DraftEditor({ workspaceId, isTemplateClone }: { workspac {view === 'map' ? ( Opening the organization map…

}> openInventory()} onOpenWorkflows={() => openInventory('workflows')} onTestImpact={() => setView('impact')} /> ) : view === 'impact' ? ( - Preparing impact testing…

}> setView('map')} /> + Preparing impact testing…

}> void continuityQuery.refetch()} onBackToMap={() => setView('map')} /> ) : ( <>