Skip to content

Commit e9f808c

Browse files
sinhavirajsinhaviraj
authoredMar 7, 2025
Decoupling changedFiles & conflicts API in PR details & file view (#171)
* decoupling conflicts API from changedFilesDiff API to load PR request details view when conflicts API takes time in larger repos or bigger PRs
1 parent 940baa2 commit e9f808c

14 files changed

+110
-30
lines changed
 

‎src/bitbucket/bitbucket-cloud/pullRequests.ts

+35-17
Original file line numberDiff line numberDiff line change
@@ -202,21 +202,47 @@ export class CloudPullRequestApi implements PullRequestApi {
202202
}));
203203
}
204204

205+
async getConflictedFiles(pr: PullRequest): Promise<string[]> {
206+
const { ownerSlug, repoSlug } = pr.site;
207+
208+
// get PRType
209+
const prTypeUrl = `https://api.bitbucket.org/internal/repositories/${ownerSlug}/${repoSlug}/pullrequests/${pr.data.id}`;
210+
let prTypeData = { data: { diff_type: '' } };
211+
try {
212+
prTypeData = await this.client.get(prTypeUrl);
213+
} catch (ex) {
214+
const error = new Error(`Fetching prTypeData failed for the PR: ${pr.data.id}} with error: ${ex}`);
215+
Logger.error(error);
216+
}
217+
const conflictedFiles: string[] = [];
218+
if (prTypeData.data.diff_type === 'TOPIC') {
219+
const conflictUrl = `https://api.bitbucket.org/internal/repositories/${ownerSlug}/${repoSlug}/pullrequests/${pr.data.id}/conflicts`;
220+
let resp = { data: [] };
221+
try {
222+
resp = await this.client.get(conflictUrl);
223+
} catch (ex) {
224+
const error = new Error(`Fetching conflict data failed for the PR: ${pr.data.id}} with error: ${ex}`);
225+
Logger.error(error);
226+
}
227+
resp.data.forEach((data: { path: '' }) => conflictedFiles.push(data.path));
228+
}
229+
return conflictedFiles;
230+
}
231+
205232
async getChangedFiles(pr: PullRequest, spec?: string): Promise<FileDiff[]> {
206233
const { ownerSlug, repoSlug } = pr.site;
207234

208235
const diffUrl = spec
209236
? `/repositories/${ownerSlug}/${repoSlug}/diffstat/${spec}`
210237
: `/repositories/${ownerSlug}/${repoSlug}/pullrequests/${pr.data.id}/diffstat`;
211-
let { data } = await this.client.get(diffUrl);
212-
213-
const conflictUrl = `https://api.bitbucket.org/internal/repositories/${ownerSlug}/${repoSlug}/pullrequests/${pr.data.id}/conflicts`;
214-
let resp = await this.client.get(conflictUrl);
215-
const conflictData = resp.data;
216-
217-
const prTypeUrl = `https://api.bitbucket.org/internal/repositories/${ownerSlug}/${repoSlug}/pullrequests/${pr.data.id}`;
218-
resp = await this.client.get(prTypeUrl);
219-
const diffType = resp.data.diff_type;
238+
let data = { values: undefined, next: undefined };
239+
try {
240+
const response = await this.client.get(diffUrl);
241+
data = response.data;
242+
} catch (ex) {
243+
const error = new Error(`Fetching diffStat failed for the PR: ${pr.data.id} with error ${ex}`);
244+
Logger.error(error);
245+
}
220246

221247
if (!data.values) {
222248
return [];
@@ -243,17 +269,9 @@ export class CloudPullRequestApi implements PullRequestApi {
243269
newPathDeletions: [],
244270
newPathContextMap: {},
245271
},
246-
isConflicted: this.isFileConflicted(diffType, diffStat, conflictData),
247272
}));
248273
}
249274

