Create Edit flow for Upload Jar Form #8458

sahil143 · 2021-03-24T15:59:21Z

Fixes: https://issues.redhat.com/browse/ODC-5466

This PR adds the flow to edit resources created from UploadJarForm.

Screen shots / Gifs for design review:

Unit test coverage report:

Test setup:

Browser conformance:

Chrome
Firefox
Safari
Edge

invincibleJai · 2021-03-26T04:23:21Z

frontend/packages/dev-console/src/components/edit-application/EditApplication.tsx

+  const pageHeading = getPageHeading(
+    _.get(initialValues, 'build.strategy', ''),
+    _.get(initialValues, 'build.source.type', undefined),
+  );
+  const validationSchema = getValidationSchema(
+    _.get(initialValues, 'build.strategy', ''),
+    _.get(initialValues, 'build.source.type', undefined),
+  );


nit: we can have strategy and source.type as const and use in getPageHeading, getValidationSchema

invincibleJai · 2021-03-26T04:30:35Z

...tend/packages/dev-console/src/components/edit-application/__tests__/edit-application-data.ts

@@ -249,6 +249,7 @@ export const appResources: AppResources = {
        annotations: {
          'app.openshift.io/vcs-ref': 'master',
          'app.openshift.io/vcs-uri': 'https://github.com/divyanshiGupta/nationalparks-py',
+          jarFileName: 'demo-app.jar',


all three of these annotations will be not be used together right?

No, jarFileName will be used to form Jar initial values and the other is for git import.

invincibleJai · 2021-03-26T04:32:40Z

...ackages/dev-console/src/components/edit-application/__tests__/edit-application-utils.spec.ts

+    expect(getFileUploadValues(editAppResource.data, buildConfig.data)).toEqual({
+      fileUpload: { name: 'demo-app.jar', value: '', javaArgs: undefined },
+      runtimeIcon: 'python',
+    });


will it make sense to update buildConfig mock data to have java as runtime as Upload Jar will always have that

This is just mock data. I don't see any difference using java or python here.

invincibleJai · 2021-03-26T04:35:41Z

frontend/packages/dev-console/src/components/import/jar/useUploadJarFormToast.ts

+import { BuildConfigModel, BuildModel } from '@console/internal/models';
+import { K8sResourceKind } from '@console/internal/module/k8s';
+import { history, resourcePathFromModel } from '@console/internal/components/utils';
+import { useTranslation } from 'react-i18next';


nit: sort imports

invincibleJai · 2021-03-26T04:38:21Z

frontend/packages/dev-console/src/components/edit-application/EditApplication.tsx

+  const renderForm = (formikProps: FormikProps<any>) => {
+    return (


nit: can use implicit return

invincibleJai · 2021-03-29T11:30:28Z

frontend/packages/dev-console/src/components/edit-application/edit-application-utils.ts

+const isFromJarUpload = (type: string): boolean => type === BuildSourceType.Binary;
+
+const getBuildSourceType = (buildConfig: K8sResourceKind): string =>
+  buildConfig?.spec?.source?.type ?? undefined;


if any value is not there in buildConfig?.spec?.source?.type then it'll return undefined only so does it make sense to return undefined explicitly as well?

invincibleJai · 2021-03-29T12:07:33Z

frontend/packages/dev-console/src/components/edit-application/edit-application-utils.ts

+  const fileName = buildConfig.metadata?.annotations?.jarFileName ?? '';
+  const javaArgs: string = resource.spec?.template?.spec?.containers
+    ?.find((container) => container.name === resourceName)
+    ?.env?.find((args) => args.name === 'JAVA_ARGS').value;


I think this will fail for a scenario where Env var name is not 'JAVA_ARGS' and will would acc values. Observed this when i added env say TEST: testwhile edit flow

invincibleJai · 2021-03-30T12:44:44Z

on edit flow, if I remove Optional Java commands i.e jai then as well the value is persisted

invincibleJai · 2021-03-31T12:52:45Z

/retest

debsmita1 · 2021-03-31T14:08:56Z

frontend/packages/dev-console/src/components/edit-application/edit-application-utils.ts

+
+export enum BuildSourceType {
+  Git = 'Git',
+  Binary = 'Binary',


how about if we move these to edit-application-types.ts

Other enums are also in this file. Not sure if it matters to move these to edit-application-types.ts.

debsmita1 · 2021-03-31T14:10:12Z

@sahil143 a test case is failing

verified the changes, it works fine

debsmita1 · 2021-03-31T16:37:58Z

/lgtm

invincibleJai · 2021-03-31T16:43:57Z

Verified works as expected for D/KSVC but for DC jar file name is not auto-populated on edit

invincibleJai · 2021-04-05T16:38:01Z

/retest

invincibleJai · 2021-04-05T16:52:30Z

/approve

Verified the changes, works as expected.

sahil143 · 2021-04-05T16:54:23Z

/retest

rottencandy

Tested locally, works as expected.
/lgtm

openshift-ci-robot · 2021-04-07T15:20:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: debsmita1, invincibleJai, rottencandy, sahil143

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/packages/dev-console/OWNERS~~ [invincibleJai]
~~frontend/packages/knative-plugin/OWNERS~~ [invincibleJai]
~~frontend/packages/topology/OWNERS~~ [invincibleJai]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot requested review from andrewballantyne and debsmita1 March 24, 2021 15:59

openshift-ci-robot added component/dev-console Related to dev-console component/knative Related to knative-plugin component/topology Related to topology labels Mar 24, 2021

sahil143 force-pushed the odc-5466 branch from 944dd0b to 6df14e6 Compare March 25, 2021 11:35

openshift-ci-robot added the kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated label Mar 25, 2021

invincibleJai reviewed Mar 26, 2021

View reviewed changes

sahil143 force-pushed the odc-5466 branch from 6df14e6 to 3a4bf4c Compare March 26, 2021 14:53

invincibleJai reviewed Mar 29, 2021

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 29, 2021

sahil143 force-pushed the odc-5466 branch from 3a4bf4c to 6480fb2 Compare March 30, 2021 06:39

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2021

sahil143 force-pushed the odc-5466 branch from 6480fb2 to da431b6 Compare March 30, 2021 12:06

sahil143 force-pushed the odc-5466 branch from da431b6 to c575aec Compare March 31, 2021 07:45

debsmita1 reviewed Mar 31, 2021

View reviewed changes

sahil143 force-pushed the odc-5466 branch from c575aec to 7c0b3ba Compare March 31, 2021 15:48

openshift-ci-robot assigned debsmita1 Mar 31, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 31, 2021

sahil143 force-pushed the odc-5466 branch from 7c0b3ba to 760e1bc Compare April 1, 2021 16:39

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 1, 2021

sahil143 force-pushed the odc-5466 branch from 760e1bc to 65bbb88 Compare April 5, 2021 09:34

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 5, 2021

sahil143 force-pushed the odc-5466 branch from 65bbb88 to 8c6c1b9 Compare April 5, 2021 17:14

Create Edit form for Upload Jar Form

0e110e2

sahil143 force-pushed the odc-5466 branch from 8c6c1b9 to 0e110e2 Compare April 6, 2021 10:01

rottencandy approved these changes Apr 7, 2021

View reviewed changes

openshift-ci-robot assigned rottencandy Apr 7, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 7, 2021

openshift-merge-robot merged commit 67f4303 into openshift:master Apr 8, 2021

spadgett added this to the v4.8 milestone Apr 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create Edit flow for Upload Jar Form #8458

Create Edit flow for Upload Jar Form #8458

sahil143 commented Mar 24, 2021 •

edited

Loading

invincibleJai Mar 26, 2021 •

edited

Loading

invincibleJai Mar 26, 2021

sahil143 Mar 26, 2021

invincibleJai Mar 26, 2021

sahil143 Mar 26, 2021

sahil143 Mar 26, 2021

invincibleJai Mar 26, 2021

invincibleJai Mar 26, 2021

invincibleJai Mar 29, 2021

invincibleJai Mar 29, 2021

invincibleJai commented Mar 30, 2021

invincibleJai commented Mar 31, 2021

debsmita1 Mar 31, 2021

sahil143 Mar 31, 2021

debsmita1 commented Mar 31, 2021

debsmita1 commented Mar 31, 2021

invincibleJai commented Mar 31, 2021

invincibleJai commented Apr 5, 2021

invincibleJai commented Apr 5, 2021

sahil143 commented Apr 5, 2021

rottencandy left a comment

openshift-ci-robot commented Apr 7, 2021

		const renderForm = (formikProps: FormikProps<any>) => {
		return (

Create Edit flow for Upload Jar Form #8458

Create Edit flow for Upload Jar Form #8458

Conversation

sahil143 commented Mar 24, 2021 • edited Loading

invincibleJai Mar 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

invincibleJai commented Mar 30, 2021

invincibleJai commented Mar 31, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

debsmita1 commented Mar 31, 2021

debsmita1 commented Mar 31, 2021

invincibleJai commented Mar 31, 2021

invincibleJai commented Apr 5, 2021

invincibleJai commented Apr 5, 2021

sahil143 commented Apr 5, 2021

rottencandy left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Apr 7, 2021

sahil143 commented Mar 24, 2021 •

edited

Loading

invincibleJai Mar 26, 2021 •

edited

Loading