From 090d8ac9b07a136b3525c4dff1b33796e2deeede Mon Sep 17 00:00:00 2001
From: keisuke-umezawa <keisuke.umezawa@gmail.com>
Date: Tue, 24 Jan 2023 10:51:00 +0900
Subject: [PATCH] Follow review comments

Fix flake8

Fix missing commits
---
 optuna_dashboard/_app.py                      | 24 +++++++++----------
 optuna_dashboard/ts/action.ts                 |  4 ++--
 optuna_dashboard/ts/apiClient.ts              |  3 +--
 optuna_dashboard/ts/components/TrialTable.tsx | 15 ++++++++++--
 4 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py
index 04a41d29..6980e16b 100644
--- a/optuna_dashboard/_app.py
+++ b/optuna_dashboard/_app.py
@@ -412,9 +412,9 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle:
         response.status = 204  # No content
         return {}
 
-    @app.post("/api/studies/<study_id:int>/<trial_id:int>/tell")
+    @app.post("/api/studies/<trial_id:int>/tell")
     @json_api_view
-    def tell_trial(study_id: int, trial_id: int) -> BottleViewReturn:
+    def tell_trial(trial_id: int) -> BottleViewReturn:
         s = request.json.get("state", None)
         vs = request.json.get("values", None)
 
@@ -431,19 +431,19 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle:
         }
         if s not in str_to_state:
             response.status = 400  # Bad request
-            return {"reason": f"You passed {s} as a state, which is not defined in Optuna."}
+            return {
+                "reason": f"You passed {s} as a state, but only 'Complete', 'Pruned' and 'Fail'"
+                "are acceptable states."
+            }
+        if s == "Complete" and values in None:
+            response.status = 400  # Bad request
+            return {
+                "reason": "When you passed 'Complete' as a state, you also need to specify values."
+            }
         state = str_to_state[s]
 
         try:
-            study_name = storage.get_study_name_from_id(study_id)
-        except KeyError:
-            response.status = 404  # Not found
-            return {"reason": f"study_id={study_id} is not found"}
-
-        study = optuna.load_study(storage=storage, study_name=study_name)
-
-        try:
-            study._storage.set_trial_state_values(trial_id, state, values)
+            storage.set_trial_state_values(trial_id, state, values)
         except Exception as e:
             response.status = 400  # Bad request
             return {"reason": e.args}
diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts
index 6a2993f8..4eb70203 100644
--- a/optuna_dashboard/ts/action.ts
+++ b/optuna_dashboard/ts/action.ts
@@ -295,7 +295,7 @@ export const actionCreator = () => {
       values === undefined
         ? `id=${trialId}, state=${state}`
         : `id=${trialId}, state=${state}, values=${values}`
-    return tellTrialAPI(studyId, trialId, state, values)
+    return tellTrialAPI(trialId, state, values)
       .then(() => {
         const index = studyDetails[studyId].trials.findIndex(
           (t) => t.trial_id === trialId
@@ -306,7 +306,7 @@ export const actionCreator = () => {
           })
           return
         }
-        setTrialState(studyId, index, state, values?.map(Number))
+        setTrialState(studyId, index, state, values)
         enqueueSnackbar(`Success to update trial (${message})`, {
           variant: "success",
         })
diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts
index f8db1eae..f5985bf7 100644
--- a/optuna_dashboard/ts/apiClient.ts
+++ b/optuna_dashboard/ts/apiClient.ts
@@ -218,7 +218,6 @@ export const saveTrialNoteAPI = (
 }
 
 export const tellTrialAPI = (
-  studyId: number,
   trialId: number,
   state: TrialStateFinished,
   values?: number[]
@@ -229,7 +228,7 @@ export const tellTrialAPI = (
   }
 
   return axiosInstance
-    .post<void>(`/api/studies/${studyId}/${trialId}/tell`, req)
+    .post<void>(`/api/studies/${trialId}/tell`, req)
     .then((res) => {
       return
     })
diff --git a/optuna_dashboard/ts/components/TrialTable.tsx b/optuna_dashboard/ts/components/TrialTable.tsx
index 5a0e6089..6616af7f 100644
--- a/optuna_dashboard/ts/components/TrialTable.tsx
+++ b/optuna_dashboard/ts/components/TrialTable.tsx
@@ -281,17 +281,28 @@ export const TrialTable: FC<{
       if (objectiveFormRefs === undefined) {
         return
       }
+      if (studyDetail === null) {
+        return
+      }
 
       e.preventDefault()
-      const studyId = (studyDetail as StudyDetail).id
+      const studyId = studyDetail.id
       const trialId = trials[index].trial_id
       const objectiveValues = objectiveFormRefs.map((ref) =>
         ref.current ? Number(ref.current.value) : NaN
       )
+      if (objectiveValues.includes(NaN)) {
+        return
+      }
+
       action.tellTrial(studyId, trialId, "Complete", objectiveValues)
     }
+
     const handleFailTrial = (e: MouseEvent<HTMLButtonElement>): void => {
-      const studyId = (studyDetail as StudyDetail).id
+      if (studyDetail === null) {
+        return
+      }
+      const studyId = studyDetail.id
       const trialId = trials[index].trial_id
       action.tellTrial(studyId, trialId, "Fail")
     }