Skip to content

Commit

Permalink
#482 New "Mark as Reviewed" & "Mark as Not Reviewed" actions on the f…
Browse files Browse the repository at this point in the history 
…ile context menu in the Commit Details View when a Code Review is in progress. Squash Merge of @dan1994's PR #483.
  • Loading branch information
@dan1994 @mhutchie
dan1994 authored and mhutchie committed Mar 14, 2021
1 parent d0412a1 commit 556e494
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 111 deletions.
33 changes: 20 additions & 13 deletions src/extensionState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,24 +359,31 @@ export class ExtensionState extends Disposable {
}

/**
* Record that a file has been reviewed in a Code Review.
* Update information for a specific Code Review.
* @param repo The repository the Code Review is in.
* @param id The ID of the Code Review.
* @param file The file that has been reviewed.
* @param remainingFiles The files remaining for review.
* @param lastViewedFile The last viewed file. If null, don't change the last viewed file.
* @returns An error message if request can't be completed.
*/
public updateCodeReviewFileReviewed(repo: string, id: string, file: string) {
let reviews = this.getCodeReviews();
if (typeof reviews[repo] !== 'undefined' && typeof reviews[repo][id] !== 'undefined') {
let i = reviews[repo][id].remainingFiles.indexOf(file);
if (i > -1) reviews[repo][id].remainingFiles.splice(i, 1);
if (reviews[repo][id].remainingFiles.length > 0) {
reviews[repo][id].lastViewedFile = file;
reviews[repo][id].lastActive = (new Date()).getTime();
} else {
removeCodeReview(reviews, repo, id);
public updateCodeReview(repo: string, id: string, remainingFiles: string[], lastViewedFile: string | null) {
const reviews = this.getCodeReviews();

if (typeof reviews[repo] === 'undefined' || typeof reviews[repo][id] === 'undefined') {
return Promise.resolve('The Code Review could not be found.');
}

if (remainingFiles.length > 0) {
reviews[repo][id].remainingFiles = remainingFiles;
reviews[repo][id].lastActive = (new Date()).getTime();
if (lastViewedFile !== null) {
reviews[repo][id].lastViewedFile = lastViewedFile;
}
this.setCodeReviews(reviews);
} else {
removeCodeReview(reviews, repo, id);
}

return this.setCodeReviews(reviews);
}

/**
Expand Down
9 changes: 6 additions & 3 deletions src/gitGraphView.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,6 @@ export class GitGraphView extends Disposable {
error: await this.dataSource.cleanUntrackedFiles(msg.repo, msg.directories)
});
break;
case 'codeReviewFileReviewed':
this.extensionState.updateCodeReviewFileReviewed(msg.repo, msg.id, msg.filePath);
break;
case 'commitDetails':
let data = await Promise.all<GitCommitDetailsData, string | null>([
msg.commitHash === UNCOMMITTED
Expand Down Expand Up @@ -576,6 +573,12 @@ export class GitGraphView extends Disposable {
...await this.dataSource.getTagDetails(msg.repo, msg.tagName)
});
break;
case 'updateCodeReview':
this.sendMessage({
command: 'updateCodeReview',
error: await this.extensionState.updateCodeReview(msg.repo, msg.id, msg.remainingFiles, msg.lastViewedFile)
});
break;
case 'viewDiff':
this.sendMessage({
command: 'viewDiff',
Expand Down
20 changes: 13 additions & 7 deletions src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,12 +645,6 @@ export interface ResponseCleanUntrackedFiles extends ResponseWithErrorInfo {
readonly command: 'cleanUntrackedFiles';
}

export interface RequestCodeReviewFileReviewed extends RepoRequest {
readonly command: 'codeReviewFileReviewed';
readonly id: string;
readonly filePath: string;
}

export interface RequestCommitDetails extends RepoRequest {
readonly command: 'commitDetails';
readonly commitHash: string;
Expand Down Expand Up @@ -1161,6 +1155,17 @@ export interface ResponseTagDetails extends ResponseWithErrorInfo {
readonly message: string;
}

export interface RequestUpdateCodeReview extends RepoRequest {
readonly command: 'updateCodeReview';
readonly id: string;
readonly remainingFiles: string[];
readonly lastViewedFile: string | null;
}

export interface ResponseUpdateCodeReview extends ResponseWithErrorInfo {
readonly command: 'updateCodeReview';
}

export interface RequestViewDiff extends RepoRequest {
readonly command: 'viewDiff';
readonly fromHash: string;
Expand Down Expand Up @@ -1207,7 +1212,6 @@ export type RequestMessage =
| RequestCheckoutCommit
| RequestCherrypickCommit
| RequestCleanUntrackedFiles
| RequestCodeReviewFileReviewed
| RequestCommitDetails
| RequestCompareCommits
| RequestCopyFilePath
Expand Down Expand Up @@ -1256,6 +1260,7 @@ export type RequestMessage =
| RequestShowErrorDialog
| RequestStartCodeReview
| RequestTagDetails
| RequestUpdateCodeReview
| RequestViewDiff
| RequestViewDiffWithWorkingFile
| RequestViewFileAtRevision
Expand Down Expand Up @@ -1315,6 +1320,7 @@ export type ResponseMessage =
| ResponseSetWorkspaceViewState
| ResponseStartCodeReview
| ResponseTagDetails
| ResponseUpdateCodeReview
| ResponseViewDiff
| ResponseViewDiffWithWorkingFile
| ResponseViewFileAtRevision
Expand Down
136 changes: 72 additions & 64 deletions tests/extensionState.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1167,124 +1167,132 @@ describe('ExtensionState', () => {
});
});

describe('updateCodeReviewFileReviewed', () => {
it('Should remove the reviewed file, set it as the last viewed file, and update the last active time', () => {
// Setup
describe('updateCodeReview', () => {
const repo = '/path/to/repo';
const id = 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2';
const startTime = 1587559257000;
const endTime = startTime + 1000;

beforeEach(() => {
const codeReviews = {
'/path/to/repo': {
'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2': {
lastActive: 1587559257000,
[repo]: {
[id]: {
lastActive: startTime,
lastViewedFile: 'file1.txt',
remainingFiles: ['file2.txt', 'file3.txt']
}
}
};

extensionContext.workspaceState.get.mockReturnValueOnce(codeReviews);
extensionContext.workspaceState.update.mockResolvedValueOnce(null);
});

it('Should update the reviewed files and change the last viewed file', async () => {
// Run
extensionState.updateCodeReviewFileReviewed('/path/to/repo', 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', 'file2.txt');
const result = await extensionState.updateCodeReview(repo, id, ['file3.txt'], 'file2.txt');

// Assert
// Asset
expect(result).toBeNull();
expect(extensionContext.workspaceState.update).toHaveBeenCalledWith('codeReviews', {
'/path/to/repo': {
'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2': {
lastActive: 1587559258000,
[repo]: {
[id]: {
lastActive: endTime,
lastViewedFile: 'file2.txt',
remainingFiles: ['file3.txt']
}
}
});
});

it('Should ignore removing reviewed files if it has already be stored as reviewed', () => {
// Setup
const codeReviews = {
'/path/to/repo': {
'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2': {
lastActive: 1587559257000,
it('Should update the reviewed files without changing the last viewed file', async () => {
// Run
const result = await extensionState.updateCodeReview(repo, id, ['file3.txt'], null);

// Assert
expect(result).toBeNull();
expect(extensionContext.workspaceState.update).toHaveBeenCalledWith('codeReviews', {
[repo]: {
[id]: {
lastActive: endTime,
lastViewedFile: 'file1.txt',
remainingFiles: ['file3.txt']
}
}
};
extensionContext.workspaceState.get.mockReturnValueOnce(codeReviews);
extensionContext.workspaceState.update.mockResolvedValueOnce(null);
});
});

it('Should update the not reviewed files without changing the last viewed file', async () => {
// Run
extensionState.updateCodeReviewFileReviewed('/path/to/repo', 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', 'file2.txt');
const result = await extensionState.updateCodeReview(repo, id, ['file2.txt', 'file3.txt', 'file4.txt'], null);

// Assert
expect(result).toBeNull();
expect(extensionContext.workspaceState.update).toHaveBeenCalledWith('codeReviews', {
'/path/to/repo': {
'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2': {
lastActive: 1587559258000,
lastViewedFile: 'file2.txt',
remainingFiles: ['file3.txt']
[repo]: {
[id]: {
lastActive: endTime,
lastViewedFile: 'file1.txt',
remainingFiles: ['file2.txt', 'file3.txt', 'file4.txt']
}
}
});
});

it('Should remove the code review the last file in it has been reviewed', () => {
// Setup
const codeReviews = {
'/path/to/repo': {
'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2': {
lastActive: 1587559257000,
it('Should set the last viewed file', async () => {
// Run
const result = await extensionState.updateCodeReview(repo, id, ['file2.txt', 'file3.txt'], 'file2.txt');

// Assert
expect(result).toBeNull();
expect(extensionContext.workspaceState.update).toHaveBeenCalledWith('codeReviews', {
[repo]: {
[id]: {
lastActive: endTime,
lastViewedFile: 'file2.txt',
remainingFiles: ['file3.txt']
remainingFiles: ['file2.txt', 'file3.txt']
}
}
};
extensionContext.workspaceState.get.mockReturnValueOnce(codeReviews);
extensionContext.workspaceState.update.mockResolvedValueOnce(null);
});
});

it('Should remove the code review the last file in it has been reviewed', async () => {
// Run
extensionState.updateCodeReviewFileReviewed('/path/to/repo', 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', 'file3.txt');
const result = await extensionState.updateCodeReview(repo, id, [], null);

// Assert
expect(result).toBeNull();
expect(extensionContext.workspaceState.update).toHaveBeenCalledWith('codeReviews', {});
});

it('Shouldn\'t change the state if no code review could be found in the specified repository', () => {
// Setup
const codeReviews = {
'/path/to/repo': {
'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2': {
lastActive: 1587559258000,
lastViewedFile: 'file1.txt',
remainingFiles: ['file2.txt', 'file3.txt']
}
}
};
extensionContext.workspaceState.get.mockReturnValueOnce(codeReviews);
it('Shouldn\'t change the state if no code review could be found in the specified repository', async () => {
// Run
const result = await extensionState.updateCodeReview(repo + '1', id, ['file3.txt'], null);

// Assert
expect(result).toBe('The Code Review could not be found.');
expect(extensionContext.workspaceState.update).toHaveBeenCalledTimes(0);
});

it('Shouldn\'t change the state if no code review could be found with the specified id', async () => {
// Run
extensionState.updateCodeReviewFileReviewed('/path/to/repo1', 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', 'file2.txt');
const result = await extensionState.updateCodeReview(repo, id + '1', ['file3.txt'], null);

// Assert
expect(result).toBe('The Code Review could not be found.');
expect(extensionContext.workspaceState.update).toHaveBeenCalledTimes(0);
});

it('Shouldn\'t change the state if no code review could be found with the specified id', () => {
it('Should return an error message when workspaceState.update rejects', async () => {
// Setup
const codeReviews = {
'/path/to/repo': {
'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2': {
lastActive: 1587559258000,
lastViewedFile: 'file1.txt',
remainingFiles: ['file2.txt', 'file3.txt']
}
}
};
extensionContext.workspaceState.get.mockReturnValueOnce(codeReviews);
extensionContext.workspaceState.update.mockReset();
extensionContext.workspaceState.update.mockRejectedValueOnce(null);

// Run
extensionState.updateCodeReviewFileReviewed('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', 'file2.txt');
const result = await extensionState.updateCodeReview(repo, id, ['file3.txt'], 'file2.txt');

// Assert
expect(extensionContext.workspaceState.update).toHaveBeenCalledTimes(0);
// Asset
expect(result).toBe('Visual Studio Code was unable to save the Git Graph Workspace State Memento.');
});
});

Expand Down
Loading

0 comments on commit 556e494

Please sign in to comment.