fix: remove error text in continue trial modal

[keita-determined]

Description

ET-32

Test Plan

• Check if no error text in continue trial modal when error occurs, but show error toast

Commentary (optional)

Checklist

• [x] Changes have been manually QA'd
• [ ] User-facing API changes need the "User-facing API Change" label.
• [ ] Release notes should be added as a separate file under docs/release-notes/. See Release Note for details.
• [ ] Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

[netlify[bot]]

✅ Deploy Preview for determined-ui ready!

Name	Link
🔨 Latest commit	ba713db1423a19e3189d682a6ce7e6a6c70b5a81
🔍 Latest deploy log	https://app.netlify.com/sites/determined-ui/deploys/65ef7af303a8680008eabc18
😎 Deploy Preview	https://deploy-preview-8923--determined-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 65.00000% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 42.75%. Comparing base (18154f6) to head (ba713db). Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8923      +/-   ##
==========================================
- Coverage   47.49%   42.75%   -4.74%     
==========================================
  Files        1168      848     -320     
  Lines      176296   137031   -39265     
  Branches     2352     2353       +1     
==========================================
- Hits        83729    58592   -25137     
+ Misses      92409    78281   -14128     
  Partials      158      158              

Flag	Coverage Δ
harness	?
web	42.83% <65.00%> (+<0.01%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...bui/react/src/components/ExperimentCreateModal.tsx	77.46% <75.00%> (+0.11%)	:arrow_up:
...i/react/src/components/ExperimentContinueModal.tsx	50.48% <58.33%> (-0.20%)	:arrow_down:

... and 320 files with indirect coverage changes

[keita-determined]

will take a look

[keita-determined]

@johnkim-det i used Alert component instead of form because code view is not a part of form component. to check the code config format, it errors out without API call in the advanced mode. in the normal mode, it shows API errors in Alert component.

[johnkim-det]

I think maybe in grooming it was unclear that this was about error handling in Full Config mode, which is different from Simple Config mode. So we thought we could just remove Message (which was used for Full Config) since the form already had input validation (which was only true for Simple Config). Actually, we need to make sure both modes have some sort of inline error messages for non-API errors.

• Using Alert sounds good for Full Config errors -- I think the submit button should also be disabled if there's an error.
• I was actually referring to Simple Config mode in my previous comment, which is why I was referring to form validation. I don't think it's displaying the error messages or disabling the submit button correctly.

[keita-determined]

@johnkim-det parsing continue experiment request: invalid resource configuration: rpc error: code = InvalidArgument desc = resource pool compute-pool does not exist or is not available to workspace id 1 error comes from backend even though it sounds like yaml format issue from web. YAML check and other validations work fine in the simple/full config.

Disabled submit button in full config page when error occurs. in the simple config page already has the functionality to disable submit button.

[johnkim-det]

Issues I'm seeing:

• Simple Config modal does not validate the form initially, so the submit button is not disabled even though the default state does not have a value for Max batches.
• Simple Config shows red outline for errors, but should include the error message that says what the error is
• Full Config can display an error and still allow submit:

also FYI created https://hpe-aiatscale.atlassian.net/browse/ET-65

[keita-determined]

• Simple Config modal does not validate the form initially, so the submit button is not disabled even though the default state does not have a value for Max batches.

I made a change like this but not quite working well. Do you have suggested code?

diff --git a/webui/react/src/components/ExperimentContinueModal.tsx b/webui/react/src/components/ExperimentContinueModal.tsx
index d0d0c29b2..6c66c127f 100644
--- a/webui/react/src/components/ExperimentContinueModal.tsx
+++ b/webui/react/src/components/ExperimentContinueModal.tsx
@@ -11,7 +11,7 @@ import { Body } from 'hew/Typography';
 import { Loaded } from 'hew/utils/loadable';
 import yaml from 'js-yaml';
 import _ from 'lodash';
-import React, { useCallback, useEffect, useId, useState } from 'react';
+import React, { useCallback, useEffect, useId, useMemo, useState } from 'react';
 
 import { paths } from 'routes/utils';
 import { continueExperiment, createExperiment } from 'services/api';
@@ -92,10 +92,12 @@ const ExperimentContinueModalComponent = ({
   const [disabled, setDisabled] = useState<boolean>(true);
   const [originalConfig, setOriginalConfig] = useState(experiment.configRaw);
   const isReactivate = type === ContinueExperimentType.Reactivate;
+  const hideSimpleConfig = isReactivate && experiment.state !== RunState.Completed;
+  const modalIsInAdvancedMode = modalState.isAdvancedMode || hideSimpleConfig;
 
   useEffect(() => setOriginalConfig(experiment.configRaw), [experiment]);
 
-  const requiredFields = [EXPERIMENT_NAME, MAX_LENGTH];
+  const requiredFields = useMemo(() => [EXPERIMENT_NAME, MAX_LENGTH], []);
 
   const handleModalClose = () => {
     setModalState(DEFAULT_MODAL_STATE);
@@ -381,15 +383,19 @@ const ExperimentContinueModalComponent = ({
       };
       return _.isEqual(prev, newModalState) ? prev : newModalState;
     });
-    setDisabled(!experiment.name); // initial disabled state set here, gets updated later in handleFieldsChange
-  }, [experiment, trial, type, isReactivate, form]);
+    const missingRequiredFields =
+      !modalIsInAdvancedMode &&
+      Object.entries(form.getFieldsValue()).some(([key, value]) => {
+        return requiredFields.includes(key) && !value;
+      });
+    setDisabled(missingRequiredFields); // initial disabled state set here, gets updated later in handleFieldsChange
+    getMaxLengthValue(experiment.configRaw);
+  }, [experiment, trial, type, isReactivate, form, requiredFields, modalIsInAdvancedMode]);
 
   if (!experiment || (!isReactivate && !trial)) return <></>;
 
-  const hideSimpleConfig = isReactivate && experiment.state !== RunState.Completed;
-
   const maxLengthType = capitalizeWord(getMaxLengthType(modalState.config) || 'batches');
-  const modalIsInAdvancedMode = modalState.isAdvancedMode || hideSimpleConfig;
+
   return (
     <Modal
       cancel

• Simple Config shows red outline for errors, but should include the error message that says what the error is

This is Hew form component issue. Error messages are set for input components, but hew form doesnt show it like antd form after error occured made a ticket for it: https://hpe-aiatscale.atlassian.net/browse/ET-70

• Full Config can display an error and still allow submit:

How can I reproduce it? I dont see the case.

[johnkim-det]

I think you can just replace the setDisabled(!experiment.name); in useEffect with form.validateFields(). Looks like setDisabled(hasError || missingRequiredFields) in handleFieldsChange will get called after that .

As for the Full Config error, I think the problem is that it doesn't change disabled state from Simple Config, so it might also stay disabled if it was disabled in Simple Config. In the case I captured in my screenshot, changing drop_capabilities from valid to invalid doesn't update the button disabled state. Don't think it matters which experiment. I think it needs a setDisabled call in handleEditorChange (and probably also toggleMode for when the modal is first changed to Full Config mode).

Also would double-check that ExperimentCreateModal also works correctly, seems like it has a lot of similar code.

Thanks for looking into this -- I know it's more than the original ticket but I think it makes sense to address as much of this as possible here and create separate tickets as necessary.

[keita-determined]

i forgot about form.validateFields(). fixed the issue with it.

---

the submit button issue in the Full config should be fixed

https://github.com/determined-ai/determined/assets/109113616/109c081a-2646-4427-86bb-c0cbbbbf3319

---

applied changes in ExperimentCreateModal too.

Let me know if there's something to fix.