Skill-management active toggle never persists — PUT payload omits isActive and the reload reverts the flip #900

New Issue

2026-07-30T04:03:33Z

spikerj commented

2026-07-30 04:03:33 +00:00

Found while adding render coverage for skill-management.component.html (wave-5 coverage loop, angular branch test/component-coverage-wave5).

Symptom

In Admin → Skill management → Definitions, flipping a definition's active slide-toggle shows a green "Deactivated"/"Activated" toast, the toggle visibly flips — and then flips straight back. The skill's active state never changes on the server. There is no error, so it reads as a UI glitch rather than a failed write.

Root cause

SkillManagementComponent.toggleActive() (projects/spikersoft/src/app/_components/admin/skill-management/skill-management.component.ts:263):

async toggleActive(def: SkillDefinitionDto): Promise<void> {
  this.processing.set(true);
  try {
    await this.skillsService.updateDefinition(def.id, { isAutoAwarded: def.isAutoAwarded });   // <-- no isActive
    this.definitions.update((defs) => defs.map((d) => (d.id === def.id ? { ...d, isActive: !d.isActive } : d)));
    this.snackBar.open(def.isActive ? ...deactivated : ...activated, ...);
    await this.loadDefinitions();          // <-- overwrites the optimistic flip
  } ...
}

Three things combine:

The PUT body is { isAutoAwarded: def.isAutoAwarded } — it re-sends the value that is already set and says nothing about isActive.
isActive is unrepresentable on this endpoint: updateDefinition(id, req: Partial<CreateSkillDefinitionRequest>) and CreateSkillDefinitionRequest has no isActive field at all (projects/spikersoft/src/app/_services/skills/skills.service.ts). So this is not a missing-field typo — the client contract cannot express the change.
The local list is flipped optimistically, then loadDefinitions() immediately refetches and overwrites it with the unchanged server row. Hence the snap-back.

Impact

Admins cannot retire a skill definition from the UI. A retired-looking skill stays active and keeps being offered in the award picker (@if (def.isActive) on the award tab) and keeps being auto-awarded if it has criteria. The success toast makes it look like it worked.

Fix sketch (needs a backend decision first)

Confirm whether PUT api/skills/definitions/{id} accepts isActive. If it does, add isActive to CreateSkillDefinitionRequest (or a dedicated UpdateSkillDefinitionRequest) and send { isActive: !def.isActive }.
If it does not, add a dedicated activate/deactivate route and call that.
Either way, drop the optimistic definitions.update(...) or keep it and drop the immediate loadDefinitions() — doing both guarantees the flicker even once the payload is fixed.

Current behavior is pinned

skill-management.render.spec.ts has a characterization test referencing this ticket which asserts the broken behavior (payload without isActive, isActive still true after the call, status badge unchanged) so the suite stays green. Update that test as part of the fix — it is written to fail once the toggle starts working.

Found while adding render coverage for `skill-management.component.html` (wave-5 coverage loop, angular branch `test/component-coverage-wave5`). ## Symptom In **Admin → Skill management → Definitions**, flipping a definition's active slide-toggle shows a green "Deactivated"/"Activated" toast, the toggle visibly flips — and then flips straight back. The skill's active state never changes on the server. There is no error, so it reads as a UI glitch rather than a failed write. ## Root cause `SkillManagementComponent.toggleActive()` (`projects/spikersoft/src/app/_components/admin/skill-management/skill-management.component.ts:263`): ```ts async toggleActive(def: SkillDefinitionDto): Promise<void> { this.processing.set(true); try { await this.skillsService.updateDefinition(def.id, { isAutoAwarded: def.isAutoAwarded }); // <-- no isActive this.definitions.update((defs) => defs.map((d) => (d.id === def.id ? { ...d, isActive: !d.isActive } : d))); this.snackBar.open(def.isActive ? ...deactivated : ...activated, ...); await this.loadDefinitions(); // <-- overwrites the optimistic flip } ... } ``` Three things combine: 1. The PUT body is `{ isAutoAwarded: def.isAutoAwarded }` — it re-sends the value that is *already* set and says nothing about `isActive`. 2. `isActive` is **unrepresentable** on this endpoint: `updateDefinition(id, req: Partial<CreateSkillDefinitionRequest>)` and `CreateSkillDefinitionRequest` has no `isActive` field at all (`projects/spikersoft/src/app/_services/skills/skills.service.ts`). So this is not a missing-field typo — the client contract cannot express the change. 3. The local list is flipped optimistically, then `loadDefinitions()` immediately refetches and overwrites it with the unchanged server row. Hence the snap-back. ## Impact Admins cannot retire a skill definition from the UI. A retired-looking skill stays active and keeps being offered in the award picker (`@if (def.isActive)` on the award tab) and keeps being auto-awarded if it has criteria. The success toast makes it look like it worked. ## Fix sketch (needs a backend decision first) - Confirm whether `PUT api/skills/definitions/{id}` accepts `isActive`. If it does, add `isActive` to `CreateSkillDefinitionRequest` (or a dedicated `UpdateSkillDefinitionRequest`) and send `{ isActive: !def.isActive }`. - If it does not, add a dedicated activate/deactivate route and call that. - Either way, drop the optimistic `definitions.update(...)` or keep it and drop the immediate `loadDefinitions()` — doing both guarantees the flicker even once the payload is fixed. ## Current behavior is pinned `skill-management.render.spec.ts` has a characterization test referencing this ticket which asserts the *broken* behavior (payload without `isActive`, `isActive` still true after the call, status badge unchanged) so the suite stays green. **Update that test as part of the fix** — it is written to fail once the toggle starts working.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#900