Skip to content

Move the onboarding path rebuild from members to the PM - #269

Open
DavidLeuter wants to merge 3 commits into
devfrom
feature/pm-only-path-rebuild
Open

DavidLeuter wants to merge 3 commits into
devfrom
feature/pm-only-path-rebuild

Conversation

@DavidLeuter

@DavidLeuter DavidLeuter commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Every member had a Rebuild button on their onboarding page. Rebuilding replaces the path and throws their progress away, so it should be the PM's call, not the member's.

Changes

  • Onboarding page: the Rebuild button and its confirmation dialog are gone. On a path whose phases are all hidden, "Try generation again" now shows only for the project's manager; members are told their PM can rebuild the path (the backend refuses members replacing an existing path).
  • Member page in the PM area (/team/:userId): new Rebuild path button in the header (Build path if the member has none yet), behind a confirmation dialog. While it runs the button shows Rebuilding…; when it finishes, the path and member progress are reloaded and a toast reports success or failure (with PM-worded error messages).
  • onboardingService.rebuildMemberPath(projectId, userId, handlers, signal) calls the new backend endpoint. The SSE handling is now shared with personalizePath (streamPathGeneration), no behaviour change there.

Needs the backend counterpart: SprintStartProject/sprintstart-backend#262 (new PM endpoint, and members can no longer rebuild or delete a built path).

Known limitation: if a PM rebuilds while the member has their onboarding page open, the member sees the new path after a reload.

Tests

  • TeamMemberDetailPage.test.tsx: the PM confirms, rebuildMemberPath is called with project and member, and the path is re-read afterwards.
  • OnBoardingPage.test.tsx: no rebuild/regenerate button on a loaded path; "Try generation again" for the manager only, a pointer to the PM for members.
  • Full routine green after merging dev: tsc -b, lint, prettier (changed files), build, 3310 unit tests.

🤖 Generated with Claude Code

Every member had a Rebuild button on their onboarding page, which threw
away their progress. It is gone from there; the PM rebuilds a member's
path from the member's page in the team area instead, through the new
POST /projects/{projectId}/onboarding/users/{userId}/path/personalize.

The SSE handling is shared between the member's own generation and the
PM's rebuild. "Try generation again" on an empty path stays for members.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kiranfin kiranfin left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

The change itself is clean, I did not find anything that should block the merge besides the merge conflict.

🟡 Worth a look

  1. Only a few failure cases get a readable message — src/pages/TeamMemberDetailPage.tsx:55-69, src/services/onboardingService.ts:48
    describeRebuildError maps not-enough-knowledge, the two blueprint cases and 403. Everything else, for example a 404 (member not in the project) or a 409 (a generation is already running), ends up as the raw string HTTP error! status: 409 The current path is unchanged. in the toast. If the backend can return those, they deserve their own sentence; if not, a generic message without the raw status would read better for a PM.

  2. A pending rebuild blocks the button for every other member until it finishes — src/pages/TeamMemberDetailPage.tsx:186-195, 804
    rebuildingUserId is only cleared by the stream events, and the abort effect only runs on unmount. If the page instance is reused when the route param changes (member A to member B), the button for B stays disabled for the whole run of A, which takes a few minutes, and the "rebuilt" toast for A is silently dropped by the shownUserId guard. It is probably acceptable, but either aborting the watch when userId changes or disabling only for rebuildingUserId === user.userId would be more predictable.

🟢 Nits / cleanup

  • In tests/unit/pages/TeamMemberDetailPage.test.tsx the new test was inserted between the comment "The knowledge-gaps overview is the project's full component roster now..." and the test it belongs to (keeps covered components out of the member's gaps panel). The comment now sits above the wrong test.
  • The same test file now mocks the whole onboardingService module with only rebuildMemberPath. PhaseCheckAdminModal, which this page renders, also uses onboardingService.fetchPhaseQuestionsForEditing, savePhaseQuestions and fetchQuestionAttempts. Any test in this file that opens the check modal will now hit undefined. Mocking with importActual and overriding only rebuildMemberPath avoids that.
  • When the member has no path yet, the button says "Build path", but the dialog still says "Rebuild ...'s onboarding path?" with the danger variant and "progress on the current path is replaced". A neutral wording for that case would fit better.
  • In streamPathGeneration, when the Keycloak token refresh fails, the function calls keycloak.login() and returns without calling any handler. For the member page that is fine because the page redirects, but for the PM rebuild the "Rebuilding..." toast and the loading state would stay until the redirect happens. Calling handlers.onError before returning would keep both call sites consistent.
  • Test coverage covers only the happy path (confirm, then onDone, then the path is reloaded). Missing: cancelling the dialog, the onError messages (especially not-enough-knowledge and 403), onInterrupted, aborting on unmount, and a service test for rebuildMemberPath next to the existing personalizePath SSE test.

Once the backend endpoint is merged and the merge conflict in OnBoardingPage.tsx is resolved, I would be fine with merging.

DavidLeuter and others added 2 commits September 29, 2026 13:38
…ebuild

# Conflicts:
#	src/pages/OnBoardingPage.tsx
The backend now refuses to replace an existing path for anyone but the
project's manager, so "Try generation again" on a path whose phases are
all hidden could only fail with 403 for a member. Managers keep the
button; members are told their PM can rebuild the path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants