[KED-2997] Create rename run and edit notes UI behavior (#665)

* add pencil/edit icon; create run-details modal; open/close of that modal working * add basic text area in modal * local creation of an Input component * create ui folder and add generic textarea and input components * use text-decoration for active state * modal gets dynamic data from selected run * run details modal tests * rename textarea to input and use that in modal; remove other input component * test written for input component * updates based on PR review * Rashida's PR reviews; hide edit run button when comparison view is on * fix typos; update general edit run modal behavior * add runDetails mutation * remove searchable text; add client to mutation so tests pass * don't format schema * Update backend schema * Fixed mypy error * fixed mypy2 * fixed pylint error * update to id from runId in graphql response * update run_id to id in response -2 * UpdateRunDetails returns Run * correct response from updateRunDetails mutation; reset mutation on error; use placeholder for notes field * fix run-selection anomoly * fix failing RunDetailsModal test * update mutation response * use the Modal component from the repo, not the QB UI one * fix failing RunDetailsModal test * remove working_directory from circleci * add working_directory to build_38 * use tmp folder instead of repo * revert and use arch * revert by removing working_directory from build_38 and test build again * clear input on trigger; remove arch from circle ci Co-authored-by: Rashida Kanchwala <[email protected]>
kedro-org · Jan 6, 2022 · 81c3bd6 · 81c3bd6
1 parent 7c9508d
commit 81c3bd6
Show file tree

Hide file tree

Showing 30 changed files with 627 additions and 131 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -39,7 +39,7 @@
     "snyk-test": "snyk test -prune-repeated-subdependencies"
   },
   "dependencies": {
-    "@apollo/client": "^3.5.5",
+    "@apollo/client": "^3.5.6",
     "@graphql-tools/schema": "7.1.5",
     "@material-ui/core": "^4.11.4",
     "@material-ui/icons": "^4.11.2",
@@ -138,4 +138,4 @@
     "not op_mini all"
   ],
   "snyk": true
-}
+}
diff --git a/package/kedro_viz/api/graphql.py b/package/kedro_viz/api/graphql.py
@@ -27,6 +27,7 @@ class JSONObject(dict):
         https://github.com/python/mypy/issues/2477
         """
 
+
 else:
     JSONObject = strawberry.scalar(
         NewType("JSONObject", dict),
@@ -46,7 +47,11 @@ def format_run(run_id: str, run_blob: Dict) -> Run:
     """
     session = data_access_manager.db_session
     git_data = run_blob.get("git")
