Add explicit active state for accepted tasks - #715
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
|
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| const matchesActiveStatus = (completion: RawTaskCompletion): boolean => | ||
| isTaskActive(completion) || isTaskComplete(completion); | ||
| const requiresActiveStatus = (statuses: string[]): boolean => | ||
| ['active', 'accept', 'accepted'].some((status) => statuses.includes(status)); | ||
| const matchesRequiredActiveStatus = ( | ||
| statuses: string[], | ||
| completion: RawTaskCompletion, | ||
| taskId: string, | ||
| isUnlockable: (taskId: string) => boolean | ||
| completion: RawTaskCompletion | ||
| ): boolean => { | ||
| if (!requiresActiveStatus(statuses)) return false; | ||
| return matchesActiveStatus(completion, taskId, isUnlockable); | ||
| return matchesActiveStatus(completion); |
There was a problem hiding this comment.
⚠️ Edge Case: Legacy accepted tasks lock downstream 'active'-requirement tasks
matchesActiveStatus in taskAvailability.ts no longer falls back to isUnlockable, and isTaskActive now returns true only when completion.active===true. Legacy accepted tasks (stored incomplete with no active field, which the PR intentionally keeps as "unknown") therefore no longer satisfy a downstream task's ['active']/accept/accepted requirement. Any task B whose taskRequirement points at such a prerequisite A will flip from available to locked for existing users who accepted A before this migration, silently hiding tasks until they re-accept. Consider treating an unknown legacy prerequisite as still satisfying an active requirement (e.g. keep an unlockable/present fallback for missing-active records) or backfilling active state for incomplete legacy rows that already gate downstream tasks.
Was this helpful? React with 👍 / 👎
| @@ -255,7 +255,7 @@ const applyStartedImports = ( | |||
| for (const taskId of startedTaskIds) { | |||
| const flags = getCompletionFlags(completions[taskId]); | |||
| const shouldStart = shouldStartImportedTask(completedTaskIds.has(taskId), flags); | |||
| if (shouldStart) store.setTaskUncompleted(taskId); | |||
| if (shouldStart) store.setTaskActive(taskId); | |||
| } | |||
There was a problem hiding this comment.
💡 Bug: Imported task-start restarts failed tasks, clobbering failed state
shouldStartImportedTask changed from !alreadyCompleted && !flags.complete && !flags.failed to !alreadyCompleted && (!flags.complete || flags.failed), so a task currently in the failed state (complete=true, failed=true) now returns true and applyStartedImports calls store.setTaskActive on it. Because applyStartedImports runs after applyCompletedImports, a task that was just marked failed (e.g. via a failed requirement) but also appears in startedTaskIds will have its failed state overwritten with active. If restarting failed tasks is intended, confirm the ordering doesn't erase failures derived within the same import; otherwise exclude failed tasks from the started set.
Was this helpful? React with 👍 / 👎
| @@ -46,6 +46,16 @@ | |||
| > | |||
| {{ t('common.complete', 'Complete') }} | |||
| </UButton> | |||
| <UButton | |||
| v-else-if="state === 'available' && !isFailed" | |||
| :size="size" | |||
| color="primary" | |||
| variant="soft" | |||
There was a problem hiding this comment.
💡 Quality: Available tasks can no longer be completed in one click on card
TaskCardActions now shows an 'Accept' button (emits 'active') for state==='available' and only shows 'Complete' for state==='active'. Users must now Accept then Complete, a two-step change from the previous single-click completion of available tasks. This also diverges from other entry points (e.g. KappaTaskRow) that still call markTaskComplete directly on available tasks, creating inconsistent behavior. If the forced Accept→Complete flow is intended, align the other completion paths; otherwise retain a direct-complete affordance for available tasks.
Was this helpful? React with 👍 / 👎
Code Review
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
1 participant
Summary
activestate for accepted ordinary tasksWhy
The existing persisted representation uses incomplete state for both accepted tasks and tasks that are merely unlocked. That makes accepted-task state impossible to query reliably and causes log-imported task-start events to lose their meaning.
This change adds the missing lifecycle distinction without mass-migrating ambiguous legacy rows. Consumers must treat a missing
activefield as unknown; onlyactive: trueis authoritative acceptance.Compatibility
Validation
This is intentionally a draft for maintainer feedback on the additive lifecycle contract and legacy-row semantics.