Skip to content

Commit

Permalink
Handle new discussion on plain text
Browse files Browse the repository at this point in the history
  • Loading branch information
Frédéric Collonval committed Feb 26, 2021
1 parent 6f1da61 commit 19dd4bc
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 90 deletions.
4 changes: 2 additions & 2 deletions jupyterlab_pullrequests/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ class CommentReply(NamedTuple):

class NewComment(NamedTuple):
text: str
commitId: str
filename: Optional[str]
position: Optional[int]
line: Optional[int]
originalLine: Optional[int]


# @dataclass
Expand Down
2 changes: 1 addition & 1 deletion jupyterlab_pullrequests/handlers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ async def post(self):
body = CommentReply(data["text"], data["discussionId"])
else:
body = NewComment(
data["text"], data["commit_id"], data["filename"], data["position"]
data["text"], filename, data.get("line"), data.get("originalLine")
)
except KeyError as e:
raise tornado.web.HTTPError(
Expand Down
56 changes: 39 additions & 17 deletions jupyterlab_pullrequests/managers/gitlab.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import asyncio
import json
from http import HTTPStatus
from urllib.parse import quote
Expand All @@ -23,6 +24,9 @@ def __init__(
access_token: Versioning service access token
"""
super().__init__(base_api_url=base_api_url, access_token=access_token)
self._merge_requests_cache = (
{}
) # Creating new file discussion required some commit sha's so we will cache them

async def get_current_user(self) -> Dict[str, str]:
git_url = url_path_join(self._base_api_url, "user")
Expand Down Expand Up @@ -59,6 +63,9 @@ async def list_prs(self, username: str, pr_filter: str) -> List[Dict[str, str]]:
"merge_requests",
str(result["iid"]),
)
# We need to request specifically the single MR endpoint to get the diff_refs
# and some other information for
self._merge_requests_cache[url] = await self._call_gitlab(url)
data.append(
{
"id": url,
Expand All @@ -69,6 +76,9 @@ async def list_prs(self, username: str, pr_filter: str) -> List[Dict[str, str]]:
}
)

all_mrs = await asyncio.gather(*[self._call_gitlab(d["id"]) for d in data])
self._merge_requests_cache = {d["id"]: e for d, e in zip(data, all_mrs)}

return data

# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -105,7 +115,7 @@ async def list_files(self, pr_id: str) -> List[Dict[str, str]]:

async def get_pr_links(self, pr_id: str, filename: str) -> Dict[str, str]:

data = await self._call_gitlab(pr_id)
data = self._merge_requests_cache[pr_id]
base_url = url_concat(
url_path_join(
self._base_api_url,
Expand All @@ -128,8 +138,7 @@ async def get_pr_links(self, pr_id: str, filename: str) -> Dict[str, str]:
),
{"ref": data["source_branch"]},
)
commit_id = data["diff_refs"]["head_sha"]
return {"baseUrl": base_url, "headUrl": head_url, "commitId": commit_id}
return {"baseUrl": base_url, "headUrl": head_url}

async def get_link_content(self, file_url: str):
try:
Expand All @@ -147,7 +156,6 @@ async def get_file_content(self, pr_id: str, filename: str) -> Dict[str, str]:
return {
"baseContent": base_content,
"headContent": head_content,
"commitId": links["commitId"],
}

# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -202,24 +210,38 @@ async def get_threads(
async def post_file_comment(
self, pr_id: str, filename: str, body: Union[CommentReply, NewComment]
):
get_logger().info(f"Get POST request with {pr_id}, {filename}, {body}")

if isinstance(body, CommentReply):
data = {"body": body.text}
git_url = url_path_join(pr_id, "discussions", body.inReplyTo, "notes")
response = await self._call_gitlab(git_url, method="POST", body=data)
get_logger().info(str(response))
return self.response_to_comment(response)
# else:
# body = {
# "body": body.text,
# "commitId": body.commit_id,
# "path": body.filename,
# "position": {"position_type": "text", "new_line": body.position},
# }
# git_url = url_path_join(pr_id, "discussions")
# response = await self._call_gitlab(git_url, method="POST", body=body)
# return self.file_comment_response(response)
else:
data = {"body": body.text}
if body.line is not None:
data["position"] = {
"position_type": "text",
"new_line": body.line,
"new_path": filename,
}
data["position"].update(self._merge_requests_cache[pr_id]["diff_refs"])
elif body.originalLine is not None:
data["position"] = {
"position_type": "text",
"old_line": body.originalLine,
"old_path": filename,
}
data["position"].update(
self._merge_requests_cache[pr_id]["diff_refs"].copy()
)

git_url = url_path_join(pr_id, "discussions")
response = await self._call_gitlab(git_url, method="POST", body=data)

comment = self.response_to_comment(response["notes"][0])
# Add the discussion ID created by GitLab
comment["inReplyTo"] = response["id"]
return comment

async def _call_gitlab(
self,
Expand All @@ -237,7 +259,7 @@ async def _call_gitlab(

headers = {
"Authorization": f"Bearer {self._access_token}",
"Accept": "application/json"
"Accept": "application/json",
}

if body is not None:
Expand Down
5 changes: 5 additions & 0 deletions src/components/diff/CommentThread.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ export class CommentThread extends Widget {
} else {
body = { ...body, discussionId: this._thread.id };
}

let endpoint = `pullrequests/files/comments?id=${encodeURIComponent(
this._thread.pullRequestId
)}`;
Expand All @@ -206,6 +207,10 @@ export class CommentThread extends Widget {
userName: response.userName,
userPicture: response.userPicture
};
// Update discussion reference
if (response.inReplyTo) {
this._thread.id = response.inReplyTo;
}
this._thread.comments.push(comment);

this._threadsContainer.replaceChild(
Expand Down
136 changes: 69 additions & 67 deletions src/components/diff/plaintext.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
/* eslint-disable @typescript-eslint/ban-ts-ignore */
import { Mode } from '@jupyterlab/codemirror';
import { mergeView } from '@jupyterlab/git/lib/components/diff/mergeview';
import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
import { Widget } from '@lumino/widgets';
import { MergeView } from 'codemirror';
import { IDiffOptions, IThread } from '../../tokens';
import { IComment, IDiffOptions, IThread } from '../../tokens';
import { generateNode } from '../../utils';
import { CommentThread } from './CommentThread';

Expand Down Expand Up @@ -35,18 +34,6 @@ export class PlainTextDiff extends Widget {
return head;
}

protected static makeThreadWidget(
thread: IThread,
renderMime: IRenderMimeRegistry
): HTMLElement {
const widget = new CommentThread({
renderMime,
thread,
handleRemove: () => null
});
return widget.node;
}

/**
* Create comment decoration node
*/
Expand All @@ -61,61 +48,20 @@ export class PlainTextDiff extends Widget {
* @param from First line in the view port
* @param to Last line in the view port
*/
protected static setCommentGutter(
protected setCommentGutter(
editor: CodeMirror.Editor,
from: number,
to: number
to: number,
side: 'line' | 'originalLine'
): void {
editor.clearGutter('jp-PullRequestCommentDecoration');
for (let lineIdx = from; lineIdx < to; lineIdx++) {
editor.setGutterMarker(
lineIdx,
'jp-PullRequestCommentDecoration',
PlainTextDiff.makeCommentDecoration()
);
}
}

protected updateOriginalView(
editor: CodeMirror.Editor,
from: number,
to: number
): void {
// Add comments
this._props.threads
.filter(
thread =>
thread.originalLine !== null &&
from < thread.originalLine &&
thread.originalLine <= to
)
.forEach(thread => {
editor.addLineWidget(
thread.originalLine - 1,
PlainTextDiff.makeThreadWidget(thread, this._props.renderMime)
);
});
}

protected updateView(
editor: CodeMirror.Editor,
from: number,
to: number
): void {
// Add comment gutters
PlainTextDiff.setCommentGutter(editor, from, to);
// Add comments
this._props.threads
.filter(
thread =>
thread.line !== null && from < thread.line && thread.line <= to
)
.forEach(thread => {
editor.addLineWidget(
thread.line - 1,
PlainTextDiff.makeThreadWidget(thread, this._props.renderMime)
);
const div = PlainTextDiff.makeCommentDecoration();
div.addEventListener('click', () => {
this.createThread(editor, lineIdx, side);
});
editor.setGutterMarker(lineIdx, 'jp-PullRequestCommentDecoration', div);
}
}

/**
Expand Down Expand Up @@ -155,21 +101,77 @@ export class PlainTextDiff extends Widget {
// @ts-ignore
const { from, to } = this._mergeView.left.orig.getViewport();
// @ts-ignore
this.updateOriginalView(this._mergeView.left.orig, from, to);
this.updateView(this._mergeView.left.orig, from, to, 'originalLine');
// @ts-ignore
this._mergeView.left.orig.on(
'viewportChange',
this.updateOriginalView.bind(this)
(editor: CodeMirror.Editor, from: number, to: number) => {
this.updateView(editor, from, to, 'originalLine');
}
);
}

{
const { from, to } = this._mergeView.editor().getViewport();
this.updateView(this._mergeView.editor(), from, to);
this._mergeView.editor().on('viewportChange', this.updateView.bind(this));
this.updateView(this._mergeView.editor(), from, to, 'line');
this._mergeView
.editor()
.on(
'viewportChange',
(editor: CodeMirror.Editor, from: number, to: number) => {
this.updateView(editor, from, to, 'line');
}
);
}
}

protected createThread(
editor: CodeMirror.Editor,
lineNo: number,
side: 'line' | 'originalLine'
): void {
const thread: IThread = {
comments: new Array<IComment>(),
pullRequestId: this._props.prId,
filename: this._props.filename
};
thread[side] = lineNo + 1;
editor.addLineWidget(lineNo, this.makeThreadWidget(thread));
}

protected makeThreadWidget(thread: IThread): HTMLElement {
const widget = new CommentThread({
renderMime: this._props.renderMime,
thread,
handleRemove: () => {
const threadIndex = this._props.threads.findIndex(
thread => thread.id === thread.id
);
this._props.threads.splice(threadIndex, 1);
}
});
return widget.node;
}

protected updateView(
editor: CodeMirror.Editor,
from: number,
to: number,
side: 'line' | 'originalLine'
): void {
// Add comment gutters
this.setCommentGutter(editor, from, to, side);
// Add comments
this._props.threads
.filter(
thread =>
thread[side] !== null && from < thread[side] && thread[side] <= to
)
.forEach(thread => {
editor.addLineWidget(thread[side] - 1, this.makeThreadWidget(thread));
});
}

protected _mergeView: MergeView.MergeViewEditor;
protected _props: IDiffOptions;
}
6 changes: 3 additions & 3 deletions src/tokens.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ export interface INewComment {
export interface IThread {
comments: IComment[];
filename?: string;
id: string | number;
id?: string | number;
// commitId: string;
line: number;
originalLine: number;
line?: number;
originalLine?: number;
pullRequestId: string;
}

Expand Down

0 comments on commit 19dd4bc

Please sign in to comment.