-    user_details = session.query(UserRunDetailsModel).filter(UserRunDetailsModel.run_id == run_id).scalar()
+    user_details = (
+        session.query(UserRunDetailsModel)
+        .filter(UserRunDetailsModel.run_id == run_id)
+        .scalar()
+    )
     run = Run(
         id=ID(run_id),
         author="",
@@ -95,7 +100,9 @@ def get_all_runs() -> List[Run]:
     return runs
 
 
-def format_run_tracking_data(tracking_data: Dict, show_diff: Optional[bool] = False) -> JSONObject:
+def format_run_tracking_data(
+    tracking_data: Dict, show_diff: Optional[bool] = False
+) -> JSONObject:
     """Convert tracking data in the front-end format.
 
     Args:
@@ -140,7 +147,9 @@ def format_run_tracking_data(tracking_data: Dict, show_diff: Optional[bool] = Fa
 
     for run_id, run_tracking_data in tracking_data.items():
         for tracking_name, data in run_tracking_data.items():
-            formatted_tracking_data[tracking_name].append({"runId": run_id, "value": data})
+            formatted_tracking_data[tracking_name].append(
+                {"runId": run_id, "value": data}
+            )
     if not show_diff:
         for tracking_key, run_tracking_data in list(formatted_tracking_data.items()):
             if len(run_tracking_data) != len(tracking_data):
@@ -149,7 +158,9 @@ def format_run_tracking_data(tracking_data: Dict, show_diff: Optional[bool] = Fa
     return JSONObject(formatted_tracking_data)
 
 
-def get_run_tracking_data(run_ids: List[ID], show_diff: Optional[bool] = False) -> List[TrackingDataset]:
+def get_run_tracking_data(
+    run_ids: List[ID], show_diff: Optional[bool] = False
+) -> List[TrackingDataset]:
     # pylint: disable=protected-access,import-outside-toplevel
     """Get all tracking data for a list of runs. Tracking data contains the data from the
     tracking MetricsDataSet and JSONDataSet instances that have been logged
@@ -179,7 +190,9 @@ def get_run_tracking_data(run_ids: List[ID], show_diff: Optional[bool] = False)
             run_id = ID(run_id)
             file_path = dataset._get_versioned_path(str(run_id))
             if Path(file_path).is_file():
-                with dataset._fs.open(file_path, **dataset._fs_open_args_load) as fs_file:
+                with dataset._fs.open(
+                    file_path, **dataset._fs_open_args_load
+                ) as fs_file:
                     json_data = json.load(fs_file)
                     all_runs[run_id] = json_data
             else:
@@ -238,7 +251,9 @@ def run_metadata(self, run_ids: List[ID]) -> List[Run]:
         return get_runs(run_ids)
 
     @strawberry.field
-    def run_tracking_data(self, run_ids: List[ID], show_diff: Optional[bool] = False) -> List[TrackingDataset]:
+    def run_tracking_data(
+        self, run_ids: List[ID], show_diff: Optional[bool] = False
+    ) -> List[TrackingDataset]:
         """Query to get data for specific runs from the session store"""
         return get_run_tracking_data(run_ids, show_diff)
 
@@ -261,18 +276,20 @@ class RunInput:
 class UpdateRunDetailsSuccess:
     """Response type for sucessful update of runs"""
 
-    run_details: JSONObject
+    run: Run
 
 
 @strawberry.type
 class UpdateRunDetailsFailure:
     """Response type for failed update of runs"""
 
-    run_id: ID
+    id: ID
     error_message: str
 
 
-Response = strawberry.union("UpdateRunDetailsResponse", (UpdateRunDetailsSuccess, UpdateRunDetailsFailure))
+Response = strawberry.union(
+    "UpdateRunDetailsResponse", (UpdateRunDetailsSuccess, UpdateRunDetailsFailure)
+)
 
 
 @strawberry.type
@@ -284,34 +301,50 @@ def update_run_details(self, run_id: ID, run_input: RunInput) -> Response:
         """Updates run details based on run inputs provided by user"""
         runs = get_runs([run_id])
         if not runs:
-            return UpdateRunDetailsFailure(run_id=run_id, error_message=f"Given run_id: {run_id} doesn't exist")
+            return UpdateRunDetailsFailure(
+                id=run_id, error_message=f"Given run_id: {run_id} doesn't exist"
+            )
         existing_run = runs[0]
-        updated_user_run_details = {
-            "run_id": run_id,
-            "bookmark": run_input.bookmark if run_input.bookmark is not None else existing_run.bookmark,
-            "notes": run_input.notes if run_input.notes is not None else existing_run.notes,
-        }
-
+        new_run = existing_run
         # if user doesn't provide a new title, use the old title.
         if run_input.title is None:
-            updated_user_run_details["title"] = existing_run.title
+            new_run.title = existing_run.title
         # if user provides an empty title, we assume they want to revert to the old timestamp title
         elif run_input.title.strip() == "":
-            updated_user_run_details["title"] = existing_run.timestamp
+            new_run.title = existing_run.timestamp
         else:
-            updated_user_run_details["title"] = run_input.title
+            new_run.title = run_input.title
+
+        new_run.bookmark = (
+            run_input.bookmark
+            if run_input.bookmark is not None
+            else existing_run.bookmark
+        )
+
+        new_run.notes = (
+            run_input.notes if run_input.notes is not None else existing_run.notes
+        )
+
+        updated_user_run_details = {
+            "run_id": run_id,
+            "title": new_run.title,
+            "bookmark": new_run.bookmark,
+            "notes": new_run.notes,
+        }
 
         session = data_access_manager.db_session
-        user_run_details = session.query(UserRunDetailsModel).filter(UserRunDetailsModel.run_id == run_id).first()
+        user_run_details = (
+            session.query(UserRunDetailsModel)
+            .filter(UserRunDetailsModel.run_id == run_id)
+            .first()
+        )
         if not user_run_details:
-            session.add(UserRunDetailsModel(**updated_user_run_details)) # type: ignore
+            session.add(UserRunDetailsModel(**updated_user_run_details))  # type: ignore
         else:
             for key, value in updated_user_run_details.items():
                 setattr(user_run_details, key, value)
         session.commit()
-        return UpdateRunDetailsSuccess(
-            run_details=JSONObject(updated_user_run_details)
-        )
+        return UpdateRunDetailsSuccess(new_run)
 
 
 schema = strawberry.Schema(query=Query, mutation=Mutation)

diff --git a/package/kedro_viz/integrations/kedro/sqlite_store.py b/package/kedro_viz/integrations/kedro/sqlite_store.py
@@ -51,7 +51,7 @@ def to_json(self) -> str:
 
                     branch = git.Repo(search_parent_directories=True).active_branch
                     value["branch"] = branch.name
-                except ImportError as exc: # pragma: no cover
+                except ImportError as exc:  # pragma: no cover
                     logger.warning("%s:%s", exc.__class__.__name__, exc.msg)
 
             if _is_json_serializable(value):

diff --git a/package/tests/test_api/test_graphql.py b/package/tests/test_api/test_graphql.py
@@ -534,23 +534,43 @@ def test_graphql_runs_tracking_data_endpoint(
 
 
 class TestGraphQLMutation:
-    @pytest.mark.parametrize("bookmark,notes,title", [
-        (False, "new notes", "new title", ),
-        (True, "new notes", "new title"),
-        (True, "", ""),
-    ])
+    @pytest.mark.parametrize(
+        "bookmark,notes,title",
+        [
+            (
+                False,
+                "new notes",
+                "new title",
+            ),
+            (True, "new notes", "new title"),
+            (True, "", ""),
+        ],
+    )
     def test_update_user_details_success(
-        self, bookmark, notes, title, client, save_version, example_runs, example_db_dataset, mocker
+        self,
+        bookmark,
+        notes,
+        title,
+        client,
+        save_version,
+        example_runs,
+        example_db_dataset,
+        mocker,
     ):
         query = f"""
             mutation updateRun {{
               updateRunDetails(runId: "{save_version}", runInput: {{bookmark: {str(bookmark).lower()}, notes: "{notes}", title: "{title}"}}) {{
                 __typename
                 ... on UpdateRunDetailsSuccess {{
-                  runDetails
+                 run {{
+                    id
+                    title
+                    bookmark
+                    notes
+                    }}
                 }}
                 ... on UpdateRunDetailsFailure {{
-                  runId
+                  id
                   errorMessage
                 }}
               }}
@@ -563,13 +583,13 @@ def test_update_user_details_success(
         ) as mock_session:
             mock_session.return_value = example_db_dataset
             mocker.patch("kedro_viz.api.graphql.get_runs").return_value = example_runs
-            response = client.post("/graphql", json={"query": query}) 
+            response = client.post("/graphql", json={"query": query})
             assert response.json() == {
                 "data": {
                     "updateRunDetails": {
                         "__typename": "UpdateRunDetailsSuccess",
-                        "runDetails": {
-                            "run_id": save_version,
+                        "run": {
+                            "id": save_version,
                             "bookmark": bookmark,
                             "title": title if title != "" else save_version,
                             "notes": notes,
@@ -586,10 +606,15 @@ def test_update_user_details_only_bookmark(
               updateRunDetails(runId: "{save_version}", runInput: {{bookmark: true}}) {{
                 __typename
                 ... on UpdateRunDetailsSuccess {{
-                  runDetails
+                  run {{
+                    id
+                    title
+                    bookmark
+                    notes
+                    }}
                 }}
                 ... on UpdateRunDetailsFailure {{
-                  runId
+                  id
                   errorMessage
                 }}
               }}
@@ -602,13 +627,13 @@ def test_update_user_details_only_bookmark(
         ) as mock_session:
             mock_session.return_value = example_db_dataset
             mocker.patch("kedro_viz.api.graphql.get_runs").return_value = example_runs
-            response = client.post("/graphql", json={"query": query}) 
+            response = client.post("/graphql", json={"query": query})
             assert response.json() == {
                 "data": {
                     "updateRunDetails": {
                         "__typename": "UpdateRunDetailsSuccess",
-                        "runDetails": {
-                            "run_id": save_version,
+                        "run": {
+                            "id": save_version,
                             "bookmark": True,
                             "title": example_runs[0].title,
                             "notes": example_runs[0].notes,
@@ -617,16 +642,23 @@ def test_update_user_details_only_bookmark(
                 }
             }
 
-    def test_update_user_details_should_add_when_it_does_not_exist(self, save_version, client, example_runs, mocker):
+    def test_update_user_details_should_add_when_it_does_not_exist(
+        self, save_version, client, example_runs, mocker
+    ):
         query = f"""
             mutation updateRun {{
               updateRunDetails(runId: "{save_version}", runInput: {{bookmark: true}}) {{
                 __typename
                 ... on UpdateRunDetailsSuccess {{
-                  runDetails
+                run {{
+                    id
+                    title
+                    bookmark
+                    notes
+                    }}
                 }}
                 ... on UpdateRunDetailsFailure {{
-                  runId
+                  id
                   errorMessage
                 }}
               }}
@@ -637,15 +669,17 @@ def test_update_user_details_should_add_when_it_does_not_exist(self, save_versio
             "kedro_viz.data_access.DataAccessManager.db_session",
             new_callable=PropertyMock,
         ) as mock_session:
-            mock_session.return_value.query.return_value.filter.return_value.first.return_value = None
+            mock_session.return_value.query.return_value.filter.return_value.first.return_value = (
+                None
+            )
             mocker.patch("kedro_viz.api.graphql.get_runs").return_value = example_runs
-            response = client.post("/graphql", json={"query": query}) 
+            response = client.post("/graphql", json={"query": query})
             assert response.json() == {
                 "data": {
                     "updateRunDetails": {
                         "__typename": "UpdateRunDetailsSuccess",
-                        "runDetails": {
-                            "run_id": save_version,
+                        "run": {
+                            "id": save_version,
                             "bookmark": True,
                             "title": example_runs[0].title,
                             "notes": example_runs[0].notes,
@@ -673,10 +707,15 @@ def test_update_user_details_fail(self, client, example_db_dataset, mocker):
                         {
                             __typename
                             ... on UpdateRunDetailsSuccess {
-                            runDetails
+                            run {
+                                id
+                                title
+                                notes
+                                bookmark
+                                }
                             }
                             ... on UpdateRunDetailsFailure {
-                            runId
+                            id
                             errorMessage
                             } }
                             }"""
@@ -686,7 +725,7 @@ def test_update_user_details_fail(self, client, example_db_dataset, mocker):
                 "data": {
                     "updateRunDetails": {
                         "__typename": "UpdateRunDetailsFailure",
-                        "runId": "2021-11-02T12.24.24.329Z",
+                        "id": "2021-11-02T12.24.24.329Z",
                         "errorMessage": "Given run_id: 2021-11-02T12.24.24.329Z doesn't exist",
                     }
                 }