feat(FN-4741): prefer rebase range for done task diffs
Fixed diff calculation for done tasks to prefer the rebase range over a stale base commit, resolving incorrect diff output when a task branch has been rebased (FN-4741). Added test coverage for the session diff routes. Fusion-Task-Id: FN-4741
This commit is contained in:
committed by
gsxdsm
parent
b772881dd3
commit
ef1343f658
@@ -304,6 +304,29 @@ describe("FN-4308 multi-commit done task aggregation", () => {
|
||||
expect(response.body.files.map((f: any) => f.path).sort()).toEqual(["one.ts", "two.ts"]);
|
||||
});
|
||||
|
||||
it("FN-4741: prefers rebaseBaseSha..commitSha over single-commit aggregate even when mergeDetails.filesChanged is absent", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ column: "done", mergeDetails: { commitSha: "tip-sha", rebaseBaseSha: "base-sha" } }));
|
||||
|
||||
gitResponses({
|
||||
"merge-base --is-ancestor tip-sha HEAD": "",
|
||||
"rev-list --parents -n 1 tip-sha": "tip-sha parent-sha",
|
||||
"diff --name-status -M parent-sha..tip-sha": "M\tonly-tip.ts",
|
||||
"diff -M parent-sha..tip-sha -- only-tip.ts": "+tip\n",
|
||||
"merge-base --is-ancestor base-sha tip-sha": "",
|
||||
"diff --name-status -M base-sha..tip-sha": "A\tfile-a.ts\nM\tfile-b.ts\nM\tonly-tip.ts",
|
||||
"diff -M base-sha..tip-sha -- file-a.ts": "+a\n",
|
||||
"diff -M base-sha..tip-sha -- file-b.ts": "+b\n",
|
||||
"diff -M base-sha..tip-sha -- only-tip.ts": "+tip\n",
|
||||
});
|
||||
|
||||
const app = createServer(store as any);
|
||||
const response = await requestDiff(app);
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body.stats.filesChanged).toBe(3);
|
||||
expect(response.body.files.map((f: any) => f.path).sort()).toEqual(["file-a.ts", "file-b.ts", "only-tip.ts"]);
|
||||
});
|
||||
|
||||
it("FN-4726: prefers rebaseBaseSha..commitSha range over partial lineage aggregate for multi-commit rebase done task", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({
|
||||
@@ -435,6 +458,29 @@ describe("FN-4308 multi-commit done task aggregation", () => {
|
||||
expect(response.body.map((f: any) => f.path).sort()).toEqual(["one.ts", "two.ts"]);
|
||||
});
|
||||
|
||||
it("FN-4741: prefers rebaseBaseSha..commitSha for file-diffs even when mergeDetails.filesChanged is absent", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ column: "done", mergeDetails: { commitSha: "tip-sha", rebaseBaseSha: "base-sha" } }));
|
||||
|
||||
gitResponses({
|
||||
"merge-base --is-ancestor tip-sha HEAD": "",
|
||||
"rev-list --parents -n 1 tip-sha": "tip-sha parent-sha",
|
||||
"diff --name-status -M parent-sha..tip-sha": "M\tonly-tip.ts",
|
||||
"diff -M parent-sha..tip-sha -- only-tip.ts": "+tip\n",
|
||||
"merge-base --is-ancestor base-sha tip-sha": "",
|
||||
"diff --name-status -M base-sha..tip-sha": "A\tfile-a.ts\nM\tfile-b.ts\nM\tonly-tip.ts",
|
||||
"diff -M base-sha..tip-sha -- file-a.ts": "+a\n",
|
||||
"diff -M base-sha..tip-sha -- file-b.ts": "+b\n",
|
||||
"diff -M base-sha..tip-sha -- only-tip.ts": "+tip\n",
|
||||
});
|
||||
|
||||
const app = createServer(store as any);
|
||||
const response = await requestFileDiffs(app);
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toHaveLength(3);
|
||||
expect(response.body.map((f: any) => f.path).sort()).toEqual(["file-a.ts", "file-b.ts", "only-tip.ts"]);
|
||||
});
|
||||
|
||||
it("uses parent-to-parent range for merge commits", async () => {
|
||||
const store = new MockStore();
|
||||
store.addTask(createTask({ column: "done", mergeDetails: { commitSha: "merge-sha" } }));
|
||||
|
||||
@@ -629,6 +629,29 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
const expectedFilesChanged = task.mergeDetails?.filesChanged ?? 0;
|
||||
const aggregationLooksComplete = expectedFilesChanged <= 0 || aggregated.stats.filesChanged >= expectedFilesChanged;
|
||||
|
||||
const rebaseBaseShaForAggregation = task.mergeDetails?.rebaseBaseSha?.trim();
|
||||
if (resolvedMergeSha && rebaseBaseShaForAggregation) {
|
||||
const rebaseDiffSpec = await resolveRebaseDiffSpec(rebaseBaseShaForAggregation, resolvedMergeSha, scopedStore.getRootDir());
|
||||
if (rebaseDiffSpec) {
|
||||
const rebaseRangeFiles = await collectDoneRangeFiles(rebaseDiffSpec.range, scopedStore.getRootDir()).catch(() => []);
|
||||
if (rebaseRangeFiles.length > 0) {
|
||||
const files = rebaseRangeFiles.map((file) => ({
|
||||
...file,
|
||||
status: file.status === "renamed" ? "modified" : file.status,
|
||||
}));
|
||||
res.json({
|
||||
files,
|
||||
stats: {
|
||||
filesChanged: files.length,
|
||||
additions: files.reduce((sum, file) => sum + file.additions, 0),
|
||||
deletions: files.reduce((sum, file) => sum + file.deletions, 0),
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (aggregated.usedAggregation && aggregated.files.length > 0 && aggregationLooksComplete) {
|
||||
res.json({
|
||||
files: aggregated.files.map((file) => ({
|
||||
@@ -641,29 +664,6 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
}
|
||||
|
||||
if (aggregated.usedAggregation && aggregated.files.length > 0) {
|
||||
const rebaseBaseSha = task.mergeDetails?.rebaseBaseSha?.trim();
|
||||
if (resolvedMergeSha && rebaseBaseSha) {
|
||||
const rebaseDiffSpec = await resolveRebaseDiffSpec(rebaseBaseSha, resolvedMergeSha, scopedStore.getRootDir());
|
||||
if (rebaseDiffSpec) {
|
||||
const rebaseRangeFiles = await collectDoneRangeFiles(rebaseDiffSpec.range, scopedStore.getRootDir()).catch(() => []);
|
||||
if (rebaseRangeFiles.length >= aggregated.stats.filesChanged) {
|
||||
const files = rebaseRangeFiles.map((file) => ({
|
||||
...file,
|
||||
status: file.status === "renamed" ? "modified" : file.status,
|
||||
}));
|
||||
res.json({
|
||||
files,
|
||||
stats: {
|
||||
filesChanged: files.length,
|
||||
additions: files.reduce((sum, file) => sum + file.additions, 0),
|
||||
deletions: files.reduce((sum, file) => sum + file.deletions, 0),
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
res.json({
|
||||
files: aggregated.files.map((file) => ({
|
||||
...file,
|
||||
@@ -884,24 +884,24 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute
|
||||
const expectedFilesChanged = task.mergeDetails?.filesChanged ?? 0;
|
||||
const aggregationLooksComplete = expectedFilesChanged <= 0 || aggregated.stats.filesChanged >= expectedFilesChanged;
|
||||
|
||||
const rebaseBaseShaForAggregation = task.mergeDetails?.rebaseBaseSha?.trim();
|
||||
if (resolvedMergeSha && rebaseBaseShaForAggregation) {
|
||||
const rebaseDiffSpec = await resolveRebaseDiffSpec(rebaseBaseShaForAggregation, resolvedMergeSha, scopedStore.getRootDir());
|
||||
if (rebaseDiffSpec) {
|
||||
const rebaseRangeFiles = await collectDoneRangeFiles(rebaseDiffSpec.range, scopedStore.getRootDir()).catch(() => []);
|
||||
if (rebaseRangeFiles.length > 0) {
|
||||
res.json(rebaseRangeFiles.map((file) => ({ path: file.path, status: file.status, diff: file.patch })));
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (aggregated.usedAggregation && aggregated.files.length > 0 && aggregationLooksComplete) {
|
||||
res.json(aggregated.files.map((file) => ({ path: file.path, status: file.status, diff: file.patch })));
|
||||
return;
|
||||
}
|
||||
|
||||
if (aggregated.usedAggregation && aggregated.files.length > 0) {
|
||||
const rebaseBaseSha = task.mergeDetails?.rebaseBaseSha?.trim();
|
||||
if (resolvedMergeSha && rebaseBaseSha) {
|
||||
const rebaseDiffSpec = await resolveRebaseDiffSpec(rebaseBaseSha, resolvedMergeSha, scopedStore.getRootDir());
|
||||
if (rebaseDiffSpec) {
|
||||
const rebaseRangeFiles = await collectDoneRangeFiles(rebaseDiffSpec.range, scopedStore.getRootDir()).catch(() => []);
|
||||
if (rebaseRangeFiles.length >= aggregated.stats.filesChanged) {
|
||||
res.json(rebaseRangeFiles.map((file) => ({ path: file.path, status: file.status, diff: file.patch })));
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
res.json(aggregated.files.map((file) => ({ path: file.path, status: file.status, diff: file.patch })));
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user