84 improve checklist reconcile and complete flow by InfinityBowman · Pull Request #86 · InfinityBowman/corates

InfinityBowman · 2025-12-18T17:28:22Z

Summary by CodeRabbit

New Features
- Confirmation dialog when marking checklists completed (locks checklist)
- Reconciliation status badge indicating ready/waiting states
Improvements
- Renamed tab to "Reconciliation" and reorganized reconciliation workflow
- Smarter checklist filtering for single vs dual-reviewer flows; reconcile button enabled only when ready
- To Do view now shows user-specific active checklists and improved empty-state UI
- Updated navbar logo
Chores
- Removed several planning documents from the repo

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…moved around correctly, and inform user of state

coderabbitai · 2025-12-18T17:28:31Z

Walkthrough

This PR deletes seven planning documents, refactors reconciliation-related UI across project tabs (todo, reconcile, completed, project view), adds a ReconcileStatusTag component, updates checklist completion flow with a confirmation dialog, tweaks navigation/counts, and changes the landing navbar logo image source.

Changes

Cohort / File(s)	Summary
Documentation Plans Removal `docs/plans/audit-logs-plan.md`, `docs/plans/cn-utility-migration.plan.md`, `docs/plans/dashboard-ux-improvements.plan.md`, `docs/plans/presence.plan.md`, `docs/plans/pricing-model.plan.md`, `docs/plans/stripe-implementation.plan.md`, `docs/plans/yjs-awareness.plan.md`	Deleted seven planning/specification documents covering audit logging, cn() migration, dashboard UX, presence, pricing, Stripe integration, and Y.js awareness.
Reconciliation UI & Counts `packages/web/src/components/project-ui/ProjectView.jsx`, `packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx`, `packages/web/src/components/project-ui/ReconcileStudyCard.jsx`	Renamed "Ready to Reconcile" → "Reconciliation"; changed filtering to include 1–2 completed checklists for dual-reviewer workflows; updated tab value to `reconcile`; added readiness gating and conditional reviewer display.
New Reconcile Status Component `packages/web/src/components/project-ui/reconcile-tab/ReconcileStatusTag.jsx`	Added `ReconcileStatusTag` component rendering "Ready" or "Waiting for {reviewer}" badges based on completed checklists and reviewer assignments.
Completed Tab & Card Logic `packages/web/src/components/project-ui/completed-tab/CompletedTab.jsx`, `packages/web/src/components/project-ui/completed-tab/CompletedStudyCard.jsx`	Differentiated single- vs dual-reviewer behavior: single-reviewer shows completed checklists; dual-reviewer shows reconciled checklists; updated display selection logic.
To‑Do Tab Filtering `packages/web/src/components/project-ui/todo-tab/ToDoTab.jsx`	Restricts studies to those where current user is a reviewer; computes user-specific active (non-completed) checklists and a `_needsChecklist` flag; filters displayed studies accordingly and updates empty-state UI.
Checklist Completion Flow `packages/web/src/components/checklist-ui/ChecklistYjsWrapper.jsx`	Made `handleToggleComplete` async; added `useConfirmDialog` confirmation before marking `completed`; treats `completed` as read-only; injects confirm dialog component into header; dynamic back-navigation based on checklist status.
Navigation Target Update `packages/web/src/components/checklist-ui/compare/ReconciliationWrapper.jsx`	Changed cancel navigation to use the new `reconcile` tab (`?tab=reconcile`).
Header Cleanup `packages/web/src/components/project-ui/ProjectHeader.jsx`	Removed a `createEffect` that logged the project name (deleted console.log side-effect).
Landing Navbar Logo `packages/landing/src/components/Navbar.jsx`	Swapped logo image source from `/CoRATES.svg` to `/logo.svg`.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–60 minutes

Areas requiring extra attention:

Reconciliation filtering and readiness logic across ReconcileTab.jsx, ReconcileStudyCard.jsx, ProjectView.jsx, and ReconcileStatusTag.jsx.
Checklist completion flow changes in ChecklistYjsWrapper.jsx (async confirmation, read-only semantics, UI injection).
Single- vs dual-reviewer branching in CompletedTab.jsx and CompletedStudyCard.jsx.
Consistency of tab value/name changes (ready-to-reconcile → reconcile) across navigation and routing.
ToDoTab user-specific checklist selection and _needsChecklist flag correctness.

Possibly related PRs

#71 — Adds the @corates/ui package and UI primitives (e.g., useConfirmDialog) now imported and used by ChecklistYjsWrapper.jsx.
#77 — Overlapping checklist and reconciliation UI changes; touches similar checklist-ui components and flows.
#83 — Modifies project UI tabs and reconciliation-related components (ProjectView, ToDoTab, ReconcileTab, CompletedTab), indicating related refactoring.

