fix(team): count changes badge by files
This commit is contained in:
parent
0d46aac5c0
commit
5d05434bbb
2 changed files with 192 additions and 28 deletions
|
|
@ -1053,6 +1053,149 @@ describe('useTeamChangesSummaries', () => {
|
|||
expect(container.textContent).not.toContain('src/app.ts');
|
||||
});
|
||||
|
||||
it('counts files instead of changed tasks in the closed-section counter', async () => {
|
||||
hoisted.getTeamTaskChangeSummaries.mockImplementation(
|
||||
async (_teamName: string, requests: TeamTaskChangeSummaryRequest[]) => ({
|
||||
teamName: 'team-a',
|
||||
computedAt: '2026-05-10T10:00:01.000Z',
|
||||
items: requests.map((request, index) => {
|
||||
const totalFiles = index === 0 ? 3 : 4;
|
||||
return {
|
||||
taskId: request.taskId,
|
||||
changeSet: {
|
||||
...changeSet(request.taskId),
|
||||
files: [
|
||||
fileChange({
|
||||
filePath: `/repo/src/${request.taskId}.ts`,
|
||||
relativePath: `src/${request.taskId}.ts`,
|
||||
}),
|
||||
],
|
||||
totalFiles,
|
||||
totalLinesAdded: totalFiles,
|
||||
},
|
||||
};
|
||||
}),
|
||||
})
|
||||
);
|
||||
|
||||
const snapshots: HookSnapshot[] = [];
|
||||
const onSnapshot = (snapshot: HookSnapshot): void => {
|
||||
snapshots.push(snapshot);
|
||||
};
|
||||
container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
React.createElement(HookHarness, {
|
||||
tasks: changedTasks(2),
|
||||
sectionOpen: false,
|
||||
onSnapshot,
|
||||
})
|
||||
);
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(hoisted.getTeamTaskChangeSummaries).toHaveBeenCalledTimes(1);
|
||||
expect(snapshots.at(-1)?.badgeCount).toBe(7);
|
||||
});
|
||||
|
||||
it('keeps the previous file counter for tasks whose summary refresh errors', async () => {
|
||||
const getTotalFilesForTaskId = (taskId: string): number => (taskId === 'changed-0' ? 3 : 4);
|
||||
|
||||
hoisted.getTeamTaskChangeSummaries
|
||||
.mockImplementationOnce(
|
||||
async (_teamName: string, requests: TeamTaskChangeSummaryRequest[]) => ({
|
||||
teamName: 'team-a',
|
||||
computedAt: '2026-05-10T10:00:01.000Z',
|
||||
items: requests.map((request) => {
|
||||
const totalFiles = getTotalFilesForTaskId(request.taskId);
|
||||
return {
|
||||
taskId: request.taskId,
|
||||
changeSet: {
|
||||
...changeSet(request.taskId),
|
||||
files: [
|
||||
fileChange({
|
||||
filePath: `/repo/src/${request.taskId}.ts`,
|
||||
relativePath: `src/${request.taskId}.ts`,
|
||||
}),
|
||||
],
|
||||
totalFiles,
|
||||
totalLinesAdded: totalFiles,
|
||||
},
|
||||
};
|
||||
}),
|
||||
})
|
||||
)
|
||||
.mockImplementationOnce(
|
||||
async (_teamName: string, requests: TeamTaskChangeSummaryRequest[]) => ({
|
||||
teamName: 'team-a',
|
||||
computedAt: '2026-05-10T10:00:02.000Z',
|
||||
items: requests.map((request) =>
|
||||
request.taskId === 'changed-0'
|
||||
? { taskId: request.taskId, changeSet: null, error: 'summary timed out' }
|
||||
: {
|
||||
taskId: request.taskId,
|
||||
changeSet: {
|
||||
...changeSet(request.taskId),
|
||||
files: [
|
||||
fileChange({
|
||||
filePath: `/repo/src/${request.taskId}.ts`,
|
||||
relativePath: `src/${request.taskId}.ts`,
|
||||
}),
|
||||
],
|
||||
totalFiles: 4,
|
||||
totalLinesAdded: 4,
|
||||
},
|
||||
}
|
||||
),
|
||||
})
|
||||
);
|
||||
|
||||
const snapshots: HookSnapshot[] = [];
|
||||
const onSnapshot = (snapshot: HookSnapshot): void => {
|
||||
snapshots.push(snapshot);
|
||||
};
|
||||
container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
|
||||
const initialTasks = changedTasks(2);
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
React.createElement(HookHarness, {
|
||||
tasks: initialTasks,
|
||||
sectionOpen: false,
|
||||
onSnapshot,
|
||||
})
|
||||
);
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(snapshots.at(-1)?.badgeCount).toBe(7);
|
||||
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
React.createElement(HookHarness, {
|
||||
tasks: initialTasks.map((item) => ({
|
||||
...item,
|
||||
updatedAt: '2026-05-10T10:05:00.000Z',
|
||||
})),
|
||||
sectionOpen: false,
|
||||
onSnapshot,
|
||||
})
|
||||
);
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(hoisted.getTeamTaskChangeSummaries).toHaveBeenCalledTimes(2);
|
||||
expect(snapshots.at(-1)?.badgeCount).toBe(7);
|
||||
});
|
||||
|
||||
it('loads staged open batches without repeating successful tasks', async () => {
|
||||
const first = createDeferred<TeamTaskChangeSummariesResponse>();
|
||||
const second = createDeferred<TeamTaskChangeSummariesResponse>();
|
||||
|
|
|
|||
|
|
@ -122,17 +122,17 @@ function hasSafeFileSummaries(changeSet: TaskChangeSetV2): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
function hasDisplayableFileSummaries(changeSet: TaskChangeSetV2): boolean {
|
||||
return (
|
||||
Array.isArray(changeSet.files) &&
|
||||
changeSet.files.some(
|
||||
(file) =>
|
||||
file &&
|
||||
typeof file === 'object' &&
|
||||
typeof file.filePath === 'string' &&
|
||||
file.filePath.trim().length > 0
|
||||
)
|
||||
);
|
||||
function countDisplayableFileSummaries(changeSet: TaskChangeSetV2): number {
|
||||
if (!Array.isArray(changeSet.files)) {
|
||||
return 0;
|
||||
}
|
||||
return changeSet.files.filter(
|
||||
(file) =>
|
||||
file &&
|
||||
typeof file === 'object' &&
|
||||
typeof file.filePath === 'string' &&
|
||||
file.filePath.trim().length > 0
|
||||
).length;
|
||||
}
|
||||
|
||||
function isMinimalPresenceChangeSet(changeSet: TaskChangeSetV2): boolean {
|
||||
|
|
@ -198,25 +198,37 @@ function shouldClearSelectedTaskChangePresence(
|
|||
);
|
||||
}
|
||||
|
||||
function isCountableTeamChangeSummary(item: TeamTaskChangeSummaryItem): boolean {
|
||||
function getTeamChangeBadgeContribution(item: TeamTaskChangeSummaryItem): number {
|
||||
if (item.error) {
|
||||
return true;
|
||||
return 1;
|
||||
}
|
||||
|
||||
const changeSet = item.changeSet;
|
||||
if (!changeSet) {
|
||||
return false;
|
||||
return 0;
|
||||
}
|
||||
if (hasDisplayableFileSummaries(changeSet)) {
|
||||
return true;
|
||||
|
||||
const totalFiles = Number(changeSet.totalFiles);
|
||||
if (Number.isFinite(totalFiles) && totalFiles > 0) {
|
||||
return Math.trunc(totalFiles);
|
||||
}
|
||||
|
||||
const displayableFileCount = countDisplayableFileSummaries(changeSet);
|
||||
if (displayableFileCount > 0) {
|
||||
return displayableFileCount;
|
||||
}
|
||||
|
||||
const reviewability = classifyTaskChangeReviewability(changeSet).reviewability;
|
||||
return reviewability === 'attention_required' || reviewability === 'diagnostic_only';
|
||||
return reviewability === 'attention_required' || reviewability === 'diagnostic_only' ? 1 : 0;
|
||||
}
|
||||
|
||||
function countChangedTasks(changeCountByTaskId: Record<string, boolean>): number {
|
||||
return Object.values(changeCountByTaskId).filter(Boolean).length;
|
||||
function sumTeamChangeBadgeContributions(changeBadgeCountByTaskId: Record<string, number>): number {
|
||||
return Object.values(changeBadgeCountByTaskId).reduce((sum, value) => {
|
||||
if (!Number.isFinite(value) || value <= 0) {
|
||||
return sum;
|
||||
}
|
||||
return sum + Math.trunc(value);
|
||||
}, 0);
|
||||
}
|
||||
|
||||
function isDocumentHidden(): boolean {
|
||||
|
|
@ -266,7 +278,9 @@ export function useTeamChangesSummaries({
|
|||
const [summariesByTaskId, setSummariesByTaskId] = useState<
|
||||
Record<string, TeamChangeSummaryState>
|
||||
>({});
|
||||
const [changeCountByTaskId, setChangeCountByTaskId] = useState<Record<string, boolean>>({});
|
||||
const [changeBadgeCountByTaskId, setChangeBadgeCountByTaskId] = useState<Record<string, number>>(
|
||||
{}
|
||||
);
|
||||
const [counterLoaded, setCounterLoaded] = useState(false);
|
||||
const [stats, setStats] = useState<TeamChangeStats>({
|
||||
eligibleCount: 0,
|
||||
|
|
@ -405,7 +419,7 @@ export function useTeamChangesSummaries({
|
|||
if (storeSummaries) {
|
||||
setSummariesByTaskId({});
|
||||
}
|
||||
setChangeCountByTaskId({});
|
||||
setChangeBadgeCountByTaskId({});
|
||||
setCounterLoaded(true);
|
||||
autoRefreshBlockedUntilRef.current = 0;
|
||||
setLoading(false);
|
||||
|
|
@ -454,16 +468,23 @@ export function useTeamChangesSummaries({
|
|||
}
|
||||
}
|
||||
|
||||
setChangeCountByTaskId((previous) => {
|
||||
const next: Record<string, boolean> = {};
|
||||
for (const [taskId, countable] of Object.entries(previous)) {
|
||||
setChangeBadgeCountByTaskId((previous) => {
|
||||
const next: Record<string, number> = {};
|
||||
for (const [taskId, badgeContribution] of Object.entries(previous)) {
|
||||
if (currentTaskIds.has(taskId) && plan.eligibleTaskIds.has(taskId)) {
|
||||
next[taskId] = countable;
|
||||
next[taskId] = badgeContribution;
|
||||
}
|
||||
}
|
||||
for (const item of responseItems) {
|
||||
if (!plan.requestOptionsByTaskId.has(item.taskId)) continue;
|
||||
next[item.taskId] = isCountableTeamChangeSummary(item);
|
||||
const nextContribution = getTeamChangeBadgeContribution(item);
|
||||
const previousContribution = previous[item.taskId];
|
||||
next[item.taskId] =
|
||||
item.error &&
|
||||
Number.isFinite(previousContribution) &&
|
||||
previousContribution > nextContribution
|
||||
? previousContribution
|
||||
: nextContribution;
|
||||
}
|
||||
return next;
|
||||
});
|
||||
|
|
@ -587,7 +608,7 @@ export function useTeamChangesSummaries({
|
|||
lastRequestedTasksFingerprintRef.current = null;
|
||||
lastCounterTasksFingerprintRef.current = null;
|
||||
setSummariesByTaskId({});
|
||||
setChangeCountByTaskId({});
|
||||
setChangeBadgeCountByTaskId({});
|
||||
setCounterLoaded(false);
|
||||
setError(null);
|
||||
setStats({ eligibleCount: 0, requestedCount: 0, deferredCount: 0 });
|
||||
|
|
@ -723,7 +744,7 @@ export function useTeamChangesSummaries({
|
|||
|
||||
return {
|
||||
summariesByTaskId,
|
||||
badgeCount: counterLoaded ? countChangedTasks(changeCountByTaskId) : null,
|
||||
badgeCount: counterLoaded ? sumTeamChangeBadgeContributions(changeBadgeCountByTaskId) : null,
|
||||
stats,
|
||||
loading,
|
||||
refreshing,
|
||||
|
|
|
|||
Loading…
Reference in a new issue