Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom diffs #138

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions kotoed-js/src/main/less/code.less
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,16 @@
}

#code-review-app {
display: flex;
flex: 1 1 100%;
}

.code-review-app-rows {
flex: 1 1 auto;
min-height: 0px;
overflow: hidden;
display: flex;
flex-flow: row;
flex-flow: column;
}

.code-review {
Expand All @@ -76,6 +81,13 @@
border-top: 1px solid #ccc;
}

.code-review-status-bar {
flex: 0 0 1.2em;
flex-flow: row;
border-top: 1px solid #ccc;
}


#code-review-left {
display: flex;
flex: none;
Expand Down Expand Up @@ -130,8 +142,11 @@
.lost-found-button-container {
flex: 0 1 auto
}
.diff-mode-button-container {
flex: 0 1 auto
}

.lost-found-button {
.review-bottom-button {
width: 100%
}

Expand Down
143 changes: 94 additions & 49 deletions kotoed-js/src/main/ts/code/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,16 @@ import {
import {
Comment, CommentsState, CommentState, FileComments, LineComments, ReviewComments
} from "./state/comments";
import {fetchDiff, fetchFile, fetchRootDir, File, FileDiffResult, FileType} from "./remote/code";
import {
DiffBase,
fetchDiff,
fetchFile,
fetchRootDir,
File,
FileDiffResponse,
FileDiffResult,
FileType, updateDiffPreference
} from "./remote/code";
import {FileNotFoundError} from "./errors";
import {push} from "react-router-redux";
import {Dispatch} from "redux";
Expand All @@ -22,7 +31,7 @@ import {
editComment as doEditComment
} from "./remote/comments";
import {Capabilities, fetchCapabilities} from "./remote/capabilities";
import {getFilePath, getNodePath} from "./util/filetree";
import {applyDiffToFileTree, getFilePath, getNodePath} from "./util/filetree";
import {NodePath} from "./state/blueprintTree";
import {makeCodeReviewCodePath, makeCodeReviewLostFoundPath} from "../util/url";
import {DbRecordWrapper, isStatusFinal} from "../data/verification";
Expand All @@ -33,6 +42,7 @@ import {fetchAnnotations} from "./remote/annotations";
import {ReviewAnnotations} from "./state/annotations";
import {CommentTemplates, fetchCommentTemplates} from "./remote/templates";
import natsort from "natsort";
import {pick, typedKeys} from "../util/common";

const actionCreator = actionCreatorFactory();

Expand Down Expand Up @@ -114,6 +124,18 @@ interface FormLockUnlockPayload {
sourceline: number
}

interface DiffBasePayload {
diffBase: DiffBase
}

interface PersistPayload {
persist: boolean
}

interface DiffResultPayload {
diff: FileDiffResult[]
}

// Local actions
export const dirExpand = actionCreator<NodePathPayload>('DIR_EXPAND');
export const dirCollapse = actionCreator<NodePathPayload>('DIR_COLLAPSE');
Expand All @@ -132,9 +154,9 @@ export const submissionFetch = actionCreator.async<SubmissionPayload, DbRecordWr


// File or dir fetch actions
export const rootFetch = actionCreator.async<SubmissionPayload, DirFetchResult, {}>('ROOT_FETCH');
export const rootFetch = actionCreator.async<SubmissionPayload, DirFetchResult & DiffResultPayload, {}>('ROOT_FETCH');
export const fileLoad = actionCreator.async<FilePathPayload & SubmissionPayload, FileFetchResult, {}>('FILE_LOAD');
export const fileDiff = actionCreator.async<FilePathPayload & SubmissionPayload, FileDiffResult | undefined, {}>('FILE_DIFF');
export const diffFetch = actionCreator.async<SubmissionPayload & DiffBasePayload, FileDiffResponse>('DIFF_FETCH')

// Annotation fetch actions
export const annotationsFetch = actionCreator.async<number, ReviewAnnotations, {}>('ANNOTATION_FETCH');
Expand Down Expand Up @@ -217,36 +239,48 @@ const naturalSorter = natsort()
const typeSorter = (a: File, b: File) => fileTypeDisplayOrder[a.type] - fileTypeDisplayOrder[b.type]

export function fetchRootDirIfNeeded(payload: SubmissionPayload) {
return (dispatch: Dispatch<CodeReviewState>, getState: () => CodeReviewState) => {
if (!getState().fileTreeState.loading)
return async (dispatch: Dispatch<CodeReviewState>, getState: () => CodeReviewState) => {
const state = getState();
if (!state.fileTreeState.loading)
return Promise.resolve();

dispatch(rootFetch.started({
submissionId: payload.submissionId
}));

return fetchRootDir(payload.submissionId).then((root) => {
const recursiveSorter = (node: File) => {
if (node.children == null) {
return
}
node.children.sort((a: File, b: File) =>
typeSorter(a, b) || naturalSorter(a.name, b.name)
)
for (let child of node.children) {
recursiveSorter(child)
}
const root = await fetchRootDir(payload.submissionId, state.diffState.base);

const recursiveSorter = (node: File) => {
if (node.children == null) {
return
}
recursiveSorter(root)
dispatch(rootFetch.done({
params: {
submissionId: payload.submissionId
},
result: {
root
}
}))
});
node.children.sort((a: File, b: File) =>
typeSorter(a, b) || naturalSorter(a.name, b.name)
)
for (let child of node.children) {
recursiveSorter(child)
}
}
recursiveSorter(root)

const diff = await fetchDiff(payload.submissionId, state.diffState.base);

dispatch(rootFetch.done({
params: {
submissionId: payload.submissionId
},
result: {
root,
diff: diff.diff
}
}))
dispatch(diffFetch.done({
params: {
submissionId: payload.submissionId,
diffBase: state.diffState.base
},
result: diff
}))
};
}

Expand Down Expand Up @@ -310,8 +344,9 @@ export function setLostFoundPath(payload: SubmissionPayload) {


export function loadFileToEditor(payload: FilePathPayload & SubmissionPayload) {
return (dispatch: Dispatch<CodeReviewState>, getState: () => CodeReviewState) => {
return async (dispatch: Dispatch<CodeReviewState>, getState: () => CodeReviewState) => {
let {filename} = payload;
const state = getState();
dispatch(fileLoad.started({
submissionId: payload.submissionId,
filename
Expand All @@ -322,28 +357,38 @@ export function loadFileToEditor(payload: FilePathPayload & SubmissionPayload) {
file: filename
})(dispatch, getState);

fetchFile(payload.submissionId, filename).then(result => {
dispatch(fileLoad.done({
params: {
submissionId: payload.submissionId,
filename
},
result: {
value: result,
displayedComments: getState().commentsState.comments.get(filename, Map<number, LineComments>()),
}
}));
});
const result = await fetchFile(payload.submissionId, filename);
dispatch(fileLoad.done({
params: {
submissionId: payload.submissionId,
filename
},
result: {
value: result,
displayedComments: state.commentsState.comments.get(filename, Map<number, LineComments>()),
}
}));

fetchDiff(payload.submissionId).then(result => {
dispatch(fileDiff.done({
params: {
submissionId: payload.submissionId,
filename
},
result: result.find((diff) => diff.toFile == filename)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff now is fetched only once, since diff endpoint returns a diff for the whole submission.

}))
});
}
}

export function updateDiff(payload: SubmissionPayload & DiffBasePayload & PersistPayload) {
return async (dispatch: Dispatch<CodeReviewState>, getState: () => CodeReviewState) => {
dispatch(diffFetch.started(payload))

const state = getState();
const patchedType = payload.diffBase.type == "SUBMISSION_ID" ? "PREVIOUS_CLOSED" : payload.diffBase.type;

if (payload.persist) {
updateDiffPreference(state.capabilitiesState.capabilities.principal, patchedType)
}

const diff = await fetchDiff(payload.submissionId, payload.diffBase);

dispatch(diffFetch.done({
params: payload,
result: diff
}))
}
}

Expand Down
Loading