Poem

🐰 I nudged the plans into a drawer,
Tabs renamed and badges soar,
Checklists lock with one brave click,
Reviewers wait, the UI's slick —
A hop, a cheer, the code's encore! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes in the PR, which focus on improving the checklist reconciliation and completion workflows across multiple components.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch 84-improve-checklist-reconcile-and-complete-flow

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 59657d8 and 0b2733a.

⛔ Files ignored due to path filters (2)

packages/landing/public/favicon.svg is excluded by !**/*.svg
packages/landing/public/logo.svg is excluded by !**/*.svg

📒 Files selected for processing (1)

packages/landing/src/components/Navbar.jsx (1 hunks)

✅ Files skipped from review due to trivial changes (1)

packages/landing/src/components/Navbar.jsx

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Workers Builds: corates

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

cloudflare-workers-and-pages · 2025-12-18T17:29:38Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	corates	`0b2733a`	Commit Preview URL	Dec 18 2025, 07:18 PM

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (7)

packages/web/src/components/project-ui/reconcile-tab/ReconcileStatusTag.jsx (2)
16-28: Edge case: waitingForReviewer() may return unexpected result when completed reviewer doesn't match study reviewers.

If the completed checklist's assignedTo doesn't match study.reviewer1, the logic assumes it must be reviewer2. However, if data is inconsistent (e.g., a checklist assigned to a removed reviewer), waitingReviewerId could incorrectly point to reviewer1, or if reviewer2 is null/undefined, the badge would display "Waiting for Unknown".

Consider adding defensive handling:
Suggested improvement
 const waitingForReviewer = () => {
   if (isReady()) return null;

   const completed = completedChecklists();
   if (completed.length !== 1) return null;

   const completedReviewerId = completed[0].assignedTo;
+  // Ensure completed reviewer is one of the assigned reviewers
+  if (completedReviewerId !== props.study.reviewer1 && completedReviewerId !== props.study.reviewer2) {
+    return null;
+  }
   // Find the other reviewer
   const waitingReviewerId =
     completedReviewerId === props.study.reviewer1 ? props.study.reviewer2 : props.study.reviewer1;

+  if (!waitingReviewerId) return null;
   return props.getAssigneeName(waitingReviewerId);
 };
30-42: Consider handling null in the fallback rendering.

If waitingForReviewer() returns null (when there are 0 or 2+ completed checklists, but isReady() is false due to data inconsistency), the badge would render "Waiting for " with no name.
Suggested improvement
-      fallback={
-        <span class='inline-flex items-center px-2.5 py-1 rounded-full text-xs font-medium bg-yellow-100 text-yellow-800'>
-          Waiting for {waitingForReviewer()}
-        </span>
-      }
+      fallback={
+        <Show when={waitingForReviewer()}>
+          {name => (
+            <span class='inline-flex items-center px-2.5 py-1 rounded-full text-xs font-medium bg-yellow-100 text-yellow-800'>
+              Waiting for {name()}
+            </span>
+          )}
+        </Show>
+      }
packages/web/src/components/project-ui/ReconcileStudyCard.jsx (1)

16-22: Note: Duplicated completedChecklists and isReady logic.

Both ReconcileStudyCard and ReconcileStatusTag define identical completedChecklists() and isReady() computations. This is acceptable for component isolation, but if the logic needs to change in the future, it will need updates in multiple places.

Consider extracting to a shared utility if this pattern expands further.
packages/web/src/components/project-ui/ProjectView.jsx (1)
183-190: The to-do filtering logic looks good.

The implementation correctly identifies studies where the user is a reviewer and has work to complete. The logic appropriately includes both studies where the user has no checklist yet and those with non-completed checklists.
Optional: Clarify the comment for precision

The comment on line 183 could more accurately reflect that the filter includes studies with no checklists assigned to the user:
-    // Count studies where user is a reviewer and has non-completed checklists to work on
+    // Count studies where user is a reviewer and has no checklist or has non-completed checklists
packages/web/src/components/project-ui/completed-tab/CompletedTab.jsx (1)
14-25: Filtering logic correctly distinguishes reviewer configurations.

The implementation properly handles two workflows:

Single-reviewer studies go directly to completed when the checklist is done

Dual-reviewer studies require reconciliation before appearing as completed

This aligns well with the overall reconciliation flow improvements.
Optional: Consider extracting the isSingleReviewer pattern

The single-reviewer check pattern (study.reviewer1 && !study.reviewer2) appears in multiple files (CompletedTab.jsx, CompletedStudyCard.jsx). You could extract this into a utility function for better maintainability:
// packages/web/src/utils/studyHelpers.js (or similar)
export const isSingleReviewerStudy = (study) => {
  return study?.reviewer1 && !study?.reviewer2;
};
Then import and use it:
-    const isSingleReviewer = study.reviewer1 && !study.reviewer2;
+    const isSingleReviewer = isSingleReviewerStudy(study);
packages/web/src/components/checklist-ui/ChecklistYjsWrapper.jsx (2)
260-288: Unreachable code: the status === 'completed' check is redundant.

