fix(report): approverName uses only included days
Move reviewer tracking logic inside the non-empty rows guard in both approved-only and include-unapproved modes. This ensures zero-row approved sheets don't pollute the reviewer set, fixing the approverName calculation to only consider reviewers from included ReportDays. Add test: approved sheet with zero rows and different reviewer correctly excluded from approver tracking. All 9 tests passing. Co-Authored-By: claude-flow <ruv@ruv.net>
This commit is contained in:
@@ -129,13 +129,13 @@ AccomplishmentReportData buildAccomplishmentReportData({
|
||||
(secondsByCategory[row.category] ?? 0) + row.seconds;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Track reviewers for approved sheets
|
||||
// Track reviewers for included approved sheets only
|
||||
if (sheet.status == DaySheetStatus.approved && sheet.reviewedBy != null) {
|
||||
allReviewers.add(sheet.reviewedBy!);
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Approved-only mode
|
||||
for (final sheet in sheetsInRange) {
|
||||
@@ -159,12 +159,12 @@ AccomplishmentReportData buildAccomplishmentReportData({
|
||||
(secondsByCategory[row.category] ?? 0) + row.seconds;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Track reviewer
|
||||
// Track reviewer for included sheets only
|
||||
if (sheet.reviewedBy != null) {
|
||||
allReviewers.add(sheet.reviewedBy!);
|
||||
}
|
||||
}
|
||||
} else if (sheet.status == DaySheetStatus.pending) {
|
||||
excludedPending++;
|
||||
} else if (sheet.status == DaySheetStatus.disapproved) {
|
||||
|
||||
@@ -606,5 +606,87 @@ void main() {
|
||||
expect(data.secondsByCategory['Meeting'], 600);
|
||||
expect(data.totalSeconds, 6900);
|
||||
});
|
||||
|
||||
test('approved sheet with zero rows and different reviewer does not affect approverName',
|
||||
() {
|
||||
final day1 = DateTime.utc(2026, 9, 1);
|
||||
final day2 = DateTime.utc(2026, 9, 2);
|
||||
final day3 = DateTime.utc(2026, 9, 3);
|
||||
|
||||
final row = DaySheetRow(
|
||||
taskId: 't1',
|
||||
title: 'Task 1',
|
||||
category: 'Software Development',
|
||||
kind: 'assignee',
|
||||
seconds: 3600,
|
||||
notes: [],
|
||||
);
|
||||
|
||||
// Two included approved sheets with same reviewer
|
||||
// One zero-row approved sheet with different reviewer
|
||||
final sheets = [
|
||||
ProgrammerDaySheet(
|
||||
id: 'sheet1',
|
||||
programmerId: 'prog1',
|
||||
status: 'approved',
|
||||
workDate: day1,
|
||||
autoSubmitted: false,
|
||||
resubmissions: 0,
|
||||
reviewedBy: 'reviewer1',
|
||||
approvedSnapshot: DaySheetSnapshot(totalSeconds: 3600, rows: [row]),
|
||||
createdAt: DateTime.utc(2026, 9, 1),
|
||||
updatedAt: DateTime.utc(2026, 9, 1),
|
||||
),
|
||||
// Zero-row approved sheet with different reviewer (should be ignored)
|
||||
ProgrammerDaySheet(
|
||||
id: 'sheet2',
|
||||
programmerId: 'prog1',
|
||||
status: 'approved',
|
||||
workDate: day2,
|
||||
autoSubmitted: false,
|
||||
resubmissions: 0,
|
||||
reviewedBy: 'reviewer2',
|
||||
approvedSnapshot: DaySheetSnapshot(totalSeconds: 0, rows: []),
|
||||
createdAt: DateTime.utc(2026, 9, 2),
|
||||
updatedAt: DateTime.utc(2026, 9, 2),
|
||||
),
|
||||
ProgrammerDaySheet(
|
||||
id: 'sheet3',
|
||||
programmerId: 'prog1',
|
||||
status: 'approved',
|
||||
workDate: day3,
|
||||
autoSubmitted: false,
|
||||
resubmissions: 0,
|
||||
reviewedBy: 'reviewer1',
|
||||
approvedSnapshot: DaySheetSnapshot(totalSeconds: 3600, rows: [row]),
|
||||
createdAt: DateTime.utc(2026, 9, 3),
|
||||
updatedAt: DateTime.utc(2026, 9, 3),
|
||||
),
|
||||
];
|
||||
|
||||
final range = ReportDateRange(
|
||||
start: DateTime.utc(2026, 9, 1),
|
||||
end: DateTime.utc(2026, 9, 4),
|
||||
label: 'Test Range',
|
||||
);
|
||||
|
||||
final data = buildAccomplishmentReportData(
|
||||
programmer: programmer,
|
||||
range: range,
|
||||
includeUnapproved: false,
|
||||
sheets: sheets,
|
||||
liveRowsByDay: {},
|
||||
tasks: [],
|
||||
profileNames: {
|
||||
'reviewer1': 'Jane Reviewer',
|
||||
'reviewer2': 'Bob Reviewer',
|
||||
},
|
||||
);
|
||||
|
||||
// Only 2 days included (day2 has zero rows)
|
||||
expect(data.days.length, 2);
|
||||
// Both included days were reviewed by reviewer1
|
||||
expect(data.approverName, 'Jane Reviewer');
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user