250-
// Topic diffs no longer indicate a conflict in the status field so we have to check the results of the conflict endpoint.
251-
private isFileConflicted(diffType: string, diffStat: any, conflictData: any[]): boolean | undefined {
252-
const oldPath = diffStat.old?.path;
253-
const newPath = diffStat.new?.path;
254-
return diffType === 'TOPIC' && conflictData.some((c) => c.path === newPath || oldPath);
255-
}
256-
257275
private mapStatusWordsToFileStatus(status: string): FileStatus {
258276
if (status === 'added') {
259277
return FileStatus.ADDED;

‎src/bitbucket/bitbucket-server/pullRequests.ts

+10
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,16 @@ export class ServerPullRequestApi implements PullRequestApi {
487487

488488
return result;
489489
}
490+
/**
491+
*
492+
* @param pr The Pull request for which conflict data is to be calculated
493+
* @returns [''] -> Empty String Array
494+
* We don't seem to have the concept of conflicted Files in Server from the getChangedFiles method above
495+
* but this has been implemented here because Server extends the common interface PullRequestApi which has this method
496+
*/
497+
async getConflictedFiles(pr: PullRequest): Promise<string[]> {
498+
return [];
499+
}
490500

491501
async getCurrentUser(site: DetailedSiteInfo): Promise<User> {
492502
const userSlug = site.userId;

‎src/bitbucket/model.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,6 @@ export interface FileDiff {
209209
// NOT using Map here as Map does not serialize to JSON
210210
newPathContextMap: Record<string, number>;
211211
};
212-
213-
// Indicates whether or not the file has a conflict. Only defined on topic diffs - recent (approx 2022 and forward) BB server diffs.
214-
// If it's undefined fall back to looking for FileStatus.CONFLICT
215-
isConflicted?: boolean;
216212
}
217213

218214
export type CreatePullRequestData = {
@@ -349,6 +345,7 @@ export interface PullRequestApi {
349345
get(site: BitbucketSite, prId: string, workspaceRepo?: WorkspaceRepo): Promise<PullRequest>;
350346
getById(site: BitbucketSite, prId: number): Promise<PullRequest>;
351347
getChangedFiles(pr: PullRequest, spec?: string): Promise<FileDiff[]>;
348+
getConflictedFiles(pr: PullRequest): Promise<string[]>;
352349
getCommits(pr: PullRequest): Promise<Commit[]>;
353350
getComments(pr: PullRequest, commitHash?: string): Promise<PaginatedComments>;
354351
editComment(

‎src/lib/ipc/toUI/pullRequestDetails.ts

+8
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export enum PullRequestDetailsMessageType {
4040
UpdateRelatedJiraIssues = 'updateRelatedJiraIssues',
4141
UpdateRelatedBitbucketIssues = 'updateRelatedBitbucketIssues',
4242
UpdateTasks = 'updateTasks',
43+
UpdateConflictedFiles = 'updateConflictedFiles',
4344
}
4445

4546
export type PullRequestDetailsMessage =
@@ -52,6 +53,7 @@ export type PullRequestDetailsMessage =
5253
| ReducerAction<PullRequestDetailsMessageType.CheckoutBranch, PullRequestDetailsCheckoutBranchMessage>
5354
| ReducerAction<PullRequestDetailsMessageType.UpdateComments, PullRequestDetailsCommentsMessage>
5455
| ReducerAction<PullRequestDetailsMessageType.UpdateFileDiffs, PullRequestDetailsFileDiffsMessage>
56+
| ReducerAction<PullRequestDetailsMessageType.UpdateConflictedFiles, PullRequestDetailsConflictedFilesMessage>
5557
| ReducerAction<PullRequestDetailsMessageType.UpdateBuildStatuses, PullRequestDetailsBuildStatusesMessage>
5658
| ReducerAction<PullRequestDetailsMessageType.UpdateMergeStrategies, PullRequestDetailsMergeStrategiesMessage>
5759
| ReducerAction<PullRequestDetailsMessageType.UpdateRelatedJiraIssues, PullRequestDetailsRelatedJiraIssuesMessage>
@@ -79,6 +81,7 @@ export interface PullRequestDetailsInitMessage {
7981
comments: Comment[];
8082
tasks: Task[];
8183
fileDiffs: FileDiff[];
84+
conflictedFiles: string[];
8285
mergeStrategies: MergeStrategy[];
8386
buildStatuses: BuildStatus[];
8487
relatedJiraIssues: MinimalIssue<DetailedSiteInfo>[];
@@ -135,6 +138,10 @@ export interface PullRequestDetailsFileDiffsMessage {
135138
fileDiffs: FileDiff[];
136139
}
137140

141+
export interface PullRequestDetailsConflictedFilesMessage {
142+
conflictedFiles: string[];
143+
}
144+
138145
export interface PullRequestDetailsMergeStrategiesMessage {
139146
mergeStrategies: MergeStrategy[];
140147
}
@@ -180,4 +187,5 @@ export const emptyPullRequestDetailsInitMessage: PullRequestDetailsInitMessage =
180187
mergeStrategies: true,
181188
buildStatuses: true,
182189
},
190+
conflictedFiles: [],
183191
};

‎src/lib/webview/controller/pullrequest/pullRequestDetailsActionApi.ts

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export interface PullRequestDetailsActionApi {
3131
editComment(comments: Comment[], pr: PullRequest, content: string, commentId: string): Promise<Comment[]>;
3232
deleteComment(pr: PullRequest, comment: Comment): Promise<Comment[]>;
3333
getFileDiffs(pr: PullRequest, inlineComments: Comment[]): Promise<FileDiff[]>;
34+
getConflictedFiles(pr: PullRequest): Promise<string[]>;
3435
openDiffViewForFile(pr: PullRequest, fileDiff: FileDiff, comments: Comment[]): Promise<void>;
3536
updateBuildStatuses(pr: PullRequest): Promise<BuildStatus[]>;
3637
updateMergeStrategies(pr: PullRequest): Promise<MergeStrategy[]>;

‎src/lib/webview/controller/pullrequest/pullRequestDetailsWebviewController.ts

+7
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,13 @@ export class PullRequestDetailsWebviewController implements WebviewController<Pu
167167
});
168168
});
169169

170+
this.api.getConflictedFiles(this.pr).then((conflictedFiles: string[]) => {
171+
this.postMessage({
172+
type: PullRequestDetailsMessageType.UpdateConflictedFiles,
173+
conflictedFiles,
174+
});
175+
});
176+
170177
//In order to get related issues, we need comments and commits. We already have comments,
171178
//so now we wait for commits. These two promises can be launched concurrently.
172179
this.commits = await commitPromise;

‎src/react/atlascode/pullrequest/CreatePullRequestPage.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,7 @@ const CreatePullRequestPage: React.FunctionComponent = () => {
516516
<DiffList
517517
fileDiffs={state.fileDiffs}
518518
openDiffHandler={controller.openDiff}
519+
conflictedFiles={[]}
519520
/>
520521
</Grid>
521522
</Grid>

‎src/react/atlascode/pullrequest/DiffList.tsx

+9-5
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ const useStyles = makeStyles((theme: Theme) => ({
5252
export const DiffList: React.FunctionComponent<{
5353
fileDiffs: FileDiff[];
5454
openDiffHandler: (filediff: FileDiff) => void;
55+
conflictedFiles: string[];
5556
}> = (props) => {
5657
const classes = useStyles();
5758

@@ -93,9 +94,11 @@ export const DiffList: React.FunctionComponent<{
9394
}
9495
};
9596

96-
const ConflictChip = (fileDiff: FileDiff) => {
97-
// Is either undefined or false
98-
if (!fileDiff.isConflicted) {
97+
const ConflictChip = (fileDiff: FileDiff, conflictedFiles: string[]) => {
98+
// Is fileDiff's old or new path doesn't exists in conflictedFiles
99+
const newPath = fileDiff.newPath || '';
100+
const oldPath = fileDiff.oldPath || '';
101+
if (!conflictedFiles.includes(newPath) || !conflictedFiles.includes(oldPath)) {
99102
return <div></div>;
100103
}
101104
return (
@@ -113,7 +116,6 @@ export const DiffList: React.FunctionComponent<{
113116
</Tooltip>
114117
);
115118
};
116-
117119
return (
118120
<TableContainer>
119121
<Table size="small" className={classes.table} aria-label="commits list">
@@ -153,7 +155,9 @@ export const DiffList: React.FunctionComponent<{
153155
/>
154156
</Tooltip>
155157
</TableCell>
156-
<TableCell className={classes.tableCell}>{ConflictChip(row)}</TableCell>
158+
<TableCell className={classes.tableCell}>
159+
{ConflictChip(row, props.conflictedFiles)}
160+
</TableCell>
157161
<TableCell className={classes.tableCell}>
158162
<Link onClick={() => props.openDiffHandler(row)}>
159163
<Typography>{row.file}</Typography>

‎src/react/atlascode/pullrequest/PullRequestDetailsPage.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@ export const PullRequestDetailsPage: React.FunctionComponent = () => {
277277
<DiffList
278278
fileDiffs={state.fileDiffs}
279279
openDiffHandler={controller.openDiff}
280+
conflictedFiles={state.conflictedFiles}
280281
/>
281282
</BasicPanel>
282283
</Grid>

‎src/react/atlascode/pullrequest/pullRequestDetailsController.ts

+17
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
PullRequestDetailsSummaryMessage,
3838
PullRequestDetailsTasksMessage,
3939
PullRequestDetailsTitleMessage,
40+
PullRequestDetailsConflictedFilesMessage,
4041
} from '../../../lib/ipc/toUI/pullRequestDetails';
4142
import { ConnectionTimeout } from '../../../util/time';
4243
import { PostMessageFunc, useMessagingApi } from '../messagingApi';
@@ -130,6 +131,7 @@ export enum PullRequestDetailsUIActionType {
130131
UpdateTasks = 'updateTasks',
131132
AddComment = 'addComment',
132133
UpdateFileDiffs = 'updateFileDiffs',
134+
UpdateConflictedFiles = 'updateConflictedFiles',
133135
UpdateBuildStatuses = 'updateBuildStatuses',
134136
UpdateMergeStrategies = 'updateMergeStrategies',
135137
UpdateRelatedJiraIssues = 'updateRelatedJiraIssues',
@@ -148,6 +150,10 @@ export type PullRequestDetailsUIAction =
148150
| ReducerAction<PullRequestDetailsUIActionType.UpdateComments, { data: PullRequestDetailsCommentsMessage }>
149151
| ReducerAction<PullRequestDetailsUIActionType.UpdateTasks, { data: PullRequestDetailsTasksMessage }>
150152
| ReducerAction<PullRequestDetailsUIActionType.UpdateFileDiffs, { data: PullRequestDetailsFileDiffsMessage }>
153+
| ReducerAction<
154+
PullRequestDetailsUIActionType.UpdateConflictedFiles,
155+
{ data: PullRequestDetailsConflictedFilesMessage }
156+
>
151157
| ReducerAction<
152158
PullRequestDetailsUIActionType.UpdateBuildStatuses,
153159
{ data: PullRequestDetailsBuildStatusesMessage }
@@ -260,6 +266,13 @@ function pullRequestDetailsReducer(
260266
case PullRequestDetailsUIActionType.UpdateFileDiffs: {
261267
return { ...state, fileDiffs: action.data.fileDiffs, loadState: { ...state.loadState, diffs: false } };
262268
}
269+
case PullRequestDetailsUIActionType.UpdateConflictedFiles: {
270+
return {
271+
...state,
272+
conflictedFiles: action.data.conflictedFiles,
273+
loadState: { ...state.loadState, diffs: false },
274+
};
275+
}
263276
case PullRequestDetailsUIActionType.UpdateBuildStatuses: {
264277
return {
265278
...state,
@@ -344,6 +357,10 @@ export function usePullRequestDetailsController(): [PullRequestDetailsState, Pul
344357
dispatch({ type: PullRequestDetailsUIActionType.UpdateFileDiffs, data: message });
345358
break;
346359
}
360+
case PullRequestDetailsMessageType.UpdateConflictedFiles: {
361+
dispatch({ type: PullRequestDetailsUIActionType.UpdateConflictedFiles, data: message });
362+
break;
363+
}
347364
case PullRequestDetailsMessageType.UpdateBuildStatuses: {
348365
dispatch({ type: PullRequestDetailsUIActionType.UpdateBuildStatuses, data: message });
349366
break;

‎src/views/nodes/commitNode.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,13 @@ export class CommitNode extends AbstractBaseNode {
3030
try {
3131
const bbApi = await clientForSite(this.pr.site);
3232
const diffs = await bbApi.pullrequests.getChangedFiles(this.pr, this.commit.hash);
33+
const conflictedFiles = await bbApi.pullrequests.getConflictedFiles(this.pr);
3334
const paginatedComments = await bbApi.pullrequests.getComments(this.pr, this.commit.hash);
3435

3536
//TODO: pass tasks if commit-level tasks exist
3637
//TODO: if there is more than one parent, there should probably be a notification about diff ambiguity, unless I can figure
3738
//out a way to resolve this
38-
const children = await createFileChangesNodes(this.pr, paginatedComments, diffs, [], {
39+
const children = await createFileChangesNodes(this.pr, paginatedComments, diffs, conflictedFiles, [], {
3940
lhs: this.commit.parentHashes?.[0] ?? '', //The only time I can think of this being undefined is for an initial commit, but what should the parent be there?
4041
rhs: this.commit.hash,
4142
});

‎src/views/pullrequest/diffViewHelper.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ function traverse(n: Comment): Comment[] {
8484
export async function getArgsForDiffView(
8585
allComments: PaginatedComments,
8686
fileDiff: FileDiff,
87+
conflictedFiles: string[],
8788
pr: PullRequest,
8889
commentController: PullRequestCommentController,
8990
commitRange?: { lhs: string; rhs: string },
@@ -203,7 +204,8 @@ export async function getArgsForDiffView(
203204
fileDisplayName: fileDisplayName,
204205
fileDiffStatus: fileDiff.status,
205206
numberOfComments: comments.length ? comments.length : 0,
206-
isConflicted: fileDiff.isConflicted,
207+
isConflicted:
208+
conflictedFiles.includes(fileDiff.newPath || '') || conflictedFiles.includes(fileDiff.oldPath || ''),
207209
},
208210
};
209211
}
@@ -260,6 +262,7 @@ export async function createFileChangesNodes(
260262
pr: PullRequest,
261263
allComments: PaginatedComments,
262264
fileDiffs: FileDiff[],
265+
conflictedFiles: string[],
263266
tasks: Task[],
264267
commitRange?: { lhs: string; rhs: string },
265268
): Promise<AbstractBaseNode[]> {
@@ -269,6 +272,7 @@ export async function createFileChangesNodes(
269272
return await getArgsForDiffView(
270273
commentsWithTasks,
271274
fileDiff,
275+
conflictedFiles,
272276
pr,
273277
Container.bitbucketContext.prCommentController,
274278
commitRange,

‎src/views/pullrequest/pullRequestNode.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,15 @@ export class PullRequestTitlesNode extends AbstractBaseNode {
8484
const bbApi = await clientForSite(this.pr.site);
8585
const promises = Promise.all([
8686
bbApi.pullrequests.getChangedFiles(this.pr),
87+
bbApi.pullrequests.getConflictedFiles(this.pr),
8788
bbApi.pullrequests.getCommits(this.pr),
8889
bbApi.pullrequests.getComments(this.pr),
8990
bbApi.pullrequests.getTasks(this.pr),
9091
]);
9192

9293
return promises.then(
9394
async (result) => {
94-
const [fileDiffs, commits, allComments, tasks] = result;
95+
const [fileDiffs, conflictedFiles, commits, allComments, tasks] = result;
9596

9697
const children: AbstractBaseNode[] = [new DescriptionNode(this.pr, this)];
9798

@@ -102,7 +103,9 @@ export class PullRequestTitlesNode extends AbstractBaseNode {
102103

103104
children.push(...(await this.createRelatedJiraIssueNode(commits, allComments)));
104105
children.push(...(await this.createRelatedBitbucketIssueNode(commits, allComments)));
105-
children.push(...(await createFileChangesNodes(this.pr, allComments, fileDiffs, tasks)));
106+
children.push(
107+
...(await createFileChangesNodes(this.pr, allComments, fileDiffs, conflictedFiles, tasks)),
108+
);
106109
return children;
107110
},
108111
(reason) => {

‎src/webview/pullrequest/vscPullRequestDetailsActionApi.ts

+8
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,18 @@ export class VSCPullRequestDetailsActionApi implements PullRequestDetailsActionA
183183
return fileDiffs;
184184
}
185185

186+
async getConflictedFiles(pr: PullRequest): Promise<string[]> {
187+
const bbApi = await clientForSite(pr.site);
188+
const conflictedFiles = await bbApi.pullrequests.getConflictedFiles(pr);
189+
return conflictedFiles;
190+
}
191+
186192
async openDiffViewForFile(pr: PullRequest, fileDiff: FileDiff, comments: Comment[]): Promise<void> {
193+
const conflictedFiles = await this.getConflictedFiles(pr);
187194
const diffViewArgs = await getArgsForDiffView(
188195
{ data: comments },
189196
fileDiff,
197+
conflictedFiles,
190198
pr,
191199
Container.bitbucketContext.prCommentController,
192200
);

0 commit comments

Comments
 (0)
Failed to load comments.