From 5e89e562541d44f62b321405565ff29f2131b073 Mon Sep 17 00:00:00 2001 From: Linus Date: Mon, 21 Sep 2026 15:17:28 +0200 Subject: [PATCH 1/4] Let the team-mode buddy edit the onboarding paths of a project's members Adds the structure half of the content area (backend#228): get_member_path, add/update/delete for phases, steps, tasks and resources, and reset_member_path. A path belongs to a person, not a project, and the by-id services load an element without asking whose it is. PathElements walks each kind up to its owner and ContentScope lets it through only when the owner is on the turn's project, at proposal and again at confirm. An element that is missing and one on somebody else's path get the same refusal, so a refusal cannot be used to probe for ids. Team mode never sets a hire's progress. update_task carries `finished` through as the task has it at confirm time, there is no tool that starts, finishes or ticks anything, and the two places where a write does touch progress say so in the preview: adding a task to a finished step reopens it, and deleting a phase, step or path names the finished work that goes with it. Updates store only the fields that change and apply them over the element as it is at confirm, so an edit made since the preview is not undone. Resource URLs must be http(s), since a hire clicks them. A member on several projects has one path; every preview says so. Co-Authored-By: Claude Sonnet 5 --- .../onboarding/service/ContentScope.kt | 93 +++++ .../onboarding/service/ContentTeamTools.kt | 92 +++++ .../onboarding/service/PathElements.kt | 214 +++++++++++ .../onboarding/service/PhaseTeamActions.kt | 228 ++++++++++++ .../service/ResetMemberPathAction.kt | 77 ++++ .../onboarding/service/ResourceTeamActions.kt | 208 +++++++++++ .../onboarding/service/StepTeamActions.kt | 292 +++++++++++++++ .../onboarding/service/TaskTeamActions.kt | 226 ++++++++++++ .../onboarding/service/ToolFields.kt | 95 +++++ .../onboarding/service/ContentFixture.kt | 111 ++++++ .../service/ContentTeamToolsTest.kt | 134 +++++++ .../onboarding/service/PathElementsTest.kt | 207 +++++++++++ .../service/PhaseTeamActionsTest.kt | 344 ++++++++++++++++++ .../service/ResetMemberPathActionTest.kt | 93 +++++ .../service/ResourceTeamActionsTest.kt | 159 ++++++++ .../onboarding/service/StepTeamActionsTest.kt | 283 ++++++++++++++ .../onboarding/service/TaskTeamActionsTest.kt | 199 ++++++++++ 17 files changed, 3055 insertions(+) create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElements.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActions.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResetMemberPathAction.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActions.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActions.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActions.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ToolFields.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElementsTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActionsTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResetMemberPathActionTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActionsTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActionsTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActionsTest.kt diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt new file mode 100644 index 00000000..035a0c61 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt @@ -0,0 +1,93 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.user.external.ProjectMember +import com.sprintstart.sprintstartbackend.user.external.ProjectMembershipApi +import com.sprintstart.sprintstartbackend.user.external.UserApi +import org.springframework.stereotype.Component +import java.util.UUID + +/** An element of somebody's path that belongs to a member of the turn's project. */ +data class ScopedElement( + val element: PathElement, + val owner: ProjectMember, +) + +/** + * Which onboarding content a project's manager may touch: the paths of the project's members. + * + * Paths belong to people, not projects — a path has one owner and no project — so an element is in + * scope exactly when its owner is a member of the turn's project. Every content tool asks this, the + * reads and the actions alike, and asks it again when a stored proposal is confirmed, so what a + * manager is shown and what they may change cannot disagree. + * + * The other half of the same fact is that a member of several projects has *one* path. Changing it + * changes it for the other projects' managers too; [alsoOn] is what lets a preview say so. + */ +@Component +class ContentScope( + private val pathElements: PathElements, + private val projectMembershipApi: ProjectMembershipApi, + private val userApi: UserApi, +) { + /** + * The [kind] element [id] names, if it sits on the path of a member of [projectId]. + * + * Null for everything else — no such element, or one on somebody else's path — and deliberately + * not distinguishable, so the model cannot use a refusal to probe for ids elsewhere. + */ + fun element(kind: PathElementKind, id: UUID?, projectId: UUID): ScopedElement? { + val found = id?.let { pathElements.find(kind, it) } ?: return null + val owner = member(found.ownerId, projectId) ?: return null + return ScopedElement(found, owner) + } + + /** The refusal for a stored proposal whose target is gone or out of scope since, or null when it is fine. */ + fun missing(kind: PathElementKind, id: UUID?, projectId: UUID): String? = + goneSince(kind).takeIf { element(kind, id, projectId) == null } + + /** The member [memberId] names on [projectId], or null. */ + fun member(memberId: UUID?, projectId: UUID): ProjectMember? = + memberId?.let { id -> projectMembershipApi.getProjectMembers(projectId).firstOrNull { it.userId == id } } + + /** The names of the other projects [userId] is on, sorted; empty when they are on this one only. */ + fun alsoOn(userId: UUID, projectId: UUID): List = + userApi + .getUsersByIds(listOf(userId)) + .firstOrNull() + ?.projects + .orEmpty() + .filter { it.projectId != projectId } + .map { it.name } + .sorted() + + /** + * The sentence a preview adds when the path it changes also belongs to other projects, or empty. + * + * Said whatever the change is: nothing about a person's path is project-scoped, so there is no + * change that stays here. + */ + fun sharedNote(owner: ProjectMember, projectId: UUID): String { + val others = alsoOn(owner.userId, projectId) + return if (others.isEmpty()) { + "" + } else { + "${owner.displayName} is also on ${others.joinToString(", ")}, and has one onboarding path for " + + "all of their projects — so this changes it there too." + } + } +} + +internal const val NOT_A_MEMBER_HERE = + "That person is not on this project. Call find_member for the people who are, and pass the member_id " + + "it gives." + +internal const val LEFT_SINCE_HERE = "That person is no longer on this project, so nothing was changed." + +/** What to tell the model when an id is not in scope, per kind. */ +internal fun notInScope(kind: PathElementKind): String = + "That ${kind.noun} is not on the onboarding path of anybody on this project. Call get_member_path for " + + "somebody who is, and pass an id from it." + +/** Why a stored proposal's target is no longer there: gone, or no longer on a member's path. */ +internal fun goneSince(kind: PathElementKind): String = + "That ${kind.noun} is gone, or no longer on the path of anybody on this project, so nothing was changed." diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt new file mode 100644 index 00000000..2ae0a5f4 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt @@ -0,0 +1,92 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import org.springframework.stereotype.Component +import org.springframework.web.server.ResponseStatusException +import java.util.UUID + +/** + * The read tools of team mode's content area: what is on the onboarding paths of the project's + * members. + * + * Paths belong to people, not projects, so everything here starts from a member of the turn's project + * and reads only what is on that member's path. The ids these print are how the content actions name + * their targets, and every action checks again that its target is on a member's path. + */ +@Component +class ContentTeamTools( + private val scope: ContentScope, + private val onboardingPathService: OnboardingPathService, + private val onboardingStepService: OnboardingStepService, +) : TeamAreaTools { + override val area = TeamArea.CONTENT + + override fun toolSpecs(): List = listOf(GET_MEMBER_PATH_SPEC) + + override fun handles(toolName: String): Boolean = toolName == GET_MEMBER_PATH + + override fun execute(call: BuddyToolCallDto, context: TeamToolContext): String = + when (call.name) { + GET_MEMBER_PATH -> memberPath(call.uuidArgument("member_id"), context.projectId) + else -> "Unknown tool: ${call.name}." + } + + /** + * One member's whole path, phases to tasks, each with the id an action needs. + * + * Descriptions are cut short: the ids are what a manager's request is turned into, and the + * full text of every step of a long path would bury them. + */ + private fun memberPath(memberId: UUID?, projectId: UUID): String { + val owner = scope.member(memberId, projectId) ?: return NOT_A_MEMBER_HERE + val path = runCatching { onboardingPathService.getOnboardingPathByUserId(owner.userId) } + .getOrElse { + if (it !is ResponseStatusException) throw it + return "${owner.displayName} has no onboarding path yet." + } + + return buildString { + appendLine("${owner.displayName}'s onboarding path:") + scope.sharedNote(owner, projectId).takeIf { it.isNotEmpty() }?.let { appendLine(it) } + path.phases.sortedBy { it.position }.forEach { phase -> + appendLine() + appendLine("Phase ${phase.position + 1}: ${phase.title} [phase_id: ${phase.id}]") + phase.description.takeIf { it.isNotBlank() }?.let { appendLine(" ${it.short()}") } + onboardingStepService.getOnboardingStepsByPhaseId(phase.id).sortedBy { it.position }.forEach { step -> + appendLine( + " Step ${step.position + 1}: ${step.title} [step_id: ${step.id}] — " + + "${step.type.name.lowercase()}, ${step.estimatedMinutes} min, ${step.status.said()}", + ) + step.description.takeIf { it.isNotBlank() }?.let { appendLine(" ${it.short()}") } + step.tasks.sortedBy { it.position }.forEach { task -> + appendLine(" - [${if (task.finished) "x" else " "}] ${task.title} [task_id: ${task.id}]") + } + step.resources.forEach { resource -> + appendLine(" - link: ${resource.title} <${resource.url}> [resource_id: ${resource.id}]") + } + } + } + }.trim() + } + + private fun StepStatus.said(): String = name.lowercase().replace('_', ' ') + + private fun String.short(): String = if (length <= SHORT_CHARS) this else take(SHORT_CHARS - 1).trimEnd() + "…" + + companion object { + const val GET_MEMBER_PATH = "get_member_path" + + private const val SHORT_CHARS = 200 + + val GET_MEMBER_PATH_SPEC = BuddyToolSpecDto( + name = GET_MEMBER_PATH, + description = "One project member's whole onboarding path: its phases, their steps, and each " + + "step's tasks and links, every one with the id an action needs. Also says where each step " + + "stands for them. Use the member_id from find_member. A path belongs to the person, not to " + + "this project, so it shows what they have across all of theirs.", + parameters = stringFields("member_id" to "The member_id from find_member.", required = listOf("member_id")), + ) + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElements.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElements.kt new file mode 100644 index 00000000..4c1de495 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElements.kt @@ -0,0 +1,214 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingStep +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingFeedbackRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingPathRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingPhaseRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingResourceRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingSkipRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingStepRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingTaskRepository +import org.springframework.stereotype.Component +import org.springframework.transaction.annotation.Transactional +import java.util.UUID + +/** The kinds of thing on an onboarding path that the content area can point at. */ +enum class PathElementKind( + val noun: String, +) { + PHASE("phase"), + STEP("step"), + TASK("task"), + RESOURCE("resource"), + SKIP("skip request"), + FEEDBACK("feedback"), +} + +/** + * One thing on somebody's onboarding path, as far as a preview needs to know about it. + * + * @property ownerId The person whose path it is. Paths belong to people, not projects, so this is + * what decides whether a project's manager may touch it. + * @property position Its place among its siblings, or null when it has none. + * @property children How many things sit directly under it, which is what a new child's position is + * validated against. + * @property contains What deleting it takes with it, in words; empty when it holds nothing. + * @property finishedSteps How many steps under it the person already got through, finished or skipped. + * @property stepStatus The status of the step it is or hangs off, or null for a phase. + */ +data class PathElement( + val kind: PathElementKind, + val id: UUID, + val ownerId: UUID, + val title: String, + val position: Int?, + val children: Int, + val contains: String, + val stepStatus: StepStatus?, + val finishedSteps: Int = 0, +) + +/** + * Finds what an id points at on an onboarding path, and whose path it is. + * + * The by-id services behind the content actions load an element without asking whose it is. This + * is the one place that walks an element up to its owner, so the check that decides whether a + * manager may touch it is the same one for every kind. + * + * Read-only transactions, because every hop up is a lazy association. + */ +@Component +class PathElements( + private val pathRepository: OnboardingPathRepository, + private val phaseRepository: OnboardingPhaseRepository, + private val stepRepository: OnboardingStepRepository, + private val taskRepository: OnboardingTaskRepository, + private val resourceRepository: OnboardingResourceRepository, + private val skipRepository: OnboardingSkipRepository, + private val feedbackRepository: OnboardingFeedbackRepository, +) { + /** The element of [kind] that [id] names, or null when there is none. */ + @Transactional(readOnly = true) + fun find(kind: PathElementKind, id: UUID): PathElement? = + when (kind) { + PathElementKind.PHASE -> phase(id) + PathElementKind.STEP -> step(id) + PathElementKind.TASK -> task(id) + PathElementKind.RESOURCE -> resource(id) + PathElementKind.SKIP -> skip(id) + PathElementKind.FEEDBACK -> feedback(id) + } + + /** A summary of somebody's whole path, for a reset; null when they have none. */ + @Transactional(readOnly = true) + fun pathOf(userId: UUID): PathSummary? { + val path = pathRepository.findByUserId(userId).orElse(null) ?: return null + val steps = path.phases.flatMap { it.steps } + return PathSummary( + phases = path.phases.size, + steps = steps.size, + finishedSteps = steps.count { it.status.isDone() }, + checks = path.phases.sumOf { it.checkQuestions.size }, + ) + } + + private fun phase(id: UUID): PathElement? = + phaseRepository.findById(id).orElse(null)?.let { phase -> + PathElement( + kind = PathElementKind.PHASE, + id = phase.id, + ownerId = phase.path.userId, + title = phase.title, + position = phase.position, + children = phase.steps.size, + contains = containedIn(phase.steps, phase.checkQuestions.size), + stepStatus = null, + finishedSteps = phase.steps.count { it.status.isDone() }, + ) + } + + private fun step(id: UUID): PathElement? = + stepRepository.findById(id).orElse(null)?.let { step -> + PathElement( + kind = PathElementKind.STEP, + id = step.id, + ownerId = step.phase.path.userId, + title = step.title, + position = step.position, + children = step.tasks.size, + contains = containedIn(listOf(step), checks = 0, includeSteps = false), + stepStatus = step.status, + finishedSteps = if (step.status.isDone()) 1 else 0, + ) + } + + private fun task(id: UUID): PathElement? = + taskRepository.findById(id).orElse(null)?.let { task -> + PathElement( + kind = PathElementKind.TASK, + id = task.id, + ownerId = task.step.phase.path.userId, + title = task.title, + position = task.position, + children = 0, + contains = "", + stepStatus = task.step.status, + ) + } + + private fun resource(id: UUID): PathElement? = + resourceRepository.findById(id).orElse(null)?.let { resource -> + PathElement( + kind = PathElementKind.RESOURCE, + id = resource.id, + ownerId = resource.step.phase.path.userId, + title = resource.title, + position = null, + children = 0, + contains = "", + stepStatus = resource.step.status, + ) + } + + private fun skip(id: UUID): PathElement? = + skipRepository.findById(id).orElse(null)?.let { skip -> + PathElement( + kind = PathElementKind.SKIP, + id = skip.id, + ownerId = skip.step.phase.path.userId, + title = skip.step.title, + position = null, + children = 0, + contains = "", + stepStatus = skip.step.status, + ) + } + + private fun feedback(id: UUID): PathElement? = + feedbackRepository.findById(id).orElse(null)?.let { feedback -> + PathElement( + kind = PathElementKind.FEEDBACK, + id = feedback.id, + ownerId = feedback.userId, + title = feedback.step?.title ?: "the path as a whole", + position = null, + children = 0, + contains = "", + stepStatus = feedback.step?.status, + ) + } + + /** What removing [steps] removes beneath them, in words, or empty when it is nothing. */ + private fun containedIn(steps: List, checks: Int, includeSteps: Boolean = true): String { + val parts = buildList { + if (includeSteps && steps.isNotEmpty()) add(counted(steps.size, "step")) + steps.sumOf { it.tasks.size }.takeIf { it > 0 }?.let { add(counted(it, "task")) } + steps.sumOf { it.resources.size }.takeIf { it > 0 }?.let { add(counted(it, "resource")) } + steps.sumOf { it.skips.size }.takeIf { it > 0 }?.let { add(counted(it, "skip request")) } + steps.sumOf { it.feedback.size }.takeIf { it > 0 }?.let { + add(counted(it, "piece of feedback", "pieces of feedback")) + } + if (checks > 0) add(counted(checks, "knowledge-check question")) + } + return when (parts.size) { + 0 -> "" + 1 -> parts.single() + else -> parts.dropLast(1).joinToString(", ") + " and " + parts.last() + } + } + + private fun counted(count: Int, singular: String, plural: String = "${singular}s"): String = + "$count ${if (count == 1) singular else plural}" +} + +/** A step counts as done once it is finished or skipped; that is what the hire's progress is made of. */ +internal fun StepStatus.isDone(): Boolean = this == StepStatus.FINISHED || this == StepStatus.SKIPPED + +/** How much a reset would remove, and how much of it the hire already got through. */ +data class PathSummary( + val phases: Int, + val steps: Int, + val finishedSteps: Int, + val checks: Int, +) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActions.kt new file mode 100644 index 00000000..5398c70d --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActions.kt @@ -0,0 +1,228 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import com.sprintstart.sprintstartbackend.onboarding.model.request.phase.CreateOnboardingPhaseRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.phase.UpdateOnboardingPhaseRequest +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.springframework.stereotype.Component + +/* + * The phase actions of team mode's content area. + * + * A phase hangs off a person's path, and the service behind these loads it by id without asking whose + * it is. So every action resolves its target through ContentScope — the path's owner must be on the + * turn's project — when drafting and again when the manager confirms. + */ + +private const val NO_PATH_YET = + "That person has no onboarding path yet, so there is nothing to add a phase to. Their path is made " + + "when they start onboarding; it cannot be started from here." + +/** Offers to add a phase to a member's path. */ +@Component +class AddPhaseAction( + private val scope: ContentScope, + private val pathElements: PathElements, + private val onboardingPhaseService: OnboardingPhaseService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "add_phase", + description = "Offer to add a phase to a member's onboarding path. Use the member_id from find_member. " + + "Omit place to add it at the end. The phase starts empty; steps are added to it afterwards. This " + + "does NOT add anything by itself; the manager confirms.", + parameters = toolFields( + ToolField("member_id", "The member_id from find_member."), + ToolField("title", "The phase's title."), + ToolField("description", "What the phase is for, in a sentence or two."), + ToolField("place", "Where in the path it goes, counting from 1. Omitted means the end.", "integer"), + required = listOf("member_id", "title"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val owner = scope.member(call.uuidArgument("member_id"), context.projectId) + ?: return TeamActionDraft.Refused(NOT_A_MEMBER_HERE) + val title = call.textArgument("title") + if (title.isEmpty()) return TeamActionDraft.Refused("A phase needs a title.") + val phases = pathElements.pathOf(owner.userId)?.phases ?: return TeamActionDraft.Refused(NO_PATH_YET) + + val placeText = call.textArgument("place") + val position = if (placeText.isEmpty()) phases else placeToPosition(placeText, phases + 1) + if (position == null) return TeamActionDraft.Refused("Place must be a number from 1 to ${phases + 1}.") + val description = call.textArgument("description") + + return TeamActionDraft.Proposed( + params = buildJsonObject { + put("member_id", owner.userId.toString()) + put("title", title) + put("description", description) + put("position", position) + }, + label = "Add phase “${title.forLabel()}” for ${owner.displayName.forLabel()}", + preview = buildString { + appendLine("Add a phase to ${owner.displayName}'s onboarding path:") + appendLine("“$title” — ${placeOf(position, phases + 1)}") + if (description.isNotEmpty()) appendLine(description) + appendLine() + if (position < phases) appendLine("The phases after it move down one place.") + append("It starts with no steps.") + scope.sharedNote(owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? { + val owner = scope.member(params.uuid("member_id"), context.projectId) ?: return LEFT_SINCE_HERE + val phases = pathElements.pathOf(owner.userId)?.phases ?: return NO_PATH_YET + val position = params.text("position").toIntOrNull() ?: return "That offer is malformed." + return "The path has fewer phases than it had — that place is gone — so nothing was changed. Offer it again." + .takeIf { position !in 0..phases } + } + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingPhaseService.createOnboardingPhaseForUserId( + requireNotNull(params.uuid("member_id")), + CreateOnboardingPhaseRequest( + position = requireNotNull(params.text("position").toIntOrNull()), + title = params.text("title"), + description = params.text("description"), + ), + ) + return "Added. “${params.text("title")}” is on the path now, with no steps yet." + } +} + +/** Offers to rename, redescribe or move a phase. */ +@Component +class UpdatePhaseAction( + private val scope: ContentScope, + private val onboardingPhaseService: OnboardingPhaseService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "update_phase", + description = "Offer to change a phase on a member's onboarding path: its title, its description, or " + + "where it sits. Pass only what should change. Use the phase_id from get_member_path. This does " + + "NOT change anything by itself; the manager confirms.", + parameters = toolFields( + ToolField("phase_id", "The phase_id from get_member_path."), + ToolField("title", "The new title."), + ToolField("description", "The new description."), + ToolField("place", "The new place in the path, counting from 1.", "integer"), + required = listOf("phase_id"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.PHASE, call.uuidArgument("phase_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.PHASE)) + val current = onboardingPhaseService.getOnboardingPhaseById(target.element.id) + val siblings = onboardingPhaseService.getOnboardingPhasesForUser(target.owner.userId).size + + val placeText = call.textArgument("place") + val requested = if (placeText.isEmpty()) current.position else placeToPosition(placeText, siblings) + if (requested == null) return TeamActionDraft.Refused("Place must be a number from 1 to $siblings.") + + val changes = ChangeSet("phase_id", target.element.id) + changes.add("title", call.changedText("title", current.title, allowBlank = false)) { + "Title: “${current.title}” becomes “$it”" + } + changes.add("description", call.changedText("description", current.description)) { + "Description becomes: ${it.ifEmpty { "(empty)" }}" + } + changes.add("position", requested.takeIf { it != current.position }?.toString()) { + "Place: ${placeOf(current.position, siblings)} becomes ${placeOf(it.toInt(), siblings)}\n" + + "The phases in between move by one place." + } + if (changes.isEmpty) return TeamActionDraft.Refused("Nothing would change: that is already how the phase is.") + + return TeamActionDraft.Proposed( + params = changes.params(), + label = "Change phase “${current.title.forLabel()}”", + preview = "Change the phase “${current.title}” on ${target.owner.displayName}'s path:\n" + + changes.preview() + + scope.sharedNote(target.owner, context.projectId).let { if (it.isEmpty()) "" else "\n\n$it" }, + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + scope.missing(PathElementKind.PHASE, params.uuid("phase_id"), context.projectId) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + val id = requireNotNull(params.uuid("phase_id")) + // Only what the manager saw is written. Everything else is read fresh, so a change made to the + // other fields since the preview is not undone by a confirm. + val current = onboardingPhaseService.getOnboardingPhaseById(id) + onboardingPhaseService.updateOnboardingPhaseById( + id, + UpdateOnboardingPhaseRequest( + position = params.text("position").toIntOrNull() ?: current.position, + title = params.text("title").ifEmpty { current.title }, + description = if (params.containsKey( + "description", + ) + ) { + params.text("description") + } else { + current.description + }, + ), + ) + return "Done. The phase is updated." + } +} + +/** Offers to delete a phase, with everything in it. */ +@Component +class DeletePhaseAction( + private val scope: ContentScope, + private val onboardingPhaseService: OnboardingPhaseService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.DESTRUCTIVE + override val spec = BuddyToolSpecDto( + name = "delete_phase", + description = "Offer to delete a phase from a member's onboarding path, with its steps, tasks, " + + "resources and knowledge checks. Use the phase_id from get_member_path. This does NOT delete " + + "anything by itself; the manager confirms.", + parameters = stringFields("phase_id" to "The phase_id from get_member_path.", required = listOf("phase_id")), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.PHASE, call.uuidArgument("phase_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.PHASE)) + val phase = target.element + + return TeamActionDraft.Proposed( + params = buildJsonObject { put("phase_id", phase.id.toString()) }, + label = "Delete phase “${phase.title.forLabel()}”", + preview = buildString { + appendLine("Delete the phase “${phase.title}” from ${target.owner.displayName}'s onboarding path.") + if (phase.contains.isNotEmpty()) appendLine("It takes ${phase.contains} with it.") + if (phase.finishedSteps > 0) { + appendLine( + "${target.owner.displayName} has already got through ${phase.finishedSteps} of its steps; " + + "that progress is deleted with them.", + ) + } + append("The phases after it move up one place. This cannot be undone.") + scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + scope.missing(PathElementKind.PHASE, params.uuid("phase_id"), context.projectId) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingPhaseService.deleteOnboardingPhaseById(requireNotNull(params.uuid("phase_id"))) + return "Deleted. The phase and everything in it are gone." + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResetMemberPathAction.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResetMemberPathAction.kt new file mode 100644 index 00000000..192aaf09 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResetMemberPathAction.kt @@ -0,0 +1,77 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.springframework.stereotype.Component + +/** + * Offers to delete a member's whole onboarding path. + * + * The largest single thing the content area can do, and the one that throws the most of a person's + * work away — so the preview says how much of it there is, in numbers, rather than leaving the + * manager to imagine it. + */ +@Component +class ResetMemberPathAction( + private val scope: ContentScope, + private val pathElements: PathElements, + private val onboardingPathService: OnboardingPathService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.DESTRUCTIVE + override val spec = BuddyToolSpecDto( + name = "reset_member_path", + description = "Offer to delete a member's whole onboarding path — every phase, step, task and link, " + + "and everything they got through. It is the biggest thing here and cannot be undone; prefer " + + "deleting the one phase or step that is wrong. Use the member_id from find_member. This does NOT " + + "delete anything by itself; the manager confirms.", + parameters = stringFields("member_id" to "The member_id from find_member.", required = listOf("member_id")), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val owner = scope.member(call.uuidArgument("member_id"), context.projectId) + ?: return TeamActionDraft.Refused(NOT_A_MEMBER_HERE) + val summary = pathElements.pathOf(owner.userId) + ?: return TeamActionDraft.Refused( + "${owner.displayName} has no onboarding path, so there is nothing to delete.", + ) + + return TeamActionDraft.Proposed( + params = buildJsonObject { put("member_id", owner.userId.toString()) }, + label = "Delete ${owner.displayName.forLabel()}'s whole path", + preview = buildString { + appendLine("Delete ${owner.displayName}'s whole onboarding path.") + appendLine() + append( + "That is ${summary.phases} phase${if (summary.phases == 1) "" else "s"} and ${summary.steps} step", + ) + append(if (summary.steps == 1) "" else "s") + appendLine(", with their tasks, links, skip requests and feedback.") + if (summary.checks > 0) appendLine("${summary.checks} knowledge-check questions go too.") + if (summary.finishedSteps > 0) { + appendLine( + "${owner.displayName} has already got through ${summary.finishedSteps} of those steps; " + + "that progress is deleted with them.", + ) + } + append("Nothing here makes a new path. This cannot be undone.") + scope.sharedNote(owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? { + val owner = scope.member(params.uuid("member_id"), context.projectId) ?: return LEFT_SINCE_HERE + return "That person's path is already gone, so nothing was changed." + .takeIf { pathElements.pathOf(owner.userId) == null } + } + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingPathService.deleteOnboardingPathByUserId(requireNotNull(params.uuid("member_id"))) + return "Deleted. The path and everything on it are gone." + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActions.kt new file mode 100644 index 00000000..ef6c5871 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActions.kt @@ -0,0 +1,208 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import com.sprintstart.sprintstartbackend.onboarding.model.request.resource.CreateOnboardingResourceRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.resource.UpdateOnboardingResourceRequest +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.springframework.stereotype.Component +import java.net.URI + +/* + * The resource actions of team mode's content area. + * + * A resource is a link a person opens from their step. The services store whatever URL they are given, + * which is fine behind an admin form and not fine behind a model: a link the hire will click has to + * be a web link, so these refuse anything else at proposal. + */ + +private const val NOT_A_WEB_LINK = "The url must be a full web address starting with http:// or https://." + +/** Whether [text] is an absolute http(s) address with a host. */ +private fun isWebLink(text: String): Boolean = + runCatching { URI(text) }.getOrNull()?.let { + (it.scheme == "http" || it.scheme == "https") && !it.host.isNullOrBlank() + } ?: false + +/** Offers to attach a link to a step of a member's path. */ +@Component +class AddResourceAction( + private val scope: ContentScope, + private val onboardingResourceService: OnboardingResourceService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "add_resource", + description = "Offer to attach a link to a step of a member's onboarding path. Use the step_id from " + + "get_member_path. This does NOT add anything by itself; the manager confirms.", + parameters = toolFields( + ToolField("step_id", "The step_id from get_member_path."), + ToolField("title", "What the link is called."), + ToolField("url", "The full web address, starting with http:// or https://."), + ToolField("description", "What the person will find there."), + required = listOf("step_id", "title", "url"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.STEP, call.uuidArgument("step_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.STEP)) + val title = call.textArgument("title") + if (title.isEmpty()) return TeamActionDraft.Refused("A resource needs a title.") + val url = call.textArgument("url") + if (!isWebLink(url)) return TeamActionDraft.Refused(NOT_A_WEB_LINK) + val description = call.textArgument("description") + + return TeamActionDraft.Proposed( + params = buildJsonObject { + put("step_id", target.element.id.toString()) + put("title", title) + put("url", url) + put("description", description) + }, + label = "Add link “${title.forLabel()}”", + preview = buildString { + appendLine("Attach a link to “${target.element.title}” on ${target.owner.displayName}'s path:") + appendLine("“$title” — $url") + if (description.isNotEmpty()) appendLine(description) + scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + scope.missing(PathElementKind.STEP, params.uuid("step_id"), context.projectId) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingResourceService.createOnboardingResourceForStepId( + requireNotNull(params.uuid("step_id")), + CreateOnboardingResourceRequest( + title = params.text("title"), + description = params.text("description"), + url = params.text("url"), + ), + ) + return "Added. “${params.text("title")}” is on the step now." + } +} + +/** Offers to change a link on a member's path. */ +@Component +class UpdateResourceAction( + private val scope: ContentScope, + private val onboardingResourceService: OnboardingResourceService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "update_resource", + description = "Offer to change a link on a member's onboarding path: its title, its address or its " + + "description. Pass only what should change. Use the resource_id from get_member_path. This does " + + "NOT change anything by itself; the manager confirms.", + parameters = toolFields( + ToolField("resource_id", "The resource_id from get_member_path."), + ToolField("title", "The new title."), + ToolField("url", "The new full web address, starting with http:// or https://."), + ToolField("description", "The new description."), + required = listOf("resource_id"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.RESOURCE, call.uuidArgument("resource_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.RESOURCE)) + val current = onboardingResourceService.getOnboardingResourceById(target.element.id) + + val url = call.changedText("url", current.url, allowBlank = false) + if (url != null && !isWebLink(url)) return TeamActionDraft.Refused(NOT_A_WEB_LINK) + + val changes = ChangeSet("resource_id", current.id) + changes.add("title", call.changedText("title", current.title, allowBlank = false)) { + "Title: “${current.title}” becomes “$it”" + } + changes.add("url", url) { "Address: ${current.url} becomes $it" } + changes.add("description", call.changedText("description", current.description)) { + "Description becomes: ${it.ifEmpty { "(empty)" }}" + } + if (changes.isEmpty) return TeamActionDraft.Refused("Nothing would change: that is already how the link is.") + + return TeamActionDraft.Proposed( + params = changes.params(), + label = "Change link “${current.title.forLabel()}”", + preview = "Change the link “${current.title}” on ${target.owner.displayName}'s path:\n" + + changes.preview() + + scope.sharedNote(target.owner, context.projectId).let { if (it.isEmpty()) "" else "\n\n$it" }, + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + scope.missing(PathElementKind.RESOURCE, params.uuid("resource_id"), context.projectId) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + val id = requireNotNull(params.uuid("resource_id")) + val current = onboardingResourceService.getOnboardingResourceById(id) + onboardingResourceService.updateOnboardingResourceById( + id, + UpdateOnboardingResourceRequest( + title = params.text("title").ifEmpty { current.title }, + description = if (params.containsKey( + "description", + ) + ) { + params.text("description") + } else { + current.description + }, + url = params.text("url").ifEmpty { current.url }, + ), + ) + return "Done. The link is updated." + } +} + +/** Offers to remove a link from a member's path. */ +@Component +class DeleteResourceAction( + private val scope: ContentScope, + private val onboardingResourceService: OnboardingResourceService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.DESTRUCTIVE + override val spec = BuddyToolSpecDto( + name = "delete_resource", + description = "Offer to remove a link from a member's onboarding path. Use the resource_id from " + + "get_member_path. This does NOT remove anything by itself; the manager confirms.", + parameters = stringFields( + "resource_id" to "The resource_id from get_member_path.", + required = listOf("resource_id"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.RESOURCE, call.uuidArgument("resource_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.RESOURCE)) + val resource = target.element + + return TeamActionDraft.Proposed( + params = buildJsonObject { put("resource_id", resource.id.toString()) }, + label = "Remove link “${resource.title.forLabel()}”", + preview = buildString { + append("Remove the link “${resource.title}” from ${target.owner.displayName}'s onboarding path. ") + append("This cannot be undone.") + scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + scope.missing(PathElementKind.RESOURCE, params.uuid("resource_id"), context.projectId) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingResourceService.deleteOnboardingResourceById(requireNotNull(params.uuid("resource_id"))) + return "Removed. The link is gone." + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActions.kt new file mode 100644 index 00000000..73c7c5c5 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActions.kt @@ -0,0 +1,292 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepType +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import com.sprintstart.sprintstartbackend.onboarding.model.request.step.CreateOnboardingStepRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.step.UpdateOnboardingStepRequest +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.springframework.stereotype.Component + +/* + * The step actions of team mode's content area. + * + * Steps are where a person's progress lives: a step is waiting, in progress, finished or skipped. + * None of these actions sets that. A new step starts waiting, an edit leaves the status alone, and + * a delete says in its preview what it throws away. + */ + +private val STEP_TYPES = StepType.entries.map { it.name } + +private const val DEFAULT_MINUTES = 15 + +private fun stepTypeOf(text: String): StepType? = StepType.entries.firstOrNull { + it.name.equals(text, ignoreCase = true) +} + +/** What a preview says about the step's status when it matters to the person. */ +private fun StepStatus?.aboutProgress(name: String): String? = + when (this) { + StepStatus.IN_PROGRESS -> "$name is working on it right now." + StepStatus.FINISHED -> "$name has already finished it; that progress is deleted with it." + StepStatus.SKIPPED -> "$name has already skipped it; that decision is deleted with it." + else -> null + } + +/** Offers to add a step to a phase of a member's path. */ +@Component +class AddStepAction( + private val scope: ContentScope, + private val onboardingStepService: OnboardingStepService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "add_step", + description = "Offer to add a step to a phase of a member's onboarding path. Use the phase_id from " + + "get_member_path. It starts waiting, like every new step; this cannot mark anything done. Omit " + + "place to add it at the end of the phase. This does NOT add anything by itself; the manager confirms.", + parameters = toolFields( + ToolField("phase_id", "The phase_id from get_member_path."), + ToolField("title", "The step's title."), + ToolField("description", "What the person does in this step."), + ToolField("type", "What kind of step it is.", values = STEP_TYPES), + ToolField("estimated_minutes", "How long it should take, in minutes.", "integer"), + ToolField("expected_outcome", "What the person should have when they are done."), + ToolField("place", "Where in the phase it goes, counting from 1. Omitted means the end.", "integer"), + required = listOf("phase_id", "title", "type"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.PHASE, call.uuidArgument("phase_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.PHASE)) + val title = call.textArgument("title") + if (title.isEmpty()) return TeamActionDraft.Refused("A step needs a title.") + val type = stepTypeOf(call.textArgument("type")) + ?: return TeamActionDraft.Refused("Type must be one of ${STEP_TYPES.joinToString(", ")}.") + val minutesText = call.textArgument("estimated_minutes") + val minutes = if (minutesText.isEmpty()) DEFAULT_MINUTES else minutesText.toIntOrNull()?.takeIf { it > 0 } + if (minutes == null) { + return TeamActionDraft.Refused("estimated_minutes must be a whole number of minutes above zero.") + } + + val slots = target.element.children + 1 + val placeText = call.textArgument("place") + val position = if (placeText.isEmpty()) slots - 1 else placeToPosition(placeText, slots) + if (position == null) return TeamActionDraft.Refused("Place must be a number from 1 to $slots.") + val description = call.textArgument("description") + val outcome = call.textArgument("expected_outcome") + + return TeamActionDraft.Proposed( + params = buildJsonObject { + put("phase_id", target.element.id.toString()) + put("title", title) + put("description", description) + put("type", type.name) + put("estimated_minutes", minutes) + put("expected_outcome", outcome) + put("position", position) + }, + label = "Add step “${title.forLabel()}”", + preview = buildString { + appendLine("Add a step to “${target.element.title}” on ${target.owner.displayName}'s path:") + appendLine("“$title” — ${type.name.lowercase()}, about $minutes min, ${placeOf(position, slots)}") + if (description.isNotEmpty()) appendLine(description) + if (outcome.isNotEmpty()) appendLine("Expected outcome: $outcome") + appendLine() + if (position < slots - 1) appendLine("The steps after it move down one place.") + append("It starts waiting; nobody is marked as having started or finished it.") + scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? { + val target = scope.element(PathElementKind.PHASE, params.uuid("phase_id"), context.projectId) + ?: return goneSince(PathElementKind.PHASE) + val position = params.text("position").toIntOrNull() ?: return "That offer is malformed." + return "The phase has fewer steps than it had — that place is gone — so nothing was changed. Offer it again." + .takeIf { position !in 0..target.element.children } + } + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingStepService.createOnboardingStepForPhaseId( + requireNotNull(params.uuid("phase_id")), + CreateOnboardingStepRequest( + position = requireNotNull(params.text("position").toIntOrNull()), + title = params.text("title"), + description = params.text("description"), + type = requireNotNull(stepTypeOf(params.text("type"))), + estimatedMinutes = params.text("estimated_minutes").toIntOrNull() ?: DEFAULT_MINUTES, + expectedOutcome = params.text("expected_outcome"), + ), + ) + return "Added. “${params.text("title")}” is on the path now, waiting." + } +} + +/** Offers to change what a step says, or where it sits. Never its status. */ +@Component +class UpdateStepAction( + private val scope: ContentScope, + private val onboardingStepService: OnboardingStepService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "update_step", + description = "Offer to change a step on a member's onboarding path: what it says, how long it is, " + + "or where it sits in its phase. Pass only what should change. Use the step_id from " + + "get_member_path. This never starts, finishes or skips a step. It does NOT change anything by " + + "itself; the manager confirms.", + parameters = toolFields( + ToolField("step_id", "The step_id from get_member_path."), + ToolField("title", "The new title."), + ToolField("description", "The new description."), + ToolField("type", "The new kind of step.", values = STEP_TYPES), + ToolField("estimated_minutes", "The new length, in minutes.", "integer"), + ToolField("expected_outcome", "The new expected outcome."), + ToolField("place", "The new place in its phase, counting from 1.", "integer"), + required = listOf("step_id"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.STEP, call.uuidArgument("step_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.STEP)) + val current = onboardingStepService.getOnboardingStepById(target.element.id) + val siblings = onboardingStepService.getOnboardingStepsByPhaseId(current.phaseId).size + + badArgument(call, siblings)?.let { return TeamActionDraft.Refused(it) } + + val changes = ChangeSet("step_id", current.id) + changes.add("title", call.changedText("title", current.title, allowBlank = false)) { + "Title: “${current.title}” becomes “$it”" + } + changes.add("description", call.changedText("description", current.description)) { + "Description becomes: ${it.ifEmpty { "(empty)" }}" + } + changes.add("type", stepTypeOf(call.textArgument("type"))?.takeIf { it != current.type }?.name) { + "Type: ${current.type.name.lowercase()} becomes ${it.lowercase()}" + } + val minutes = call.textArgument("estimated_minutes").toIntOrNull()?.takeIf { it != current.estimatedMinutes } + changes.add("estimated_minutes", minutes?.toString()) { + "Length: ${current.estimatedMinutes} min becomes $it min" + } + changes.add( + "expected_outcome", + call.changedText("expected_outcome", current.expectedOutcomes.firstOrNull().orEmpty()), + ) { + "Expected outcome becomes: ${it.ifEmpty { "(empty)" }}" + } + val position = placeToPosition(call.textArgument("place"), siblings)?.takeIf { it != current.position } + changes.add("position", position?.toString()) { + "Place: ${placeOf(current.position, siblings)} becomes ${placeOf(it.toInt(), siblings)}" + } + if (changes.isEmpty) return TeamActionDraft.Refused("Nothing would change: that is already how the step is.") + + return TeamActionDraft.Proposed( + params = changes.params(), + label = "Change step “${current.title.forLabel()}”", + preview = "Change the step “${current.title}” on ${target.owner.displayName}'s path:\n" + + changes.preview() + "\n" + + "Its status stays ${current.status.name.lowercase().replace('_', ' ')}." + + scope.sharedNote(target.owner, context.projectId).let { if (it.isEmpty()) "" else "\n\n$it" }, + ) + } + + /** Why the call's type, length or place cannot be used, or null when each is either absent or fine. */ + private fun badArgument(call: BuddyToolCallDto, slots: Int): String? = + when { + call.textArgument("type").let { it.isNotEmpty() && stepTypeOf(it) == null } -> + "Type must be one of ${STEP_TYPES.joinToString(", ")}." + call.textArgument("estimated_minutes").let { + it.isNotEmpty() && + it.toIntOrNull()?.takeIf { m -> m > 0 } == null + } -> + "estimated_minutes must be a whole number of minutes above zero." + call.textArgument("place").let { it.isNotEmpty() && placeToPosition(it, slots) == null } -> + "Place must be a number from 1 to $slots." + else -> null + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + scope.missing(PathElementKind.STEP, params.uuid("step_id"), context.projectId) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + val id = requireNotNull(params.uuid("step_id")) + // Only what the manager saw is written; the rest is read fresh. + val current = onboardingStepService.getOnboardingStepById(id) + onboardingStepService.updateOnboardingStepById( + id, + UpdateOnboardingStepRequest( + position = params.text("position").toIntOrNull() ?: current.position, + title = params.text("title").ifEmpty { current.title }, + description = if (params.containsKey( + "description", + ) + ) { + params.text("description") + } else { + current.description + }, + type = stepTypeOf(params.text("type")) ?: current.type, + estimatedMinutes = params.text("estimated_minutes").toIntOrNull() ?: current.estimatedMinutes, + expectedOutcome = if (params.containsKey("expected_outcome")) { + params.text("expected_outcome") + } else { + current.expectedOutcomes.firstOrNull().orEmpty() + }, + ), + ) + return "Done. The step is updated; its status is as it was." + } +} + +/** Offers to delete a step, with its tasks and resources. */ +@Component +class DeleteStepAction( + private val scope: ContentScope, + private val onboardingStepService: OnboardingStepService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.DESTRUCTIVE + override val spec = BuddyToolSpecDto( + name = "delete_step", + description = "Offer to delete a step from a member's onboarding path, with its tasks and resources. " + + "Use the step_id from get_member_path. This does NOT delete anything by itself; the manager " + + "confirms.", + parameters = stringFields("step_id" to "The step_id from get_member_path.", required = listOf("step_id")), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.STEP, call.uuidArgument("step_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.STEP)) + val step = target.element + + return TeamActionDraft.Proposed( + params = buildJsonObject { put("step_id", step.id.toString()) }, + label = "Delete step “${step.title.forLabel()}”", + preview = buildString { + appendLine("Delete the step “${step.title}” from ${target.owner.displayName}'s onboarding path.") + if (step.contains.isNotEmpty()) appendLine("It takes ${step.contains} with it.") + step.stepStatus.aboutProgress(target.owner.displayName)?.let { appendLine(it) } + append("The steps after it move up one place. This cannot be undone.") + scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + scope.missing(PathElementKind.STEP, params.uuid("step_id"), context.projectId) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingStepService.deleteOnboardingStepById(requireNotNull(params.uuid("step_id"))) + return "Deleted. The step and what was in it are gone." + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActions.kt new file mode 100644 index 00000000..0f8be3d4 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActions.kt @@ -0,0 +1,226 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import com.sprintstart.sprintstartbackend.onboarding.model.request.task.CreateOnboardingTaskRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.task.UpdateOnboardingTaskRequest +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.springframework.stereotype.Component + +/* + * The task actions of team mode's content area. + * + * A task is what a person ticks off. The update service takes `finished` alongside the text, so an + * edit here carries the tick through exactly as it is: team mode never sets a hire's progress, and an + * edit that quietly un-ticked a task would be doing exactly that. + */ + +/** Offers to add a task to a step of a member's path. */ +@Component +class AddTaskAction( + private val scope: ContentScope, + private val onboardingTaskService: OnboardingTaskService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "add_task", + description = "Offer to add a task to a step of a member's onboarding path. Use the step_id from " + + "get_member_path. It starts unfinished. Omit place to add it at the end of the step. This does " + + "NOT add anything by itself; the manager confirms.", + parameters = toolFields( + ToolField("step_id", "The step_id from get_member_path."), + ToolField("title", "The task's title."), + ToolField("description", "What the person does."), + ToolField("place", "Where in the step it goes, counting from 1. Omitted means the end.", "integer"), + required = listOf("step_id", "title"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.STEP, call.uuidArgument("step_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.STEP)) + val title = call.textArgument("title") + if (title.isEmpty()) return TeamActionDraft.Refused("A task needs a title.") + + val slots = target.element.children + 1 + val placeText = call.textArgument("place") + val position = if (placeText.isEmpty()) slots - 1 else placeToPosition(placeText, slots) + if (position == null) return TeamActionDraft.Refused("Place must be a number from 1 to $slots.") + val description = call.textArgument("description") + + return TeamActionDraft.Proposed( + params = buildJsonObject { + put("step_id", target.element.id.toString()) + put("title", title) + put("description", description) + put("position", position) + }, + label = "Add task “${title.forLabel()}”", + preview = buildString { + appendLine("Add a task to “${target.element.title}” on ${target.owner.displayName}'s path:") + appendLine("“$title” — ${placeOf(position, slots)}") + if (description.isNotEmpty()) appendLine(description) + appendLine() + if (position < slots - 1) appendLine("The tasks after it move down one place.") + append("It starts unfinished.") + if (target.element.stepStatus == StepStatus.FINISHED) { + // The service does this, and it is a change to progress the manager has to agree to. + append( + "\n\nThe step is already finished. A step can only stay finished while all its tasks " + + "are done, so this reopens it: it goes back to in progress for " + + "${target.owner.displayName}.", + ) + } + scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? { + val target = scope.element(PathElementKind.STEP, params.uuid("step_id"), context.projectId) + ?: return goneSince(PathElementKind.STEP) + val position = params.text("position").toIntOrNull() ?: return "That offer is malformed." + return "The step has fewer tasks than it had — that place is gone — so nothing was changed. Offer it again." + .takeIf { position !in 0..target.element.children } + } + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingTaskService.createOnboardingTaskForStepId( + requireNotNull(params.uuid("step_id")), + CreateOnboardingTaskRequest( + position = requireNotNull(params.text("position").toIntOrNull()), + title = params.text("title"), + description = params.text("description"), + ), + ) + return "Added. “${params.text("title")}” is on the step now, unfinished." + } +} + +/** Offers to change what a task says, or where it sits. Never whether it is ticked. */ +@Component +class UpdateTaskAction( + private val scope: ContentScope, + private val onboardingTaskService: OnboardingTaskService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "update_task", + description = "Offer to change a task on a member's onboarding path: its title, its description or " + + "where it sits in its step. Pass only what should change. Use the task_id from get_member_path. " + + "This can never tick or un-tick a task. It does NOT change anything by itself; the manager " + + "confirms.", + parameters = toolFields( + ToolField("task_id", "The task_id from get_member_path."), + ToolField("title", "The new title."), + ToolField("description", "The new description."), + ToolField("place", "The new place in its step, counting from 1.", "integer"), + required = listOf("task_id"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.TASK, call.uuidArgument("task_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.TASK)) + val current = onboardingTaskService.getOnboardingTaskById(target.element.id) + val siblings = onboardingTaskService.getOnboardingTasksByStepId(current.stepId).size + + val placeText = call.textArgument("place") + val requested = if (placeText.isEmpty()) current.position else placeToPosition(placeText, siblings) + if (requested == null) return TeamActionDraft.Refused("Place must be a number from 1 to $siblings.") + + val changes = ChangeSet("task_id", current.id) + changes.add("title", call.changedText("title", current.title, allowBlank = false)) { + "Title: “${current.title}” becomes “$it”" + } + changes.add("description", call.changedText("description", current.description)) { + "Description becomes: ${it.ifEmpty { "(empty)" }}" + } + changes.add("position", requested.takeIf { it != current.position }?.toString()) { + "Place: ${placeOf(current.position, siblings)} becomes ${placeOf(it.toInt(), siblings)}" + } + if (changes.isEmpty) return TeamActionDraft.Refused("Nothing would change: that is already how the task is.") + + return TeamActionDraft.Proposed( + params = changes.params(), + label = "Change task “${current.title.forLabel()}”", + preview = "Change the task “${current.title}” on ${target.owner.displayName}'s path:\n" + + changes.preview() + "\n" + + "Whether it is ticked off stays as it is (${if (current.finished) "done" else "not done"})." + + scope.sharedNote(target.owner, context.projectId).let { if (it.isEmpty()) "" else "\n\n$it" }, + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + scope.missing(PathElementKind.TASK, params.uuid("task_id"), context.projectId) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + val id = requireNotNull(params.uuid("task_id")) + // `finished` is read now and passed back unchanged: the request carries it, and it must never + // be the thing an edit from here decides. + val current = onboardingTaskService.getOnboardingTaskById(id) + onboardingTaskService.updateOnboardingTaskById( + id, + UpdateOnboardingTaskRequest( + position = params.text("position").toIntOrNull() ?: current.position, + title = params.text("title").ifEmpty { current.title }, + description = if (params.containsKey( + "description", + ) + ) { + params.text("description") + } else { + current.description + }, + finished = current.finished, + ), + ) + return "Done. The task is updated; whether it is ticked off is as it was." + } +} + +/** Offers to delete a task. */ +@Component +class DeleteTaskAction( + private val scope: ContentScope, + private val onboardingTaskService: OnboardingTaskService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.DESTRUCTIVE + override val spec = BuddyToolSpecDto( + name = "delete_task", + description = "Offer to delete a task from a member's onboarding path. Use the task_id from " + + "get_member_path. This does NOT delete anything by itself; the manager confirms.", + parameters = stringFields("task_id" to "The task_id from get_member_path.", required = listOf("task_id")), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.TASK, call.uuidArgument("task_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.TASK)) + val task = target.element + + return TeamActionDraft.Proposed( + params = buildJsonObject { put("task_id", task.id.toString()) }, + label = "Delete task “${task.title.forLabel()}”", + preview = buildString { + appendLine("Delete the task “${task.title}” from ${target.owner.displayName}'s onboarding path.") + append("The tasks after it move up one place, and any tick on it goes with it. This cannot be undone.") + scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + scope.missing(PathElementKind.TASK, params.uuid("task_id"), context.projectId) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingTaskService.deleteOnboardingTaskById(requireNotNull(params.uuid("task_id"))) + return "Deleted. The task is gone." + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ToolFields.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ToolFields.kt new file mode 100644 index 00000000..21e5750a --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ToolFields.kt @@ -0,0 +1,95 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.add +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonArray +import kotlinx.serialization.json.putJsonObject +import java.util.UUID + +/** + * One argument of a content tool. + * + * The content tools take more than text: a place in a list, an amount of minutes, one of a fixed + * set of values. [stringFields] can only say "string", which would leave a model guessing that + * `type` must be `VIDEO`, `DOCUMENT` or `TASK`. + */ +internal data class ToolField( + val name: String, + val description: String, + val type: String = "string", + val values: List = emptyList(), +) + +/** A JSON schema of typed [fields], for a content tool's definition. */ +internal fun toolFields(vararg fields: ToolField, required: List): JsonObject = + buildJsonObject { + put("type", "object") + putJsonObject("properties") { + fields.forEach { field -> + putJsonObject(field.name) { + put("type", field.type) + put("description", field.description) + if (field.values.isNotEmpty()) putJsonArray("enum") { field.values.forEach { add(it) } } + } + } + } + putJsonArray("required") { required.forEach { add(it) } } + } + +/** + * A 1-based place the model gave, as the 0-based position the services take. + * + * Null when it is not a whole number or lies outside the [slots] places there are. People and + * models both count "the first phase" as 1; the services count from 0, and the conversion lives here + * so that neither of them has to think about it. + */ +internal fun placeToPosition(place: String, slots: Int): Int? = + place.toIntOrNull()?.minus(1)?.takeIf { it in 0 until slots } + +/** A 0-based position said the way a person counts: "place 1 of 3". */ +internal fun placeOf(position: Int, total: Int): String = "place ${position + 1} of $total" + +/** + * The new value of a text argument, or null when the call leaves it alone or only repeats what is + * already there. + * + * An argument the model left out is not the same as one it sent empty: the second is a request to + * clear the field, and is honoured unless [allowBlank] is false, as it is for a title. + */ +internal fun BuddyToolCallDto.changedText(name: String, now: String, allowBlank: Boolean = true): String? = + textArgument(name).takeIf { arguments.containsKey(name) && (allowBlank || it.isNotEmpty()) && it != now } + +/** + * The edits one update call asks for, collected once and read two ways: what a confirm stores, and + * what the manager is shown. + * + * Only what actually changes goes in. What a confirm stores is applied over the element as it is + * *then*, so a change made to any other field since the preview is not undone by it. + */ +internal class ChangeSet( + private val idName: String, + private val id: UUID, +) { + private val stored = linkedMapOf() + private val lines = mutableListOf() + + val isEmpty: Boolean get() = stored.isEmpty() + + /** Records [value] under [name] and the line describing it, unless it is null — no change. */ + fun add(name: String, value: String?, line: (String) -> String) { + if (value == null) return + stored[name] = value + lines += line(value) + } + + fun params(): JsonObject = + buildJsonObject { + put(idName, id.toString()) + stored.forEach { (name, value) -> put(name, value) } + } + + fun preview(): String = lines.joinToString("\n") +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt new file mode 100644 index 00000000..9e1dd09c --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt @@ -0,0 +1,111 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.user.external.ProjectMember +import com.sprintstart.sprintstartbackend.user.external.ProjectMembershipApi +import com.sprintstart.sprintstartbackend.user.external.UserApi +import com.sprintstart.sprintstartbackend.user.external.dto.ProjectDto +import com.sprintstart.sprintstartbackend.user.external.dto.UserDto +import io.mockk.every +import io.mockk.mockk +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonNull +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.assertj.core.api.Assertions.assertThat +import java.util.UUID + +/** + * A project with one member, and a [ContentScope] over it, for the content actions' tests. + * + * The scope is the real one over mocked sources, so a test of an action also tests that the action + * asks it — an action that forgot to would find its target and fail the outsider test. + */ +internal class ContentFixture { + val pathElements: PathElements = mockk(relaxed = true) + val projectMembershipApi: ProjectMembershipApi = mockk(relaxed = true) + val userApi: UserApi = mockk(relaxed = true) + val scope = ContentScope(pathElements, projectMembershipApi, userApi) + + val projectId: UUID = UUID.randomUUID() + val memberId: UUID = UUID.randomUUID() + val outsiderId: UUID = UUID.randomUUID() + val context = TeamToolContext(userId = UUID.randomUUID(), authId = "auth|pm", projectId = projectId) + + init { + every { projectMembershipApi.getProjectMembers(projectId) } returns + listOf(ProjectMember(memberId, "Sam Rivera", githubLogin = null, joinedAt = null)) + onlyThisProject() + } + + /** The member is on this project and no other, which is what most tests want. */ + fun onlyThisProject() = alsoOn() + + /** The member is on this project and on the projects named. */ + fun alsoOn(vararg names: String) { + every { userApi.getUsersByIds(listOf(memberId)) } returns + listOf( + UserDto( + id = memberId, + username = "sam", + firstname = "Sam", + lastname = "Rivera", + avatarUrl = null, + profileIcon = null, + projects = setOf(ProjectDto(projectId, "This one", null)) + + names.map { ProjectDto(UUID.randomUUID(), it, null) }, + projectRoles = emptyList(), + ), + ) + } + + /** An element that exists, on [owner]'s path. */ + fun element( + kind: PathElementKind, + id: UUID = UUID.randomUUID(), + owner: UUID = memberId, + title: String = "The ${kind.noun}", + position: Int? = 0, + children: Int = 0, + contains: String = "", + stepStatus: StepStatus? = null, + finishedSteps: Int = 0, + ): PathElement { + val element = PathElement(kind, id, owner, title, position, children, contains, stepStatus, finishedSteps) + every { pathElements.find(kind, id) } returns element + return element + } + + /** An element that is gone. */ + fun gone(kind: PathElementKind, id: UUID) { + every { pathElements.find(kind, id) } returns null + } + + fun call(name: String, vararg args: Pair) = + BuddyToolCallDto(id = "c1", name = name, arguments = json(*args)) + + fun json(vararg args: Pair): JsonObject = + buildJsonObject { + args.forEach { (key, value) -> + when (value) { + null -> put(key, JsonNull) + is Number -> put(key, value) + is JsonElement -> put(key, value) + else -> put(key, JsonPrimitive(value.toString())) + } + } + } + + fun proposed(draft: TeamActionDraft): TeamActionDraft.Proposed { + assertThat(draft).isInstanceOf(TeamActionDraft.Proposed::class.java) + return draft as TeamActionDraft.Proposed + } + + fun refusal(draft: TeamActionDraft): String { + assertThat(draft).isInstanceOf(TeamActionDraft.Refused::class.java) + return (draft as TeamActionDraft.Refused).reason + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt new file mode 100644 index 00000000..a808f3ef --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt @@ -0,0 +1,134 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepType +import com.sprintstart.sprintstartbackend.onboarding.model.response.path.GetOnboardingPathResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.phase.GetOnboardingPhasesResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.resource.GetOnboardingResourcesResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.step.GetOnboardingStepResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.task.GetOnboardingTasksResponse +import io.mockk.every +import io.mockk.mockk +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.http.HttpStatus +import org.springframework.web.server.ResponseStatusException +import java.time.Instant +import java.util.UUID + +class ContentTeamToolsTest { + private val f = ContentFixture() + private val pathService: OnboardingPathService = mockk(relaxed = true) + private val stepService: OnboardingStepService = mockk(relaxed = true) + private val tools = ContentTeamTools(f.scope, pathService, stepService) + + private val phaseId = UUID.randomUUID() + private val stepId = UUID.randomUUID() + private val taskId = UUID.randomUUID() + private val resourceId = UUID.randomUUID() + + private fun path() { + every { pathService.getOnboardingPathByUserId(f.memberId) } returns + GetOnboardingPathResponse( + id = UUID.randomUUID(), + userId = f.memberId, + createdAt = Instant.now(), + phases = listOf( + GetOnboardingPhasesResponse(phaseId, UUID.randomUUID(), 0, "Setup", "Get the machine ready"), + ), + ) + every { stepService.getOnboardingStepsByPhaseId(phaseId) } returns + listOf( + GetOnboardingStepResponse( + id = stepId, + phaseId = phaseId, + position = 0, + title = "Install", + description = "Install the toolchain", + type = StepType.TASK, + estimatedMinutes = 20, + isAiAssisted = false, + tasks = listOf(GetOnboardingTasksResponse(taskId, stepId, 0, "Clone", "d", finished = true)), + resources = listOf( + GetOnboardingResourcesResponse(resourceId, stepId, "Docs", "d", "https://docs.example.com"), + ), + status = StepStatus.IN_PROGRESS, + completedAt = null, + skip = null, + ), + ) + } + + private fun read(memberId: Any) = tools.execute(f.call("get_member_path", "member_id" to memberId), f.context) + + @Test + fun `mounts exactly the reads of the area, in the content area`() { + assertThat(tools.area).isEqualTo(TeamArea.CONTENT) + assertThat(tools.toolSpecs().map { it.name }).containsExactly("get_member_path") + assertThat(tools.handles("get_member_path")).isTrue() + assertThat(tools.handles("add_phase")).isFalse() + } + + @Test + fun `prints the whole path with the id every action needs`() { + path() + + val text = read(f.memberId) + + assertThat(text).contains( + "Sam Rivera's onboarding path", + "Phase 1: Setup [phase_id: $phaseId]", + "Step 1: Install [step_id: $stepId]", + "task, 20 min, in progress", + "[x] Clone [task_id: $taskId]", + "link: Docs [resource_id: $resourceId]", + ) + } + + @Test + fun `says when the path is also somebody else's`() { + path() + f.alsoOn("Payments") + + assertThat(read(f.memberId)).contains("also on Payments", "one onboarding path") + } + + @Test + fun `long descriptions are cut short so the ids stay findable`() { + path() + val long = "word ".repeat(200) + every { pathService.getOnboardingPathByUserId(f.memberId) } returns + GetOnboardingPathResponse( + UUID.randomUUID(), + f.memberId, + Instant.now(), + listOf(GetOnboardingPhasesResponse(phaseId, UUID.randomUUID(), 0, "Setup", long)), + ) + + val text = read(f.memberId) + + assertThat(text).contains("…") + assertThat(text.length).isLessThan(long.length) + } + + @Test + fun `somebody not on the project cannot be read, and there is nothing to probe`() { + path() + + assertThat(read(f.outsiderId)).contains("not on this project") + assertThat(read("not-an-id")).contains("not on this project") + } + + @Test + fun `a member with no path is said to have none`() { + every { pathService.getOnboardingPathByUserId(f.memberId) } throws + ResponseStatusException(HttpStatus.NOT_FOUND) + + assertThat(read(f.memberId)).isEqualTo("Sam Rivera has no onboarding path yet.") + } + + @Test + fun `an unknown tool is named as such`() { + assertThat(tools.execute(f.call("nope"), f.context)).isEqualTo("Unknown tool: nope.") + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElementsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElementsTest.kt new file mode 100644 index 00000000..93ba1ab5 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElementsTest.kt @@ -0,0 +1,207 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepType +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingFeedback +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingPath +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingPhase +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingResource +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingSkip +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingStep +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingTask +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingFeedbackRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingPathRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingPhaseRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingResourceRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingSkipRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingStepRepository +import com.sprintstart.sprintstartbackend.onboarding.repository.OnboardingTaskRepository +import com.sprintstart.sprintstartbackend.shared.crypto.CryptoConfiguration +import jakarta.persistence.EntityManager +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest +import org.springframework.context.annotation.Import +import org.springframework.test.context.ActiveProfiles +import java.util.UUID + +/** + * Whose path an element is on decides whether a manager may touch it, so the walk from each kind + * up to its owner meets a real database: a mocked repository would only prove the mapping. + */ +@ActiveProfiles("test") +@DataJpaTest +// The same slice configuration as the other repository tests, so it reuses their cached context: one +// more distinct context is one more Spring container held in a test JVM that is already near its heap. +@Import(CryptoConfiguration::class) +class PathElementsTest { + @Autowired + private lateinit var entityManager: EntityManager + + @Autowired + private lateinit var pathRepository: OnboardingPathRepository + + @Autowired + private lateinit var phaseRepository: OnboardingPhaseRepository + + @Autowired + private lateinit var stepRepository: OnboardingStepRepository + + @Autowired + private lateinit var taskRepository: OnboardingTaskRepository + + @Autowired + private lateinit var resourceRepository: OnboardingResourceRepository + + @Autowired + private lateinit var skipRepository: OnboardingSkipRepository + + @Autowired + private lateinit var feedbackRepository: OnboardingFeedbackRepository + + private val pathElements: PathElements by lazy { + PathElements( + pathRepository, + phaseRepository, + stepRepository, + taskRepository, + resourceRepository, + skipRepository, + feedbackRepository, + ) + } + + private val owner = UUID.randomUUID() + + private class Built( + val path: OnboardingPath, + val phase: OnboardingPhase, + val step: OnboardingStep, + val task: OnboardingTask, + val resource: OnboardingResource, + val skip: OnboardingSkip, + val feedback: OnboardingFeedback, + ) + + /** One phase with one step holding a task, a resource, a skip request and feedback. */ + private fun pathFor(userId: UUID, status: StepStatus = StepStatus.WAITING): Built { + val path = OnboardingPath(userId = userId) + val phase = OnboardingPhase(path = path, position = 0, title = "Setup", description = "d") + val step = OnboardingStep( + phase = phase, + position = 0, + title = "Install", + description = "d", + type = StepType.TASK, + estimatedMinutes = 10, + expectedOutcome = "it runs", + status = status, + ) + val task = OnboardingTask(step = step, position = 0, title = "Clone", description = "d") + val resource = OnboardingResource(step = step, title = "Docs", description = "d", url = "https://docs") + val skip = OnboardingSkip(step = step, reason = "already know it") + val feedback = OnboardingFeedback(userId = userId, step = step, message = "too long") + step.tasks += task + step.resources += resource + step.skips += skip + step.feedback += feedback + phase.steps += step + path.phases += phase + entityManager.persist(path) + entityManager.flush() + entityManager.clear() + return Built(path, phase, step, task, resource, skip, feedback) + } + + @Test + fun `every kind is walked up to the person whose path it is on`() { + val built = pathFor(owner) + + val found = mapOf( + PathElementKind.PHASE to built.phase.id, + PathElementKind.STEP to built.step.id, + PathElementKind.TASK to built.task.id, + PathElementKind.RESOURCE to built.resource.id, + PathElementKind.SKIP to built.skip.id, + PathElementKind.FEEDBACK to built.feedback.id, + ).mapValues { (kind, id) -> pathElements.find(kind, id) } + + found.forEach { (kind, element) -> + assertThat(element).describedAs("$kind").isNotNull + assertThat(element!!.ownerId).describedAs("$kind").isEqualTo(owner) + } + } + + @Test + fun `an element on somebody else's path is theirs, not the first path's`() { + val other = UUID.randomUUID() + val mine = pathFor(owner) + val theirs = pathFor(other) + + assertThat(pathElements.find(PathElementKind.TASK, mine.task.id)!!.ownerId).isEqualTo(owner) + assertThat(pathElements.find(PathElementKind.TASK, theirs.task.id)!!.ownerId).isEqualTo(other) + assertThat(pathElements.find(PathElementKind.SKIP, theirs.skip.id)!!.ownerId).isEqualTo(other) + } + + @Test + fun `an id of one kind is not found as another kind`() { + val built = pathFor(owner) + + assertThat(pathElements.find(PathElementKind.STEP, built.task.id)).isNull() + assertThat(pathElements.find(PathElementKind.TASK, built.phase.id)).isNull() + assertThat(pathElements.find(PathElementKind.FEEDBACK, UUID.randomUUID())).isNull() + } + + @Test + fun `a phase says what deleting it would take with it`() { + val built = pathFor(owner, StepStatus.FINISHED) + + val phase = pathElements.find(PathElementKind.PHASE, built.phase.id)!! + + assertThat(phase.contains).isEqualTo( + "1 step, 1 task, 1 resource, 1 skip request and 1 piece of feedback", + ) + assertThat(phase.children).isEqualTo(1) + assertThat(phase.finishedSteps).isEqualTo(1) + } + + @Test + fun `a step carries its own status and counts its tasks as its children`() { + val built = pathFor(owner, StepStatus.IN_PROGRESS) + + val step = pathElements.find(PathElementKind.STEP, built.step.id)!! + + assertThat(step.stepStatus).isEqualTo(StepStatus.IN_PROGRESS) + assertThat(step.children).isEqualTo(1) + assertThat(step.contains).doesNotContain("step") + } + + @Test + fun `feedback on no step is described as being about the path as a whole`() { + pathFor(owner) + val general = OnboardingFeedback(userId = owner, step = null, message = "overall fine") + entityManager.persist(general) + entityManager.flush() + entityManager.clear() + + val found = pathElements.find(PathElementKind.FEEDBACK, general.id)!! + + assertThat(found.title).isEqualTo("the path as a whole") + assertThat(found.stepStatus).isNull() + } + + @Test + fun `a path summary counts finished and skipped steps as got through`() { + val built = pathFor(owner, StepStatus.SKIPPED) + + val summary = pathElements.pathOf(built.path.userId)!! + + assertThat(summary).isEqualTo(PathSummary(phases = 1, steps = 1, finishedSteps = 1, checks = 0)) + } + + @Test + fun `somebody with no path has no summary`() { + assertThat(pathElements.pathOf(UUID.randomUUID())).isNull() + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActionsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActionsTest.kt new file mode 100644 index 00000000..e135aed6 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActionsTest.kt @@ -0,0 +1,344 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.model.request.phase.CreateOnboardingPhaseRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.phase.UpdateOnboardingPhaseRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.phase.GetOnboardingPhaseResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.phase.GetOnboardingPhasesResponse +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import java.util.UUID + +class PhaseTeamActionsTest { + private val f = ContentFixture() + private val phaseService: OnboardingPhaseService = mockk(relaxed = true) + + private fun summary(id: UUID, position: Int) = + GetOnboardingPhasesResponse( + id = id, + pathId = UUID.randomUUID(), + position = position, + title = "t$position", + description = "", + ) + + private fun current(id: UUID, position: Int = 1, title: String = "Setup", description: String = "Get going") { + every { phaseService.getOnboardingPhaseById(id) } returns + GetOnboardingPhaseResponse( + id = id, + pathId = UUID.randomUUID(), + position = position, + title = title, + description = description, + steps = emptyList(), + ) + } + + @Nested + inner class Add { + private val action = AddPhaseAction(f.scope, f.pathElements, phaseService) + + private fun pathOf(phases: Int) { + every { f.pathElements.pathOf(f.memberId) } returns PathSummary(phases, 0, 0, 0) + } + + @Test + fun `is a standard change in the content area`() { + assertThat(action.area).isEqualTo(TeamArea.CONTENT) + assertThat(action.risk).isEqualTo(BuddyProposalRisk.STANDARD) + } + + @Test + fun `appends by default and says the phase starts empty`() { + pathOf(3) + + val draft = f.proposed( + action.draft( + f.call("add_phase", "member_id" to f.memberId, "title" to "Review", "description" to "How"), + f.context, + ), + ) + + assertThat(draft.params.text("position")).isEqualTo("3") + assertThat(draft.preview).contains("Sam Rivera", "“Review”", "place 4 of 4", "no steps") + assertThat(draft.preview).doesNotContain("move down") + } + + @Test + fun `inserting before others says they move down`() { + pathOf(3) + + val draft = f.proposed( + action.draft( + f.call("add_phase", "member_id" to f.memberId, "title" to "First", "place" to 1), + f.context, + ), + ) + + assertThat(draft.params.text("position")).isEqualTo("0") + assertThat(draft.preview).contains("move down one place") + } + + @Test + fun `a place outside the path is refused`() { + pathOf(3) + + val reason = f.refusal( + action.draft(f.call("add_phase", "member_id" to f.memberId, "title" to "x", "place" to 9), f.context), + ) + + assertThat(reason).contains("from 1 to 4") + } + + @Test + fun `somebody not on the project is refused`() { + val reason = f.refusal( + action.draft(f.call("add_phase", "member_id" to f.outsiderId, "title" to "x"), f.context), + ) + + assertThat(reason).contains("not on this project") + } + + @Test + fun `a member with no path is refused rather than given one`() { + every { f.pathElements.pathOf(f.memberId) } returns null + + val reason = f.refusal( + action.draft(f.call("add_phase", "member_id" to f.memberId, "title" to "x"), f.context), + ) + + assertThat(reason).contains("no onboarding path") + } + + @Test + fun `a blank title is refused`() { + pathOf(1) + + val reason = f.refusal( + action.draft(f.call("add_phase", "member_id" to f.memberId, "title" to " "), f.context), + ) + + assertThat(reason).contains("needs a title") + } + + @Test + fun `a member on other projects has it said in the preview`() { + pathOf(1) + f.alsoOn("Payments", "Search") + + val draft = f.proposed( + action.draft(f.call("add_phase", "member_id" to f.memberId, "title" to "x"), f.context), + ) + + assertThat( + draft.preview, + ).contains("also on Payments, Search", "one onboarding path", "changes it there too") + } + + @Test + fun `nothing is written while drafting`() { + pathOf(1) + + action.draft(f.call("add_phase", "member_id" to f.memberId, "title" to "x"), f.context) + + verify(exactly = 0) { phaseService.createOnboardingPhaseForUserId(any(), any()) } + } + + @Test + fun `a confirm for somebody who left is turned down`() { + val params = f.json("member_id" to f.outsiderId, "title" to "x", "position" to 0) + + assertThat(action.recheck(params, f.context)).contains("no longer on this project") + } + + @Test + fun `a confirm for a place that has gone is turned down`() { + pathOf(1) + val params = f.json("member_id" to f.memberId, "title" to "x", "position" to 5) + + assertThat(action.recheck(params, f.context)).contains("fewer phases") + } + + @Test + fun `performing creates the phase with what was stored`() = runTest { + val request = slot() + every { phaseService.createOnboardingPhaseForUserId(f.memberId, capture(request)) } returns + mockk(relaxed = true) + + action.perform( + f.json( + "member_id" to f.memberId, + "title" to "Review", + "description" to "How", + "position" to 2, + ), + f.context, + ) + + assertThat(request.captured).isEqualTo(CreateOnboardingPhaseRequest(2, "Review", "How")) + } + } + + @Nested + inner class Update { + private val action = UpdatePhaseAction(f.scope, phaseService) + private val phaseId = UUID.randomUUID() + + private fun onMembersPath(siblings: Int = 3) { + f.element(PathElementKind.PHASE, phaseId, title = "Setup", position = 1) + current(phaseId) + every { phaseService.getOnboardingPhasesForUser(f.memberId) } returns + (0 until siblings).map { summary(UUID.randomUUID(), it) } + } + + @Test + fun `stores only what changes, and previews before and after`() { + onMembersPath() + + val draft = f.proposed( + action.draft( + f.call("update_phase", "phase_id" to phaseId, "title" to "Onboarding setup", "place" to 3), + f.context, + ), + ) + + assertThat(draft.params.text("title")).isEqualTo("Onboarding setup") + assertThat(draft.params.text("position")).isEqualTo("2") + assertThat(draft.params.containsKey("description")).isFalse() + assertThat( + draft.preview, + ).contains("“Setup” becomes “Onboarding setup”", "place 2 of 3 becomes place 3 of 3") + } + + @Test + fun `repeating what is already there is refused as no change`() { + onMembersPath() + + val reason = f.refusal( + action.draft( + f.call("update_phase", "phase_id" to phaseId, "title" to "Setup", "place" to 2), + f.context, + ), + ) + + assertThat(reason).contains("Nothing would change") + } + + @Test + fun `an empty description is a request to clear it, and is kept`() { + onMembersPath() + + val draft = f.proposed( + action.draft(f.call("update_phase", "phase_id" to phaseId, "description" to ""), f.context), + ) + + assertThat(draft.params.containsKey("description")).isTrue() + assertThat(draft.preview).contains("(empty)") + } + + @Test + fun `a phase on somebody else's path is refused, as is one that does not exist`() { + f.element(PathElementKind.PHASE, phaseId, owner = f.outsiderId) + val missing = UUID.randomUUID() + f.gone(PathElementKind.PHASE, missing) + + val outsider = f.refusal( + action.draft(f.call("update_phase", "phase_id" to phaseId, "title" to "x"), f.context), + ) + val absent = f.refusal( + action.draft(f.call("update_phase", "phase_id" to missing, "title" to "x"), f.context), + ) + + assertThat(outsider).contains("not on the onboarding path of anybody on this project") + assertThat(absent).isEqualTo(outsider) + } + + @Test + fun `a place outside the path is refused`() { + onMembersPath(siblings = 2) + + val reason = f.refusal(action.draft(f.call("update_phase", "phase_id" to phaseId, "place" to 7), f.context)) + + assertThat(reason).contains("from 1 to 2") + } + + @Test + fun `a confirm after the phase moved to somebody who left the project is turned down`() { + f.element(PathElementKind.PHASE, phaseId, owner = f.outsiderId) + + assertThat(action.recheck(f.json("phase_id" to phaseId), f.context)).contains("no longer on the path") + } + + @Test + fun `performing writes the stored change over the phase as it is now`() = runTest { + current(phaseId, position = 4, title = "Renamed since", description = "Edited since") + val request = slot() + every { phaseService.updateOnboardingPhaseById(phaseId, capture(request)) } returns mockk(relaxed = true) + + action.perform(f.json("phase_id" to phaseId, "position" to 0), f.context) + + assertThat(request.captured).isEqualTo(UpdateOnboardingPhaseRequest(0, "Renamed since", "Edited since")) + } + } + + @Nested + inner class Delete { + private val action = DeletePhaseAction(f.scope, phaseService) + private val phaseId = UUID.randomUUID() + + @Test + fun `is destructive`() { + assertThat(action.risk).isEqualTo(BuddyProposalRisk.DESTRUCTIVE) + } + + @Test + fun `says what goes with the phase and how much progress it holds`() { + f.element( + PathElementKind.PHASE, + phaseId, + title = "Setup", + contains = "4 steps and 9 tasks", + finishedSteps = 2, + ) + + val draft = f.proposed(action.draft(f.call("delete_phase", "phase_id" to phaseId), f.context)) + + assertThat( + draft.preview, + ).contains("Setup", "takes 4 steps and 9 tasks with it", "got through 2 of its steps") + assertThat(draft.preview).contains("cannot be undone") + } + + @Test + fun `an empty phase makes no claim about what is inside`() { + f.element(PathElementKind.PHASE, phaseId, title = "Empty") + + val draft = f.proposed(action.draft(f.call("delete_phase", "phase_id" to phaseId), f.context)) + + assertThat(draft.preview).doesNotContain("takes") + assertThat(draft.preview).doesNotContain("got through") + } + + @Test + fun `a phase on somebody else's path is refused at proposal and at confirm`() { + f.element(PathElementKind.PHASE, phaseId, owner = f.outsiderId) + + assertThat(f.refusal(action.draft(f.call("delete_phase", "phase_id" to phaseId), f.context))) + .contains("not on the onboarding path") + assertThat(action.recheck(f.json("phase_id" to phaseId), f.context)).isNotNull() + } + + @Test + fun `performing deletes the phase by id`() = runTest { + action.perform(f.json("phase_id" to phaseId), f.context) + + verify { phaseService.deleteOnboardingPhaseById(phaseId) } + } + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResetMemberPathActionTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResetMemberPathActionTest.kt new file mode 100644 index 00000000..160d357c --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResetMemberPathActionTest.kt @@ -0,0 +1,93 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +class ResetMemberPathActionTest { + private val f = ContentFixture() + private val pathService: OnboardingPathService = mockk(relaxed = true) + private val action = ResetMemberPathAction(f.scope, f.pathElements, pathService) + + private fun call(id: Any) = f.call("reset_member_path", "member_id" to id) + + @Test + fun `is destructive`() { + assertThat(action.risk).isEqualTo(BuddyProposalRisk.DESTRUCTIVE) + } + + @Test + fun `puts numbers on how much is thrown away, including progress`() { + every { f.pathElements.pathOf(f.memberId) } returns + PathSummary(phases = 4, steps = 17, finishedSteps = 9, checks = 6) + + val draft = f.proposed(action.draft(call(f.memberId), f.context)) + + assertThat(draft.preview).contains( + "Sam Rivera's whole onboarding path", + "4 phases and 17 steps", + "6 knowledge-check questions", + "got through 9 of those steps", + "cannot be undone", + ) + assertThat(draft.preview).contains("Nothing here makes a new path") + } + + @Test + fun `a path nobody has started makes no claim about progress`() { + every { f.pathElements.pathOf(f.memberId) } returns PathSummary(1, 1, 0, 0) + + val draft = f.proposed(action.draft(call(f.memberId), f.context)) + + assertThat(draft.preview).contains("1 phase and 1 step").doesNotContain("got through") + } + + @Test + fun `says so when the path is shared with other projects`() { + every { f.pathElements.pathOf(f.memberId) } returns PathSummary(1, 1, 0, 0) + f.alsoOn("Payments") + + val draft = f.proposed(action.draft(call(f.memberId), f.context)) + + assertThat(draft.preview).contains("also on Payments", "changes it there too") + } + + @Test + fun `refuses somebody not on the project, and somebody with no path`() { + assertThat(f.refusal(action.draft(call(f.outsiderId), f.context))).contains("not on this project") + + every { f.pathElements.pathOf(f.memberId) } returns null + assertThat(f.refusal(action.draft(call(f.memberId), f.context))).contains("no onboarding path") + } + + @Test + fun `a confirm for somebody who left, or whose path is already gone, is turned down`() { + assertThat(action.recheck(f.json("member_id" to f.outsiderId), f.context)).contains("no longer on this project") + + every { f.pathElements.pathOf(f.memberId) } returns null + assertThat(action.recheck(f.json("member_id" to f.memberId), f.context)).contains("already gone") + } + + @Test + fun `a live path passes the recheck`() { + every { f.pathElements.pathOf(f.memberId) } returns PathSummary(1, 1, 0, 0) + + assertThat(action.recheck(f.json("member_id" to f.memberId), f.context)).isNull() + } + + @Test + fun `drafting deletes nothing, performing deletes the path of the member named`() = + runTest { + every { f.pathElements.pathOf(f.memberId) } returns PathSummary(1, 1, 0, 0) + + action.draft(call(f.memberId), f.context) + verify(exactly = 0) { pathService.deleteOnboardingPathByUserId(any()) } + + action.perform(f.json("member_id" to f.memberId), f.context) + verify { pathService.deleteOnboardingPathByUserId(f.memberId) } + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActionsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActionsTest.kt new file mode 100644 index 00000000..60128e37 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActionsTest.kt @@ -0,0 +1,159 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.model.request.resource.CreateOnboardingResourceRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.resource.UpdateOnboardingResourceRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.resource.GetOnboardingResourceResponse +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import java.util.UUID + +class ResourceTeamActionsTest { + private val f = ContentFixture() + private val resourceService: OnboardingResourceService = mockk(relaxed = true) + + @Nested + inner class Add { + private val action = AddResourceAction(f.scope, resourceService) + private val stepId = UUID.randomUUID() + + private fun call(url: String) = f.call("add_resource", "step_id" to stepId, "title" to "Docs", "url" to url) + + @Test + fun `shows the address the hire will be sent to`() { + f.element(PathElementKind.STEP, stepId, title = "Install") + + val draft = f.proposed(action.draft(call("https://docs.example.com/start"), f.context)) + + assertThat(draft.preview).contains("“Install”", "“Docs” — https://docs.example.com/start") + } + + @Test + fun `only web links are accepted, because a hire clicks them`() { + f.element(PathElementKind.STEP, stepId) + + listOf("javascript:alert(1)", "file:///etc/passwd", "docs.example.com", "https://", "not a url").forEach { + assertThat(f.refusal(action.draft(call(it), f.context))).describedAs(it).contains("http") + } + assertThat(action.draft(call("http://wiki.internal/setup"), f.context)) + .isInstanceOf(TeamActionDraft.Proposed::class.java) + } + + @Test + fun `a step on another person's path is refused`() { + f.element(PathElementKind.STEP, stepId, owner = f.outsiderId) + + assertThat(f.refusal(action.draft(call("https://x.io"), f.context))).contains("not on the onboarding path") + } + + @Test + fun `performing creates the resource with what was stored`() = + runTest { + val request = slot() + every { resourceService.createOnboardingResourceForStepId(stepId, capture(request)) } returns + mockk(relaxed = true) + + action.perform( + f.json("step_id" to stepId, "title" to "Docs", "url" to "https://x.io", "description" to "d"), + f.context, + ) + + assertThat(request.captured).isEqualTo(CreateOnboardingResourceRequest("Docs", "d", "https://x.io")) + } + } + + @Nested + inner class Update { + private val action = UpdateResourceAction(f.scope, resourceService) + private val resourceId = UUID.randomUUID() + + private fun existing() { + f.element(PathElementKind.RESOURCE, resourceId) + every { resourceService.getOnboardingResourceById(resourceId) } returns + GetOnboardingResourceResponse(resourceId, UUID.randomUUID(), "Docs", "old", "https://old.io") + } + + @Test + fun `previews the address change`() { + existing() + + val draft = f.proposed( + action.draft( + f.call("update_resource", "resource_id" to resourceId, "url" to "https://new.io"), + f.context, + ), + ) + + assertThat(draft.preview).contains("https://old.io becomes https://new.io") + } + + @Test + fun `a new address that is not a web link is refused`() { + existing() + + val reason = f.refusal( + action.draft( + f.call("update_resource", "resource_id" to resourceId, "url" to "javascript:x"), + f.context, + ), + ) + + assertThat(reason).contains("http") + } + + @Test + fun `repeating what is there is refused as no change`() { + existing() + + val reason = f.refusal( + action.draft(f.call("update_resource", "resource_id" to resourceId, "title" to "Docs"), f.context), + ) + + assertThat(reason).contains("Nothing would change") + } + + @Test + fun `performing keeps what was not stored`() = + runTest { + existing() + val request = slot() + every { resourceService.updateOnboardingResourceById(resourceId, capture(request)) } returns + mockk(relaxed = true) + + action.perform(f.json("resource_id" to resourceId, "url" to "https://new.io"), f.context) + + assertThat(request.captured).isEqualTo(UpdateOnboardingResourceRequest("Docs", "old", "https://new.io")) + } + } + + @Nested + inner class Delete { + private val action = DeleteResourceAction(f.scope, resourceService) + private val resourceId = UUID.randomUUID() + + @Test + fun `names the link and refuses one on another person's path`() { + f.element(PathElementKind.RESOURCE, resourceId, title = "Docs") + assertThat( + f.proposed(action.draft(f.call("delete_resource", "resource_id" to resourceId), f.context)).preview, + ).contains("“Docs”") + + f.element(PathElementKind.RESOURCE, resourceId, owner = f.outsiderId) + assertThat(f.refusal(action.draft(f.call("delete_resource", "resource_id" to resourceId), f.context))) + .contains("not on the onboarding path") + } + + @Test + fun `performing deletes the resource by id`() = + runTest { + action.perform(f.json("resource_id" to resourceId), f.context) + + verify { resourceService.deleteOnboardingResourceById(resourceId) } + } + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActionsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActionsTest.kt new file mode 100644 index 00000000..664893ba --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActionsTest.kt @@ -0,0 +1,283 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepType +import com.sprintstart.sprintstartbackend.onboarding.model.request.step.CreateOnboardingStepRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.step.UpdateOnboardingStepRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.step.GetOnboardingStepResponse +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import java.util.UUID + +class StepTeamActionsTest { + private val f = ContentFixture() + private val stepService: OnboardingStepService = mockk(relaxed = true) + + private fun step( + id: UUID = UUID.randomUUID(), + phaseId: UUID = UUID.randomUUID(), + position: Int = 1, + title: String = "Install", + description: String = "Do it", + type: StepType = StepType.TASK, + minutes: Int = 20, + outcome: String = "It runs", + status: StepStatus = StepStatus.IN_PROGRESS, + ) = GetOnboardingStepResponse( + id = id, + phaseId = phaseId, + position = position, + title = title, + description = description, + type = type, + estimatedMinutes = minutes, + isAiAssisted = false, + expectedOutcomes = listOf(outcome), + tasks = emptyList(), + resources = emptyList(), + status = status, + completedAt = null, + skip = null, + ) + + @Nested + inner class Add { + private val action = AddStepAction(f.scope, stepService) + private val phaseId = UUID.randomUUID() + + private fun inPhaseOf(steps: Int) { + f.element(PathElementKind.PHASE, phaseId, title = "Setup", children = steps) + } + + private fun addCall(vararg extra: Pair) = + f.call("add_step", "phase_id" to phaseId, "title" to "Run tests", "type" to "task", *extra) + + @Test + fun `appends by default, and says it starts waiting`() { + inPhaseOf(2) + + val draft = f.proposed(action.draft(addCall("estimated_minutes" to 30), f.context)) + + assertThat(draft.params.text("position")).isEqualTo("2") + assertThat(draft.params.text("type")).isEqualTo("TASK") + assertThat(draft.preview).contains("“Setup”", "about 30 min", "place 3 of 3", "starts waiting") + } + + @Test + fun `minutes default when left out`() { + inPhaseOf(0) + + val draft = f.proposed(action.draft(addCall(), f.context)) + + assertThat(draft.params.text("estimated_minutes")).isEqualTo("15") + } + + @Test + fun `a type outside the set is refused, naming the ones there are`() { + inPhaseOf(0) + + val reason = f.refusal( + action.draft(f.call("add_step", "phase_id" to phaseId, "title" to "x", "type" to "QUIZ"), f.context), + ) + + assertThat(reason).contains("VIDEO", "DOCUMENT", "TASK") + } + + @Test + fun `minutes that are not a positive whole number are refused`() { + inPhaseOf(0) + + val reason = f.refusal(action.draft(addCall("estimated_minutes" to "soon"), f.context)) + + assertThat(reason).contains("whole number of minutes") + } + + @Test + fun `a phase on another person's path is refused`() { + f.element(PathElementKind.PHASE, phaseId, owner = f.outsiderId) + + val reason = f.refusal(action.draft(addCall(), f.context)) + + assertThat(reason).contains("not on the onboarding path of anybody on this project") + } + + @Test + fun `a confirm after the phase lost steps is turned down`() { + inPhaseOf(1) + + val reason = action.recheck(f.json("phase_id" to phaseId, "position" to 4), f.context) + + assertThat(reason).contains("fewer steps") + } + + @Test + fun `performing creates the step with what was stored`() = + runTest { + val request = slot() + every { stepService.createOnboardingStepForPhaseId(phaseId, capture(request)) } returns + mockk(relaxed = true) + + action.perform( + f.json( + "phase_id" to phaseId, + "title" to "Run tests", + "description" to "d", + "type" to "TASK", + "estimated_minutes" to 30, + "expected_outcome" to "green", + "position" to 1, + ), + f.context, + ) + + assertThat(request.captured) + .isEqualTo(CreateOnboardingStepRequest(1, "Run tests", "d", StepType.TASK, 30, "green")) + } + } + + @Nested + inner class Update { + private val action = UpdateStepAction(f.scope, stepService) + private val stepId = UUID.randomUUID() + private val phaseId = UUID.randomUUID() + + private fun existing(status: StepStatus = StepStatus.IN_PROGRESS) { + f.element(PathElementKind.STEP, stepId, title = "Install") + every { stepService.getOnboardingStepById(stepId) } returns + step(id = stepId, phaseId = phaseId, status = status) + every { stepService.getOnboardingStepsByPhaseId(phaseId) } returns + (0 until 3).map { step(position = it) } + } + + private fun updateCall(vararg args: Pair) = f.call("update_step", "step_id" to stepId, *args) + + @Test + fun `previews each change and says the status is not touched`() { + existing() + + val draft = f.proposed( + action.draft( + updateCall("type" to "video", "estimated_minutes" to 45, "expected_outcome" to ""), + f.context, + ), + ) + + assertThat(draft.params.text("type")).isEqualTo("VIDEO") + assertThat(draft.preview).contains("task becomes video", "20 min becomes 45 min") + assertThat(draft.preview).contains("Expected outcome becomes: (empty)", "status stays in progress") + } + + @Test + fun `nothing that differs is refused as no change`() { + existing() + + val reason = f.refusal( + action.draft(updateCall("title" to "Install", "type" to "TASK", "estimated_minutes" to 20), f.context), + ) + + assertThat(reason).contains("Nothing would change") + } + + @Test + fun `a bad type, length or place is refused`() { + existing() + + assertThat(f.refusal(action.draft(updateCall("type" to "QUIZ"), f.context))) + .contains("Type must be one of") + assertThat(f.refusal(action.draft(updateCall("estimated_minutes" to 0), f.context))) + .contains("above zero") + assertThat(f.refusal(action.draft(updateCall("place" to 8), f.context))) + .contains("from 1 to 3") + } + + @Test + fun `a step on another person's path is refused at proposal and at confirm`() { + f.element(PathElementKind.STEP, stepId, owner = f.outsiderId) + + assertThat(f.refusal(action.draft(updateCall("title" to "x"), f.context))) + .contains("not on the onboarding path") + assertThat(action.recheck(f.json("step_id" to stepId), f.context)).isNotNull() + } + + @Test + fun `performing changes the stored fields and carries the rest over as they are now`() = + runTest { + every { stepService.getOnboardingStepById(stepId) } returns + step( + id = stepId, + title = "Renamed since", + description = "Edited since", + minutes = 25, + outcome = "Now green", + ) + val request = slot() + every { stepService.updateOnboardingStepById(stepId, capture(request)) } returns + mockk(relaxed = true) + + action.perform(f.json("step_id" to stepId, "estimated_minutes" to 40), f.context) + + assertThat(request.captured).isEqualTo( + UpdateOnboardingStepRequest(1, "Renamed since", "Edited since", StepType.TASK, 40, "Now green"), + ) + } + } + + @Nested + inner class Delete { + private val action = DeleteStepAction(f.scope, stepService) + private val stepId = UUID.randomUUID() + + @Test + fun `is destructive`() { + assertThat(action.risk).isEqualTo(BuddyProposalRisk.DESTRUCTIVE) + } + + @Test + fun `says what goes with it, and that a finished step's progress goes too`() { + f.element( + PathElementKind.STEP, + stepId, + title = "Install", + contains = "3 tasks and 1 resource", + stepStatus = StepStatus.FINISHED, + ) + + val draft = f.proposed(action.draft(f.call("delete_step", "step_id" to stepId), f.context)) + + assertThat(draft.preview).contains("takes 3 tasks and 1 resource with it", "already finished it") + } + + @Test + fun `says so when the person is working on it right now`() { + f.element(PathElementKind.STEP, stepId, stepStatus = StepStatus.IN_PROGRESS) + + val draft = f.proposed(action.draft(f.call("delete_step", "step_id" to stepId), f.context)) + + assertThat(draft.preview).contains("Sam Rivera is working on it right now") + } + + @Test + fun `a waiting step makes no claim about progress`() { + f.element(PathElementKind.STEP, stepId, stepStatus = StepStatus.WAITING) + + val draft = f.proposed(action.draft(f.call("delete_step", "step_id" to stepId), f.context)) + + assertThat(draft.preview).doesNotContain("working on it").doesNotContain("already") + } + + @Test + fun `performing deletes the step by id`() = + runTest { + action.perform(f.json("step_id" to stepId), f.context) + + verify { stepService.deleteOnboardingStepById(stepId) } + } + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActionsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActionsTest.kt new file mode 100644 index 00000000..34b5b737 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActionsTest.kt @@ -0,0 +1,199 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.model.request.task.CreateOnboardingTaskRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.task.UpdateOnboardingTaskRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.task.GetOnboardingTaskResponse +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import java.util.UUID + +class TaskTeamActionsTest { + private val f = ContentFixture() + private val taskService: OnboardingTaskService = mockk(relaxed = true) + + private fun task( + id: UUID = UUID.randomUUID(), + stepId: UUID = UUID.randomUUID(), + position: Int = 0, + title: String = "Clone", + description: String = "git clone", + finished: Boolean = false, + ) = GetOnboardingTaskResponse(id, stepId, position, title, description, finished) + + @Nested + inner class Add { + private val action = AddTaskAction(f.scope, taskService) + private val stepId = UUID.randomUUID() + + private fun addCall(vararg extra: Pair) = + f.call("add_task", "step_id" to stepId, "title" to "Build", *extra) + + @Test + fun `appends by default and starts unfinished`() { + f.element(PathElementKind.STEP, stepId, title = "Install", children = 2, stepStatus = StepStatus.WAITING) + + val draft = f.proposed(action.draft(addCall(), f.context)) + + assertThat(draft.params.text("position")).isEqualTo("2") + assertThat(draft.preview).contains("“Install”", "place 3 of 3", "starts unfinished") + assertThat(draft.preview).doesNotContain("reopens") + } + + @Test + fun `adding to a finished step says it reopens the step`() { + f.element(PathElementKind.STEP, stepId, children = 1, stepStatus = StepStatus.FINISHED) + + val draft = f.proposed(action.draft(addCall(), f.context)) + + assertThat(draft.preview).contains("already finished", "reopens it", "back to in progress for Sam Rivera") + } + + @Test + fun `a skipped step is not claimed to reopen`() { + f.element(PathElementKind.STEP, stepId, children = 1, stepStatus = StepStatus.SKIPPED) + + val draft = f.proposed(action.draft(addCall(), f.context)) + + assertThat(draft.preview).doesNotContain("reopens") + } + + @Test + fun `a step on another person's path is refused`() { + f.element(PathElementKind.STEP, stepId, owner = f.outsiderId) + + val reason = f.refusal(action.draft(addCall(), f.context)) + + assertThat(reason).contains("not on the onboarding path") + } + + @Test + fun `a blank title and an impossible place are refused`() { + f.element(PathElementKind.STEP, stepId, children = 1) + + assertThat(f.refusal(action.draft(f.call("add_task", "step_id" to stepId, "title" to ""), f.context))) + .contains("needs a title") + assertThat(f.refusal(action.draft(addCall("place" to 5), f.context))).contains("from 1 to 2") + } + + @Test + fun `a confirm after the step lost tasks is turned down`() { + f.element(PathElementKind.STEP, stepId, children = 0) + + assertThat(action.recheck(f.json("step_id" to stepId, "position" to 3), f.context)) + .contains("fewer tasks") + } + + @Test + fun `performing creates the task with what was stored`() = + runTest { + val request = slot() + every { taskService.createOnboardingTaskForStepId(stepId, capture(request)) } returns + mockk(relaxed = true) + + action.perform( + f.json("step_id" to stepId, "title" to "Build", "description" to "d", "position" to 1), + f.context, + ) + + assertThat(request.captured).isEqualTo(CreateOnboardingTaskRequest(1, "Build", "d")) + } + } + + @Nested + inner class Update { + private val action = UpdateTaskAction(f.scope, taskService) + private val taskId = UUID.randomUUID() + private val stepId = UUID.randomUUID() + + private fun existing(finished: Boolean) { + f.element(PathElementKind.TASK, taskId) + every { taskService.getOnboardingTaskById(taskId) } returns + task(taskId, stepId, position = 1, finished = finished) + every { taskService.getOnboardingTasksByStepId(stepId) } returns (0 until 3).map { task(position = it) } + } + + @Test + fun `previews the change and says the tick stays as it is`() { + existing(finished = true) + + val draft = f.proposed( + action.draft(f.call("update_task", "task_id" to taskId, "title" to "Clone the repo"), f.context), + ) + + assertThat(draft.preview).contains("“Clone” becomes “Clone the repo”", "stays as it is (done)") + } + + @Test + fun `nothing new is refused`() { + existing(finished = false) + + val reason = f.refusal( + action.draft(f.call("update_task", "task_id" to taskId, "title" to "Clone"), f.context), + ) + + assertThat(reason).contains("Nothing would change") + } + + @Test + fun `there is no way to ask for a tick`() { + val properties = action.spec.parameters["properties"].toString() + + assertThat(properties).doesNotContain("finished").doesNotContain("done") + } + + @Test + fun `performing passes the tick through exactly as the task has it now`() = + runTest { + every { taskService.getOnboardingTaskById(taskId) } returns task(taskId, stepId, finished = true) + val request = slot() + every { taskService.updateOnboardingTaskById(taskId, capture(request)) } returns + mockk(relaxed = true) + + action.perform(f.json("task_id" to taskId, "title" to "Renamed"), f.context) + + assertThat(request.captured) + .isEqualTo(UpdateOnboardingTaskRequest(0, "Renamed", "git clone", finished = true)) + } + + @Test + fun `a task on another person's path is refused at proposal and at confirm`() { + f.element(PathElementKind.TASK, taskId, owner = f.outsiderId) + + assertThat(f.refusal(action.draft(f.call("update_task", "task_id" to taskId, "title" to "x"), f.context))) + .contains("not on the onboarding path") + assertThat(action.recheck(f.json("task_id" to taskId), f.context)).isNotNull() + } + } + + @Nested + inner class Delete { + private val action = DeleteTaskAction(f.scope, taskService) + private val taskId = UUID.randomUUID() + + @Test + fun `is destructive and says it cannot be undone`() { + f.element(PathElementKind.TASK, taskId, title = "Clone") + + val draft = f.proposed(action.draft(f.call("delete_task", "task_id" to taskId), f.context)) + + assertThat(action.risk).isEqualTo(BuddyProposalRisk.DESTRUCTIVE) + assertThat(draft.preview).contains("“Clone”", "cannot be undone") + } + + @Test + fun `performing deletes the task by id`() = + runTest { + action.perform(f.json("task_id" to taskId), f.context) + + verify { taskService.deleteOnboardingTaskById(taskId) } + } + } +} From f771182b364c277eea8b8cd14ceae2b2635c8535 Mon Sep 17 00:00:00 2001 From: Linus Date: Mon, 21 Sep 2026 15:31:23 +0200 Subject: [PATCH 2/4] Let the team-mode buddy answer skips, read feedback and edit checks and packets Adds the rest of the content area (backend#228): list_pending_skips, list_feedback, get_phase_checks and get_orientation_packet, and the actions accept_skip, deny_skip, delete_skip, mark_feedback_read, replace_phase_checks, author_orientation_packet and revert_orientation_packet. Accepting and denying a skip are the two actions in this area that change a hire's progress, as the admin surface they mirror does, so their previews say what happens to the step: an accepted skip marks it skipped and can finish the hire's onboarding, a denied one puts a step they had started back to waiting. A denial without a comment is refused because the hire is shown it, and the comment is quoted in full in the preview. replace_phase_checks takes the whole list because the service deletes any question it is not given, along with everybody's answers to it. Every id is checked against what the phase has, the preview lists what is kept, new and deleted, and a confirm is turned down if the phase's questions changed since. Orientation packets are keyed by task and project but the service only checks that the task exists. ContentScope resolves the task through its repository's project links, at proposal and at confirm. Authoring shows the full text the hire will read and what it replaces, and a person's packet is named as such; a confirm is turned down if the packet was replaced since the preview. Co-Authored-By: Claude Sonnet 5 --- .../onboarding/service/ContentScope.kt | 35 +- .../onboarding/service/ContentTeamTools.kt | 200 +++++++++++- .../service/MarkFeedbackReadAction.kt | 75 +++++ .../service/OrientationTeamActions.kt | 289 +++++++++++++++++ .../service/ReplacePhaseChecksAction.kt | 293 +++++++++++++++++ .../onboarding/service/ResourceTeamActions.kt | 4 +- .../onboarding/service/SkipTeamActions.kt | 266 ++++++++++++++++ .../onboarding/service/ContentFixture.kt | 16 +- .../service/ContentTeamToolsReadsTest.kt | 194 ++++++++++++ .../service/ContentTeamToolsTest.kt | 21 +- .../service/MarkFeedbackReadActionTest.kt | 104 ++++++ .../service/OrientationTeamActionsTest.kt | 298 ++++++++++++++++++ .../service/ReplacePhaseChecksActionTest.kt | 296 +++++++++++++++++ .../onboarding/service/SkipTeamActionsTest.kt | 227 +++++++++++++ 14 files changed, 2294 insertions(+), 24 deletions(-) create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/MarkFeedbackReadAction.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/OrientationTeamActions.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksAction.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/SkipTeamActions.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsReadsTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/MarkFeedbackReadActionTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/OrientationTeamActionsTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksActionTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/SkipTeamActionsTest.kt diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt index 035a0c61..302e679f 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt @@ -1,5 +1,7 @@ package com.sprintstart.sprintstartbackend.onboarding.service +import com.sprintstart.sprintstartbackend.onboarding.model.entity.StarterWorkTaskProposal +import com.sprintstart.sprintstartbackend.onboarding.repository.StarterWorkTaskProposalRepository import com.sprintstart.sprintstartbackend.user.external.ProjectMember import com.sprintstart.sprintstartbackend.user.external.ProjectMembershipApi import com.sprintstart.sprintstartbackend.user.external.UserApi @@ -28,6 +30,8 @@ class ContentScope( private val pathElements: PathElements, private val projectMembershipApi: ProjectMembershipApi, private val userApi: UserApi, + private val starterWorkTaskProposalRepository: StarterWorkTaskProposalRepository, + private val starterWorkScope: StarterWorkScope, ) { /** * The [kind] element [id] names, if it sits on the path of a member of [projectId]. @@ -45,6 +49,21 @@ class ContentScope( fun missing(kind: PathElementKind, id: UUID?, projectId: UUID): String? = goneSince(kind).takeIf { element(kind, id, projectId) == null } + /** + * The starter-work task [taskId] names, if its repository is linked to [projectId]. + * + * An orientation packet belongs to a task and a project together, but the services behind it only + * check that the task exists. The task's source is what ties it to a project, so that is what is + * asked here — the same question the starter-work area asks of the pool. + */ + fun proposal(taskId: UUID?, projectId: UUID): StarterWorkTaskProposal? = + taskId + ?.let { starterWorkTaskProposalRepository.findById(it).orElse(null) } + ?.takeIf { starterWorkScope.covers(it.sourceId, projectId) } + + /** Everyone on [projectId]. */ + fun members(projectId: UUID): List = projectMembershipApi.getProjectMembers(projectId) + /** The member [memberId] names on [projectId], or null. */ fun member(memberId: UUID?, projectId: UUID): ProjectMember? = memberId?.let { id -> projectMembershipApi.getProjectMembers(projectId).firstOrNull { it.userId == id } } @@ -81,12 +100,22 @@ internal const val NOT_A_MEMBER_HERE = "That person is not on this project. Call find_member for the people who are, and pass the member_id " + "it gives." +internal const val TASK_NOT_HERE = + "That task is not from a repository linked to this project. Call list_starter_work_pool for the tasks " + + "that are, and pass the task_id it gives." + internal const val LEFT_SINCE_HERE = "That person is no longer on this project, so nothing was changed." /** What to tell the model when an id is not in scope, per kind. */ -internal fun notInScope(kind: PathElementKind): String = - "That ${kind.noun} is not on the onboarding path of anybody on this project. Call get_member_path for " + - "somebody who is, and pass an id from it." +internal fun notInScope(kind: PathElementKind): String { + val where = when (kind) { + PathElementKind.SKIP -> "list_pending_skips" + PathElementKind.FEEDBACK -> "list_feedback" + else -> "get_member_path for somebody who is" + } + return "That ${kind.noun} is not on the onboarding path of anybody on this project. Call $where, and pass " + + "an id from it." +} /** Why a stored proposal's target is no longer there: gone, or no longer on a member's path. */ internal fun goneSince(kind: PathElementKind): String = diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt index 2ae0a5f4..6bd1e946 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt @@ -1,35 +1,92 @@ package com.sprintstart.sprintstartbackend.onboarding.service +import com.sprintstart.sprintstartbackend.onboarding.external.enums.SkipStatus import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import com.sprintstart.sprintstartbackend.onboarding.model.response.orientation.OrientationPacketResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.question.QuestionForAdminResponse +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put import org.springframework.stereotype.Component import org.springframework.web.server.ResponseStatusException +import java.time.ZoneOffset import java.util.UUID +private const val SHORT_CHARS = 200 + +private fun StepStatus.said(): String = name.lowercase().replace('_', ' ') + +private fun String.short(): String = if (length <= SHORT_CHARS) this else take(SHORT_CHARS - 1).trimEnd() + "…" + +/** Questions with the ids and correct answers a replacement has to start from. */ +private fun renderChecks(questions: List): String = + buildString { + questions.forEachIndexed { index, question -> + appendLine( + "${index + 1}. ${question.question} [question_id: ${question.id}] — " + + question.type.name + .lowercase() + .replace('_', ' '), + ) + question.correctAnswer?.let { appendLine(" Correct answer: $it") } + question.options.forEach { + appendLine(" ${if (it.correct) "[correct]" else "[wrong] "} ${it.label} [option_id: ${it.id}]") + } + question.explanation?.let { appendLine(" Explanation: $it") } + } + }.trim() + +/** A packet in full: replacing it means starting from every word of it. */ +private fun renderPacket(packet: OrientationPacketResponse): String = + buildString { + appendLine("Packet for “${packet.taskTitle}”, ${packet.origin.author()}.") + packet.summary?.let { appendLine("Summary: $it") } + packet.sections.forEach { section -> + appendLine() + appendLine("${section.step.name.lowercase().replace('_', ' ')} — ${section.title}") + appendLine(section.body) + section.citations.forEach { + appendLine( + " source: ${it.filename}${it.sourceUrl?.let { u -> " <$u>" }.orEmpty()}", + ) + } + } + }.trim() + /** * The read tools of team mode's content area: what is on the onboarding paths of the project's - * members. + * members, and what waits for the manager to answer. * * Paths belong to people, not projects, so everything here starts from a member of the turn's project - * and reads only what is on that member's path. The ids these print are how the content actions name - * their targets, and every action checks again that its target is on a member's path. + * and reads only what is on that member's path — or, for an orientation packet, from a task whose + * repository is linked to the project. The ids these print are how the content actions name their + * targets, and every action checks again that its target is in scope. */ @Component class ContentTeamTools( private val scope: ContentScope, + private val pathElements: PathElements, private val onboardingPathService: OnboardingPathService, private val onboardingStepService: OnboardingStepService, + private val onboardingSkipService: OnboardingSkipService, + private val onboardingFeedbackService: OnboardingFeedbackService, + private val questionAttemptService: QuestionAttemptService, + private val taskOrientationService: TaskOrientationService, ) : TeamAreaTools { override val area = TeamArea.CONTENT - override fun toolSpecs(): List = listOf(GET_MEMBER_PATH_SPEC) + override fun toolSpecs(): List = SPECS - override fun handles(toolName: String): Boolean = toolName == GET_MEMBER_PATH + override fun handles(toolName: String): Boolean = SPECS.any { it.name == toolName } override fun execute(call: BuddyToolCallDto, context: TeamToolContext): String = when (call.name) { GET_MEMBER_PATH -> memberPath(call.uuidArgument("member_id"), context.projectId) + LIST_PENDING_SKIPS -> pendingSkips(context.projectId) + LIST_FEEDBACK -> feedback(call.booleanArgument("include_read") == true, context.projectId) + GET_PHASE_CHECKS -> phaseChecks(call.uuidArgument("phase_id"), context.projectId) + GET_ORIENTATION_PACKET -> orientationPacket(call.uuidArgument("task_id"), context.projectId) else -> "Unknown tool: ${call.name}." } @@ -71,22 +128,135 @@ class ContentTeamTools( }.trim() } - private fun StepStatus.said(): String = name.lowercase().replace('_', ' ') + /** Requests to skip a step that nobody has answered, from members of this project only. */ + private fun pendingSkips(projectId: UUID): String { + val pending = scope.members(projectId).flatMap { member -> + onboardingSkipService + .getAllSkipsByUserId(member.userId) + .filter { it.status == SkipStatus.PENDING } + .map { member to it } + } + if (pending.isEmpty()) return "Nobody on this project has a skip request waiting for an answer." + + return buildString { + appendLine("Skip requests waiting for an answer, oldest first:") + pending.sortedBy { it.second.createdAt }.forEach { (member, skip) -> + val step = pathElements.find(PathElementKind.STEP, skip.stepId) + append("- ${member.displayName} asks to skip “${step?.title ?: "a step"}”") + step?.stepStatus?.let { append(" (${it.said()})") } + appendLine(" [skip_id: ${skip.id}]") + appendLine(" asked ${skip.createdAt.atZone(ZoneOffset.UTC).toLocalDate()}: “${skip.reason}”") + } + }.trim() + } + + /** Feedback hires left on their onboarding, unread first — the read ones only when asked for. */ + private fun feedback(includeRead: Boolean, projectId: UUID): String { + val all = scope.members(projectId).flatMap { member -> + onboardingFeedbackService.getAllFeedbackByUserId(member.userId).map { member to it } + } + val shown = all.filter { includeRead || !it.second.read } + if (shown.isEmpty()) { + return if (all.isEmpty()) { + "Nobody on this project has left onboarding feedback." + } else { + "There is no unread feedback. Pass include_read to see the ${all.size} already read." + } + } + + return buildString { + appendLine("Onboarding feedback from this project's members, oldest first:") + shown.sortedBy { it.second.createdAt }.forEach { (member, item) -> + val about = item.stepTitle?.let { "“$it”" } ?: "their path as a whole" + appendLine( + "- ${member.displayName} on $about [feedback_id: ${item.id}]${if (item.read) " (read)" else ""}", + ) + appendLine(" ${item.createdAt.atZone(ZoneOffset.UTC).toLocalDate()}: “${item.message}”") + } + }.trim() + } + + /** A phase's knowledge-check questions, with correct answers — the starting point of any replacement. */ + private fun phaseChecks(phaseId: UUID?, projectId: UUID): String { + val target = + scope.element(PathElementKind.PHASE, phaseId, projectId) ?: return notInScope(PathElementKind.PHASE) + val questions = questionAttemptService.getPhaseQuestions(target.element.id).questions + if (questions.isEmpty()) return "“${target.element.title}” has no knowledge-check questions." + return "Knowledge checks of “${target.element.title}” on ${target.owner.displayName}'s path:\n" + + renderChecks(questions.sortedBy { it.position }) + } - private fun String.short(): String = if (length <= SHORT_CHARS) this else take(SHORT_CHARS - 1).trimEnd() + "…" + /** The orientation packet a hire gets for a task, or the fact that there is none. */ + private fun orientationPacket(taskId: UUID?, projectId: UUID): String { + val task = scope.proposal(taskId, projectId) ?: return TASK_NOT_HERE + val packet = taskOrientationService.getForAuthoring(task.id, projectId).packet + ?: return "There is no orientation packet for “${task.title}” on this project yet. One is assembled " + + "from the docs the first time a hire opens the task, or a person can write it." + return renderPacket(packet) + } companion object { const val GET_MEMBER_PATH = "get_member_path" + const val LIST_PENDING_SKIPS = "list_pending_skips" + const val LIST_FEEDBACK = "list_feedback" + const val GET_PHASE_CHECKS = "get_phase_checks" + const val GET_ORIENTATION_PACKET = "get_orientation_packet" - private const val SHORT_CHARS = 200 + private fun noArgs() = + buildJsonObject { + put("type", "object") + put("properties", buildJsonObject { }) + } - val GET_MEMBER_PATH_SPEC = BuddyToolSpecDto( - name = GET_MEMBER_PATH, - description = "One project member's whole onboarding path: its phases, their steps, and each " + - "step's tasks and links, every one with the id an action needs. Also says where each step " + - "stands for them. Use the member_id from find_member. A path belongs to the person, not to " + - "this project, so it shows what they have across all of theirs.", - parameters = stringFields("member_id" to "The member_id from find_member.", required = listOf("member_id")), + private val SPECS = listOf( + BuddyToolSpecDto( + name = GET_MEMBER_PATH, + description = "One project member's whole onboarding path: its phases, their steps, and each " + + "step's tasks and links, every one with the id an action needs. Also says where each step " + + "stands for them. Use the member_id from find_member. A path belongs to the person, not to " + + "this project, so it shows what they have across all of theirs.", + parameters = stringFields( + "member_id" to "The member_id from find_member.", + required = listOf("member_id"), + ), + ), + BuddyToolSpecDto( + name = LIST_PENDING_SKIPS, + description = "The requests to skip a step that hires on this project are waiting to have " + + "answered, each with the reason the hire gave and a skip_id. Read one before offering to " + + "accept or deny it. Takes no arguments.", + parameters = noArgs(), + ), + BuddyToolSpecDto( + name = LIST_FEEDBACK, + description = "Feedback hires on this project left on their onboarding, each with a " + + "feedback_id. Unread only unless include_read is true. Use it for 'what are hires saying " + + "about onboarding?'; quote them rather than summarising away what they said.", + parameters = toolFields( + ToolField("include_read", "True to include feedback already marked as read.", "boolean"), + required = emptyList(), + ), + ), + BuddyToolSpecDto( + name = GET_PHASE_CHECKS, + description = "The knowledge-check questions at the end of one phase of a member's path, with " + + "their correct answers and ids. Read it before offering replace_phase_checks, which takes " + + "the whole list. Use the phase_id from get_member_path.", + parameters = stringFields( + "phase_id" to "The phase_id from get_member_path.", + required = listOf("phase_id"), + ), + ), + BuddyToolSpecDto( + name = GET_ORIENTATION_PACKET, + description = "The orientation packet hires get for one starter-work task on this project, in " + + "full, and whether a person or the AI wrote it. Read it before offering to write or drop " + + "one. Use the task_id from list_starter_work_pool.", + parameters = stringFields( + "task_id" to "The task_id from list_starter_work_pool.", + required = listOf("task_id"), + ), + ), ) } } diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/MarkFeedbackReadAction.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/MarkFeedbackReadAction.kt new file mode 100644 index 00000000..a1e0f08a --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/MarkFeedbackReadAction.kt @@ -0,0 +1,75 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.springframework.stereotype.Component +import java.util.UUID + +/** + * Offers to mark a hire's feedback as read. + * + * The admin service has no read-by-id, and its by-id write loads the feedback without asking whose it + * is — so the owner is resolved through [ContentScope] first, and the message is read back through + * that owner's own feedback so the preview can show what is being marked. + */ +@Component +class MarkFeedbackReadAction( + private val scope: ContentScope, + private val onboardingFeedbackService: OnboardingFeedbackService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "mark_feedback_read", + description = "Offer to mark one piece of a hire's onboarding feedback as read, once the manager has " + + "seen it. It only clears it from the unread list; nothing is sent to the hire. Use the " + + "feedback_id from list_feedback. This does NOT mark anything by itself; the manager confirms.", + parameters = stringFields( + "feedback_id" to "The feedback_id from list_feedback.", + required = listOf("feedback_id"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val scoped = scope.element(PathElementKind.FEEDBACK, call.uuidArgument("feedback_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.FEEDBACK)) + val feedback = onboardingFeedbackService + .getAllFeedbackByUserId(scoped.owner.userId) + .firstOrNull { it.id == scoped.element.id } + ?: return TeamActionDraft.Refused(goneSince(PathElementKind.FEEDBACK)) + if (feedback.read) return TeamActionDraft.Refused("That feedback is already marked as read.") + + return TeamActionDraft.Proposed( + params = buildJsonObject { put("feedback_id", feedback.id.toString()) }, + label = "Mark ${scoped.owner.displayName.forLabel()}'s feedback read", + preview = buildString { + appendLine("Mark this feedback from ${scoped.owner.displayName} as read:") + appendLine( + "On ${feedback.stepTitle?.let { "“$it”" } ?: "their path as a whole"}: “${feedback.message}”", + ) + appendLine() + append("It only leaves the unread list. They are not told, and nothing else changes.") + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? { + val scoped = scope.element(PathElementKind.FEEDBACK, params.uuid("feedback_id"), context.projectId) + ?: return goneSince(PathElementKind.FEEDBACK) + val alreadyRead = onboardingFeedbackService + .getAllFeedbackByUserId(scoped.owner.userId) + .firstOrNull { it.id == scoped.element.id } + ?.read + return "Somebody already marked that feedback as read, so nothing was changed.".takeIf { alreadyRead == true } + } + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + val id: UUID = requireNotNull(params.uuid("feedback_id")) + onboardingFeedbackService.markFeedbackAsRead(id) + return "Marked as read." + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/OrientationTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/OrientationTeamActions.kt new file mode 100644 index 00000000..f6166cfa --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/OrientationTeamActions.kt @@ -0,0 +1,289 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.enums.OrientationOrigin +import com.sprintstart.sprintstartbackend.onboarding.external.enums.OrientationStep +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import com.sprintstart.sprintstartbackend.onboarding.model.request.orientation.AuthorOrientationCitationRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.orientation.AuthorOrientationRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.orientation.AuthorOrientationSectionRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.orientation.OrientationPacketResponse +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.add +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonArray +import kotlinx.serialization.json.putJsonObject +import org.springframework.stereotype.Component +import java.util.UUID + +/* + * The orientation-packet actions of team mode's content area. + * + * A packet is keyed by a starter-work task and a project, but the service behind it only checks that + * the task exists. The task's source is what ties it to a project, so both actions resolve the task + * through ContentScope — its repository must be linked to the turn's project — at proposal and at + * confirm. + * + * A packet somebody wrote is served exactly as written and never regenerated, so replacing or dropping + * one throws that person's text away. The previews say so, and a confirm is turned down if the packet + * was replaced since the preview. + */ + +private const val PACKET_CHANGED = + "The packet for that task changed since this was proposed, so nothing was changed. Call " + + "get_orientation_packet and offer it again." + +private val ORIENTATION_STEPS = OrientationStep.entries.map { it.name } + +private fun orientationStepOf(text: String): OrientationStep? = + OrientationStep.entries.firstOrNull { it.name.equals(text, ignoreCase = true) } + +/** Who wrote a packet, as the previews say it. */ +internal fun OrientationOrigin.author(): String = if (this == + OrientationOrigin.HUMAN +) { + "written by a person" +} else { + "assembled by the AI" +} + +/** What identifies the packet a preview was written against; changes whenever the packet is replaced. */ +private fun OrientationPacketResponse?.version(): String = this?.assembledAt?.toString().orEmpty() + +private fun currentPacket(service: TaskOrientationService, taskId: UUID, projectId: UUID): OrientationPacketResponse? = + service.getForAuthoring(taskId, projectId).packet + +/** The sections the model gave, or the reason they cannot be used. */ +private fun readSections(raw: List): Pair?, String?> { + if (raw.isEmpty()) return null to "A packet needs at least one section." + val sections = raw.mapIndexed { index, item -> + val step = orientationStepOf(item.text("step")) + ?: return null to "Section ${index + 1} needs a step: ${ORIENTATION_STEPS.joinToString(", ")}." + if (item.text("title").isEmpty() || item.text("body").isEmpty()) { + return null to "Section ${index + 1} needs both a title and a body." + } + val citations = item.objectArray("citations").map { citation -> + val url = citation.text("source_url") + if (url.isNotEmpty() && + !isWebLink(url) + ) { + return null to "Section ${index + 1} has a link that is not a web address." + } + if (citation.text("filename").isEmpty()) return null to "Section ${index + 1} has a source with no name." + AuthorOrientationCitationRequest(citation.text("filename"), url.takeIf { it.isNotEmpty() }) + } + AuthorOrientationSectionRequest(step, item.text("title"), item.text("body"), citations) + } + return sections to null +} + +private fun AuthorOrientationRequest.stored(): JsonObject = + buildJsonObject { + summary?.let { put("summary", it) } + putJsonArray("sections") { + sections.forEach { section -> + add( + buildJsonObject { + put("step", section.step.name) + put("title", section.title) + put("body", section.body) + putJsonArray("citations") { + section.citations.forEach { citation -> + add( + buildJsonObject { + put("filename", citation.filename) + citation.sourceUrl?.let { put("source_url", it) } + }, + ) + } + } + }, + ) + } + } + } + +/** The sections as the hire will read them, in full — what the manager is agreeing to is the text. */ +private fun AuthorOrientationRequest.readable(): String = + buildString { + summary?.let { appendLine("Summary: $it") } + sections.forEach { section -> + appendLine() + appendLine("${section.step.name.lowercase().replace('_', ' ')} — ${section.title}") + appendLine(section.body) + section.citations.forEach { + appendLine( + " source: ${it.filename}${it.sourceUrl?.let { url -> " <$url>" }.orEmpty()}", + ) + } + } + }.trim() + +private fun orientationSectionSchema() = + buildJsonObject { + put("type", "object") + putJsonObject("properties") { + putJsonObject("step") { + put("type", "string") + put("description", "Which step of the path to a first pull request this section belongs to.") + putJsonArray("enum") { ORIENTATION_STEPS.forEach { add(it) } } + } + putJsonObject("title") { put("type", "string") } + putJsonObject("body") { put("type", "string") } + putJsonObject("citations") { + put("type", "array") + put("description", "Optional sources to point at.") + putJsonObject("items") { + put("type", "object") + putJsonObject("properties") { + putJsonObject("filename") { put("type", "string") } + putJsonObject("source_url") { put("type", "string") } + } + } + } + } + } + +/** Offers to write the orientation packet a hire gets for a starter-work task. */ +@Component +class AuthorOrientationPacketAction( + private val scope: ContentScope, + private val taskOrientationService: TaskOrientationService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "author_orientation_packet", + description = "Offer to write the orientation packet hires get for a starter-work task on this project, " + + "replacing whatever packet is there. Once written it is served exactly as written and the AI never " + + "rewrites it. Call get_orientation_packet first and start from what is there. Use the task_id from " + + "list_starter_work_pool. This does NOT save anything by itself; the manager confirms the full text.", + parameters = buildJsonObject { + put("type", "object") + putJsonObject("properties") { + putJsonObject("task_id") { + put("type", "string") + put("description", "The task_id from list_starter_work_pool.") + } + putJsonObject("summary") { + put("type", "string") + put("description", "Optional. A short overview shown above the sections.") + } + putJsonObject("sections") { + put("type", "array") + put("description", "At least one, in reading order.") + put("items", orientationSectionSchema()) + } + } + putJsonArray("required") { + add("task_id") + add("sections") + } + }, + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val task = scope.proposal(call.uuidArgument("task_id"), context.projectId) + ?: return TeamActionDraft.Refused(TASK_NOT_HERE) + val (sections, problem) = readSections(call.arguments.objectArray("sections")) + if (sections == null) return TeamActionDraft.Refused(problem.orEmpty()) + val request = AuthorOrientationRequest(call.textArgument("summary").takeIf { it.isNotEmpty() }, sections) + val existing = currentPacket(taskOrientationService, task.id, context.projectId) + + return TeamActionDraft.Proposed( + params = buildJsonObject { + put("task_id", task.id.toString()) + // What the packet was when this was written, so a confirm can tell it was replaced since. + put("base_version", existing.version()) + put("packet", request.stored()) + }, + label = "Write the orientation for “${task.title.forLabel()}”", + preview = buildString { + appendLine("Write the orientation packet for “${task.title}” on this project. Hires read this:") + appendLine() + appendLine(request.readable()) + appendLine() + existing?.let { + appendLine("It replaces the packet that is there now, ${it.origin.author()}.") + } ?: appendLine("There is no packet for this task yet.") + append("Once saved it is served exactly as written; the AI does not regenerate it.") + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? { + val task = scope.proposal(params.uuid("task_id"), context.projectId) ?: return TASK_NOT_HERE + val now = currentPacket(taskOrientationService, task.id, context.projectId).version() + return PACKET_CHANGED.takeIf { now != params.text("base_version") } + } + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + val packet = params["packet"] as JsonObject + val (sections, _) = readSections(packet.objectArray("sections")) + taskOrientationService.authorPacket( + requireNotNull(params.uuid("task_id")), + context.projectId, + AuthorOrientationRequest(packet.text("summary").takeIf { it.isNotEmpty() }, requireNotNull(sections)), + ) + return "Saved. Hires get exactly this text for the task." + } +} + +/** Offers to drop a task's orientation packet. */ +@Component +class RevertOrientationPacketAction( + private val scope: ContentScope, + private val taskOrientationService: TaskOrientationService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.DESTRUCTIVE + override val spec = BuddyToolSpecDto( + name = "revert_orientation_packet", + description = "Offer to drop the orientation packet for a starter-work task on this project, so the " + + "next hire who opens it gets one assembled from the docs again. If a person wrote the packet, " + + "their text is deleted. Use the task_id from list_starter_work_pool. This does NOT drop anything " + + "by itself; the manager confirms.", + parameters = stringFields( + "task_id" to "The task_id from list_starter_work_pool.", + required = listOf("task_id"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val task = scope.proposal(call.uuidArgument("task_id"), context.projectId) + ?: return TeamActionDraft.Refused(TASK_NOT_HERE) + val existing = currentPacket(taskOrientationService, task.id, context.projectId) + ?: return TeamActionDraft.Refused("There is no packet for that task, so there is nothing to drop.") + + return TeamActionDraft.Proposed( + params = buildJsonObject { + put("task_id", task.id.toString()) + put("base_version", existing.version()) + }, + label = "Drop the orientation for “${task.title.forLabel()}”", + preview = buildString { + appendLine( + "Drop the orientation packet for “${task.title}” on this project, ${existing.origin.author()}.", + ) + appendLine() + if (existing.origin == OrientationOrigin.HUMAN) { + appendLine("A person wrote it, and their text is deleted. This cannot be undone.") + } + append("The next hire who opens the task gets a packet assembled from the docs again.") + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? { + val task = scope.proposal(params.uuid("task_id"), context.projectId) ?: return TASK_NOT_HERE + val now = currentPacket(taskOrientationService, task.id, context.projectId).version() + return PACKET_CHANGED.takeIf { now != params.text("base_version") } + } + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + taskOrientationService.revertToAi(requireNotNull(params.uuid("task_id")), context.projectId) + return "Dropped. The next hire gets a packet assembled from the docs." + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksAction.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksAction.kt new file mode 100644 index 00000000..dcdf28fc --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksAction.kt @@ -0,0 +1,293 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.enums.CheckQuestionType +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import com.sprintstart.sprintstartbackend.onboarding.model.request.question.UpdateOptionRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.question.UpdatePhaseQuestionsRequest +import com.sprintstart.sprintstartbackend.onboarding.model.request.question.UpdateQuestionRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.question.QuestionForAdminResponse +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.add +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonArray +import kotlinx.serialization.json.putJsonObject +import org.springframework.http.HttpStatus +import org.springframework.stereotype.Component +import org.springframework.web.server.ResponseStatusException +import java.util.UUID + +/* + * Replacing a phase's knowledge checks. + * + * The service takes the whole list: a question it is given with a known id is edited in place, one + * without an id is created, and one it is *not* given is deleted — along with the hire's history on it, + * and any link that made a step wait for it. So the action takes the whole list too, checks every id in + * it against what the phase has, and previews what stays, what is new and what goes. + */ + +private const val CHECKS_CHANGED = + "The phase's questions changed since this was proposed, so nothing was changed. Call get_phase_checks " + + "and offer it again." + +private val QUESTION_TYPES = CheckQuestionType.entries.map { it.name } + +/** What reading the model's questions made of them. */ +private sealed interface Checks { + data class Valid( + val questions: List, + ) : Checks + + data class Invalid( + val reason: String, + ) : Checks +} + +private fun questionTypeOf(text: String): CheckQuestionType? = + CheckQuestionType.entries.firstOrNull { it.name.equals(text, ignoreCase = true) } + +/** The reason [question] cannot be stored as asked, or null. Mirrors what the service refuses at the write. */ +private fun problemWith(question: UpdateQuestionRequest, number: Int): String? = + when { + question.question.isBlank() -> "Question $number has no text." + question.type == CheckQuestionType.SHORT_TEXT && question.correctAnswer.isNullOrBlank() -> + "Question $number is a short-text question and needs a correct_answer." + question.type == CheckQuestionType.MULTIPLE_CHOICE && question.options.size < 2 -> + "Question $number is multiple choice and needs at least two options." + question.type == CheckQuestionType.MULTIPLE_CHOICE && question.options.none { it.correct } -> + "Question $number is multiple choice and needs at least one correct option." + question.options.any { it.label.isBlank() } -> "Question $number has an option with no text." + else -> null + } + +private fun readOption(raw: JsonObject, index: Int) = + UpdateOptionRequest( + id = raw.uuid("id"), + position = index, + label = raw.text("label"), + correct = raw.boolean("correct") == true, + ) + +/** One question as the model wrote it, or the reason it cannot be read; position is its place in the list. */ +private fun readQuestion(raw: JsonObject, index: Int): Pair { + val type = questionTypeOf(raw.text("type")) + ?: return null to "Question ${index + 1} needs a type: ${QUESTION_TYPES.joinToString(" or ")}." + if (raw.text("id").isNotEmpty() && raw.uuid("id") == null) { + return null to "Question ${index + 1} has an id that is not one from get_phase_checks." + } + return UpdateQuestionRequest( + id = raw.uuid("id"), + position = index, + type = type, + question = raw.text("question"), + explanation = raw.text("explanation").takeIf { it.isNotEmpty() }, + correctAnswer = raw.text("correct_answer").takeIf { type == CheckQuestionType.SHORT_TEXT && it.isNotEmpty() }, + options = if (type == CheckQuestionType.MULTIPLE_CHOICE) { + raw.objectArray("options").mapIndexed { i, option -> readOption(option, i) } + } else { + emptyList() + }, + ) to null +} + +/** Why an id in [question] is not one the phase has, so nothing is silently created or lost; null when all are. */ +private fun unknownId(question: UpdateQuestionRequest, current: List, number: Int): String? { + val id = question.id ?: return null + val existing = current.firstOrNull { it.id == id } + ?: return "Question $number has an id that is not one of this phase's questions. Call get_phase_checks " + + "and use its ids, or leave the id out for a new question." + val known = existing.options.map { it.id }.toSet() + return "Question $number has an option id that is not one of that question's options." + .takeIf { question.options.any { it.id != null && it.id !in known } } +} + +private fun readChecks(raw: List, current: List): Checks { + val questions = mutableListOf() + raw.forEachIndexed { index, item -> + val (question, unreadable) = readQuestion(item, index) + val problem = unreadable ?: question?.let { problemWith(it, index + 1) ?: unknownId(it, current, index + 1) } + if (problem != null || question == null) return Checks.Invalid(problem.orEmpty()) + questions += question + } + val ids = questions.mapNotNull { it.id } + return if (ids.size != ids.toSet().size) { + Checks.Invalid("The same question id is listed twice.") + } else { + Checks.Valid(questions) + } +} + +/** The stored form of a valid list: the same shape the model sent, so a confirm reads it the same way. */ +private fun UpdateQuestionRequest.stored(): JsonObject = + buildJsonObject { + id?.let { put("id", it.toString()) } + put("type", type.name) + put("question", question) + explanation?.let { put("explanation", it) } + correctAnswer?.let { put("correct_answer", it) } + if (options.isNotEmpty()) { + putJsonArray("options") { + options.forEach { option -> + add( + buildJsonObject { + option.id?.let { put("id", it.toString()) } + put("label", option.label) + put("correct", option.correct) + }, + ) + } + } + } + } + +private fun UpdateQuestionRequest.described(number: Int, kept: Boolean): String = + buildString { + append("$number. ${if (kept) "(kept) " else "(new) "}${type.name.lowercase().replace('_', ' ')}: $question") + correctAnswer?.let { append("\n Correct answer: $it") } + options.forEach { append("\n ${if (it.correct) "[correct]" else "[wrong] "} ${it.label}") } + explanation?.let { append("\n Explanation: $it") } + } + +/** Offers to replace the knowledge-check questions of a phase on a member's path. */ +@Component +class ReplacePhaseChecksAction( + private val scope: ContentScope, + private val questionAttemptService: QuestionAttemptService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "replace_phase_checks", + description = "Offer to replace the knowledge-check questions at the end of a phase on a member's " + + "onboarding path. It takes the WHOLE new list: a question with its id is edited in place, one " + + "without an id is new, and any current question left out is deleted with the hire's answers to " + + "it. Always call get_phase_checks first and start from what it shows. This does NOT change " + + "anything by itself; the manager confirms.", + parameters = buildJsonObject { + put("type", "object") + putJsonObject("properties") { + putJsonObject("phase_id") { + put("type", "string") + put("description", "The phase_id from get_member_path.") + } + putJsonObject("questions") { + put("type", "array") + put("description", "The complete list of questions, in order. An empty list removes them all.") + putJsonObject("items") { + put("type", "object") + putJsonObject("properties") { + putJsonObject("id") { + put("type", "string") + put("description", "The question's id from get_phase_checks; leave out for a new one.") + } + putJsonObject("type") { + put("type", "string") + putJsonArray("enum") { QUESTION_TYPES.forEach { add(it) } } + } + putJsonObject("question") { put("type", "string") } + putJsonObject("explanation") { + put("type", "string") + put("description", "Optional. Shown to the hire after they answer.") + } + putJsonObject("correct_answer") { + put("type", "string") + put("description", "SHORT_TEXT only: the answer that counts as right.") + } + putJsonObject("options") { + put("type", "array") + put("description", "MULTIPLE_CHOICE only: at least two, at least one correct.") + putJsonObject("items") { + put("type", "object") + putJsonObject("properties") { + putJsonObject("id") { put("type", "string") } + putJsonObject("label") { put("type", "string") } + putJsonObject("correct") { put("type", "boolean") } + } + } + } + } + } + } + } + putJsonArray("required") { + add("phase_id") + add("questions") + } + }, + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val target = scope.element(PathElementKind.PHASE, call.uuidArgument("phase_id"), context.projectId) + ?: return TeamActionDraft.Refused(notInScope(PathElementKind.PHASE)) + if (call.arguments["questions"] !is JsonArray) { + return TeamActionDraft.Refused( + "Pass questions as the complete list, even an empty one. Call get_phase_checks to see the " + + "current ones.", + ) + } + val current = questionAttemptService.getPhaseQuestions(target.element.id).questions + val questions = when (val read = readChecks(call.arguments.objectArray("questions"), current)) { + is Checks.Invalid -> return TeamActionDraft.Refused(read.reason) + is Checks.Valid -> read.questions + } + if (questions.isEmpty() && current.isEmpty()) { + return TeamActionDraft.Refused("The phase has no questions and none were given, so nothing would change.") + } + + return TeamActionDraft.Proposed( + params = buildJsonObject { + put("phase_id", target.element.id.toString()) + // What the phase had when this was written, so a confirm can tell it changed since. + putJsonArray("base_ids") { current.forEach { add(it.id.toString()) } } + putJsonArray("questions") { questions.forEach { add(it.stored()) } } + }, + label = "Replace the checks of “${target.element.title.forLabel()}”", + preview = previewOf(target, questions, current, scope.sharedNote(target.owner, context.projectId)), + ) + } + + private fun previewOf( + target: ScopedElement, + questions: List, + current: List, + sharedNote: String, + ): String = + buildString { + appendLine( + "Replace the knowledge checks of “${target.element.title}” on ${target.owner.displayName}'s path.", + ) + appendLine("It will have ${questions.size} question${if (questions.size == 1) "" else "s"}:") + questions.forEachIndexed { i, q -> appendLine(q.described(i + 1, kept = q.id != null)) } + val removed = current.filter { existing -> questions.none { it.id == existing.id } } + if (removed.isNotEmpty()) { + appendLine() + appendLine("These are deleted, with everybody's past answers to them:") + removed.forEach { appendLine("- ${it.question}") } + appendLine("A step that was waiting on one of them no longer waits for it.") + } + if (sharedNote.isNotEmpty()) append("\n$sharedNote") + }.trim() + + override fun recheck(params: JsonObject, context: TeamToolContext): String? { + val phaseId = params.uuid("phase_id") + scope.missing(PathElementKind.PHASE, phaseId, context.projectId)?.let { return it } + val base = (params["base_ids"] as? JsonArray).orEmpty().mapNotNull { (it as? JsonPrimitive)?.content }.toSet() + val now = questionAttemptService.getPhaseQuestions(requireNotNull(phaseId)).questions.map { it.id.toString() } + return CHECKS_CHANGED.takeIf { now.toSet() != base } + } + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + val phaseId: UUID = requireNotNull(params.uuid("phase_id")) + val current = questionAttemptService.getPhaseQuestions(phaseId).questions + val questions = when (val read = readChecks(params.objectArray("questions"), current)) { + is Checks.Invalid -> throw ResponseStatusException(HttpStatus.BAD_REQUEST, read.reason) + is Checks.Valid -> read.questions + } + questionAttemptService.replacePhaseQuestions(phaseId, UpdatePhaseQuestionsRequest(questions)) + return "Done. The phase now has ${questions.size} question${if (questions.size == 1) "" else "s"}." + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActions.kt index ef6c5871..37d54728 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActions.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ResourceTeamActions.kt @@ -19,10 +19,10 @@ import java.net.URI * be a web link, so these refuse anything else at proposal. */ -private const val NOT_A_WEB_LINK = "The url must be a full web address starting with http:// or https://." +internal const val NOT_A_WEB_LINK = "The url must be a full web address starting with http:// or https://." /** Whether [text] is an absolute http(s) address with a host. */ -private fun isWebLink(text: String): Boolean = +internal fun isWebLink(text: String): Boolean = runCatching { URI(text) }.getOrNull()?.let { (it.scheme == "http" || it.scheme == "https") && !it.host.isNullOrBlank() } ?: false diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/SkipTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/SkipTeamActions.kt new file mode 100644 index 00000000..aa0ca27c --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/SkipTeamActions.kt @@ -0,0 +1,266 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.enums.SkipStatus +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolSpecDto +import com.sprintstart.sprintstartbackend.onboarding.model.request.skip.ReviewOnboardingSkipRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.skip.GetOnboardingSkipResponse +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.springframework.stereotype.Component +import org.springframework.web.server.ResponseStatusException +import java.util.UUID + +/* + * The skip-request actions of team mode's content area. + * + * A hire asks to skip a step and the request waits for somebody to answer it. Accepting and denying + * are the two places in this area where an action changes a hire's progress — an accepted skip marks + * the step skipped, which counts as done — and both are explicitly the manager's decision in the + * admin surface these mirror. Each preview says what the answer does to the step. + * + * Only a pending request can be answered or deleted; the services say so at the write, and these + * say so at proposal and again at confirm, so a request somebody else answered in between is turned + * down rather than failing halfway. + */ + +private const val NOT_PENDING = "That skip request has already been answered, so there is nothing to decide." + +/** A pending skip request on a member's path. */ +private class PendingSkip( + val scoped: ScopedElement, + val skip: GetOnboardingSkipResponse, +) + +/** What looking up a skip request found. */ +private sealed interface SkipLookup { + data class Found( + val pending: PendingSkip, + ) : SkipLookup + + data class Refused( + val reason: String, + ) : SkipLookup +} + +private fun lookUpPendingSkip( + scope: ContentScope, + skips: OnboardingSkipService, + id: UUID?, + projectId: UUID, +): SkipLookup { + val scoped = scope.element(PathElementKind.SKIP, id, projectId) + ?: return SkipLookup.Refused(notInScope(PathElementKind.SKIP)) + // Gone between the scope check and this read is the same answer as gone before it. + val skip = try { + skips.getSkipById(scoped.element.id) + } catch (_: ResponseStatusException) { + return SkipLookup.Refused(goneSince(PathElementKind.SKIP)) + } + if (skip.status != SkipStatus.PENDING) return SkipLookup.Refused(NOT_PENDING) + return SkipLookup.Found(PendingSkip(scoped, skip)) +} + +/** What the step's status says about what an answer does, beyond what the answer itself does. */ +private fun StepStatus?.beforeDenying(name: String): String = + if (this == StepStatus.IN_PROGRESS) { + "$name had started it; denying puts the step back to waiting. " + } else { + "" + } + +private fun SkipLookup.pendingOrNull(): PendingSkip? = (this as? SkipLookup.Found)?.pending + +private fun SkipLookup.reason(): String = (this as? SkipLookup.Refused)?.reason.orEmpty() + +private fun reviewParams(pending: PendingSkip, comment: String): JsonObject = + buildJsonObject { + put("skip_id", pending.skip.id.toString()) + put("review_comment", comment) + } + +/** The comment, quoted in full, or a sentence saying there is none. */ +private fun commentLine(comment: String): String = + if (comment.isEmpty()) "No comment goes with it." else "It goes with this comment: “$comment”" + +/** Offers to let a hire skip a step. */ +@Component +class AcceptSkipAction( + private val scope: ContentScope, + private val onboardingSkipService: OnboardingSkipService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "accept_skip", + description = "Offer to accept a hire's request to skip a step. The step is marked skipped and counts " + + "as done for them. Use the skip_id from list_pending_skips and read what they wrote first. A " + + "comment to the hire is optional. This does NOT accept anything by itself; the manager confirms.", + parameters = stringFields( + "skip_id" to "The skip_id from list_pending_skips.", + "review_comment" to "Optional. A note the hire will see with the answer.", + required = listOf("skip_id"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val lookup = lookUpPendingSkip(scope, onboardingSkipService, call.uuidArgument("skip_id"), context.projectId) + val pending = lookup.pendingOrNull() ?: return TeamActionDraft.Refused(lookup.reason()) + val comment = call.textArgument("review_comment") + val owner = pending.scoped.owner.displayName + val step = pending.scoped.element + + return TeamActionDraft.Proposed( + params = reviewParams(pending, comment), + label = "Let $owner skip “${step.title.forLabel()}”", + preview = buildString { + appendLine("Accept $owner's request to skip the step “${step.title}”.") + appendLine("Their reason: “${pending.skip.reason}”") + appendLine(commentLine(comment)) + appendLine() + append( + "The step is marked skipped and counts as done in their progress. If it was the last " + + "step they had open, that finishes their onboarding.", + ) + scope + .sharedNote(pending.scoped.owner, context.projectId) + .takeIf { it.isNotEmpty() } + ?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + recheckPending(scope, onboardingSkipService, params, context) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingSkipService.acceptSkipById( + requireNotNull(params.uuid("skip_id")), + ReviewOnboardingSkipRequest(reviewComment = params.text("review_comment")), + ) + return "Accepted. The step is skipped, and it counts as done for them." + } +} + +/** Offers to refuse a hire's request to skip a step. */ +@Component +class DenySkipAction( + private val scope: ContentScope, + private val onboardingSkipService: OnboardingSkipService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.STANDARD + override val spec = BuddyToolSpecDto( + name = "deny_skip", + description = "Offer to deny a hire's request to skip a step. A denial needs a comment saying why, " + + "which the hire will see; ask the manager for one rather than inventing it. Use the skip_id from " + + "list_pending_skips. This does NOT deny anything by itself; the manager confirms.", + parameters = stringFields( + "skip_id" to "The skip_id from list_pending_skips.", + "review_comment" to "Why it is denied, in the manager's words. Required.", + required = listOf("skip_id", "review_comment"), + ), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val comment = call.textArgument("review_comment") + if (comment.isEmpty()) { + return TeamActionDraft.Refused( + "A denial needs a comment saying why, because the hire sees it. Ask the manager what to write.", + ) + } + val lookup = lookUpPendingSkip(scope, onboardingSkipService, call.uuidArgument("skip_id"), context.projectId) + val pending = lookup.pendingOrNull() ?: return TeamActionDraft.Refused(lookup.reason()) + val owner = pending.scoped.owner.displayName + val step = pending.scoped.element + + return TeamActionDraft.Proposed( + params = reviewParams(pending, comment), + label = "Deny $owner's skip of “${step.title.forLabel()}”", + preview = buildString { + appendLine("Deny $owner's request to skip the step “${step.title}”.") + appendLine("Their reason: “${pending.skip.reason}”") + appendLine("They are told: “$comment”") + appendLine() + append(step.stepStatus.beforeDenying(owner)) + append("The step stays for them to do.") + scope + .sharedNote(pending.scoped.owner, context.projectId) + .takeIf { it.isNotEmpty() } + ?.let { append("\n\n$it") } + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + recheckPending(scope, onboardingSkipService, params, context) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingSkipService.denySkipById( + requireNotNull(params.uuid("skip_id")), + ReviewOnboardingSkipRequest(reviewComment = params.text("review_comment")), + ) + return "Denied. They have your comment, and the step is theirs to do." + } +} + +/** Offers to delete a pending skip request without answering it. */ +@Component +class DeleteSkipAction( + private val scope: ContentScope, + private val onboardingSkipService: OnboardingSkipService, +) : TeamActionHandler { + override val area = TeamArea.CONTENT + override val risk = BuddyProposalRisk.DESTRUCTIVE + override val spec = BuddyToolSpecDto( + name = "delete_skip", + description = "Offer to delete a pending skip request without answering it, so the hire hears nothing. " + + "Prefer accept_skip or deny_skip, which tell them. Use the skip_id from list_pending_skips. This " + + "does NOT delete anything by itself; the manager confirms.", + parameters = stringFields("skip_id" to "The skip_id from list_pending_skips.", required = listOf("skip_id")), + ) + + override fun draft(call: BuddyToolCallDto, context: TeamToolContext): TeamActionDraft { + val lookup = lookUpPendingSkip(scope, onboardingSkipService, call.uuidArgument("skip_id"), context.projectId) + val pending = lookup.pendingOrNull() ?: return TeamActionDraft.Refused(lookup.reason()) + val owner = pending.scoped.owner.displayName + val step = pending.scoped.element + + return TeamActionDraft.Proposed( + params = buildJsonObject { put("skip_id", pending.skip.id.toString()) }, + label = "Delete $owner's skip request", + preview = buildString { + appendLine("Delete $owner's request to skip “${step.title}”, without answering it.") + appendLine("Their reason: “${pending.skip.reason}”") + appendLine() + append( + "They are not told anything, and the step stays as it is. Accepting or denying it would " + + "tell them. This cannot be undone.", + ) + }.trim(), + ) + } + + override fun recheck(params: JsonObject, context: TeamToolContext): String? = + recheckPending(scope, onboardingSkipService, params, context) + + override suspend fun perform(params: JsonObject, context: TeamToolContext): String { + onboardingSkipService.deleteSkipById(requireNotNull(params.uuid("skip_id"))) + return "Deleted. The request is gone; they were not told." + } +} + +/** The refusal for a stored skip proposal whose request is gone, out of scope, or already answered. */ +private fun recheckPending( + scope: ContentScope, + skips: OnboardingSkipService, + params: JsonObject, + context: TeamToolContext, +): String? = + when (val found = lookUpPendingSkip(scope, skips, params.uuid("skip_id"), context.projectId)) { + is SkipLookup.Refused -> found.reason + is SkipLookup.Found -> null + } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt index 9e1dd09c..53a02b64 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt @@ -2,6 +2,8 @@ package com.sprintstart.sprintstartbackend.onboarding.service import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus import com.sprintstart.sprintstartbackend.onboarding.external.model.BuddyToolCallDto +import com.sprintstart.sprintstartbackend.onboarding.model.entity.StarterWorkTaskProposal +import com.sprintstart.sprintstartbackend.onboarding.repository.StarterWorkTaskProposalRepository import com.sprintstart.sprintstartbackend.user.external.ProjectMember import com.sprintstart.sprintstartbackend.user.external.ProjectMembershipApi import com.sprintstart.sprintstartbackend.user.external.UserApi @@ -16,6 +18,7 @@ import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.buildJsonObject import kotlinx.serialization.json.put import org.assertj.core.api.Assertions.assertThat +import java.util.Optional import java.util.UUID /** @@ -28,7 +31,9 @@ internal class ContentFixture { val pathElements: PathElements = mockk(relaxed = true) val projectMembershipApi: ProjectMembershipApi = mockk(relaxed = true) val userApi: UserApi = mockk(relaxed = true) - val scope = ContentScope(pathElements, projectMembershipApi, userApi) + val proposals: StarterWorkTaskProposalRepository = mockk(relaxed = true) + val starterWorkScope: StarterWorkScope = mockk(relaxed = true) + val scope = ContentScope(pathElements, projectMembershipApi, userApi, proposals, starterWorkScope) val projectId: UUID = UUID.randomUUID() val memberId: UUID = UUID.randomUUID() @@ -39,6 +44,7 @@ internal class ContentFixture { every { projectMembershipApi.getProjectMembers(projectId) } returns listOf(ProjectMember(memberId, "Sam Rivera", githubLogin = null, joinedAt = null)) onlyThisProject() + every { proposals.findById(any()) } returns Optional.empty() } /** The member is on this project and no other, which is what most tests want. */ @@ -79,6 +85,14 @@ internal class ContentFixture { return element } + /** A starter-work task, from a repository that is linked to this project or not. */ + fun task(linked: Boolean = true, title: String = "Fix the typo"): StarterWorkTaskProposal { + val task = StarterWorkTaskProposal(sourceId = "github:acme/app:ISSUE:${UUID.randomUUID()}", title = title) + every { proposals.findById(task.id) } returns Optional.of(task) + every { starterWorkScope.covers(task.sourceId, projectId) } returns linked + return task + } + /** An element that is gone. */ fun gone(kind: PathElementKind, id: UUID) { every { pathElements.find(kind, id) } returns null diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsReadsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsReadsTest.kt new file mode 100644 index 00000000..1e1d1601 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsReadsTest.kt @@ -0,0 +1,194 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.CheckQuestionType +import com.sprintstart.sprintstartbackend.onboarding.external.enums.OrientationOrigin +import com.sprintstart.sprintstartbackend.onboarding.external.enums.OrientationStep +import com.sprintstart.sprintstartbackend.onboarding.external.enums.SkipStatus +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.model.response.feedback.GetAdminOnboardingFeedbackResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.orientation.MyOrientationResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.orientation.OrientationCitationResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.orientation.OrientationPacketResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.orientation.OrientationSectionResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.question.GetPhaseQuestionsResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.question.QuestionForAdminResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.question.QuestionOptionForAdminResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.skip.GetOnboardingSkipResponse +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import java.time.Instant +import java.util.UUID + +/** The reads of the content area that are about what waits for the manager, rather than the path itself. */ +class ContentTeamToolsReadsTest { + private val f = ContentFixture() + private val skipService: OnboardingSkipService = mockk(relaxed = true) + private val feedbackService: OnboardingFeedbackService = mockk(relaxed = true) + private val questionService: QuestionAttemptService = mockk(relaxed = true) + private val orientationService: TaskOrientationService = mockk(relaxed = true) + private val tools = ContentTeamTools( + f.scope, + f.pathElements, + mockk(relaxed = true), + mockk(relaxed = true), + skipService, + feedbackService, + questionService, + orientationService, + ) + + private fun read(name: String, vararg args: Pair) = tools.execute(f.call(name, *args), f.context) + + @Test + fun `pending skips list only requests still waiting, from members of this project`() { + val stepId = UUID.randomUUID() + f.element(PathElementKind.STEP, stepId, title = "Install", stepStatus = StepStatus.IN_PROGRESS) + every { skipService.getAllSkipsByUserId(f.memberId) } returns + listOf( + GetOnboardingSkipResponse( + UUID.randomUUID(), + stepId, + SkipStatus.PENDING, + "I know it", + createdAt = Instant.parse("2026-09-01T10:00:00Z"), + ), + GetOnboardingSkipResponse(UUID.randomUUID(), stepId, SkipStatus.ACCEPTED, "old one"), + ) + + val text = read("list_pending_skips") + + assertThat(text).contains( + "Sam Rivera asks to skip “Install” (in progress)", + "[skip_id:", + "“I know it”", + "2026-09-01", + ) + assertThat(text).doesNotContain("old one") + } + + @Test + fun `no pending skips is said plainly`() { + every { skipService.getAllSkipsByUserId(f.memberId) } returns emptyList() + + assertThat(read("list_pending_skips")).contains("skip request waiting") + } + + private fun feedbackOf(read: Boolean, message: String) = + GetAdminOnboardingFeedbackResponse( + id = UUID.randomUUID(), + userId = f.memberId, + stepId = UUID.randomUUID(), + stepTitle = "Install", + message = message, + read = read, + createdAt = Instant.parse("2026-09-02T10:00:00Z"), + ) + + @Test + fun `feedback shows unread by default and read only on request`() { + every { feedbackService.getAllFeedbackByUserId(f.memberId) } returns + listOf(feedbackOf(read = false, message = "Needed Docker"), feedbackOf(read = true, message = "Fine")) + + val unread = read("list_feedback") + val everything = read("list_feedback", "include_read" to true) + + assertThat(unread).contains("Sam Rivera on “Install”", "[feedback_id:", "“Needed Docker”") + assertThat(unread).doesNotContain("Fine") + assertThat(everything).contains("“Needed Docker”", "“Fine”", "(read)") + } + + @Test + fun `when everything is read the answer says how many there are`() { + every { feedbackService.getAllFeedbackByUserId(f.memberId) } returns + listOf(feedbackOf(read = true, message = "Fine")) + + assertThat(read("list_feedback")).contains("no unread feedback", "already read") + } + + @Test + fun `phase checks show the ids and correct answers a replacement starts from`() { + val phaseId = UUID.randomUUID() + f.element(PathElementKind.PHASE, phaseId, title = "Setup") + val questionId = UUID.randomUUID() + val optionId = UUID.randomUUID() + every { questionService.getPhaseQuestions(phaseId) } returns + GetPhaseQuestionsResponse( + phaseId, + listOf( + QuestionForAdminResponse( + id = questionId, + position = 0, + type = CheckQuestionType.MULTIPLE_CHOICE, + question = "Where do logs go?", + explanation = "See runbook", + options = listOf(QuestionOptionForAdminResponse(optionId, 0, "stdout", true)), + ), + ), + ) + + val text = read("get_phase_checks", "phase_id" to phaseId) + + assertThat(text).contains("Setup", "Sam Rivera", "[question_id: $questionId]", "See runbook") + assertThat(text).contains("[correct] stdout [option_id: $optionId]") + } + + @Test + fun `phase checks of a phase on another person's path are not shown`() { + val phaseId = UUID.randomUUID() + f.element(PathElementKind.PHASE, phaseId, owner = f.outsiderId) + + val text = read("get_phase_checks", "phase_id" to phaseId) + + assertThat(text).contains("not on the onboarding path") + verify(exactly = 0) { questionService.getPhaseQuestions(any()) } + } + + @Test + fun `an orientation packet is shown in full, and says who wrote it`() { + val task = f.task() + val citation = OrientationCitationResponse("README.md", "c1", "https://x.io/readme") + every { orientationService.getForAuthoring(task.id, f.projectId) } returns + MyOrientationResponse( + task.id, + task.title, + null, + OrientationPacketResponse( + taskId = task.id, + taskTitle = task.title, + summary = "Start here", + sections = listOf( + OrientationSectionResponse( + OrientationStep.SET_UP, + "Install", + "Run make setup.", + listOf(citation), + ), + ), + sources = emptyList(), + assembledAt = Instant.now(), + origin = OrientationOrigin.HUMAN, + ), + null, + ) + + val text = read("get_orientation_packet", "task_id" to task.id) + + assertThat(text).contains("written by a person", "Summary: Start here", "set up — Install", "Run make setup.") + assertThat(text).contains("README.md ") + } + + @Test + fun `a task with no packet says so, and one from an unlinked repository is refused`() { + val task = f.task() + every { orientationService.getForAuthoring(task.id, f.projectId) } returns + MyOrientationResponse(task.id, task.title, null, null, null) + val unlinked = f.task(linked = false) + + assertThat(read("get_orientation_packet", "task_id" to task.id)).contains("no orientation packet") + assertThat(read("get_orientation_packet", "task_id" to unlinked.id)).contains("not from a repository linked") + verify(exactly = 0) { orientationService.getForAuthoring(unlinked.id, any()) } + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt index a808f3ef..13a32ac6 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt @@ -20,7 +20,16 @@ class ContentTeamToolsTest { private val f = ContentFixture() private val pathService: OnboardingPathService = mockk(relaxed = true) private val stepService: OnboardingStepService = mockk(relaxed = true) - private val tools = ContentTeamTools(f.scope, pathService, stepService) + private val tools = ContentTeamTools( + f.scope, + f.pathElements, + pathService, + stepService, + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + ) private val phaseId = UUID.randomUUID() private val stepId = UUID.randomUUID() @@ -64,8 +73,14 @@ class ContentTeamToolsTest { @Test fun `mounts exactly the reads of the area, in the content area`() { assertThat(tools.area).isEqualTo(TeamArea.CONTENT) - assertThat(tools.toolSpecs().map { it.name }).containsExactly("get_member_path") - assertThat(tools.handles("get_member_path")).isTrue() + assertThat(tools.toolSpecs().map { it.name }).containsExactly( + "get_member_path", + "list_pending_skips", + "list_feedback", + "get_phase_checks", + "get_orientation_packet", + ) + assertThat(tools.toolSpecs().all { tools.handles(it.name) }).isTrue() assertThat(tools.handles("add_phase")).isFalse() } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/MarkFeedbackReadActionTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/MarkFeedbackReadActionTest.kt new file mode 100644 index 00000000..5cf5eaad --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/MarkFeedbackReadActionTest.kt @@ -0,0 +1,104 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.model.response.feedback.GetAdminOnboardingFeedbackResponse +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import java.time.Instant +import java.util.UUID + +class MarkFeedbackReadActionTest { + private val f = ContentFixture() + private val feedbackService: OnboardingFeedbackService = mockk(relaxed = true) + private val action = MarkFeedbackReadAction(f.scope, feedbackService) + private val feedbackId = UUID.randomUUID() + + private fun feedback(read: Boolean = false, stepTitle: String? = "Install") { + f.element(PathElementKind.FEEDBACK, feedbackId, owner = f.memberId) + every { feedbackService.getAllFeedbackByUserId(f.memberId) } returns + listOf( + GetAdminOnboardingFeedbackResponse( + id = feedbackId, + userId = f.memberId, + stepId = UUID.randomUUID(), + stepTitle = stepTitle, + message = "This step assumed I had Docker", + read = read, + createdAt = Instant.now(), + ), + ) + } + + private fun call(id: Any = feedbackId) = f.call("mark_feedback_read", "feedback_id" to id) + + @Test + fun `shows the message being marked and says nobody is told`() { + feedback() + + val draft = f.proposed(action.draft(call(), f.context)) + + assertThat(draft.preview).contains( + "feedback from Sam Rivera", + "“Install”", + "“This step assumed I had Docker”", + "They are not told", + ) + } + + @Test + fun `feedback on no step is described as about the path as a whole`() { + feedback(stepTitle = null) + + assertThat(f.proposed(action.draft(call(), f.context)).preview).contains("their path as a whole") + } + + @Test + fun `feedback already read is refused`() { + feedback(read = true) + + assertThat(f.refusal(action.draft(call(), f.context))).contains("already marked as read") + } + + @Test + fun `feedback from somebody off the project is refused, and looks the same as feedback that is not there`() { + f.element(PathElementKind.FEEDBACK, feedbackId, owner = f.outsiderId) + val missing = UUID.randomUUID() + f.gone(PathElementKind.FEEDBACK, missing) + + val outsider = f.refusal(action.draft(call(), f.context)) + val absent = f.refusal(action.draft(call(missing), f.context)) + + assertThat(outsider).contains("list_feedback") + assertThat(absent).isEqualTo(outsider) + } + + @Test + fun `a confirm for feedback somebody else already read is turned down`() { + feedback(read = true) + + assertThat(action.recheck(f.json("feedback_id" to feedbackId), f.context)) + .contains("already marked that feedback as read") + } + + @Test + fun `a confirm for unread feedback still on a member's path passes`() { + feedback() + + assertThat(action.recheck(f.json("feedback_id" to feedbackId), f.context)).isNull() + } + + @Test + fun `drafting marks nothing, performing marks the feedback named`() = + runTest { + feedback() + + action.draft(call(), f.context) + verify(exactly = 0) { feedbackService.markFeedbackAsRead(any()) } + + action.perform(f.json("feedback_id" to feedbackId), f.context) + verify { feedbackService.markFeedbackAsRead(feedbackId) } + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/OrientationTeamActionsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/OrientationTeamActionsTest.kt new file mode 100644 index 00000000..dd7911af --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/OrientationTeamActionsTest.kt @@ -0,0 +1,298 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.enums.OrientationOrigin +import com.sprintstart.sprintstartbackend.onboarding.external.enums.OrientationStep +import com.sprintstart.sprintstartbackend.onboarding.model.request.orientation.AuthorOrientationRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.orientation.MyOrientationResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.orientation.OrientationPacketResponse +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonObject +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import java.time.Instant +import java.util.UUID + +class OrientationTeamActionsTest { + private val f = ContentFixture() + private val service: TaskOrientationService = mockk(relaxed = true) + + private val assembledAt: Instant = Instant.parse("2026-09-01T10:00:00Z") + + private fun packetFor(taskId: UUID, origin: OrientationOrigin, at: Instant = assembledAt) = + OrientationPacketResponse( + taskId = taskId, + taskTitle = "Fix the typo", + summary = null, + sections = emptyList(), + sources = emptyList(), + assembledAt = at, + origin = origin, + ) + + /** What the service says the task has now: nothing, or a packet written by [origin]. */ + private fun current(taskId: UUID, origin: OrientationOrigin?, at: Instant = assembledAt) { + every { service.getForAuthoring(taskId, f.projectId) } returns + MyOrientationResponse( + taskId = taskId, + taskTitle = "Fix the typo", + taskUrl = null, + packet = origin?.let { packetFor(taskId, it, at) }, + reason = null, + ) + } + + private fun section( + step: String = "SET_UP", + title: String = "Install", + body: String = "Run make setup.", + citations: JsonArray = JsonArray(emptyList()), + ) = f.json("step" to step, "title" to title, "body" to body, "citations" to citations) + + @Nested + inner class Author { + private val action = AuthorOrientationPacketAction(f.scope, service) + + private fun author(taskId: UUID, vararg sections: JsonObject, summary: String? = null) = + f.call( + "author_orientation_packet", + "task_id" to taskId, + "summary" to summary, + "sections" to JsonArray(sections.toList()), + ) + + @Test + fun `is a standard change`() { + assertThat(action.risk).isEqualTo(BuddyProposalRisk.STANDARD) + assertThat(action.area).isEqualTo(TeamArea.CONTENT) + } + + @Test + fun `shows the full text a hire will read`() { + val task = f.task() + current(task.id, null) + + val draft = f.proposed( + action.draft( + author( + task.id, + section(body = "Run make setup, then open the dev container."), + summary = "Start here", + ), + f.context, + ), + ) + + assertThat(draft.preview).contains( + "orientation packet for “Fix the typo”", + "Summary: Start here", + "set up — Install", + "Run make setup, then open the dev container.", + "There is no packet for this task yet", + "AI does not regenerate it", + ) + } + + @Test + fun `says what it replaces, and that a person's text is among it`() { + val task = f.task() + current(task.id, OrientationOrigin.HUMAN) + + val draft = f.proposed(action.draft(author(task.id, section()), f.context)) + + assertThat(draft.preview).contains("replaces the packet that is there now, written by a person") + } + + @Test + fun `an assembled packet is named as such`() { + val task = f.task() + current(task.id, OrientationOrigin.AI) + + val draft = f.proposed(action.draft(author(task.id, section()), f.context)) + + assertThat(draft.preview).contains("assembled by the AI") + } + + @Test + fun `a task whose repository is not linked to this project is refused`() { + val task = f.task(linked = false) + + assertThat( + f.refusal(action.draft(author(task.id, section()), f.context)), + ).contains("not from a repository linked") + } + + @Test + fun `a task that does not exist is refused the same way`() { + val reason = f.refusal(action.draft(author(UUID.randomUUID(), section()), f.context)) + + assertThat(reason).contains("not from a repository linked") + } + + @Test + fun `sections the service would refuse are refused at proposal`() { + val task = f.task() + current(task.id, null) + + assertThat(f.refusal(action.draft(author(task.id), f.context))).contains("at least one section") + assertThat(f.refusal(action.draft(author(task.id, section(title = " ")), f.context))) + .contains("both a title and a body") + assertThat(f.refusal(action.draft(author(task.id, section(step = "DEPLOY")), f.context))) + .contains("needs a step", "SET_UP", "OPEN_THE_PR") + } + + @Test + fun `a citation link that is not a web address is refused`() { + val task = f.task() + current(task.id, null) + val bad = section(citations = JsonArray(listOf(f.json("filename" to "x", "source_url" to "javascript:x")))) + + assertThat(f.refusal(action.draft(author(task.id, bad), f.context))).contains("not a web address") + } + + @Test + fun `a confirm is turned down when the packet was replaced since the preview`() { + val task = f.task() + current(task.id, OrientationOrigin.AI, at = assembledAt) + val proposed = f.proposed(action.draft(author(task.id, section()), f.context)) + + current(task.id, OrientationOrigin.HUMAN, at = assembledAt.plusSeconds(60)) + + assertThat(action.recheck(proposed.params, f.context)).contains("changed since this was proposed") + } + + @Test + fun `a confirm with the packet as it was passes, and one for a task that left the project does not`() { + val task = f.task() + current(task.id, OrientationOrigin.AI) + val proposed = f.proposed(action.draft(author(task.id, section()), f.context)) + + assertThat(action.recheck(proposed.params, f.context)).isNull() + + f.task(linked = false).let { unlinked -> + assertThat( + action.recheck(f.json("task_id" to unlinked.id), f.context), + ).contains("not from a repository linked") + } + } + + @Test + fun `nothing is saved while drafting`() { + val task = f.task() + current(task.id, null) + + action.draft(author(task.id, section()), f.context) + + verify(exactly = 0) { service.authorPacket(any(), any(), any()) } + } + + @Test + fun `performing saves the stored text for this project`() = + runTest { + val task = f.task() + current(task.id, null) + val proposed = f.proposed( + action.draft( + author( + task.id, + section(step = "check_locally", title = "Test", body = "make test"), + summary = "Hi", + ), + f.context, + ), + ) + val request = slot() + every { service.authorPacket(task.id, f.projectId, capture(request)) } returns mockk(relaxed = true) + + action.perform(proposed.params, f.context) + + assertThat(request.captured.summary).isEqualTo("Hi") + assertThat( + request.captured.sections + .single() + .step, + ).isEqualTo(OrientationStep.CHECK_LOCALLY) + assertThat( + request.captured.sections + .single() + .body, + ).isEqualTo("make test") + } + } + + @Nested + inner class Revert { + private val action = RevertOrientationPacketAction(f.scope, service) + + @Test + fun `is destructive`() { + assertThat(action.risk).isEqualTo(BuddyProposalRisk.DESTRUCTIVE) + } + + @Test + fun `dropping a person's packet says their text is deleted`() { + val task = f.task() + current(task.id, OrientationOrigin.HUMAN) + + val draft = f.proposed(action.draft(f.call("revert_orientation_packet", "task_id" to task.id), f.context)) + + assertThat(draft.preview).contains("written by a person", "their text is deleted", "cannot be undone") + } + + @Test + fun `dropping an assembled packet makes no such claim`() { + val task = f.task() + current(task.id, OrientationOrigin.AI) + + val draft = f.proposed(action.draft(f.call("revert_orientation_packet", "task_id" to task.id), f.context)) + + assertThat(draft.preview).doesNotContain("their text is deleted").contains("assembled from the docs again") + } + + @Test + fun `a task with no packet is refused`() { + val task = f.task() + current(task.id, null) + + assertThat(f.refusal(action.draft(f.call("revert_orientation_packet", "task_id" to task.id), f.context))) + .contains("no packet") + } + + @Test + fun `a task whose repository is not linked to this project is refused`() { + val task = f.task(linked = false) + + assertThat(f.refusal(action.draft(f.call("revert_orientation_packet", "task_id" to task.id), f.context))) + .contains("not from a repository linked") + } + + @Test + fun `a confirm is turned down when the packet was replaced since`() { + val task = f.task() + current(task.id, OrientationOrigin.HUMAN) + val proposed = f.proposed( + action.draft(f.call("revert_orientation_packet", "task_id" to task.id), f.context), + ) + + current(task.id, OrientationOrigin.HUMAN, at = assembledAt.plusSeconds(1)) + + assertThat(action.recheck(proposed.params, f.context)).contains("changed since") + } + + @Test + fun `performing drops the packet of this project`() = + runTest { + val task = f.task() + + action.perform(f.json("task_id" to task.id), f.context) + + verify { service.revertToAi(task.id, f.projectId) } + } + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksActionTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksActionTest.kt new file mode 100644 index 00000000..e610e3cf --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksActionTest.kt @@ -0,0 +1,296 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.CheckQuestionType +import com.sprintstart.sprintstartbackend.onboarding.model.request.question.UpdatePhaseQuestionsRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.question.GetPhaseQuestionsResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.question.QuestionForAdminResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.question.QuestionOptionForAdminResponse +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonObject +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import java.util.UUID + +class ReplacePhaseChecksActionTest { + private val f = ContentFixture() + private val questionService: QuestionAttemptService = mockk(relaxed = true) + private val action = ReplacePhaseChecksAction(f.scope, questionService) + private val phaseId = UUID.randomUUID() + + private val shortId = UUID.randomUUID() + private val choiceId = UUID.randomUUID() + private val optionA = UUID.randomUUID() + private val optionB = UUID.randomUUID() + + /** The phase as it stands: one short-text question and one multiple-choice one. */ + private fun existing() { + f.element(PathElementKind.PHASE, phaseId, title = "Setup") + every { questionService.getPhaseQuestions(phaseId) } returns + GetPhaseQuestionsResponse( + phaseId, + listOf( + QuestionForAdminResponse( + shortId, + 0, + CheckQuestionType.SHORT_TEXT, + "Which command builds it?", + null, + "make", + ), + QuestionForAdminResponse( + id = choiceId, + position = 1, + type = CheckQuestionType.MULTIPLE_CHOICE, + question = "Where do logs go?", + explanation = "See the runbook", + options = listOf( + QuestionOptionForAdminResponse(optionA, 0, "stdout", true), + QuestionOptionForAdminResponse(optionB, 1, "a file", false), + ), + ), + ), + ) + } + + private fun short(id: UUID? = null, text: String = "Which command builds it?", answer: String? = "make") = + f.json( + *listOfNotNull( + id?.let { "id" to it }, + "type" to "SHORT_TEXT", + "question" to text, + answer?.let { + "correct_answer" to + it + }, + ).toTypedArray(), + ) + + private fun choice(id: UUID? = null, options: List) = + f.json( + *listOfNotNull( + id?.let { "id" to it }, + "type" to "MULTIPLE_CHOICE", + "question" to "Where do logs go?", + "options" to JsonArray(options), + ).toTypedArray(), + ) + + private fun option(id: UUID? = null, label: String, correct: Boolean) = + f.json(*listOfNotNull(id?.let { "id" to it }, "label" to label, "correct" to correct).toTypedArray()) + + private fun replace(vararg questions: JsonObject) = + f.call("replace_phase_checks", "phase_id" to phaseId, "questions" to JsonArray(questions.toList())) + + @Test + fun `previews what is kept, what is new and what is deleted`() { + existing() + + val draft = f.proposed( + action.draft( + replace(short(shortId), short(text = "How do you run the tests?", answer = "make test")), + f.context, + ), + ) + + assertThat(draft.preview).contains( + "It will have 2 questions", + "1. (kept) short text: Which command builds it?", + "2. (new) short text: How do you run the tests?", + "Correct answer: make test", + "These are deleted, with everybody's past answers to them:", + "- Where do logs go?", + "no longer waits for it", + ) + } + + @Test + fun `keeping everything deletes nothing and says nothing about deletion`() { + existing() + + val draft = f.proposed( + action.draft( + replace( + short(shortId), + choice(choiceId, listOf(option(optionA, "stdout", true), option(optionB, "a file", false))), + ), + f.context, + ), + ) + + assertThat(draft.preview).doesNotContain("deleted") + } + + @Test + fun `an empty list removes them all, and says so`() { + existing() + + val draft = f.proposed(action.draft(replace(), f.context)) + + assertThat(draft.preview).contains("It will have 0 questions", "Which command builds it?", "Where do logs go?") + } + + @Test + fun `an empty list for a phase with no questions is refused as no change`() { + f.element(PathElementKind.PHASE, phaseId) + every { questionService.getPhaseQuestions(phaseId) } returns GetPhaseQuestionsResponse(phaseId, emptyList()) + + assertThat(f.refusal(action.draft(replace(), f.context))).contains("nothing would change") + } + + @Test + fun `a call that leaves out the list is refused rather than read as clear-all`() { + existing() + + val reason = f.refusal(action.draft(f.call("replace_phase_checks", "phase_id" to phaseId), f.context)) + + assertThat(reason).contains("complete list") + } + + @Test + fun `an id that is not one of the phase's questions is refused`() { + existing() + + val reason = f.refusal(action.draft(replace(short(UUID.randomUUID())), f.context)) + + assertThat(reason).contains("Question 1", "not one of this phase's questions") + } + + @Test + fun `an option id that is not one of the question's options is refused`() { + existing() + + val reason = f.refusal( + action.draft( + replace(choice(choiceId, listOf(option(UUID.randomUUID(), "x", true), option(optionB, "y", false)))), + f.context, + ), + ) + + assertThat(reason).contains("option id that is not one of that question's options") + } + + @Test + fun `the same question listed twice is refused`() { + existing() + + assertThat(f.refusal(action.draft(replace(short(shortId), short(shortId)), f.context))) + .contains("listed twice") + } + + @Test + fun `questions the service would refuse are refused at proposal`() { + existing() + + assertThat(f.refusal(action.draft(replace(short(answer = null)), f.context))).contains("needs a correct_answer") + assertThat(f.refusal(action.draft(replace(short(text = " ")), f.context))).contains("has no text") + assertThat( + f.refusal( + action.draft(replace(choice(options = listOf(option(label = "one", correct = true)))), f.context), + ), + ).contains("at least two options") + assertThat( + f.refusal( + action.draft( + replace( + choice( + options = listOf( + option(label = "a", correct = false), + option(label = "b", correct = false), + ), + ), + ), + f.context, + ), + ), + ).contains("at least one correct option") + } + + @Test + fun `an unknown question type is refused, naming the ones there are`() { + existing() + val odd = f.json("type" to "ESSAY", "question" to "Discuss") + + assertThat(f.refusal(action.draft(replace(odd), f.context))).contains("MULTIPLE_CHOICE", "SHORT_TEXT") + } + + @Test + fun `a phase on somebody else's path is refused`() { + f.element(PathElementKind.PHASE, phaseId, owner = f.outsiderId) + + assertThat(f.refusal(action.draft(replace(short()), f.context))).contains("not on the onboarding path") + } + + @Test + fun `nothing is written while drafting`() { + existing() + + action.draft(replace(short(shortId)), f.context) + + verify(exactly = 0) { questionService.replacePhaseQuestions(any(), any()) } + } + + @Test + fun `a confirm is turned down when the phase's questions changed since the preview`() { + existing() + val proposed = f.proposed(action.draft(replace(short(shortId)), f.context)) + + every { questionService.getPhaseQuestions(phaseId) } returns + GetPhaseQuestionsResponse( + phaseId, + listOf( + QuestionForAdminResponse( + UUID.randomUUID(), + 0, + CheckQuestionType.SHORT_TEXT, + "Added since", + null, + "x", + ), + ), + ) + + assertThat(action.recheck(proposed.params, f.context)).contains("changed since this was proposed") + } + + @Test + fun `a confirm with the phase as it was passes`() { + existing() + val proposed = f.proposed(action.draft(replace(short(shortId)), f.context)) + + assertThat(action.recheck(proposed.params, f.context)).isNull() + } + + @Test + fun `performing sends the whole list, keeping ids and numbering places from the list order`() = + runTest { + existing() + val proposed = f.proposed( + action.draft( + replace( + short(text = "New first"), + choice( + choiceId, + listOf(option(optionA, "stdout", true), option(label = "syslog", correct = false)), + ), + ), + f.context, + ), + ) + val request = slot() + every { questionService.replacePhaseQuestions(phaseId, capture(request)) } returns mockk(relaxed = true) + + action.perform(proposed.params, f.context) + + val sent = request.captured.questions + assertThat(sent.map { it.position }).containsExactly(0, 1) + assertThat(sent[0].id).isNull() + assertThat(sent[1].id).isEqualTo(choiceId) + assertThat(sent[1].options.map { it.id }).containsExactly(optionA, null) + assertThat(sent[1].options.map { it.correct }).containsExactly(true, false) + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/SkipTeamActionsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/SkipTeamActionsTest.kt new file mode 100644 index 00000000..555f149b --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/SkipTeamActionsTest.kt @@ -0,0 +1,227 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.BuddyProposalRisk +import com.sprintstart.sprintstartbackend.onboarding.external.enums.SkipStatus +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.model.request.skip.ReviewOnboardingSkipRequest +import com.sprintstart.sprintstartbackend.onboarding.model.response.skip.GetOnboardingSkipResponse +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.springframework.http.HttpStatus +import org.springframework.web.server.ResponseStatusException +import java.util.UUID + +class SkipTeamActionsTest { + private val f = ContentFixture() + private val skipService: OnboardingSkipService = mockk(relaxed = true) + private val skipId = UUID.randomUUID() + + private fun pending( + status: SkipStatus = SkipStatus.PENDING, + stepStatus: StepStatus = StepStatus.WAITING, + owner: UUID = f.memberId, + ) { + f.element(PathElementKind.SKIP, skipId, owner = owner, title = "Install", stepStatus = stepStatus) + every { skipService.getSkipById(skipId) } returns + GetOnboardingSkipResponse(skipId, UUID.randomUUID(), status, "I already know the toolchain") + } + + @Nested + inner class Accept { + private val action = AcceptSkipAction(f.scope, skipService) + + @Test + fun `shows the hire's reason and what accepting does to the step`() { + pending() + + val draft = f.proposed( + action.draft( + f.call("accept_skip", "skip_id" to skipId, "review_comment" to "Fine, go ahead"), + f.context, + ), + ) + + assertThat(action.risk).isEqualTo(BuddyProposalRisk.STANDARD) + assertThat(draft.preview).contains( + "Accept Sam Rivera's request to skip the step “Install”", + "“I already know the toolchain”", + "“Fine, go ahead”", + "marked skipped and counts as done", + "finishes their onboarding", + ) + } + + @Test + fun `a comment is optional for an acceptance`() { + pending() + + val draft = f.proposed(action.draft(f.call("accept_skip", "skip_id" to skipId), f.context)) + + assertThat(draft.preview).contains("No comment goes with it") + } + + @Test + fun `a request that was already answered is refused, at proposal and at confirm`() { + pending(status = SkipStatus.ACCEPTED) + + assertThat(f.refusal(action.draft(f.call("accept_skip", "skip_id" to skipId), f.context))) + .contains("already been answered") + assertThat(action.recheck(f.json("skip_id" to skipId), f.context)).contains("already been answered") + } + + @Test + fun `a request from somebody off the project is refused, and looks the same as one that is not there`() { + pending(owner = f.outsiderId) + val missing = UUID.randomUUID() + f.gone(PathElementKind.SKIP, missing) + + val outsider = f.refusal(action.draft(f.call("accept_skip", "skip_id" to skipId), f.context)) + val absent = f.refusal(action.draft(f.call("accept_skip", "skip_id" to missing), f.context)) + + assertThat(outsider).contains("not on the onboarding path of anybody on this project") + assertThat(absent).isEqualTo(outsider) + } + + @Test + fun `a request deleted between the scope check and the read is refused, not thrown`() { + f.element(PathElementKind.SKIP, skipId) + every { skipService.getSkipById(skipId) } throws ResponseStatusException(HttpStatus.NOT_FOUND) + + assertThat(action.recheck(f.json("skip_id" to skipId), f.context)).contains("is gone") + } + + @Test + fun `says so when the hire's path is shared with other projects`() { + pending() + f.alsoOn("Payments") + + val draft = f.proposed(action.draft(f.call("accept_skip", "skip_id" to skipId), f.context)) + + assertThat(draft.preview).contains("also on Payments") + } + + @Test + fun `performing accepts with the stored comment`() = + runTest { + val request = slot() + every { skipService.acceptSkipById(skipId, capture(request)) } returns mockk(relaxed = true) + + action.perform(f.json("skip_id" to skipId, "review_comment" to "ok"), f.context) + + assertThat(request.captured.reviewComment).isEqualTo("ok") + } + } + + @Nested + inner class Deny { + private val action = DenySkipAction(f.scope, skipService) + + @Test + fun `a denial without a comment is refused, because the hire is shown it`() { + pending() + + val reason = f.refusal(action.draft(f.call("deny_skip", "skip_id" to skipId), f.context)) + + assertThat(reason).contains("needs a comment") + } + + @Test + fun `a blank comment is no comment`() { + pending() + + val reason = f.refusal( + action.draft(f.call("deny_skip", "skip_id" to skipId, "review_comment" to " "), f.context), + ) + + assertThat(reason).contains("needs a comment") + } + + @Test + fun `shows the comment in full`() { + pending() + val comment = "Please do this one, it is where the team's conventions are explained. " + + "Ask on the channel if you get stuck." + + val draft = f.proposed( + action.draft(f.call("deny_skip", "skip_id" to skipId, "review_comment" to comment), f.context), + ) + + assertThat(draft.preview).contains("They are told: “$comment”", "stays for them to do") + assertThat(draft.params.text("review_comment")).isEqualTo(comment) + } + + @Test + fun `says when a step they had started goes back to waiting`() { + pending(stepStatus = StepStatus.IN_PROGRESS) + + val draft = f.proposed( + action.draft(f.call("deny_skip", "skip_id" to skipId, "review_comment" to "no"), f.context), + ) + + assertThat(draft.preview).contains("Sam Rivera had started it", "back to waiting") + } + + @Test + fun `a step that was only waiting makes no such claim`() { + pending(stepStatus = StepStatus.WAITING) + + val draft = f.proposed( + action.draft(f.call("deny_skip", "skip_id" to skipId, "review_comment" to "no"), f.context), + ) + + assertThat(draft.preview).doesNotContain("had started") + } + + @Test + fun `performing denies with the stored comment`() = + runTest { + val request = slot() + every { skipService.denySkipById(skipId, capture(request)) } returns mockk(relaxed = true) + + action.perform(f.json("skip_id" to skipId, "review_comment" to "not this one"), f.context) + + assertThat(request.captured.reviewComment).isEqualTo("not this one") + } + } + + @Nested + inner class Delete { + private val action = DeleteSkipAction(f.scope, skipService) + + @Test + fun `is destructive and says the hire is not told`() { + pending() + + val draft = f.proposed(action.draft(f.call("delete_skip", "skip_id" to skipId), f.context)) + + assertThat(action.risk).isEqualTo(BuddyProposalRisk.DESTRUCTIVE) + assertThat(draft.preview).contains("without answering it", "not told anything", "cannot be undone") + } + + @Test + fun `only a pending request can be deleted`() { + pending(status = SkipStatus.DENIED) + + assertThat(f.refusal(action.draft(f.call("delete_skip", "skip_id" to skipId), f.context))) + .contains("already been answered") + } + + @Test + fun `drafting deletes nothing, performing deletes the request named`() = + runTest { + pending() + + action.draft(f.call("delete_skip", "skip_id" to skipId), f.context) + verify(exactly = 0) { skipService.deleteSkipById(any()) } + + action.perform(f.json("skip_id" to skipId), f.context) + verify { skipService.deleteSkipById(skipId) } + } + } +} From f23145242169c3e7e9ac84c8606081ae2eb7b4de Mon Sep 17 00:00:00 2001 From: Linus Date: Mon, 21 Sep 2026 15:37:03 +0200 Subject: [PATCH 3/4] Make the content previews match what the writes do, and let a path be deleted Self-review of the content area found four places where a preview said something other than what happens, or a write would not have happened at all. - reset_member_path would have failed on every confirm. OnboardingPath's deleteByUserId is a derived delete, which throws TransactionRequiredException when it runs without a transaction, and neither the service nor the buddy's confirm path has one. It is now @Transactional on the repository, with a test that runs outside the test's own transaction, which is what hid it. The admin endpoints DELETE /users/{userId}/path and DELETE /me/path reach the same method and had the same problem. - replace_phase_checks said a removed question is deleted "with everybody's past answers". Attempts reference a question by plain id with no foreign key, so the answers stay as rows and simply stop counting. It now says that. - Deleting the last phase, step or task said the ones after it move up. It now says so only when there are later siblings. - get_member_path listed phases the hire is never shown, as if they were live. They are marked as not shown. Adds a mount test for the acceptance criteria that are about the set of tools: exactly these tools, no two sharing a name (handlers are keyed by it), and none that can tick a task or start, finish or complete a step. Co-Authored-By: Claude Sonnet 5 --- .../repository/OnboardingPathRepository.kt | 9 ++ .../onboarding/service/ContentScope.kt | 3 + .../onboarding/service/ContentTeamTools.kt | 3 +- .../onboarding/service/PathElements.kt | 6 + .../onboarding/service/PhaseTeamActions.kt | 3 +- .../service/ReplacePhaseChecksAction.kt | 13 +- .../onboarding/service/StepTeamActions.kt | 3 +- .../onboarding/service/TaskTeamActions.kt | 3 +- .../repository/OnboardingPathDeleteTest.kt | 67 ++++++++++ .../service/ContentAreaMountTest.kt | 126 ++++++++++++++++++ .../onboarding/service/ContentFixture.kt | 4 +- .../service/ContentTeamToolsTest.kt | 24 ++++ .../service/PhaseTeamActionsTest.kt | 11 ++ .../service/ReplacePhaseChecksActionTest.kt | 2 +- .../onboarding/service/StepTeamActionsTest.kt | 11 ++ .../onboarding/service/TaskTeamActionsTest.kt | 11 ++ 16 files changed, 287 insertions(+), 12 deletions(-) create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/repository/OnboardingPathDeleteTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentAreaMountTest.kt diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/repository/OnboardingPathRepository.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/repository/OnboardingPathRepository.kt index b3ce8dac..a3a12dda 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/repository/OnboardingPathRepository.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/repository/OnboardingPathRepository.kt @@ -2,12 +2,21 @@ package com.sprintstart.sprintstartbackend.onboarding.repository import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingPath import org.springframework.data.jpa.repository.JpaRepository +import org.springframework.transaction.annotation.Transactional import java.util.Optional import java.util.UUID interface OnboardingPathRepository : JpaRepository { fun findOnboardingPathByUserId(userId: UUID): Optional + /** + * Deletes a user's path with everything under it. + * + * Transactional here rather than left to each caller: a derived delete throws + * `TransactionRequiredException` when it runs with no transaction, and the services that call it + * are not themselves transactional. + */ + @Transactional fun deleteByUserId(userId: UUID) fun existsByUserId(userId: UUID): Boolean diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt index 302e679f..72fa0404 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentScope.kt @@ -106,6 +106,9 @@ internal const val TASK_NOT_HERE = internal const val LEFT_SINCE_HERE = "That person is no longer on this project, so nothing was changed." +/** Whether anything sits after this element among its siblings, so deleting it moves something up. */ +internal fun PathElement.hasLaterSiblings(): Boolean = position != null && position < siblings - 1 + /** What to tell the model when an id is not in scope, per kind. */ internal fun notInScope(kind: PathElementKind): String { val where = when (kind) { diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt index 6bd1e946..2f340da8 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt @@ -109,7 +109,8 @@ class ContentTeamTools( scope.sharedNote(owner, projectId).takeIf { it.isNotEmpty() }?.let { appendLine(it) } path.phases.sortedBy { it.position }.forEach { phase -> appendLine() - appendLine("Phase ${phase.position + 1}: ${phase.title} [phase_id: ${phase.id}]") + val hidden = if (phase.generationStatus.isHiddenFromUser()) " — not shown to them" else "" + appendLine("Phase ${phase.position + 1}: ${phase.title} [phase_id: ${phase.id}]$hidden") phase.description.takeIf { it.isNotBlank() }?.let { appendLine(" ${it.short()}") } onboardingStepService.getOnboardingStepsByPhaseId(phase.id).sortedBy { it.position }.forEach { step -> appendLine( diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElements.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElements.kt index 4c1de495..387a54e8 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElements.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PathElements.kt @@ -34,6 +34,8 @@ enum class PathElementKind( * @property children How many things sit directly under it, which is what a new child's position is * validated against. * @property contains What deleting it takes with it, in words; empty when it holds nothing. + * @property siblings How many things share its parent, itself included; what tells a preview whether + * deleting it moves anything after it. * @property finishedSteps How many steps under it the person already got through, finished or skipped. * @property stepStatus The status of the step it is or hangs off, or null for a phase. */ @@ -47,6 +49,7 @@ data class PathElement( val contains: String, val stepStatus: StepStatus?, val finishedSteps: Int = 0, + val siblings: Int = 0, ) /** @@ -105,6 +108,7 @@ class PathElements( contains = containedIn(phase.steps, phase.checkQuestions.size), stepStatus = null, finishedSteps = phase.steps.count { it.status.isDone() }, + siblings = phase.path.phases.size, ) } @@ -120,6 +124,7 @@ class PathElements( contains = containedIn(listOf(step), checks = 0, includeSteps = false), stepStatus = step.status, finishedSteps = if (step.status.isDone()) 1 else 0, + siblings = step.phase.steps.size, ) } @@ -134,6 +139,7 @@ class PathElements( children = 0, contains = "", stepStatus = task.step.status, + siblings = task.step.tasks.size, ) } diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActions.kt index 5398c70d..105567e7 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActions.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActions.kt @@ -212,7 +212,8 @@ class DeletePhaseAction( "that progress is deleted with them.", ) } - append("The phases after it move up one place. This cannot be undone.") + if (phase.hasLaterSiblings()) append("The phases after it move up one place. ") + append("This cannot be undone.") scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } }.trim(), ) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksAction.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksAction.kt index dcdf28fc..7231d4fa 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksAction.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksAction.kt @@ -25,9 +25,10 @@ import java.util.UUID * Replacing a phase's knowledge checks. * * The service takes the whole list: a question it is given with a known id is edited in place, one - * without an id is created, and one it is *not* given is deleted — along with the hire's history on it, - * and any link that made a step wait for it. So the action takes the whole list too, checks every id in - * it against what the phase has, and previews what stays, what is new and what goes. + * without an id is created, and one it is *not* given is deleted — after which the hires' answers to it, + * which are kept only by its id, count for nothing, and no step waits on it any more. So the action + * takes the whole list too, checks every id in it against what the phase has, and previews what stays, + * what is new and what goes. */ private const val CHECKS_CHANGED = @@ -164,8 +165,8 @@ class ReplacePhaseChecksAction( name = "replace_phase_checks", description = "Offer to replace the knowledge-check questions at the end of a phase on a member's " + "onboarding path. It takes the WHOLE new list: a question with its id is edited in place, one " + - "without an id is new, and any current question left out is deleted with the hire's answers to " + - "it. Always call get_phase_checks first and start from what it shows. This does NOT change " + + "without an id is new, and any current question left out is deleted and the hires' answers to " + + "it stop counting. Always call get_phase_checks first and start from what it shows. This does NOT change " + "anything by itself; the manager confirms.", parameters = buildJsonObject { put("type", "object") @@ -265,7 +266,7 @@ class ReplacePhaseChecksAction( val removed = current.filter { existing -> questions.none { it.id == existing.id } } if (removed.isNotEmpty()) { appendLine() - appendLine("These are deleted, with everybody's past answers to them:") + appendLine("These are deleted, and nobody's past answers to them count any more:") removed.forEach { appendLine("- ${it.question}") } appendLine("A step that was waiting on one of them no longer waits for it.") } diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActions.kt index 73c7c5c5..c2ca35fa 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActions.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActions.kt @@ -276,7 +276,8 @@ class DeleteStepAction( appendLine("Delete the step “${step.title}” from ${target.owner.displayName}'s onboarding path.") if (step.contains.isNotEmpty()) appendLine("It takes ${step.contains} with it.") step.stepStatus.aboutProgress(target.owner.displayName)?.let { appendLine(it) } - append("The steps after it move up one place. This cannot be undone.") + if (step.hasLaterSiblings()) append("The steps after it move up one place. ") + append("This cannot be undone.") scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } }.trim(), ) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActions.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActions.kt index 0f8be3d4..35daaf31 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActions.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActions.kt @@ -210,7 +210,8 @@ class DeleteTaskAction( label = "Delete task “${task.title.forLabel()}”", preview = buildString { appendLine("Delete the task “${task.title}” from ${target.owner.displayName}'s onboarding path.") - append("The tasks after it move up one place, and any tick on it goes with it. This cannot be undone.") + if (task.hasLaterSiblings()) append("The tasks after it move up one place. ") + append("Any tick on it goes with it. This cannot be undone.") scope.sharedNote(target.owner, context.projectId).takeIf { it.isNotEmpty() }?.let { append("\n\n$it") } }.trim(), ) diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/repository/OnboardingPathDeleteTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/repository/OnboardingPathDeleteTest.kt new file mode 100644 index 00000000..f4ab18f3 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/repository/OnboardingPathDeleteTest.kt @@ -0,0 +1,67 @@ +package com.sprintstart.sprintstartbackend.onboarding.repository + +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus +import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepType +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingPath +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingPhase +import com.sprintstart.sprintstartbackend.onboarding.model.entity.OnboardingStep +import com.sprintstart.sprintstartbackend.shared.crypto.CryptoConfiguration +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest +import org.springframework.context.annotation.Import +import org.springframework.test.context.ActiveProfiles +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.annotation.Propagation +import org.springframework.transaction.annotation.Transactional +import org.springframework.transaction.support.TransactionTemplate +import java.util.UUID + +/** + * `OnboardingPathService.deleteOnboardingPathByUserId` is not transactional, so it reaches + * `deleteByUserId` with no transaction of its own — which is how the buddy's confirm calls it, on a + * coroutine with none around it. Every other test of the path runs inside the test's transaction, where + * a derived delete cannot fail this way, so this one deliberately opts out of it. + */ +@ActiveProfiles("test") +@DataJpaTest +@Import(CryptoConfiguration::class) +@Transactional(propagation = Propagation.NOT_SUPPORTED) +class OnboardingPathDeleteTest { + @Autowired + private lateinit var repository: OnboardingPathRepository + + @Autowired + private lateinit var transactionManager: PlatformTransactionManager + + private fun inTransaction(block: () -> Unit) { + TransactionTemplate(transactionManager).executeWithoutResult { block() } + } + + @Test + fun `deleting a path outside any transaction removes it with what hangs off it`() { + val userId = UUID.randomUUID() + inTransaction { + val path = OnboardingPath(userId = userId) + val phase = OnboardingPhase(path = path, position = 0, title = "Setup", description = "d") + phase.steps += OnboardingStep( + phase = phase, + position = 0, + title = "Install", + description = "d", + type = StepType.TASK, + estimatedMinutes = 5, + expectedOutcome = "it runs", + status = StepStatus.WAITING, + ) + path.phases += phase + repository.save(path) + } + assertThat(repository.existsByUserId(userId)).isTrue() + + repository.deleteByUserId(userId) + + assertThat(repository.existsByUserId(userId)).isFalse() + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentAreaMountTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentAreaMountTest.kt new file mode 100644 index 00000000..017d71b0 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentAreaMountTest.kt @@ -0,0 +1,126 @@ +package com.sprintstart.sprintstartbackend.onboarding.service + +import io.mockk.every +import io.mockk.mockk +import kotlinx.serialization.json.JsonObject +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.ObjectProvider +import java.time.Clock + +/** + * What opening the content area hands the model, checked through the same mounting the buddy uses. + * + * The acceptance criteria that are about the set of tools rather than about one of them live here: + * exactly these tools, and none that can set a hire's progress. + */ +class ContentAreaMountTest { + private val f = ContentFixture() + + private val handlers: List = listOf( + AddPhaseAction(f.scope, f.pathElements, mockk()), + UpdatePhaseAction(f.scope, mockk()), + DeletePhaseAction(f.scope, mockk()), + AddStepAction(f.scope, mockk()), + UpdateStepAction(f.scope, mockk()), + DeleteStepAction(f.scope, mockk()), + AddTaskAction(f.scope, mockk()), + UpdateTaskAction(f.scope, mockk()), + DeleteTaskAction(f.scope, mockk()), + AddResourceAction(f.scope, mockk()), + UpdateResourceAction(f.scope, mockk()), + DeleteResourceAction(f.scope, mockk()), + ReplacePhaseChecksAction(f.scope, mockk()), + AcceptSkipAction(f.scope, mockk()), + DenySkipAction(f.scope, mockk()), + DeleteSkipAction(f.scope, mockk()), + MarkFeedbackReadAction(f.scope, mockk()), + ResetMemberPathAction(f.scope, f.pathElements, mockk()), + AuthorOrientationPacketAction(f.scope, mockk()), + RevertOrientationPacketAction(f.scope, mockk()), + ) + + private val reads = ContentTeamTools( + f.scope, + f.pathElements, + mockk(), + mockk(), + mockk(), + mockk(), + mockk(), + mockk(), + ) + + private val proposals: BuddyProposalService = run { + val provider: ObjectProvider = mockk() + every { provider.orderedStream() } answers { handlers.stream() } + BuddyProposalService(mockk(), mockk(), provider, Clock.systemUTC()) + } + + private fun propertyNames(schema: JsonObject): Set = + (schema["properties"] as? JsonObject)?.keys.orEmpty() + + @Test + fun `opening the content area mounts exactly its reads and actions`() { + val mounted = proposals.actionSpecs(setOf(TeamArea.CONTENT)).map { it.name } + reads.toolSpecs().map { it.name } + + assertThat(mounted).containsExactlyInAnyOrder( + "get_member_path", + "list_pending_skips", + "list_feedback", + "get_phase_checks", + "get_orientation_packet", + "add_phase", + "update_phase", + "delete_phase", + "add_step", + "update_step", + "delete_step", + "add_task", + "update_task", + "delete_task", + "add_resource", + "update_resource", + "delete_resource", + "replace_phase_checks", + "accept_skip", + "deny_skip", + "delete_skip", + "mark_feedback_read", + "reset_member_path", + "author_orientation_packet", + "revert_orientation_packet", + ) + } + + @Test + fun `no two tools share a name, because handlers are keyed by it`() { + val names = handlers.map { it.spec.name } + reads.toolSpecs().map { it.name } + + assertThat(names).doesNotHaveDuplicates() + } + + @Test + fun `every action belongs to the content area and its risk is declared, not taken from the model`() { + assertThat(handlers.map { it.area }).containsOnly(TeamArea.CONTENT) + handlers.forEach { assertThat(propertyNames(it.spec.parameters)).doesNotContain("risk") } + } + + @Test + fun `nothing in the area can tick a task or start, finish or complete a step`() { + val names = handlers.map { it.spec.name } + assertThat(names.filter { Regex("start|finish|complete|tick|done").containsMatchIn(it) }).isEmpty() + + handlers.forEach { handler -> + assertThat(propertyNames(handler.spec.parameters)) + .describedAs(handler.spec.name) + .doesNotContain("finished", "status", "completed", "started", "done") + } + } + + @Test + fun `an unopened area's tools are not mounted`() { + assertThat(proposals.actionSpecs(setOf(TeamArea.TEAM))).isEmpty() + assertThat(proposals.actionAreas()).containsExactly(TeamArea.CONTENT) + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt index 53a02b64..e227d28e 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentFixture.kt @@ -79,8 +79,10 @@ internal class ContentFixture { contains: String = "", stepStatus: StepStatus? = null, finishedSteps: Int = 0, + siblings: Int = 0, ): PathElement { - val element = PathElement(kind, id, owner, title, position, children, contains, stepStatus, finishedSteps) + val element = + PathElement(kind, id, owner, title, position, children, contains, stepStatus, finishedSteps, siblings) every { pathElements.find(kind, id) } returns element return element } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt index 13a32ac6..5c93160f 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt @@ -1,5 +1,6 @@ package com.sprintstart.sprintstartbackend.onboarding.service +import com.sprintstart.sprintstartbackend.onboarding.external.enums.GenerationStatus import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepType import com.sprintstart.sprintstartbackend.onboarding.model.response.path.GetOnboardingPathResponse @@ -100,6 +101,29 @@ class ContentTeamToolsTest { ) } + @Test + fun `a phase the hire is not shown is marked as such`() { + path() + every { pathService.getOnboardingPathByUserId(f.memberId) } returns + GetOnboardingPathResponse( + UUID.randomUUID(), + f.memberId, + Instant.now(), + listOf( + GetOnboardingPhasesResponse( + phaseId, + UUID.randomUUID(), + 0, + "Setup", + "d", + generationStatus = GenerationStatus.FAILED, + ), + ), + ) + + assertThat(read(f.memberId)).contains("[phase_id: $phaseId] — not shown to them") + } + @Test fun `says when the path is also somebody else's`() { path() diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActionsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActionsTest.kt index e135aed6..3d8c1331 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActionsTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/PhaseTeamActionsTest.kt @@ -315,6 +315,17 @@ class PhaseTeamActionsTest { assertThat(draft.preview).contains("cannot be undone") } + @Test + fun `says the later phases move up only when there are later phases`() { + f.element(PathElementKind.PHASE, phaseId, position = 1, siblings = 3) + assertThat(f.proposed(action.draft(f.call("delete_phase", "phase_id" to phaseId), f.context)).preview) + .contains("The phases after it move up one place") + + f.element(PathElementKind.PHASE, phaseId, position = 2, siblings = 3) + assertThat(f.proposed(action.draft(f.call("delete_phase", "phase_id" to phaseId), f.context)).preview) + .doesNotContain("move up") + } + @Test fun `an empty phase makes no claim about what is inside`() { f.element(PathElementKind.PHASE, phaseId, title = "Empty") diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksActionTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksActionTest.kt index e610e3cf..85c0b51f 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksActionTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ReplacePhaseChecksActionTest.kt @@ -102,7 +102,7 @@ class ReplacePhaseChecksActionTest { "1. (kept) short text: Which command builds it?", "2. (new) short text: How do you run the tests?", "Correct answer: make test", - "These are deleted, with everybody's past answers to them:", + "These are deleted, and nobody's past answers to them count any more:", "- Where do logs go?", "no longer waits for it", ) diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActionsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActionsTest.kt index 664893ba..de36eb51 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActionsTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/StepTeamActionsTest.kt @@ -263,6 +263,17 @@ class StepTeamActionsTest { assertThat(draft.preview).contains("Sam Rivera is working on it right now") } + @Test + fun `says the later steps move up only when there are later steps`() { + f.element(PathElementKind.STEP, stepId, position = 0, siblings = 2) + assertThat(f.proposed(action.draft(f.call("delete_step", "step_id" to stepId), f.context)).preview) + .contains("The steps after it move up one place") + + f.element(PathElementKind.STEP, stepId, position = 1, siblings = 2) + assertThat(f.proposed(action.draft(f.call("delete_step", "step_id" to stepId), f.context)).preview) + .doesNotContain("move up") + } + @Test fun `a waiting step makes no claim about progress`() { f.element(PathElementKind.STEP, stepId, stepStatus = StepStatus.WAITING) diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActionsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActionsTest.kt index 34b5b737..092d6d3b 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActionsTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/TaskTeamActionsTest.kt @@ -188,6 +188,17 @@ class TaskTeamActionsTest { assertThat(draft.preview).contains("“Clone”", "cannot be undone") } + @Test + fun `says the later tasks move up only when there are later tasks`() { + f.element(PathElementKind.TASK, taskId, position = 0, siblings = 2) + assertThat(f.proposed(action.draft(f.call("delete_task", "task_id" to taskId), f.context)).preview) + .contains("The tasks after it move up one place", "Any tick on it goes with it") + + f.element(PathElementKind.TASK, taskId, position = 1, siblings = 2) + assertThat(f.proposed(action.draft(f.call("delete_task", "task_id" to taskId), f.context)).preview) + .doesNotContain("move up") + } + @Test fun `performing deletes the task by id`() = runTest { From b170fc0a484fefb023f6d3ef3aa4c17085f6f0b3 Mon Sep 17 00:00:00 2001 From: Linus Date: Mon, 21 Sep 2026 15:58:42 +0200 Subject: [PATCH 4/4] Read a member's path the way dev now serves it dev changed OnboardingPathService.getOnboardingPathByUserId to return the path as the person has it. Phases whose generation produced nothing are no longer listed; they come back as generationIssues instead. get_member_path now lists those under "not shown to them" with their ids, in place of the per-phase marker that could no longer fire. Co-Authored-By: Claude Sonnet 5 --- .../onboarding/service/ContentTeamTools.kt | 12 ++++- .../service/ContentTeamToolsTest.kt | 44 +++++++++++-------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt index 2f340da8..317500e7 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamTools.kt @@ -109,8 +109,7 @@ class ContentTeamTools( scope.sharedNote(owner, projectId).takeIf { it.isNotEmpty() }?.let { appendLine(it) } path.phases.sortedBy { it.position }.forEach { phase -> appendLine() - val hidden = if (phase.generationStatus.isHiddenFromUser()) " — not shown to them" else "" - appendLine("Phase ${phase.position + 1}: ${phase.title} [phase_id: ${phase.id}]$hidden") + appendLine("Phase ${phase.position + 1}: ${phase.title} [phase_id: ${phase.id}]") phase.description.takeIf { it.isNotBlank() }?.let { appendLine(" ${it.short()}") } onboardingStepService.getOnboardingStepsByPhaseId(phase.id).sortedBy { it.position }.forEach { step -> appendLine( @@ -126,6 +125,15 @@ class ContentTeamTools( } } } + if (path.generationIssues.isNotEmpty()) { + appendLine() + appendLine("Not shown to them, because generating them produced nothing usable:") + path.generationIssues.forEach { + appendLine( + "- ${it.title} [phase_id: ${it.phaseId}] (${it.status.name.lowercase().replace('_', ' ')})", + ) + } + } }.trim() } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt index 5c93160f..b1d85585 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/onboarding/service/ContentTeamToolsTest.kt @@ -3,8 +3,9 @@ package com.sprintstart.sprintstartbackend.onboarding.service import com.sprintstart.sprintstartbackend.onboarding.external.enums.GenerationStatus import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepStatus import com.sprintstart.sprintstartbackend.onboarding.external.enums.StepType -import com.sprintstart.sprintstartbackend.onboarding.model.response.path.GetOnboardingPathResponse -import com.sprintstart.sprintstartbackend.onboarding.model.response.phase.GetOnboardingPhasesResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.path.GetOnboardingPathForUserResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.path.OnboardingGenerationIssueResponse +import com.sprintstart.sprintstartbackend.onboarding.model.response.phase.GetOnboardingPhaseForUserResponse import com.sprintstart.sprintstartbackend.onboarding.model.response.resource.GetOnboardingResourcesResponse import com.sprintstart.sprintstartbackend.onboarding.model.response.step.GetOnboardingStepResponse import com.sprintstart.sprintstartbackend.onboarding.model.response.task.GetOnboardingTasksResponse @@ -37,14 +38,25 @@ class ContentTeamToolsTest { private val taskId = UUID.randomUUID() private val resourceId = UUID.randomUUID() + private fun phase(description: String) = + GetOnboardingPhaseForUserResponse( + phaseId, + UUID.randomUUID(), + 0, + "Setup", + description, + locked = false, + steps = emptyList(), + ) + private fun path() { every { pathService.getOnboardingPathByUserId(f.memberId) } returns - GetOnboardingPathResponse( + GetOnboardingPathForUserResponse( id = UUID.randomUUID(), userId = f.memberId, createdAt = Instant.now(), phases = listOf( - GetOnboardingPhasesResponse(phaseId, UUID.randomUUID(), 0, "Setup", "Get the machine ready"), + phase("Get the machine ready"), ), ) every { stepService.getOnboardingStepsByPhaseId(phaseId) } returns @@ -102,26 +114,20 @@ class ContentTeamToolsTest { } @Test - fun `a phase the hire is not shown is marked as such`() { - path() + fun `phases that produced nothing are listed as not shown to them`() { + val hiddenId = UUID.randomUUID() every { pathService.getOnboardingPathByUserId(f.memberId) } returns - GetOnboardingPathResponse( + GetOnboardingPathForUserResponse( UUID.randomUUID(), f.memberId, Instant.now(), - listOf( - GetOnboardingPhasesResponse( - phaseId, - UUID.randomUUID(), - 0, - "Setup", - "d", - generationStatus = GenerationStatus.FAILED, - ), + emptyList(), + generationIssues = listOf( + OnboardingGenerationIssueResponse(hiddenId, "Deep dive", GenerationStatus.FAILED), ), ) - assertThat(read(f.memberId)).contains("[phase_id: $phaseId] — not shown to them") + assertThat(read(f.memberId)).contains("Not shown to them", "Deep dive [phase_id: $hiddenId] (failed)") } @Test @@ -137,11 +143,11 @@ class ContentTeamToolsTest { path() val long = "word ".repeat(200) every { pathService.getOnboardingPathByUserId(f.memberId) } returns - GetOnboardingPathResponse( + GetOnboardingPathForUserResponse( UUID.randomUUID(), f.memberId, Instant.now(), - listOf(GetOnboardingPhasesResponse(phaseId, UUID.randomUUID(), 0, "Setup", long)), + listOf(phase(long)), ) val text = read(f.memberId)