Refactor diff using new @jupyterlab/git version (#39)

* Use jlab/git endpoint for nb diff * Refactor diff logic with new jlab-git version * Clean style * Homogeneous diff tab title with jlab-git
jupyterlab · Apr 5, 2021 · caba62f · caba62f
1 parent 3dcc532
commit caba62f
Show file tree

Hide file tree

Showing 15 changed files with 132 additions and 627 deletions.
diff --git a/jupyterlab_pullrequests/handlers.py b/jupyterlab_pullrequests/handlers.py
@@ -152,33 +152,6 @@ async def post(self):
         self.finish(json.dumps(result))
 
 
-class PullRequestsFileNBDiffHandler(PullRequestsAPIHandler):
-    """
-    Returns nbdime diff of given notebook base content and remote content
-    """
-
-    @tornado.web.authenticated
-    async def post(self):
-        data = get_body_value(self)
-        try:
-            prev_content = data["previousContent"]
-            curr_content = data["currentContent"]
-        except KeyError as e:
-            self._jp_log.error(f"Missing key in POST request.", exc_info=e)
-            raise tornado.web.HTTPError(
-                status_code=HTTPStatus.BAD_REQUEST, reason=f"Missing POST key: {e}"
-            )
-        try:
-            content = await self._manager.get_file_nbdiff(prev_content, curr_content)
-        except Exception as e:
-            self._jp_log.error(f"Error computing notebook diff.", exc_info=e)
-            raise tornado.web.HTTPError(
-                status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
-                reason=f"Error diffing content: {e}.",
-            ) from e
-        self.finish(json.dumps(content))
-
-
 # -----------------------------------------------------------------------------
 # Handler utilities
 # -----------------------------------------------------------------------------
@@ -222,7 +195,6 @@ def get_body_value(handler):
     ("prs/files", ListPullRequestsFilesHandler),
     ("files/content", PullRequestsFileContentHandler),
     ("files/comments", PullRequestsFileCommentsHandler),
-    ("files/nbdiff", PullRequestsFileNBDiffHandler),
 ]
 
 

diff --git a/jupyterlab_pullrequests/managers/manager.py b/jupyterlab_pullrequests/managers/manager.py
@@ -6,7 +6,6 @@
 
 import nbformat
 import tornado
-from nbdime import diff_notebooks
 from notebook.utils import url_path_join
 
 from .._version import __version__
@@ -62,32 +61,6 @@ async def get_file_diff(self, pr_id: str, filename: str) -> dict:
         """
         raise NotImplementedError()
 
-    async def get_file_nbdiff(
-        self, prev_content: str, curr_content: str
-    ) -> Dict[str, str]:
-        """Compute the diff between two notebooks.
-
-        Args:
-            prev_content: Notebook previous content
-            curr_content: Notebook current content
-        Returns:
-            {"base": Dict, "diff": Dict}
-        """
-
-        def read_notebook(content):
-            if not content:
-                return nbformat.v4.new_notebook()
-            return nbformat.reads(content, as_version=4)
-
-        current_loop = tornado.ioloop.IOLoop.current()
-        prev_nb = await current_loop.run_in_executor(None, read_notebook, prev_content)
-        curr_nb = await current_loop.run_in_executor(None, read_notebook, curr_content)
-        thediff = await current_loop.run_in_executor(
-            None, diff_notebooks, prev_nb, curr_nb
-        )
-
-        return {"base": prev_nb, "diff": thediff}
-
     @abc.abstractmethod
     async def get_threads(
         self, pr_id: str, filename: Optional[str] = None

diff --git a/jupyterlab_pullrequests/tests/sample_responses/github/ipynb_base.json b/jupyterlab_pullrequests/tests/sample_responses/github/ipynb_base.json
diff --git a/jupyterlab_pullrequests/tests/sample_responses/github/ipynb_nbdiff.json b/jupyterlab_pullrequests/tests/sample_responses/github/ipynb_nbdiff.json
diff --git a/jupyterlab_pullrequests/tests/sample_responses/github/ipynb_remote.json b/jupyterlab_pullrequests/tests/sample_responses/github/ipynb_remote.json
diff --git a/jupyterlab_pullrequests/tests/test_handlers_integration.py b/jupyterlab_pullrequests/tests/test_handlers_integration.py
@@ -248,39 +248,3 @@ def test_id_invalid(self):
         )
         self.assertEqual(response.code, 400)
         self.assertIn("Invalid response", response.reason)
-
-
-# Test get PR comments
-class TestPostPullRequestsNBDiffHandler(TestPullRequest):
-
-    # Test empty body
-    def test_body_empty(self):
-        response = self.fetch("/pullrequests/files/nbdiff", method="POST", body="")
-        self.assertEqual(response.code, 400)
-        self.assertIn("Invalid POST body", response.reason)
-
-    # Test invalid body JSON
-    def test_body_invalid(self):
-        response = self.fetch("/pullrequests/files/nbdiff", method="POST", body="{)")
-        self.assertEqual(response.code, 400)
-        self.assertIn("Invalid POST body", response.reason)
-
-    # Test invalid body JSON
-    def test_body_missingkey(self):
-        response = self.fetch(
-            "/pullrequests/files/nbdiff",
-            method="POST",
-            body='{"body": 123, "currentContent": "test"}',
-        )
-        self.assertEqual(response.code, 400)
-        self.assertIn("Missing POST key", response.reason)
-
-    # Test invalid body JSON
-    def test_body_invalid_(self):
-        response = self.fetch(
-            "/pullrequests/files/nbdiff",
-            method="POST",
-            body='{"previousContent": "bad", "currentContent": "bad"}',
-        )
-        self.assertEqual(response.code, 500)
-        self.assertIn("Error diffing content", response.reason)
diff --git a/jupyterlab_pullrequests/tests/test_manager.py b/jupyterlab_pullrequests/tests/test_manager.py
@@ -18,24 +18,6 @@ def read_sample_response(filename):
     )
 
 
-@pytest.mark.asyncio
-async def test_GitHubManager_get_file_nbdiff():
-    manager = GitHubManager(access_token="valid")
-    prev_content = (
-        HERE / "sample_responses" / "github" / "ipynb_base.json"
-    ).read_text()
-    curr_content = (
-        HERE / "sample_responses" / "github" / "ipynb_remote.json"
-    ).read_text()
-
-    result = await manager.get_file_nbdiff(prev_content, curr_content)
-
-    expected_result = json.loads(
-        (HERE / "sample_responses" / "github" / "ipynb_nbdiff.json").read_text()
-    )
-    assert result == expected_result
-
-
 @pytest.mark.asyncio
 @patch("tornado.httpclient.AsyncHTTPClient.fetch", new_callable=AsyncMock)
 async def test_GitHubManager_call_provider_bad_gitresponse(mock_fetch):

diff --git a/package.json b/package.json
@@ -49,7 +49,7 @@
     "@jupyterlab/coreutils": "^4.0.0",
     "@jupyterlab/docregistry": "^2.0.0",
     "@jupyterlab/filebrowser": "^2.0.0",
-    "@jupyterlab/git": "0.21.0 - 0.30.0",
+    "@jupyterlab/git": "0.24.0 - 0.30.0",
     "@jupyterlab/mainmenu": "^2.0.0",
     "@jupyterlab/nbformat": "^2.0.0",
     "@jupyterlab/rendermime": "^2.0.0",