Since isReadOnly() (line 261) already returns true when status === 'completed' (see lines 49-50), the function will return early before reaching the check on line 266. The toast message "Checklist Locked" will never be shown.

Consider removing the redundant block or restructuring if the specific toast message is desired for this case.
🔎 Option 1: Remove the redundant block
 async function handleToggleComplete() {
   if (isReadOnly()) return;
   const checklist = currentChecklist();
   if (!checklist) return;

-  // If already completed, don't allow toggle back (checklist is locked)
-  if (checklist.status === 'completed') {
-    showToast.info('Checklist Locked', 'Completed checklists cannot be edited.');
-    return;
-  }
-
   // Show confirmation dialog before marking complete
   const confirmed = await confirmDialog.open({
🔎 Option 2: Show specific toast for completed status

If the distinct toast message is intentional, check for completed status before the generic isReadOnly() check:
 async function handleToggleComplete() {
+  const checklist = currentChecklist();
+  if (!checklist) return;
+
+  // If already completed, show specific locked message
+  if (checklist.status === 'completed') {
+    showToast.info('Checklist Locked', 'Completed checklists cannot be edited.');
+    return;
+  }
+
   if (isReadOnly()) return;
-  const checklist = currentChecklist();
-  if (!checklist) return;
-
-  // If already completed, don't allow toggle back (checklist is locked)
-  if (checklist.status === 'completed') {
-    showToast.info('Checklist Locked', 'Completed checklists cannot be edited.');
-    return;
-  }

   // Show confirmation dialog before marking complete
356-365: Dead code: button's completed state is unreachable.

This button is only rendered when !isReadOnly() is true. Since isReadOnly() returns true when status === 'completed', the button will never be visible in the completed state. The ternary conditions checking for status === 'completed' (styling and text) will always evaluate to the false branch.

Consider simplifying to remove the unreachable paths:
🔎 Simplify the button
         <button
           onClick={handleToggleComplete}
-          class={`px-3 py-1.5 text-sm font-medium rounded-lg transition-colors ${
-            currentChecklist()?.status === 'completed' ?
-              'bg-green-100 text-green-700 hover:bg-green-200'
-            : 'bg-blue-600 text-white hover:bg-blue-700'
-          }`}
+          class='px-3 py-1.5 text-sm font-medium rounded-lg transition-colors bg-blue-600 text-white hover:bg-blue-700'
         >
-          {currentChecklist()?.status === 'completed' ? 'Completed' : 'Mark Complete'}
+          Mark Complete
         </button>

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d68cdf2 and 59657d8.

📒 Files selected for processing (17)

docs/plans/audit-logs-plan.md (0 hunks)
docs/plans/cn-utility-migration.plan.md (0 hunks)
docs/plans/dashboard-ux-improvements.plan.md (0 hunks)
docs/plans/presence.plan.md (0 hunks)
docs/plans/pricing-model.plan.md (0 hunks)
docs/plans/stripe-implementation.plan.md (0 hunks)
docs/plans/yjs-awareness.plan.md (0 hunks)
packages/web/src/components/checklist-ui/ChecklistYjsWrapper.jsx (6 hunks)
packages/web/src/components/checklist-ui/compare/ReconciliationWrapper.jsx (1 hunks)
packages/web/src/components/project-ui/ProjectHeader.jsx (1 hunks)
packages/web/src/components/project-ui/ProjectView.jsx (3 hunks)
packages/web/src/components/project-ui/ReconcileStudyCard.jsx (2 hunks)
packages/web/src/components/project-ui/completed-tab/CompletedStudyCard.jsx (3 hunks)
packages/web/src/components/project-ui/completed-tab/CompletedTab.jsx (1 hunks)
packages/web/src/components/project-ui/reconcile-tab/ReconcileStatusTag.jsx (1 hunks)
packages/web/src/components/project-ui/reconcile-tab/ReconcileTab.jsx (2 hunks)
packages/web/src/components/project-ui/todo-tab/ToDoTab.jsx (3 hunks)

💤 Files with no reviewable changes (7)

docs/plans/yjs-awareness.plan.md
docs/plans/dashboard-ux-improvements.plan.md
docs/plans/cn-utility-migration.plan.md
docs/plans/pricing-model.plan.md
docs/plans/audit-logs-plan.md
docs/plans/presence.plan.md
docs/plans/stripe-implementation.plan.md

🧰 Additional context used

📓 Path-based instructions (8)

**/*.{jsx,tsx,js,ts}