diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java index f8e7fab58..2d70a060b 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializer.java @@ -63,33 +63,44 @@ public void run(ApplicationArguments args) { log.info("Bootstrap admin is disabled"); return; } - if (localCredentialRepository.existsByUsernameIgnoreCase(bootstrapAdminProperties.getUsername())) { - log.info("Bootstrap admin already exists, skipping"); + LocalCredential existingCredential = localCredentialRepository + .findByUsernameIgnoreCase(bootstrapAdminProperties.getUsername()) + .orElse(null); + if (existingCredential != null + && !bootstrapAdminProperties.getUserId().equals(existingCredential.getUserId())) { + log.info("Bootstrap admin username '{}' is already bound to another user, skipping", + bootstrapAdminProperties.getUsername()); return; } - // 1. Create admin user account + // 1. Create admin user account. When the credential already belongs to the + // configured bootstrap user, only backfill role/membership and preserve + // any existing profile fields managed elsewhere. UserAccount admin = userAccountRepository.findById(bootstrapAdminProperties.getUserId()) - .orElseGet(() -> userAccountRepository.save( - new UserAccount( - bootstrapAdminProperties.getUserId(), - bootstrapAdminProperties.getDisplayName(), - bootstrapAdminProperties.getEmail(), - null - ) - )); - admin.setDisplayName(bootstrapAdminProperties.getDisplayName()); - admin.setEmail(bootstrapAdminProperties.getEmail()); - admin = userAccountRepository.save(admin); + .orElse(null); + if (admin == null) { + admin = userAccountRepository.save(new UserAccount( + bootstrapAdminProperties.getUserId(), + bootstrapAdminProperties.getDisplayName(), + bootstrapAdminProperties.getEmail(), + null + )); + } else if (existingCredential == null) { + admin.setDisplayName(bootstrapAdminProperties.getDisplayName()); + admin.setEmail(bootstrapAdminProperties.getEmail()); + admin = userAccountRepository.save(admin); + } // 2. Create local credential (username/password) - localCredentialRepository.save( - new LocalCredential( - admin.getId(), - bootstrapAdminProperties.getUsername(), - passwordEncoder.encode(bootstrapAdminProperties.getPassword()) - ) - ); + if (existingCredential == null) { + localCredentialRepository.save( + new LocalCredential( + admin.getId(), + bootstrapAdminProperties.getUsername(), + passwordEncoder.encode(bootstrapAdminProperties.getPassword()) + ) + ); + } // 3. Assign SUPER_ADMIN role Role superAdmin = roleRepository.findByCode("SUPER_ADMIN") diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 1fea61ca5..68540961e 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -167,3 +167,4 @@ validation.auth.password.reset.email.invalid=Email format is invalid validation.auth.password.reset.code.notBlank=Verification code cannot be blank validation.auth.password.reset.code.invalid=Verification code must be 6 digits validation.auth.password.reset.newPassword.notBlank=New password cannot be blank +promotion.target_skill_conflict=The target global skill "{0}" already exists diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 05bd09054..c1f782f02 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -167,3 +167,4 @@ validation.auth.password.reset.email.invalid=邮箱格式不正确 validation.auth.password.reset.code.notBlank=验证码不能为空 validation.auth.password.reset.code.invalid=验证码必须为 6 位数字 validation.auth.password.reset.newPassword.notBlank=新密码不能为空 +promotion.target_skill_conflict=目标全局技能“{0}”已存在 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java index a151d2a89..ace28d26a 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BootstrapAdminInitializerTest.java @@ -73,7 +73,6 @@ void shouldSeedBootstrapAdminWithCredentialRoleAndMembership() throws Exception setField(superAdminRole, "id", 1L); setField(superAdminRole, "code", "SUPER_ADMIN"); - when(localCredentialRepository.existsByUsernameIgnoreCase("admin")).thenReturn(false); when(userAccountRepository.findById("docker-admin")).thenReturn(Optional.empty()); when(userAccountRepository.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); when(passwordEncoder.encode("ChangeMe!2026")).thenReturn("encoded-password"); @@ -111,7 +110,8 @@ void shouldSeedBootstrapAdminWithCredentialRoleAndMembership() throws Exception @Test void shouldSkipWhenBootstrapAdminCredentialAlreadyExists() { bootstrapAdminProperties.setEnabled(true); - when(localCredentialRepository.existsByUsernameIgnoreCase("admin")).thenReturn(true); + LocalCredential conflictingCredential = new LocalCredential("someone-else", "admin", "encoded-password"); + when(localCredentialRepository.findByUsernameIgnoreCase("admin")).thenReturn(Optional.of(conflictingCredential)); initializer.run(new DefaultApplicationArguments(new String[0])); @@ -121,13 +121,70 @@ void shouldSkipWhenBootstrapAdminCredentialAlreadyExists() { verify(namespaceMemberRepository, never()).save(any(NamespaceMember.class)); } + @Test + void shouldStillEnsureRoleAndMembershipWhenBootstrapCredentialExistsForConfiguredUser() throws Exception { + bootstrapAdminProperties.setEnabled(true); + Namespace global = new Namespace("global", "Global", "system"); + setField(global, "id", 1L); + + Role superAdminRole = new Role(); + setField(superAdminRole, "id", 1L); + setField(superAdminRole, "code", "SUPER_ADMIN"); + + LocalCredential existingCredential = new LocalCredential("docker-admin", "admin", "encoded-password"); + + when(localCredentialRepository.findByUsernameIgnoreCase("admin")).thenReturn(Optional.of(existingCredential)); + when(userAccountRepository.findById("docker-admin")).thenReturn(Optional.empty()); + when(userAccountRepository.save(any(UserAccount.class))).thenAnswer(invocation -> invocation.getArgument(0)); + when(roleRepository.findByCode("SUPER_ADMIN")).thenReturn(Optional.of(superAdminRole)); + when(userRoleBindingRepository.findByUserId("docker-admin")).thenReturn(List.of()); + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(global)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, "docker-admin")).thenReturn(Optional.empty()); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + verify(localCredentialRepository, never()).save(any(LocalCredential.class)); + verify(userRoleBindingRepository).save(any(UserRoleBinding.class)); + verify(namespaceMemberRepository).save(any(NamespaceMember.class)); + } + + @Test + void shouldPreserveExistingUserProfileWhenBackfillingRoleAndMembership() throws Exception { + bootstrapAdminProperties.setEnabled(true); + Namespace global = new Namespace("global", "Global", "system"); + setField(global, "id", 1L); + + Role superAdminRole = new Role(); + setField(superAdminRole, "id", 1L); + setField(superAdminRole, "code", "SUPER_ADMIN"); + + LocalCredential existingCredential = new LocalCredential("docker-admin", "admin", "encoded-password"); + UserAccount existingUser = new UserAccount("docker-admin", "Existing Admin", "existing-admin@example.com", null); + + when(localCredentialRepository.findByUsernameIgnoreCase("admin")).thenReturn(Optional.of(existingCredential)); + when(userAccountRepository.findById("docker-admin")).thenReturn(Optional.of(existingUser)); + when(roleRepository.findByCode("SUPER_ADMIN")).thenReturn(Optional.of(superAdminRole)); + when(userRoleBindingRepository.findByUserId("docker-admin")).thenReturn(List.of()); + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(global)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(1L, "docker-admin")).thenReturn(Optional.empty()); + + initializer.run(new DefaultApplicationArguments(new String[0])); + + assertEquals("Existing Admin", existingUser.getDisplayName()); + assertEquals("existing-admin@example.com", existingUser.getEmail()); + verify(userAccountRepository, never()).save(any(UserAccount.class)); + verify(localCredentialRepository, never()).save(any(LocalCredential.class)); + verify(userRoleBindingRepository).save(any(UserRoleBinding.class)); + verify(namespaceMemberRepository).save(any(NamespaceMember.class)); + } + @Test void shouldSkipWhenBootstrapAdminIsDisabled() { bootstrapAdminProperties.setEnabled(false); initializer.run(new DefaultApplicationArguments(new String[0])); - verify(localCredentialRepository, never()).existsByUsernameIgnoreCase(any()); + verify(localCredentialRepository, never()).findByUsernameIgnoreCase(any()); verify(userAccountRepository, never()).save(any(UserAccount.class)); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/PromotionApprovalFlowIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/PromotionApprovalFlowIntegrationTest.java new file mode 100644 index 000000000..44d397125 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/PromotionApprovalFlowIntegrationTest.java @@ -0,0 +1,193 @@ +package com.iflytek.skillhub.controller.portal; + +import com.iflytek.skillhub.SkillhubApplication; +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.governance.GovernanceNotificationService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceType; +import com.iflytek.skillhub.domain.review.PromotionRequest; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.infra.jpa.NamespaceJpaRepository; +import com.iflytek.skillhub.infra.jpa.PromotionRequestJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillVersionJpaRepository; +import com.iflytek.skillhub.infra.jpa.UserAccountJpaRepository; +import com.iflytek.skillhub.notification.service.NotificationDispatcher; +import java.time.Instant; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +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.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest(classes = SkillhubApplication.class) +@AutoConfigureMockMvc +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class PromotionApprovalFlowIntegrationTest { + + private static final String SUBMITTER_ID = "promotion-owner"; + private static final String REVIEWER_ID = "docker-admin"; + + @Autowired + private MockMvc mockMvc; + + @Autowired + private UserAccountJpaRepository userAccountRepository; + + @Autowired + private NamespaceJpaRepository namespaceRepository; + + @Autowired + private SkillJpaRepository skillRepository; + + @Autowired + private SkillVersionJpaRepository skillVersionRepository; + + @Autowired + private PromotionRequestJpaRepository promotionRequestRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @MockBean + private RbacService rbacService; + + @MockBean + private GovernanceNotificationService governanceNotificationService; + + @MockBean + private NotificationDispatcher notificationDispatcher; + + @BeforeEach + void setUp() { + when(rbacService.getUserRoleCodes(REVIEWER_ID)).thenReturn(Set.of("SUPER_ADMIN")); + } + + @Test + void approvePromotion_persistsTargetSkillAndReturnsSuccessForBootstrapAdmin() throws Exception { + PromotionGraph graph = createPromotionGraph(); + + mockMvc.perform(post("/api/web/promotions/" + graph.request().getId() + "/approve") + .contentType("application/json") + .content("{\"comment\":\"ship it\"}") + .with(authentication(portalAuth(REVIEWER_ID, "SUPER_ADMIN"))) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.id").value(graph.request().getId())) + .andExpect(jsonPath("$.data.status").value("APPROVED")) + .andExpect(jsonPath("$.data.reviewedBy").value(REVIEWER_ID)) + .andExpect(jsonPath("$.data.reviewComment").value("ship it")); + + PromotionRequest savedRequest = promotionRequestRepository.findAllById(List.of(graph.request().getId())) + .stream() + .findFirst() + .orElseThrow(); + assertThat(savedRequest.getStatus()).isEqualTo(ReviewTaskStatus.APPROVED); + assertThat(savedRequest.getReviewedBy()).isEqualTo(REVIEWER_ID); + assertThat(savedRequest.getTargetSkillId()).isNotNull(); + + Skill targetSkill = skillRepository.findAllById(List.of(savedRequest.getTargetSkillId())) + .stream() + .findFirst() + .orElseThrow(); + assertThat(targetSkill.getNamespaceId()).isEqualTo(graph.globalNamespace().getId()); + assertThat(targetSkill.getSlug()).isEqualTo(graph.sourceSkill().getSlug()); + assertThat(targetSkill.getSourceSkillId()).isEqualTo(graph.sourceSkill().getId()); + assertThat(targetSkill.getLatestVersionId()).isNotNull(); + + List targetVersions = skillVersionRepository.findBySkillId(targetSkill.getId()); + assertThat(targetVersions).hasSize(1); + assertThat(targetVersions.get(0).getStatus()).isEqualTo(SkillVersionStatus.PUBLISHED); + } + + private PromotionGraph createPromotionGraph() { + String suffix = UUID.randomUUID().toString().substring(0, 8); + + userAccountRepository.saveAndFlush( + new UserAccount(SUBMITTER_ID, "Promotion Owner", "owner-" + suffix + "@example.com", null) + ); + userAccountRepository.saveAndFlush( + new UserAccount(REVIEWER_ID, "Admin", "admin-" + suffix + "@example.com", null) + ); + + Namespace globalNamespace = new Namespace("global-" + suffix, "Global " + suffix, REVIEWER_ID); + globalNamespace.setType(NamespaceType.GLOBAL); + globalNamespace = namespaceRepository.saveAndFlush(globalNamespace); + + Namespace teamNamespace = new Namespace("team-" + suffix, "Team " + suffix, SUBMITTER_ID); + teamNamespace = namespaceRepository.saveAndFlush(teamNamespace); + + Skill sourceSkill = new Skill(teamNamespace.getId(), "promote-skill-" + suffix, SUBMITTER_ID, SkillVisibility.PUBLIC); + sourceSkill.setDisplayName("Promote Skill " + suffix); + sourceSkill.setSummary("Used to verify promotion approval flow."); + sourceSkill.setCreatedBy(SUBMITTER_ID); + sourceSkill.setUpdatedBy(SUBMITTER_ID); + sourceSkill = skillRepository.saveAndFlush(sourceSkill); + + SkillVersion sourceVersion = new SkillVersion(sourceSkill.getId(), "1.0.0", SUBMITTER_ID); + sourceVersion.setStatus(SkillVersionStatus.PUBLISHED); + sourceVersion.setPublishedAt(Instant.now()); + sourceVersion.setRequestedVisibility(SkillVisibility.PUBLIC); + sourceVersion.setParsedMetadataJson("{\"name\":\"promotion-flow\"}"); + sourceVersion.setManifestJson("{\"version\":\"1.0.0\"}"); + sourceVersion = skillVersionRepository.saveAndFlush(sourceVersion); + + sourceSkill.setLatestVersionId(sourceVersion.getId()); + sourceSkill.setUpdatedBy(SUBMITTER_ID); + sourceSkill = skillRepository.saveAndFlush(sourceSkill); + + PromotionRequest request = promotionRequestRepository.saveAndFlush( + new PromotionRequest(sourceSkill.getId(), sourceVersion.getId(), globalNamespace.getId(), SUBMITTER_ID) + ); + + return new PromotionGraph(globalNamespace, sourceSkill, sourceVersion, request); + } + + private UsernamePasswordAuthenticationToken portalAuth(String userId, String... roles) { + PlatformPrincipal principal = new PlatformPrincipal( + userId, + userId, + userId + "@example.com", + "", + "session", + Set.of(roles) + ); + List authorities = java.util.Arrays.stream(roles) + .map(role -> new SimpleGrantedAuthority("ROLE_" + role)) + .toList(); + return new UsernamePasswordAuthenticationToken(principal, null, authorities); + } + + private record PromotionGraph(Namespace globalNamespace, + Skill sourceSkill, + SkillVersion sourceVersion, + PromotionRequest request) { + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java index eb533b8a9..4ce5d3c9d 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java @@ -16,6 +16,7 @@ import com.iflytek.skillhub.domain.skill.*; import jakarta.persistence.EntityManager; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -197,9 +198,8 @@ public PromotionRequest approvePromotion(Long promotionId, String reviewerId, if (updated == 0) { throw new ConcurrentModificationException("Promotion request was modified concurrently"); } - syncPromotionRequestState(request, ReviewTaskStatus.APPROVED, reviewerId, comment); - entityManager.detach(request); - PromotionRequest approvedRequest = request; + PromotionRequest approvedRequest = promotionRequestRepository.findById(promotionId) + .orElseThrow(() -> new DomainNotFoundException("promotion.not_found", promotionId)); Skill sourceSkill = skillRepository.findById(approvedRequest.getSourceSkillId()) .orElseThrow(() -> new DomainNotFoundException("skill.not_found", approvedRequest.getSourceSkillId())); @@ -207,6 +207,8 @@ public PromotionRequest approvePromotion(Long promotionId, String reviewerId, SkillVersion sourceVersion = skillVersionRepository.findById(approvedRequest.getSourceVersionId()) .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", approvedRequest.getSourceVersionId())); + assertTargetSkillNotExists(approvedRequest, sourceSkill); + // Create new skill in global namespace Skill newSkill = new Skill(approvedRequest.getTargetNamespaceId(), sourceSkill.getSlug(), sourceSkill.getOwnerId(), SkillVisibility.PUBLIC); @@ -215,7 +217,11 @@ public PromotionRequest approvePromotion(Long promotionId, String reviewerId, newSkill.setSourceSkillId(sourceSkill.getId()); newSkill.setCreatedBy(reviewerId); newSkill.setUpdatedBy(reviewerId); - newSkill = skillRepository.save(newSkill); + try { + newSkill = skillRepository.save(newSkill); + } catch (DataIntegrityViolationException ex) { + throw duplicateTargetSkillConflict(sourceSkill.getSlug(), ex); + } // Create new version copying metadata from source SkillVersion newVersion = new SkillVersion(newSkill.getId(), sourceVersion.getVersion(), @@ -264,6 +270,24 @@ public PromotionRequest approvePromotion(Long promotionId, String reviewerId, return savedRequest; } + private void assertTargetSkillNotExists(PromotionRequest approvedRequest, Skill sourceSkill) { + skillRepository.findByNamespaceIdAndSlugAndOwnerId( + approvedRequest.getTargetNamespaceId(), + sourceSkill.getSlug(), + sourceSkill.getOwnerId() + ).ifPresent(existing -> { + throw duplicateTargetSkillConflict(sourceSkill.getSlug(), null); + }); + } + + private DomainBadRequestException duplicateTargetSkillConflict(String slug, Exception cause) { + DomainBadRequestException ex = new DomainBadRequestException("promotion.target_skill_conflict", slug); + if (cause != null) { + ex.initCause(cause); + } + return ex; + } + /** * Rejects a pending promotion request without changing the source skill. */ diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java index ea00a4493..8586f96d2 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java @@ -19,6 +19,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.DataIntegrityViolationException; import java.time.Clock; import java.time.Instant; @@ -119,6 +120,16 @@ private PromotionRequest createPendingPromotion() { return pr; } + private PromotionRequest approvedPromotion(PromotionRequest original, String comment) { + PromotionRequest approved = createPendingPromotion(); + approved.setStatus(ReviewTaskStatus.APPROVED); + approved.setReviewedBy(REVIEWER_ID); + approved.setReviewComment(comment); + approved.setReviewedAt(Instant.now(CLOCK)); + setField(approved, "version", original.getVersion() + 1); + return approved; + } + private List createSourceFiles() { return List.of( new SkillFile(SOURCE_VERSION_ID, "main.py", 500L, "text/x-python", "sha1", "storage/key1"), @@ -347,6 +358,7 @@ class ReviewPromotion { @Test void shouldNotifySubmitterWhenPromotionApproved() { PromotionRequest request = createPendingPromotion(); + PromotionRequest approvedRequest = approvedPromotion(request, "ok"); Skill sourceSkill = createSourceSkill(); SkillVersion sourceVersion = createPublishedVersion(); Skill newSkill = new Skill(TARGET_NAMESPACE_ID, "my-skill", REVIEWER_ID, SkillVisibility.PUBLIC); @@ -354,7 +366,8 @@ void shouldNotifySubmitterWhenPromotionApproved() { SkillVersion newVersion = new SkillVersion(NEW_SKILL_ID, sourceVersion.getVersion(), REVIEWER_ID); setField(newVersion, "id", NEW_VERSION_ID); - when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(request)); + when(promotionRequestRepository.findById(PROMOTION_ID)) + .thenReturn(Optional.of(request), Optional.of(approvedRequest)); when(permissionChecker.canReviewPromotion(request, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); when(promotionRequestRepository.updateStatusWithVersion( PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "ok", null, request.getVersion())) @@ -364,6 +377,7 @@ void shouldNotifySubmitterWhenPromotionApproved() { when(skillRepository.save(any(Skill.class))).thenReturn(newSkill); when(skillVersionRepository.save(any(SkillVersion.class))).thenReturn(newVersion); when(skillFileRepository.findByVersionId(SOURCE_VERSION_ID)).thenReturn(List.of()); + when(promotionRequestRepository.save(approvedRequest)).thenReturn(approvedRequest); promotionService.approvePromotion(PROMOTION_ID, REVIEWER_ID, "ok", Set.of("SKILL_ADMIN")); @@ -393,11 +407,13 @@ class ApprovePromotion { @Test void shouldApprovePromotionSuccessfully() { PromotionRequest pr = createPendingPromotion(); + PromotionRequest approvedRequest = approvedPromotion(pr, "LGTM"); Skill sourceSkill = createSourceSkill(); SkillVersion sourceVersion = createPublishedVersion(); List sourceFiles = createSourceFiles(); - when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(pr)); + when(promotionRequestRepository.findById(PROMOTION_ID)) + .thenReturn(Optional.of(pr), Optional.of(approvedRequest)); when(permissionChecker.canReviewPromotion(pr, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); when(promotionRequestRepository.updateStatusWithVersion( PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "LGTM", null, pr.getVersion())) @@ -416,7 +432,7 @@ void shouldApprovePromotionSuccessfully() { }); when(skillFileRepository.findByVersionId(SOURCE_VERSION_ID)).thenReturn(sourceFiles); when(skillFileRepository.saveAll(anyList())).thenAnswer(inv -> inv.getArgument(0)); - when(promotionRequestRepository.save(pr)).thenReturn(pr); + when(promotionRequestRepository.save(approvedRequest)).thenReturn(approvedRequest); PromotionRequest result = promotionService.approvePromotion( PROMOTION_ID, REVIEWER_ID, "LGTM", Set.of("SKILL_ADMIN")); @@ -467,8 +483,8 @@ void shouldApprovePromotionSuccessfully() { assertEquals(REVIEWER_ID, event.publisherId()); // Verify targetSkillId updated on promotion request - verify(promotionRequestRepository).save(pr); - assertEquals(NEW_SKILL_ID, pr.getTargetSkillId()); + verify(promotionRequestRepository).save(approvedRequest); + assertEquals(NEW_SKILL_ID, approvedRequest.getTargetSkillId()); } @Test @@ -511,13 +527,39 @@ void shouldThrowOnConcurrentModification() { () -> promotionService.approvePromotion(PROMOTION_ID, REVIEWER_ID, "ok", Set.of("SKILL_ADMIN"))); } + @Test + void shouldTranslateDuplicateTargetSkillIntoBadRequest() { + PromotionRequest pr = createPendingPromotion(); + PromotionRequest approvedRequest = approvedPromotion(pr, "ok"); + Skill sourceSkill = createSourceSkill(); + SkillVersion sourceVersion = createPublishedVersion(); + + when(promotionRequestRepository.findById(PROMOTION_ID)) + .thenReturn(Optional.of(pr), Optional.of(approvedRequest)); + when(permissionChecker.canReviewPromotion(pr, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); + when(promotionRequestRepository.updateStatusWithVersion( + PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "ok", null, pr.getVersion())) + .thenReturn(1); + when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); + when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion)); + when(skillRepository.save(any(Skill.class))) + .thenThrow(new DataIntegrityViolationException("duplicate key value violates unique constraint")); + + DomainBadRequestException ex = assertThrows(DomainBadRequestException.class, + () -> promotionService.approvePromotion(PROMOTION_ID, REVIEWER_ID, "ok", Set.of("SKILL_ADMIN"))); + + assertEquals("promotion.target_skill_conflict", ex.messageCode()); + } + @Test void shouldCopyDisplayNameAndSummaryToNewSkill() { PromotionRequest pr = createPendingPromotion(); + PromotionRequest approvedRequest = approvedPromotion(pr, "ok"); Skill sourceSkill = createSourceSkill(); SkillVersion sourceVersion = createPublishedVersion(); - when(promotionRequestRepository.findById(PROMOTION_ID)).thenReturn(Optional.of(pr)); + when(promotionRequestRepository.findById(PROMOTION_ID)) + .thenReturn(Optional.of(pr), Optional.of(approvedRequest)); when(permissionChecker.canReviewPromotion(pr, REVIEWER_ID, Set.of("SKILL_ADMIN"))).thenReturn(true); when(promotionRequestRepository.updateStatusWithVersion(any(), any(), any(), any(), any(), any())).thenReturn(1); when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(sourceSkill)); @@ -543,6 +585,7 @@ void shouldCopyDisplayNameAndSummaryToNewSkill() { assertEquals("My Skill", newSkill.getDisplayName()); assertEquals("A test skill", newSkill.getSummary()); } + } @Nested