From 3f0d3897be18cd5a50d420ff07d989f911c2af76 Mon Sep 17 00:00:00 2001 From: "EVOFORGE\\dimay" Date: Fri, 3 Nov 2023 08:55:52 -0400 Subject: [PATCH 1/3] #2464: fixed learning path validation to consider whether the badge is actually configured currently on the learning path or will be on the learning path after this addition --- .../admin/CircularLearningPathChecker.groovy | 39 ++++++++++++++----- ...LearningPathValidationEndpointSpecs.groovy | 25 ++++++++++++ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/service/src/main/java/skills/services/admin/CircularLearningPathChecker.groovy b/service/src/main/java/skills/services/admin/CircularLearningPathChecker.groovy index b28e28529f..8e68b655f9 100644 --- a/service/src/main/java/skills/services/admin/CircularLearningPathChecker.groovy +++ b/service/src/main/java/skills/services/admin/CircularLearningPathChecker.groovy @@ -36,6 +36,7 @@ class CircularLearningPathChecker { // private private BadgeAndSkillsLookup badgeAndSkillsLookup = new BadgeAndSkillsLookup() private PrerequisiteNodeLookup prerequisiteNodeLookup = new PrerequisiteNodeLookup() + private Set allItemIdsOnFinalLearningPath private SkillInfo start // contains all of the badges by following start node in the opposite direction of prerequisite path @@ -182,6 +183,7 @@ class CircularLearningPathChecker { SkillInfo skillInfo = new SkillInfo(projectId: skillDef.projectId, skillId: skillDef.skillId, name: skillDef.name, type: skillDef.type) SkillInfo prereqSkillInfo = new SkillInfo(projectId: prereqSkillDef.projectId, skillId: prereqSkillDef.skillId, name: prereqSkillDef.name, type: prereqSkillDef.type) List path = [skillInfo] + allItemIdsOnFinalLearningPath = constructAllItemIdsOnFinalLearningPath(edgePairs, skillInfo, prereqSkillInfo) startNodeBadgesOnParentPath = [] collectParentBadges(skillInfo, startNodeBadgesOnParentPath, 0) @@ -274,7 +276,9 @@ class CircularLearningPathChecker { badgesOnPath = badgesOnPath.findAll { it.type == SkillDef.ContainerType.Badge } for (SkillInfo badgeOnPathSkillInfo : badgesOnPath) { BadgeAndSkills badge = badgeAndSkillsLookup.getBadgeByBadgeId(badgeOnPathSkillInfo.skillId) - if (badge.badgeHasSkillId(current.skillId) && badge.badgeGraphNode.skillId != current.circularCheckBadgeLoadedDueToPreviousSkillFollowingRouteOfBadgeId) { + if (isBadgeOnCurrentLearningPath(badge) && + badge.badgeHasSkillId(current.skillId) && + badge.badgeGraphNode.skillId != current.circularCheckBadgeLoadedDueToPreviousSkillFollowingRouteOfBadgeId) { return new DependencyCheckResult(possible: false, failureType: DependencyCheckResult.FailureType.BadgeSkillIsAlreadyOnPath, violatingSkillId: current.skillId, @@ -327,21 +331,36 @@ class CircularLearningPathChecker { private DependencyCheckResult checkBadgesForSkillOverlap(BadgeAndSkills badge, List checkAgainst) { for (SkillInfo badgeOnPathSkillInfo : checkAgainst) { BadgeAndSkills badgeOnPath = badgeAndSkillsLookup.getBadgeByBadgeId(badgeOnPathSkillInfo.skillId) - SkillInfo found = badgeOnPath.skills.find { SkillInfo searchFor -> badge.skills.find { searchFor.skillId == it.skillId } } - if (found) { - return new DependencyCheckResult(possible: false, - failureType: DependencyCheckResult.FailureType.BadgeOverlappingSkills, - violatingSkillInBadgeId: badge.badgeGraphNode.skillId, - violatingSkillInBadgeName: badge.badgeGraphNode.name, - violatingSkillId: found.skillId, - violatingSkillName: found.name, - reason: "Multiple badges on the same Learning path cannot have overlapping skills. Both badge [${badge.badgeGraphNode.name}] and [${badgeOnPath.badgeGraphNode.name}] badge have [${found.name}] skill.") + if (isBadgeOnCurrentLearningPath(badgeOnPath)) { + SkillInfo found = badgeOnPath.skills.find { SkillInfo searchFor -> badge.skills.find { searchFor.skillId == it.skillId } } + if (found) { + return new DependencyCheckResult(possible: false, + failureType: DependencyCheckResult.FailureType.BadgeOverlappingSkills, + violatingSkillInBadgeId: badge.badgeGraphNode.skillId, + violatingSkillInBadgeName: badge.badgeGraphNode.name, + violatingSkillId: found.skillId, + violatingSkillName: found.name, + reason: "Multiple badges on the same Learning path cannot have overlapping skills. Both badge [${badge.badgeGraphNode.name}] and [${badgeOnPath.badgeGraphNode.name}] badge have [${found.name}] skill.") + } } } return new DependencyCheckResult() } + private Set constructAllItemIdsOnFinalLearningPath(List edgePairs, SkillInfo skillInfo, SkillInfo prereqSkillInfo) { + Set res = edgePairs.collect { + ["${it.node.projectId}-${it.node.skillId}".toString(), "${it.prerequisite.projectId}-${it.prerequisite.skillId}".toString()] + }.flatten().toSet() + res.add("${skillInfo.projectId}-${skillInfo.skillId}".toString()) + res.add("${prereqSkillInfo.projectId}-${prereqSkillInfo.skillId}".toString()) + return res + } + private boolean isBadgeOnCurrentLearningPath(BadgeAndSkills badge) { + boolean isBadgeOnPath = allItemIdsOnFinalLearningPath.contains("${badge.badgeGraphNode.projectId}-${badge.badgeGraphNode.skillId}".toString()) + return isBadgeOnPath + } + private void collectParentBadges(SkillInfo start, List badges, int currentIteration) { if (currentIteration > 1000) { throw new IllegalStateException("Number of [1000] iterations exceeded when checking for circular dependency for [${start.skillId}]") diff --git a/service/src/test/java/skills/intTests/dependentSkills/LearningPathValidationEndpointSpecs.groovy b/service/src/test/java/skills/intTests/dependentSkills/LearningPathValidationEndpointSpecs.groovy index 10525a2bdc..88017fc6c8 100644 --- a/service/src/test/java/skills/intTests/dependentSkills/LearningPathValidationEndpointSpecs.groovy +++ b/service/src/test/java/skills/intTests/dependentSkills/LearningPathValidationEndpointSpecs.groovy @@ -742,4 +742,29 @@ class LearningPathValidationEndpointSpecs extends DefaultIntSpec { !result.violatingSkillName result.reason == "Discovered circular prerequisite [Skill:skill1 -> Skill:skill3 -> Badge:badge1(Skill:skill1)]" } + + def "skills can be belong to multiple badges"() { + def p1 = createProject(1) + def p1subj1 = createSubject(1, 1) + def p1Skills = createSkills(4, 1, 1, 100) + skillsService.createProjectAndSubjectAndSkills(p1, p1subj1, p1Skills) + + def badge1 = SkillsFactory.createBadge(1, 1) + skillsService.createBadge(badge1) + skillsService.assignSkillToBadge([projectId: p1.projectId, badgeId: badge1.badgeId, skillId: p1Skills[2].skillId]) + skillsService.assignSkillToBadge([projectId: p1.projectId, badgeId: badge1.badgeId, skillId: p1Skills[3].skillId]) + + def badge2 = SkillsFactory.createBadge(1, 2) + skillsService.createBadge(badge2) + skillsService.assignSkillToBadge([projectId: p1.projectId, badgeId: badge2.badgeId, skillId: p1Skills[0].skillId]) + skillsService.assignSkillToBadge([projectId: p1.projectId, badgeId: badge2.badgeId, skillId: p1Skills[3].skillId]) + + skillsService.addLearningPathPrerequisite(p1.projectId, p1Skills[3].skillId, p1Skills[2].skillId) + + when: + def result = skillsService.vadlidateLearningPathPrerequisite(p1.projectId, p1Skills[1].skillId, p1.projectId, p1Skills[0].skillId) + + then: + result.possible == true + } } From cb8d3d3b2afbd8150192aa370dd4084a8f738e66 Mon Sep 17 00:00:00 2001 From: "EVOFORGE\\dimay" Date: Mon, 6 Nov 2023 07:38:42 -0500 Subject: [PATCH 2/3] Revert "change poms to match master" This reverts commit 949b1a5876817bbb2637e1669bda5f9da0251058. --- client-display/pom.xml | 2 +- dashboard/pom.xml | 2 +- pom.xml | 2 +- service/pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client-display/pom.xml b/client-display/pom.xml index 5085340f27..e7cec46b7f 100644 --- a/client-display/pom.xml +++ b/client-display/pom.xml @@ -5,7 +5,7 @@ skills-service-parent skill-tree - 2.12.0-SNAPSHOT + 2.11.2-SNAPSHOT 4.0.0 diff --git a/dashboard/pom.xml b/dashboard/pom.xml index 77fb78e757..95bc083646 100644 --- a/dashboard/pom.xml +++ b/dashboard/pom.xml @@ -5,7 +5,7 @@ skills-service-parent skill-tree - 2.12.0-SNAPSHOT + 2.11.2-SNAPSHOT 4.0.0 diff --git a/pom.xml b/pom.xml index 9ed8a1a2a4..3eb12fee58 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ skill-tree skills-service-parent pom - 2.12.0-SNAPSHOT + 2.11.2-SNAPSHOT dashboard client-display diff --git a/service/pom.xml b/service/pom.xml index 36c914a28f..3bf334e498 100644 --- a/service/pom.xml +++ b/service/pom.xml @@ -5,7 +5,7 @@ skills-service-parent skill-tree - 2.12.0-SNAPSHOT + 2.11.2-SNAPSHOT 4.0.0 From 4ee02f6ad5c6a18b728785ad6058fabf02afd8fd Mon Sep 17 00:00:00 2001 From: "EVOFORGE\\dimay" Date: Mon, 6 Nov 2023 10:45:21 -0500 Subject: [PATCH 3/3] 2.11.X -> master merge --- client-display/pom.xml | 2 +- dashboard/pom.xml | 2 +- pom.xml | 2 +- service/pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client-display/pom.xml b/client-display/pom.xml index e7cec46b7f..5085340f27 100644 --- a/client-display/pom.xml +++ b/client-display/pom.xml @@ -5,7 +5,7 @@ skills-service-parent skill-tree - 2.11.2-SNAPSHOT + 2.12.0-SNAPSHOT 4.0.0 diff --git a/dashboard/pom.xml b/dashboard/pom.xml index 95bc083646..77fb78e757 100644 --- a/dashboard/pom.xml +++ b/dashboard/pom.xml @@ -5,7 +5,7 @@ skills-service-parent skill-tree - 2.11.2-SNAPSHOT + 2.12.0-SNAPSHOT 4.0.0 diff --git a/pom.xml b/pom.xml index 3eb12fee58..9ed8a1a2a4 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ skill-tree skills-service-parent pom - 2.11.2-SNAPSHOT + 2.12.0-SNAPSHOT dashboard client-display diff --git a/service/pom.xml b/service/pom.xml index 3bf334e498..36c914a28f 100644 --- a/service/pom.xml +++ b/service/pom.xml @@ -5,7 +5,7 @@ skills-service-parent skill-tree - 2.11.2-SNAPSHOT + 2.12.0-SNAPSHOT 4.0.0