Skip to content

Commit 14d2417

Browse files
committed
Added checkboxes for file reviewing
1 parent 70dab38 commit 14d2417

File tree

9 files changed

+349
-58
lines changed

9 files changed

+349
-58
lines changed

src/container.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ import { Logger } from './logger';
7575
import { SearchJiraHelper } from './views/jira/searchJiraHelper';
7676
import { featureFlagClientInitializedEvent } from './analytics';
7777
import { openPullRequest } from './commands/bitbucket/pullRequest';
78+
import { CheckboxStateManager } from './views/nodes/checkBoxStateManager';
7879

7980
const isDebuggingRegex = /^--(debug|inspect)\b(-brk\b|(?!-))=?/;
8081
const ConfigTargetKey = 'configurationTarget';
@@ -87,6 +88,8 @@ export class Container {
8788
static async initialize(context: ExtensionContext, config: IConfig, version: string) {
8889
const analyticsEnv: string = this.isDebugging ? 'staging' : 'prod';
8990

91+
this._checkboxStateManager = new CheckboxStateManager(context);
92+
9093
this._analyticsClient = analyticsClient({
9194
origin: 'desktop',
9295
env: analyticsEnv,
@@ -531,4 +534,9 @@ export class Container {
531534
public static get pmfStats() {
532535
return this._pmfStats;
533536
}
537+
538+
private static _checkboxStateManager: CheckboxStateManager;
539+
public static get checkboxStateManager() {
540+
return this._checkboxStateManager;
541+
}
534542
}

src/views/BitbucketExplorer.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,46 @@ import { DescriptionNode, PullRequestTitlesNode } from './pullrequest/pullReques
1212
import { PullRequestNodeDataProvider } from './pullRequestNodeDataProvider';
1313
import { RefreshTimer } from './RefreshTimer';
1414
import { BitbucketActivityMonitor } from './BitbucketActivityMonitor';
15+
import { PullRequestFilesNode } from './nodes/pullRequestFilesNode';
16+
import { DirectoryNode } from './nodes/directoryNode';
17+
import { TreeView } from 'vscode';
18+
import { AbstractBaseNode } from './nodes/abstractBaseNode';
1519

1620
export abstract class BitbucketExplorer extends Explorer implements Disposable {
1721
private _disposable: Disposable;
1822

1923
private monitor: BitbucketActivityMonitor | undefined;
2024
private _refreshTimer: RefreshTimer;
25+
private _onDidChangeTreeData = new vscode.EventEmitter<AbstractBaseNode | undefined>();
26+
27+
protected newTreeView(): TreeView<AbstractBaseNode> | undefined {
28+
super.newTreeView();
29+
this.setupCheckboxHandling();
30+
return this.treeView;
31+
}
32+
33+
private setupCheckboxHandling(): void {
34+
if (!this.treeView) {
35+
return;
36+
}
37+
this.treeView.onDidChangeCheckboxState((event) => {
38+
event.items.forEach(([item, state]) => {
39+
const checked = state === vscode.TreeItemCheckboxState.Checked;
40+
if (item instanceof PullRequestFilesNode || item instanceof DirectoryNode) {
41+
item.checked = checked;
42+
this._onDidChangeTreeData.fire(item);
43+
}
44+
});
45+
});
46+
}
2147

2248
constructor(protected ctx: BitbucketContext) {
2349
super(() => this.dispose());
2450

51+
setTimeout(() => {
52+
this.setupCheckboxHandling();
53+
}, 50);
54+
2555
Container.context.subscriptions.push(configuration.onDidChange(this._onConfigurationChanged, this));
2656

2757
this._refreshTimer = new RefreshTimer(this.explorerEnabledConfiguration(), this.refreshConfiguration(), () =>

src/views/nodes/FilesRootNode.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import * as vscode from 'vscode';
2+
import { AbstractBaseNode } from './abstractBaseNode';
3+
4+
export class FilesRootNode extends AbstractBaseNode {
5+
constructor(
6+
private fileNodes: AbstractBaseNode[],
7+
parent?: AbstractBaseNode,
8+
) {
9+
super(parent);
10+
}
11+
12+
getTreeItem(): vscode.TreeItem {
13+
const item = new vscode.TreeItem('Files', vscode.TreeItemCollapsibleState.Collapsed);
14+
item.contextValue = 'files-root';
15+
return item;
16+
}
17+
18+
async getChildren(): Promise<AbstractBaseNode[]> {
19+
return this.fileNodes;
20+
}
21+
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { Logger } from 'src/logger';
2+
import vscode from 'vscode';
3+
interface CheckboxState {
4+
id: string;
5+
timestamp: number;
6+
}
7+
export class CheckboxStateManager {
8+
private readonly STATE_EXPIRY = 24 * 60 * 60 * 1000 * 30; // 30 days
9+
private readonly STORAGE_KEY = 'bitbucket.viewedFiles';
10+
11+
constructor(private context: vscode.ExtensionContext) {
12+
this.cleanup();
13+
}
14+
15+
private async cleanup(): Promise<void> {
16+
const states = this.context.workspaceState.get<CheckboxState[]>(this.STORAGE_KEY, []);
17+
const now = Date.now();
18+
19+
const validStates = states.filter((state) => {
20+
const isValid = now - state.timestamp < this.STATE_EXPIRY;
21+
if (!isValid) {
22+
Logger.debug(`Removing expired checkbox state: ${state.id}`);
23+
}
24+
return isValid;
25+
});
26+
27+
await this.context.workspaceState.update(this.STORAGE_KEY, validStates);
28+
Logger.debug(`Cleanup complete. Remaining states: ${validStates.length}`);
29+
}
30+
31+
isChecked(id: string): boolean {
32+
const states = this.context.workspaceState.get<CheckboxState[]>(this.STORAGE_KEY, []);
33+
const state = states.find((state) => state.id === id);
34+
35+
if (state) {
36+
if (Date.now() - state.timestamp >= this.STATE_EXPIRY) {
37+
this.setChecked(id, false);
38+
return false;
39+
}
40+
return true;
41+
}
42+
return false;
43+
}
44+
45+
setChecked(id: string, checked: boolean): void {
46+
const states = this.context.workspaceState.get<CheckboxState[]>(this.STORAGE_KEY, []);
47+
48+
if (checked) {
49+
const existingIndex = states.findIndex((state) => state.id === id);
50+
if (existingIndex !== -1) {
51+
states.splice(existingIndex, 1);
52+
}
53+
states.push({ id, timestamp: Date.now() });
54+
} else {
55+
const index = states.findIndex((state) => state.id === id);
56+
if (index !== -1) {
57+
states.splice(index, 1);
58+
}
59+
}
60+
61+
this.context.workspaceState.update(this.STORAGE_KEY, states);
62+
Logger.debug(`Checkbox state updated: ${id} = ${checked}`);
63+
}
64+
}

src/views/nodes/commitNode.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,18 @@ export class CommitNode extends AbstractBaseNode {
3636
//TODO: pass tasks if commit-level tasks exist
3737
//TODO: if there is more than one parent, there should probably be a notification about diff ambiguity, unless I can figure
3838
//out a way to resolve this
39-
const children = await createFileChangesNodes(this.pr, paginatedComments, diffs, conflictedFiles, [], {
40-
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?
41-
rhs: this.commit.hash,
42-
});
39+
const children = await createFileChangesNodes(
40+
this.pr,
41+
paginatedComments,
42+
diffs,
43+
conflictedFiles,
44+
[],
45+
{
46+
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?
47+
rhs: this.commit.hash,
48+
},
49+
'commits',
50+
);
4351
return children;
4452
} catch (e) {
4553
Logger.debug('error fetching changed files', e);

src/views/nodes/directoryNode.ts

Lines changed: 111 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,129 @@ import * as vscode from 'vscode';
22
import { PRDirectory } from '../pullrequest/diffViewHelper';
33
import { AbstractBaseNode } from './abstractBaseNode';
44
import { PullRequestFilesNode } from './pullRequestFilesNode';
5+
import { Container } from 'src/container';
6+
import { Logger } from 'src/logger';
7+
import { PullRequest } from 'src/bitbucket/model';
8+
import * as crypto from 'crypto';
59

610
export class DirectoryNode extends AbstractBaseNode {
7-
constructor(private directoryData: PRDirectory) {
11+
isRootFilesDirectory: boolean | undefined;
12+
constructor(
13+
private directoryData: PRDirectory,
14+
private section: 'files' | 'commits' = 'files',
15+
private pr: PullRequest,
16+
) {
817
super();
918
}
1019

20+
private _isDirectClick = false;
21+
22+
get directoryId(): string {
23+
const prUrl = this.pr.data.url;
24+
const prUrlPath = vscode.Uri.parse(prUrl).path;
25+
const prId = prUrlPath.slice(prUrlPath.lastIndexOf('/') + 1);
26+
const repoUrl = prUrl.slice(0, prUrl.indexOf('/pull-requests'));
27+
const repoId = repoUrl.slice(repoUrl.lastIndexOf('/') + 1);
28+
const dirPath = this.directoryData.dirPath;
29+
30+
const dirId =
31+
this.section === 'commits'
32+
? `repo-${repoId}-pr-${prId}-section-${this.section}-commit-${this.pr.data.source.commitHash}-directory-${dirPath}`
33+
: `repo-${repoId}-pr-${prId}-section-${this.section}-directory-${dirPath}`;
34+
return crypto.createHash('md5').update(dirId).digest('hex');
35+
}
36+
37+
private areAllChildrenChecked(): boolean {
38+
const allFilesChecked = this.directoryData.files.every((file) => {
39+
const fileNode = new PullRequestFilesNode(file, this.section, this.pr);
40+
return fileNode.checked;
41+
});
42+
43+
const allSubdirsChecked = Array.from(this.directoryData.subdirs.values()).every((subdir) => {
44+
const subdirNode = new DirectoryNode(subdir, this.section, this.pr);
45+
return subdirNode.checked;
46+
});
47+
48+
return allFilesChecked && allSubdirsChecked;
49+
}
50+
51+
set checked(value: boolean) {
52+
// This is to avoid infinite loops when setting the checked state
53+
if (this._isDirectClick) {
54+
return;
55+
}
56+
57+
try {
58+
this._isDirectClick = true;
59+
Logger.debug(`Setting directory ${this.directoryId} checked state to ${value}`);
60+
Container.checkboxStateManager.setChecked(this.directoryId, value);
61+
62+
Logger.debug('Propagating state to children');
63+
this.directoryData.files.forEach((file) => {
64+
const fileNode = new PullRequestFilesNode(file, this.section, this.pr);
65+
Container.checkboxStateManager.setChecked(fileNode.fileId, value);
66+
});
67+
this.directoryData.subdirs.forEach((subdir) => {
68+
const subdirNode = new DirectoryNode(subdir, this.section, this.pr);
69+
Container.checkboxStateManager.setChecked(subdirNode.directoryId, value);
70+
});
71+
} finally {
72+
this._isDirectClick = false;
73+
}
74+
}
75+
76+
get checked(): boolean {
77+
const hasExplicitState = Container.checkboxStateManager.isChecked(this.directoryId);
78+
if (!hasExplicitState) {
79+
return this.areAllChildrenChecked();
80+
}
81+
return hasExplicitState;
82+
}
83+
1184
async getTreeItem(): Promise<vscode.TreeItem> {
12-
const item = new vscode.TreeItem(this.directoryData.name, vscode.TreeItemCollapsibleState.Expanded);
85+
this.isRootFilesDirectory =
86+
this.section === 'files' && this.directoryData.name === 'Files' && this.directoryData.dirPath === '';
87+
88+
const item = new vscode.TreeItem(
89+
this.directoryData.name,
90+
this.isRootFilesDirectory
91+
? vscode.TreeItemCollapsibleState.Collapsed
92+
: vscode.TreeItemCollapsibleState.Expanded,
93+
);
1394
item.tooltip = this.directoryData.name;
14-
item.iconPath = vscode.ThemeIcon.Folder;
95+
96+
if (!this.isRootFilesDirectory) {
97+
item.iconPath = vscode.ThemeIcon.Folder;
98+
}
99+
100+
const allChecked = this.areAllChildrenChecked();
101+
102+
if (!this.isRootFilesDirectory) {
103+
item.checkboxState = this.checked
104+
? vscode.TreeItemCheckboxState.Checked
105+
: vscode.TreeItemCheckboxState.Unchecked;
106+
item.contextValue = `directory${allChecked ? '.checked' : ''}`;
107+
}
108+
109+
Logger.debug('directoryId', this.directoryId);
110+
111+
if (!this.isRootFilesDirectory) {
112+
item.id = this.directoryId;
113+
}
114+
15115
return item;
16116
}
17117

18-
async getChildren(element?: AbstractBaseNode): Promise<AbstractBaseNode[]> {
118+
async getChildren(): Promise<AbstractBaseNode[]> {
119+
const fileNodes: AbstractBaseNode[] = this.directoryData.files.map(
120+
(diffViewArg) => new PullRequestFilesNode(diffViewArg, this.section, this.pr),
121+
);
122+
19123
const directoryNodes: DirectoryNode[] = Array.from(
20124
this.directoryData.subdirs.values(),
21-
(subdir) => new DirectoryNode(subdir),
125+
(subdir) => new DirectoryNode(subdir, this.section, this.pr),
22126
);
23-
const fileNodes: AbstractBaseNode[] = this.directoryData.files.map(
24-
(diffViewArg) => new PullRequestFilesNode(diffViewArg),
25-
);
26-
return fileNodes.concat(directoryNodes);
127+
128+
return [...directoryNodes, ...fileNodes];
27129
}
28130
}

src/views/nodes/pullRequestFilesNode.ts

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,49 @@
11
import path from 'path';
22
import * as vscode from 'vscode';
3-
import { FileStatus } from '../../bitbucket/model';
3+
import { FileStatus, PullRequest } from '../../bitbucket/model';
44
import { Commands } from '../../commands';
55
import { configuration } from '../../config/configuration';
66
import { Resources } from '../../resources';
77
import { DiffViewArgs } from '../pullrequest/diffViewHelper';
88
import { PullRequestContextValue } from '../pullrequest/pullRequestNode';
99
import { AbstractBaseNode } from './abstractBaseNode';
10+
import { Container } from '../../container';
11+
import { DirectoryNode } from './directoryNode';
12+
import * as crypto from 'crypto';
1013

1114
export class PullRequestFilesNode extends AbstractBaseNode {
12-
constructor(private diffViewData: DiffViewArgs) {
15+
constructor(
16+
private diffViewData: DiffViewArgs,
17+
private section: 'files' | 'commits' = 'files',
18+
private pr: PullRequest,
19+
) {
1320
super();
1421
}
1522

23+
get fileId(): string {
24+
const prUrl = this.pr.data.url;
25+
const repoUrl = prUrl.slice(0, prUrl.indexOf('/pull-requests'));
26+
const repoId = repoUrl.slice(repoUrl.lastIndexOf('/') + 1);
27+
28+
const fileId =
29+
this.section === 'commits'
30+
? `repo-${repoId}-pr-${this.pr.data.id}-section-${this.section}-commit-${this.pr.data.source.commitHash}-file-${this.diffViewData.fileDisplayData.fileDisplayName}-${this.diffViewData.latestFileContentHash}`
31+
: `repo-${repoId}-pr-${this.pr.data.id}-section-${this.section}-file-${this.diffViewData.fileDisplayData.fileDisplayName}-${this.diffViewData.latestFileContentHash}`;
32+
return crypto.createHash('md5').update(fileId).digest('hex');
33+
}
34+
35+
get checked(): boolean {
36+
return Container.checkboxStateManager.isChecked(this.fileId);
37+
}
38+
39+
set checked(value: boolean) {
40+
Container.checkboxStateManager.setChecked(this.fileId, value);
41+
42+
if (this.getParent instanceof DirectoryNode) {
43+
this.getTreeItem();
44+
}
45+
}
46+
1647
async getTreeItem(): Promise<vscode.TreeItem> {
1748
const itemData = this.diffViewData.fileDisplayData;
1849
let fileDisplayString = itemData.fileDisplayName;
@@ -23,14 +54,21 @@ export class PullRequestFilesNode extends AbstractBaseNode {
2354
`${itemData.numberOfComments > 0 ? '💬 ' : ''}${fileDisplayString}`,
2455
vscode.TreeItemCollapsibleState.None,
2556
);
57+
58+
item.checkboxState = this.checked
59+
? vscode.TreeItemCheckboxState.Checked
60+
: vscode.TreeItemCheckboxState.Unchecked;
61+
62+
item.id = this.fileId;
63+
2664
item.tooltip = itemData.fileDisplayName;
2765
item.command = {
2866
command: Commands.ViewDiff,
2967
title: 'Diff file',
3068
arguments: this.diffViewData.diffArgs,
3169
};
3270

33-
item.contextValue = PullRequestContextValue;
71+
item.contextValue = `${PullRequestContextValue}${this.checked ? '.checked' : ''}`;
3472
item.resourceUri = vscode.Uri.parse(`${itemData.prUrl}#chg-${itemData.fileDisplayName}`);
3573
switch (itemData.fileDiffStatus) {
3674
case FileStatus.ADDED:

0 commit comments

Comments
 (0)