Suggest run name based on the pipeline version used to create run (#2731

)

* Suggest run name

* refactor to combine similiar code block

* Tests

* format

* Add handling for a case which we shouldn't run into now but just for
robustness: we arrive at newrun page with version id but no pipeline id

* Attach random number to suggested run name; remove a code block that is
currently not used

* format

* address comments

* format

* refactor spy func

* refactor random string

frontend/src/pages/NewRun.test.tsx

@@ -218,6 +218,20 @@ describe('NewRun', () => {
     expect(tree.state()).toHaveProperty('runName', 'run name');
   });
 
+  it('reports validation error when missing the run name', async () => {
+    const props = generateProps();
+    props.location.search = `?${QUERY_PARAMS.pipelineId}=${MOCK_PIPELINE.id}&${
+      QUERY_PARAMS.pipelineVersionId
+    }=${MOCK_PIPELINE.default_version!.id}`;
+
+    tree = shallow(<TestNewRun {...props} />);
+    await TestUtils.flushPromises();
+
+    (tree.instance() as TestNewRun).handleChange('runName')({ target: { value: null } });
+
+    expect(tree.state()).toHaveProperty('errorMessage', 'Run name is required');
+  });
+
   it('allows updating the run description', async () => {
     tree = shallow(<TestNewRun {...(generateProps() as any)} />);
     await TestUtils.flushPromises();
@@ -353,6 +367,9 @@ describe('NewRun', () => {
   });
 
   it('fetches the associated pipeline if one is present in the query params', async () => {
+    const randomSpy = jest.spyOn(Math, 'random');
+    randomSpy.mockImplementation(() => 0.5);
+
     const props = generateProps();
     props.location.search = `?${QUERY_PARAMS.pipelineId}=${MOCK_PIPELINE.id}&${
       QUERY_PARAMS.pipelineVersionId
@@ -364,8 +381,10 @@ describe('NewRun', () => {
     expect(tree.state()).toHaveProperty('pipeline', MOCK_PIPELINE);
     expect(tree.state()).toHaveProperty('pipelineName', MOCK_PIPELINE.name);
     expect(tree.state()).toHaveProperty('pipelineVersion', MOCK_PIPELINE_VERSION);
-    expect(tree.state()).toHaveProperty('errorMessage', 'Run name is required');
+    expect((tree.state() as any).runName).toMatch(/Run of original mock pipeline version name/);
     expect(tree).toMatchSnapshot();
+
+    randomSpy.mockRestore();
   });
 
   it('shows a page error if getPipeline fails', async () => {
@@ -1052,6 +1071,8 @@ describe('NewRun', () => {
         `&${QUERY_PARAMS.pipelineVersionId}=${MOCK_PIPELINE_VERSION.id}`;
 
       tree = shallow(<TestNewRun {...props} />);
+      await TestUtils.flushPromises();
+
       (tree.instance() as TestNewRun).handleChange('runName')({
         target: { value: 'test run name' },
       });
@@ -1386,6 +1407,8 @@ describe('NewRun', () => {
         `&${QUERY_PARAMS.pipelineVersionId}=${MOCK_PIPELINE_VERSION.id}`;
 
       tree = shallow(<TestNewRun {...props} />);
+      await TestUtils.flushPromises();
+
       (tree.instance() as TestNewRun).handleChange('runName')({
         target: { value: 'test run name' },
       });
@@ -1435,6 +1458,8 @@ describe('NewRun', () => {
 
       tree = TestUtils.mountWithRouter(<TestNewRun {...props} />);
       const instance = tree.instance() as TestNewRun;
+      await TestUtils.flushPromises();
+
       instance.handleChange('runName')({ target: { value: 'test run name' } });
       instance.handleChange('description')({ target: { value: 'test run description' } });
       await TestUtils.flushPromises();

frontend/src/pages/NewRun.tsx

@@ -617,6 +617,38 @@ class NewRun extends Page<{}, NewRunState> {
             pipeline,
             pipelineName: (pipeline && pipeline.name) || '',
           });
+          const possiblePipelineVersionId =
+            urlParser.get(QUERY_PARAMS.pipelineVersionId) ||
+            (pipeline.default_version && pipeline.default_version.id);
+          if (possiblePipelineVersionId) {
+            try {
+              const pipelineVersion = await Apis.pipelineServiceApi.getPipelineVersion(
+                possiblePipelineVersionId,
+              );
+              this.setStateSafe({
+                parameters: pipelineVersion.parameters || [],
+                pipelineVersion,
+                pipelineVersionName: (pipelineVersion && pipelineVersion.name) || '',
+                runName: this._getRunNameFromPipelineVersion(
+                  (pipelineVersion && pipelineVersion.name) || '',
+                ),
+              });
+            } catch (err) {
+              urlParser.clear(QUERY_PARAMS.pipelineVersionId);
+              await this.showPageError(
+                `Error: failed to retrieve pipeline version: ${possiblePipelineVersionId}.`,
+                err,
+              );
+              logger.error(
+                `Failed to retrieve pipeline version: ${possiblePipelineVersionId}`,
+                err,
+              );
+            }
+          } else {
+            this.setStateSafe({
+              runName: this._getRunNameFromPipelineVersion((pipeline && pipeline.name) || ''),
+            });
+          }
         } catch (err) {
           urlParser.clear(QUERY_PARAMS.pipelineId);
           await this.showPageError(
@@ -626,26 +658,6 @@ class NewRun extends Page<{}, NewRunState> {
           logger.error(`Failed to retrieve pipeline: ${possiblePipelineId}`, err);
         }
       }
-      const possiblePipelineVersionId = urlParser.get(QUERY_PARAMS.pipelineVersionId);
-      if (possiblePipelineVersionId) {
-        try {
-          const pipelineVersion = await Apis.pipelineServiceApi.getPipelineVersion(
-            possiblePipelineVersionId,
-          );
-          this.setStateSafe({
-            parameters: pipelineVersion.parameters || [],
-            pipelineVersion,
-            pipelineVersionName: (pipelineVersion && pipelineVersion.name) || '',
-          });
-        } catch (err) {
-          urlParser.clear(QUERY_PARAMS.pipelineVersionId);
-          await this.showPageError(
-            `Error: failed to retrieve pipeline version: ${possiblePipelineVersionId}.`,
-            err,
-          );
-          logger.error(`Failed to retrieve pipeline version: ${possiblePipelineVersionId}`, err);
-        }
-      }
     }
 
     let experiment: ApiExperiment | undefined;
@@ -1045,6 +1057,24 @@ class NewRun extends Page<{}, NewRunState> {
     }
   }
 
+  private _generateRandomString(length: number): string {
+    let d = 0;
+    function randomChar(): string {
+      const r = Math.trunc((d + Math.random() * 16) % 16);
+      d = Math.floor(d / 16);
+      return r.toString(16);
+    }
+    let str = '';
+    for (let i = 0; i < length; ++i) {
+      str += randomChar();
+    }
+    return str;
+  }
+
+  private _getRunNameFromPipelineVersion(pipelineVersionName: string): string {
+    return 'Run of ' + pipelineVersionName + ' (' + this._generateRandomString(5) + ')';
+  }
+
   private _validate(): void {
     // Validate state
     const { pipelineVersion, workflowFromRun, maxConcurrentRuns, runName, trigger } = this.state;

frontend/src/pages/__snapshots__/NewRun.test.tsx.snap

@@ -2923,7 +2923,7 @@ exports[`NewRun fetches the associated pipeline if one is present in the query p
       label="Run name"
       onChange={[Function]}
       required={true}
-      value=""
+      value="Run of original mock pipeline version name (88888)"
       variant="outlined"
     />
     <Component
@@ -3006,7 +3006,7 @@ exports[`NewRun fetches the associated pipeline if one is present in the query p
       <BusyButton
         busy={false}
         className="buttonAction"
-        disabled={true}
+        disabled={false}
         id="startNewRunBtn"
         onClick={[Function]}
         title="Start"
@@ -3024,9 +3024,7 @@ exports[`NewRun fetches the associated pipeline if one is present in the query p
             "color": "red",
           }
         }
-      >
-        Run name is required
-      </div>
+      />
     </div>
   </div>
 </div>