Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions packages/web/src/components/checklist/ChecklistYjsWrapper.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ export default function ChecklistYjsWrapper() {
const study = currentStudy();
if (!checklist || !study) return;

// If already completed, don't allow toggle back (checklist is locked)
if (checklist.status === CHECKLIST_STATUS.COMPLETED) {
// If already finalized, don't allow toggle back (checklist is locked)
if (checklist.status === CHECKLIST_STATUS.FINALIZED) {
showToast.info('Checklist Locked', 'Completed checklists cannot be edited.');
return;
}
Expand Down Expand Up @@ -290,7 +290,7 @@ export default function ChecklistYjsWrapper() {
updateChecklist(params.studyId, params.checklistId, { status: nextStatus });

const statusLabel =
nextStatus === CHECKLIST_STATUS.COMPLETED ? 'completed' : 'awaiting reconciliation';
nextStatus === CHECKLIST_STATUS.FINALIZED ? 'completed' : 'awaiting reconciliation';
showToast.success(
'Checklist Completed',
`This checklist has been marked as ${statusLabel} and is now locked.`,
Expand Down Expand Up @@ -362,12 +362,12 @@ export default function ChecklistYjsWrapper() {
fallback={
<span
class={`rounded-lg px-3 py-1.5 text-sm font-medium ${
currentChecklist()?.status === CHECKLIST_STATUS.COMPLETED ?
currentChecklist()?.status === CHECKLIST_STATUS.FINALIZED ?
'bg-green-100 text-green-700'
: 'bg-gray-100 text-gray-700'
}`}
>
{currentChecklist()?.status === CHECKLIST_STATUS.COMPLETED ?
{currentChecklist()?.status === CHECKLIST_STATUS.FINALIZED ?
'Completed'
: 'Read-only'}
</span>
Expand All @@ -382,13 +382,13 @@ export default function ChecklistYjsWrapper() {
: undefined
}
class={`rounded-lg px-3 py-1.5 text-sm font-medium transition-colors ${
currentChecklist()?.status === CHECKLIST_STATUS.COMPLETED ?
currentChecklist()?.status === CHECKLIST_STATUS.FINALIZED ?
'bg-green-100 text-green-700 hover:bg-green-200'
: !isChecklistValid() ? 'cursor-not-allowed bg-gray-300 text-gray-500 opacity-60'
: 'bg-blue-600 text-white hover:bg-blue-700'
}`}
>
{currentChecklist()?.status === CHECKLIST_STATUS.COMPLETED ?
{currentChecklist()?.status === CHECKLIST_STATUS.FINALIZED ?
'Completed'
: 'Mark Complete'}
</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,9 @@ export default function ReconciliationWrapper() {
progress.checklist2Id === params.checklist2Id &&
progress.reconciledChecklistId
) {
// Verify it still exists and is not completed
// Verify it still exists and is not finalized
const existingChecklist = study.checklists?.find(
c => c.id === progress.reconciledChecklistId && c.status !== CHECKLIST_STATUS.COMPLETED,
c => c.id === progress.reconciledChecklistId && c.status !== CHECKLIST_STATUS.FINALIZED,
);
if (existingChecklist) {
setReconciledChecklistId(progress.reconciledChecklistId);
Expand All @@ -249,7 +249,7 @@ export default function ReconciliationWrapper() {

// Check if a reconciled checklist already exists (another client may have created it)
const existingReconciled = findReconciledChecklist(study);
if (existingReconciled && existingReconciled.status !== CHECKLIST_STATUS.COMPLETED) {
if (existingReconciled && existingReconciled.status !== CHECKLIST_STATUS.FINALIZED) {
// Found existing - save reference in progress and use it
saveReconciliationProgress(params.studyId, {
checklist1Id: params.checklist1Id,
Expand All @@ -273,9 +273,9 @@ export default function ReconciliationWrapper() {
return;
}

// Mark it as in-progress (reconciled checklist starts as in-progress)
// Mark it as reconciling (reconciled checklist starts as reconciling)
updateChecklist(params.studyId, newChecklistId, {
status: CHECKLIST_STATUS.IN_PROGRESS,
status: CHECKLIST_STATUS.RECONCILING,
title: 'Reconciled Checklist',
});

Expand Down Expand Up @@ -365,19 +365,14 @@ export default function ReconciliationWrapper() {
throw new Error('No reconciled checklist found');
}

// Mark the reconciled checklist as completed
// Mark the reconciled checklist as finalized
updateChecklist(params.studyId, id, {
status: CHECKLIST_STATUS.COMPLETED,
status: CHECKLIST_STATUS.FINALIZED,
title: reconciledName || 'Reconciled Checklist',
});

// Mark the individual reviewer checklists as completed
updateChecklist(params.studyId, params.checklist1Id, {
status: CHECKLIST_STATUS.COMPLETED,
});
updateChecklist(params.studyId, params.checklist2Id, {
status: CHECKLIST_STATUS.COMPLETED,
});
// Individual reviewer checklists remain as REVIEWER_COMPLETED (they were already set when reviewers completed them)
// No need to update them - they're historical records

// Keep reconciliation progress (checklist1Id and checklist2Id) so users can view previous reviewers
// The progress data is needed for the "View Previous" button in the completed tab
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ export default function AMSTAR2ResultsTable(props) {
const reconciledChecklist = checklists.find(
c => c.id === study.reconciliation.reconciledChecklistId && c.type === 'AMSTAR2',
);
if (reconciledChecklist && reconciledChecklist.status === CHECKLIST_STATUS.COMPLETED) {
if (reconciledChecklist && reconciledChecklist.status === CHECKLIST_STATUS.FINALIZED) {
checklistToScore = reconciledChecklist;
}
}

// If no reconciled checklist, use first completed AMSTAR2 checklist
// If no reconciled checklist, use first finalized AMSTAR2 checklist
if (!checklistToScore) {
checklistToScore = checklists.find(
c => c.type === 'AMSTAR2' && c.status === CHECKLIST_STATUS.COMPLETED,
c => c.type === 'AMSTAR2' && c.status === CHECKLIST_STATUS.FINALIZED,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ export default function ChartSection(props) {
if (checklists.length === 0) continue;

for (const checklist of checklists) {
// Only include completed AMSTAR2 checklists
if (checklist.status !== CHECKLIST_STATUS.COMPLETED) continue;
// Only include finalized AMSTAR2 checklists
if (checklist.status !== CHECKLIST_STATUS.FINALIZED) continue;
if (checklist.type !== 'AMSTAR2') continue;

// Answers are pre-computed during sync and stored on the checklist
Expand Down
10 changes: 6 additions & 4 deletions packages/web/src/components/project/overview-tab/OverviewTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@ export default function OverviewTab() {
const readyToReconcile = () =>
studies().filter(s => {
const checklists = s.checklists || [];
const completedChecklists = checklists.filter(c => c.status === CHECKLIST_STATUS.COMPLETED);
const completedChecklists = checklists.filter(
c => c.status === CHECKLIST_STATUS.REVIEWER_COMPLETED,
);
return completedChecklists.length === 2;
}).length;
}).length / 2; // Divide by 2 because we need to count the number of studies that have both reviewers completed, not the number of checklists
Comment on lines 46 to +53
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove the incorrect division by 2.

The division by 2 on Line 53 is incorrect. The filter already counts studies (not checklists) where exactly 2 reviewers have REVIEWER_COMPLETED status. Each qualifying study is counted once, so dividing by 2 will incorrectly report half the actual number of studies ready to reconcile.

The comment's reasoning is flawed—the filter's completedChecklists.length === 2 condition already ensures we're counting studies with both reviewers completed, not counting individual checklists.

Proposed fix
 const readyToReconcile = () =>
   studies().filter(s => {
     const checklists = s.checklists || [];
     const completedChecklists = checklists.filter(
       c => c.status === CHECKLIST_STATUS.REVIEWER_COMPLETED,
     );
     return completedChecklists.length === 2;
-  }).length / 2; // Divide by 2 because we need to count the number of studies that have both reviewers completed, not the number of checklists
+  }).length;
🤖 Prompt for AI Agents
In packages/web/src/components/project/overview-tab/OverviewTab.jsx around lines
46 to 53, the function readyToReconcile incorrectly divides the filtered studies
count by 2; remove the " / 2" so the function returns the actual number of
studies whose checklists have exactly two REVIEWER_COMPLETED entries. Update the
trailing comment to reflect that each filtered item is one study (no division
needed).


const completedStudies = () =>
studies().filter(s => shouldShowInTab(s, 'completed', null)).length;
Expand Down Expand Up @@ -84,8 +86,8 @@ export default function OverviewTab() {
const userChecklists = checklists.filter(c => c.assignedTo === userId);
const hasCompleted = userChecklists.some(
c =>
c.status === CHECKLIST_STATUS.COMPLETED ||
c.status === CHECKLIST_STATUS.AWAITING_RECONCILE,
c.status === CHECKLIST_STATUS.FINALIZED ||
c.status === CHECKLIST_STATUS.REVIEWER_COMPLETED,
);

if (hasCompleted) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { isReconciledChecklist } from '@/lib/checklist-domain.js';
export default function ReconcileStatusTag(props) {
const awaitingReconcileChecklists = () =>
(props.study.checklists || []).filter(
c => !isReconciledChecklist(c) && c.status === CHECKLIST_STATUS.AWAITING_RECONCILE,
c => !isReconciledChecklist(c) && c.status === CHECKLIST_STATUS.REVIEWER_COMPLETED,
);

const isReady = () => awaitingReconcileChecklists().length === 2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export default function ReconcileStudyRow(props) {
// Get the individual reviewer checklists awaiting reconciliation
const awaitingReconcileChecklists = () => {
return (study().checklists || []).filter(
c => !isReconciledChecklist(c) && c.status === CHECKLIST_STATUS.AWAITING_RECONCILE,
c => !isReconciledChecklist(c) && c.status === CHECKLIST_STATUS.REVIEWER_COMPLETED,
);
};

Expand Down
45 changes: 27 additions & 18 deletions packages/web/src/constants/checklist-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
export const CHECKLIST_STATUS = {
PENDING: 'pending',
IN_PROGRESS: 'in-progress',
COMPLETED: 'completed',
AWAITING_RECONCILE: 'awaiting-reconcile',
REVIEWER_COMPLETED: 'reviewer-completed',
RECONCILING: 'reconciling',
FINALIZED: 'finalized',
};

/**
Expand All @@ -18,7 +19,11 @@ export const CHECKLIST_STATUS = {
* @returns {boolean} True if the checklist can be edited
*/
export function isEditable(status) {
return status !== CHECKLIST_STATUS.COMPLETED && status !== CHECKLIST_STATUS.AWAITING_RECONCILE;
return (
status !== CHECKLIST_STATUS.FINALIZED &&
status !== CHECKLIST_STATUS.REVIEWER_COMPLETED &&
status !== CHECKLIST_STATUS.RECONCILING
);
}

/**
Expand All @@ -32,10 +37,12 @@ export function getStatusLabel(status) {
return 'Pending';
case CHECKLIST_STATUS.IN_PROGRESS:
return 'In Progress';
case CHECKLIST_STATUS.COMPLETED:
return 'Completed';
case CHECKLIST_STATUS.AWAITING_RECONCILE:
return 'Awaiting Reconcile';
case CHECKLIST_STATUS.REVIEWER_COMPLETED:
return 'Reviewer Completed';
case CHECKLIST_STATUS.RECONCILING:
return 'Reconciling';
case CHECKLIST_STATUS.FINALIZED:
return 'Finalized';
default:
return status || 'Pending';
}
Expand All @@ -48,12 +55,14 @@ export function getStatusLabel(status) {
*/
export function getStatusStyle(status) {
switch (status) {
case CHECKLIST_STATUS.COMPLETED:
case CHECKLIST_STATUS.FINALIZED:
return 'bg-green-100 text-green-800';
case CHECKLIST_STATUS.IN_PROGRESS:
return 'bg-yellow-100 text-yellow-800';
case CHECKLIST_STATUS.AWAITING_RECONCILE:
case CHECKLIST_STATUS.REVIEWER_COMPLETED:
return 'bg-blue-100 text-blue-800';
case CHECKLIST_STATUS.RECONCILING:
return 'bg-purple-100 text-purple-800';
case CHECKLIST_STATUS.PENDING:
default:
return 'bg-gray-100 text-gray-800';
Expand All @@ -75,23 +84,23 @@ export function canTransitionTo(currentStatus, newStatus) {
return true;
}

// Can transition from in-progress to completed or awaiting-reconcile
// Can transition from in-progress to reviewer-completed or finalized
if (currentStatus === CHECKLIST_STATUS.IN_PROGRESS) {
return (
newStatus === CHECKLIST_STATUS.COMPLETED || newStatus === CHECKLIST_STATUS.AWAITING_RECONCILE
newStatus === CHECKLIST_STATUS.REVIEWER_COMPLETED || newStatus === CHECKLIST_STATUS.FINALIZED
);
}

// Can transition from awaiting-reconcile to completed (after reconciliation)
if (
currentStatus === CHECKLIST_STATUS.AWAITING_RECONCILE &&
newStatus === CHECKLIST_STATUS.COMPLETED
) {
// Can transition from reconciling to finalized (after reconciliation is complete)
if (currentStatus === CHECKLIST_STATUS.RECONCILING && newStatus === CHECKLIST_STATUS.FINALIZED) {
return true;
}

// Cannot transition from completed to anything else (locked)
if (currentStatus === CHECKLIST_STATUS.COMPLETED) {
// Cannot transition from finalized or reviewer-completed to anything else (locked)
if (
currentStatus === CHECKLIST_STATUS.FINALIZED ||
currentStatus === CHECKLIST_STATUS.REVIEWER_COMPLETED
) {
return false;
}

Expand Down
Loading