Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UI] Make sure form validation displays non-valid fields as red in all forms #7064

Conversation

rm3l
Copy link
Member

@rm3l rm3l commented Sep 1, 2023

What type of PR is this:
/area Web-UI

What does this PR do / why we need it:
This makes sure invalid fields are highlighted as red in all forms.

Add an Exec Command

image

Add an Image Command

image

Add an Apply Command

image

Add a Composite Command

image

Add a new container

image

image

Add a new image

image

Add a new resource

image

Add a new volume

image

Which issue(s) this PR fixes:
Fixes #7054

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@openshift-ci openshift-ci bot added area/Dev-UI Issues or PRs related to the odo dev Web UI, a.k.a Devfile Builder do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. labels Sep 1, 2023
@netlify
Copy link

netlify bot commented Sep 1, 2023

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 7055b6e
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/64f71aaa40cf3d0008a1f9af

@openshift-ci
Copy link

openshift-ci bot commented Sep 1, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@odo-robot
Copy link

odo-robot bot commented Sep 1, 2023

OpenShift Unauthenticated Tests on commit 00cd2e5 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 1, 2023

NoCluster Tests on commit 00cd2e5 finished successfully.
View logs: TXT HTML

@rm3l rm3l added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Sep 1, 2023
@odo-robot
Copy link

odo-robot bot commented Sep 1, 2023

Unit Tests on commit 00cd2e5 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 1, 2023

Validate Tests on commit 00cd2e5 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 1, 2023

Kubernetes Tests on commit 00cd2e5 finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 1, 2023

Windows Tests (OCP) on commit 00cd2e5 finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 1, 2023

OpenShift Tests on commit 00cd2e5 finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 1, 2023

Kubernetes Docs Tests on commit a658121 finished successfully.
View logs: TXT HTML

@rm3l rm3l force-pushed the 7054-ui-form-validation-should-display-non-valid-fields-as-red-in-all-forms branch 2 times, most recently from ccad415 to 4879b59 Compare September 4, 2023 15:43
@rm3l rm3l changed the title [WIP] [UI] Make sure form validation display non-valid fields as red in all forms [UI] Make sure form validation display non-valid fields as red in all forms Sep 4, 2023
@rm3l rm3l marked this pull request as ready for review September 4, 2023 15:44
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Sep 4, 2023
@rm3l rm3l requested a review from feloy September 4, 2023 15:44
@openshift-ci openshift-ci bot requested a review from kadel September 4, 2023 15:44
@rm3l rm3l changed the title [UI] Make sure form validation display non-valid fields as red in all forms [UI] Make sure form validation displays non-valid fields as red in all forms Sep 5, 2023
@rm3l rm3l force-pushed the 7054-ui-form-validation-should-display-non-valid-fields-as-red-in-all-forms branch from 41083ef to 7832533 Compare September 5, 2023 09:30
Copy link
Member Author

@rm3l rm3l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@feloy As discussed together, I've made the necessary changes to simplify the form control logic. Please take a look when you get a chance. Thanks.

rm3l and others added 6 commits September 5, 2023 11:33
This covers the following forms:
- Add commands when adding a Composite Command

Co-authored-by: Philippe Martin <[email protected]>
This covers the following forms:
- Add Environment variables in Create Container
- Add Deployment annotations in Create Container
- Add Service annotations in Create Container

Co-authored-by: Philippe Martin <[email protected]>
This covers the following forms:
- Add Command in Create Container
- Add Args in Create Container
- Add Args in Create Image

Co-authored-by: Philippe Martin <[email protected]>
This covers the following forms:
- Select or Create container in Add Exec Command
- Select or create image component in Add Image Command
- Select or create Resource in Add Apply command

Co-authored-by: Philippe Martin <[email protected]>
This covers the following forms:
- Select or Create volume mount in Create container

Co-authored-by: Philippe Martin <[email protected]>
@rm3l rm3l force-pushed the 7054-ui-form-validation-should-display-non-valid-fields-as-red-in-all-forms branch from 7832533 to b35c5ca Compare September 5, 2023 09:34
@@ -62,15 +62,15 @@ describe('devfile editor spec', () => {
cy.getByDataCy('container-env-value-2').type("val3");

cy.getByDataCy('volume-mount-add').click();
cy.getByDataCy('volume-mount-path-0').type("/mnt/vol1");
cy.getByDataCy('volume-mount-path-0').type("/mnt/vol1", {force: true});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why, but I had to make Cypress forcibly bypass some checks with force: true. Otherwise, Cypress would complain that the element was covered by another one:

CypressError: Timed out retrying after 4000ms: `cy.type()` failed because this element:

`<input _ngcontent-xcd-c181="" formcontrolname="path" matinput="" class="mat-mdc-input-element ng-tns-c84-53 ng-untouched ng-pristine ng-invalid mat-mdc-form-field-input-control mdc-text-field__input cdk-text-field-autofill-monitored" ng-reflect-name="path" data-cy="volume-mount-path-0" id="mat-input-32" required="" aria-required="true">`

is being covered by another element:

`<span aria-hidden="true" class="mat-mdc-form-field-required-marker mdc-floating-label--required ng-tns-c84-53 ng-star-inserted"></span>`

Fix this problem, or use {force: true} to disable error checking.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 5, 2023
@rm3l
Copy link
Member Author

rm3l commented Sep 5, 2023

  [FAILED] in [It] - /go/odo_1/tests/helper/helper_cmd_wrapper.go:120 @ 09/05/23 12:54:55.574
  Running kubectl with args [kubectl get namespace cmd-devfile-deploy-test695stb -o jsonpath={.metadata.name}] and odo env: []
  [kubectl] cmd-devfile-deploy-test695stbDeleting project: cmd-devfile-deploy-test695stb
  Running kubectl with args [kubectl delete namespaces cmd-devfile-deploy-test695stb --wait=false] and odo env: []
  [kubectl] namespace "cmd-devfile-deploy-test695stb" deleted
  Setting current dir to: /go/odo_1/tests/integration
  Deleting dir: /tmp/3472070801
  Deleting dir: /tmp/2108200904
  << Timeline

  [FAILED] Timed out after 600.000s.
  Expected process to exit.  It did not.
  In [It] at: /go/odo_1/tests/helper/helper_cmd_wrapper.go:120 @ 09/05/23 12:54:55.574
...
Summarizing 1 Failure:
  [FAIL] odo devfile deploy command tests deploying devfile with long-running exec when Automount volumes are present in the namespace [It] should mount the volumes
  /go/odo_1/tests/helper/helper_cmd_wrapper.go:120

Ran 409 of 954 Specs in 1327.923 seconds
FAIL! -- 408 Passed | 1 Failed | 0 Pending | 545 Skipped

UI-only PR, so not related to the failures here.

/override Kubernetes-Integration-Tests/Kubernetes-Integration-Tests

@openshift-ci
Copy link

openshift-ci bot commented Sep 5, 2023

@rm3l: Overrode contexts on behalf of rm3l: Kubernetes-Integration-Tests/Kubernetes-Integration-Tests

In response to this:

 [FAILED] in [It] - /go/odo_1/tests/helper/helper_cmd_wrapper.go:120 @ 09/05/23 12:54:55.574
 Running kubectl with args [kubectl get namespace cmd-devfile-deploy-test695stb -o jsonpath={.metadata.name}] and odo env: []
 [kubectl] cmd-devfile-deploy-test695stbDeleting project: cmd-devfile-deploy-test695stb
 Running kubectl with args [kubectl delete namespaces cmd-devfile-deploy-test695stb --wait=false] and odo env: []
 [kubectl] namespace "cmd-devfile-deploy-test695stb" deleted
 Setting current dir to: /go/odo_1/tests/integration
 Deleting dir: /tmp/3472070801
 Deleting dir: /tmp/2108200904
 << Timeline

 [FAILED] Timed out after 600.000s.
 Expected process to exit.  It did not.
 In [It] at: /go/odo_1/tests/helper/helper_cmd_wrapper.go:120 @ 09/05/23 12:54:55.574
...
Summarizing 1 Failure:
 [FAIL] odo devfile deploy command tests deploying devfile with long-running exec when Automount volumes are present in the namespace [It] should mount the volumes
 /go/odo_1/tests/helper/helper_cmd_wrapper.go:120

Ran 409 of 954 Specs in 1327.923 seconds
FAIL! -- 408 Passed | 1 Failed | 0 Pending | 545 Skipped

UI-only PR, so not related to the failures here.

/override Kubernetes-Integration-Tests/Kubernetes-Integration-Tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@rm3l
Copy link
Member Author

rm3l commented Sep 5, 2023

       âš   0/3 nodes are available: 3 pod has unbound immediate PersistentVolumeClaims. preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling.
       âš   Pod is Pending
       âš   0/3 nodes are available: 3 pod has unbound immediate PersistentVolumeClaims. preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling.
       âš   AttachVolume.Attach failed for volume "pvc-1606fb7f-4869-4b2a-b0d5-9fd061018b19" : timed out waiting for external-attacher of vpc.block.csi.ibm.io CSI driver to attach volume r010-ead57323-f582-4fee-a68b-c7fcbd5c5092
       âš   AttachVolume.Attach failed for volume "pvc-8521c60b-5aa0-433d-86dc-8bc09d2ba498" : timed out waiting for external-attacher of vpc.block.csi.ibm.io CSI driver to attach volume r010-cafd8507-9798-4a46-8a0f-440ba39d1c04
       âš   Unable to attach or mount volumes: unmounted volumes=[firstvol-jwjpal-app-vol secondvol-jwjpal-app-vol], unattached volumes=[odo-shared-data firstvol-jwjpal-app-vol secondvol-jwjpal-app-vol kube-api-access-xtxx6 odo-projects]: timed out waiting for the condition

...
Summarizing 3 Failures:
  [FAIL] odo devfile deploy command tests deploying devfile with long-running exec when Automount volumes are present in the namespace [It] should mount the volumes
  /go/odo_1/tests/helper/helper_cmd_wrapper.go:120
  [FAIL] odo dev command tests when running odo dev with devfile containing volume-component - without metadata.name [BeforeEach] should successfully use the volume components in container components
  /go/odo_1/tests/helper/helper_run.go:54
  [FAIL] odo dev command tests when running odo dev with devfile containing volume-component - with metadata.name [BeforeEach] should successfully use the volume components in container components
  /go/odo_1/tests/helper/helper_run.go:54

Ran 404 of 954 Specs in 1598.498 seconds
FAIL! -- 401 Passed | 3 Failed | 0 Pending | 550 Skipped

Looks like there are some issues with volumes. Let's keep an eye out for those failures to see if they happen again.
Overriding because this is a UI-only PR, so not related to the failures here.

/override OpenShift-Integration-tests/OpenShift-Integration-tests

@openshift-ci
Copy link

openshift-ci bot commented Sep 5, 2023

@rm3l: Overrode contexts on behalf of rm3l: OpenShift-Integration-tests/OpenShift-Integration-tests

In response to this:

      âš   0/3 nodes are available: 3 pod has unbound immediate PersistentVolumeClaims. preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling.
      âš   Pod is Pending
      âš   0/3 nodes are available: 3 pod has unbound immediate PersistentVolumeClaims. preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling.
      âš   AttachVolume.Attach failed for volume "pvc-1606fb7f-4869-4b2a-b0d5-9fd061018b19" : timed out waiting for external-attacher of vpc.block.csi.ibm.io CSI driver to attach volume r010-ead57323-f582-4fee-a68b-c7fcbd5c5092
      âš   AttachVolume.Attach failed for volume "pvc-8521c60b-5aa0-433d-86dc-8bc09d2ba498" : timed out waiting for external-attacher of vpc.block.csi.ibm.io CSI driver to attach volume r010-cafd8507-9798-4a46-8a0f-440ba39d1c04
      âš   Unable to attach or mount volumes: unmounted volumes=[firstvol-jwjpal-app-vol secondvol-jwjpal-app-vol], unattached volumes=[odo-shared-data firstvol-jwjpal-app-vol secondvol-jwjpal-app-vol kube-api-access-xtxx6 odo-projects]: timed out waiting for the condition

...
Summarizing 3 Failures:
 [FAIL] odo devfile deploy command tests deploying devfile with long-running exec when Automount volumes are present in the namespace [It] should mount the volumes
 /go/odo_1/tests/helper/helper_cmd_wrapper.go:120
 [FAIL] odo dev command tests when running odo dev with devfile containing volume-component - without metadata.name [BeforeEach] should successfully use the volume components in container components
 /go/odo_1/tests/helper/helper_run.go:54
 [FAIL] odo dev command tests when running odo dev with devfile containing volume-component - with metadata.name [BeforeEach] should successfully use the volume components in container components
 /go/odo_1/tests/helper/helper_run.go:54

Ran 404 of 954 Specs in 1598.498 seconds
FAIL! -- 401 Passed | 3 Failed | 0 Pending | 550 Skipped

Looks like there are some issues with volumes. Let's keep an eye out for those failures to see if they happen again.
Overriding because this is a UI-only PR, so not related to the failures here.

/override OpenShift-Integration-tests/OpenShift-Integration-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@rm3l
Copy link
Member Author

rm3l commented Sep 5, 2023


  [FAILED] Timed out after 420.012s.
  Expected
      <string>:   __
       /  \__     Developing using the "wmrbzj" Devfile
       \__/  \    Namespace: cmd-dev-test577yhx
       /  \__/    odo version: v3.14.0
       \__/
      
      - Running on the cluster in Dev mode
       -  Waiting for Kubernetes resources  ...
       !  Pod is Pending
       !  0/3 nodes are available: 3 pod has unbound immediate PersistentVolumeClaims. preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling.
       !  0/3 nodes are available: 3 pod has unbound immediate PersistentVolumeClaims. preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling.
       !  AttachVolume.Attach failed for volume "pvc-0e9d83df-b6c5-479c-b131-bbe589324d8a" : timed out waiting for external-attacher of vpc.block.csi.ibm.io CSI driver to attach volume r010-75cd0332-23ad-484f-b11d-7c4827c29cf6
       !  Unable to attach or mount volumes: unmounted volumes=[odo-projects], unattached volumes=[odo-projects odo-shared-data kube-api-access-mphwf]: timed out waiting for the condition
      
  to contain substring
      <string>: [Ctrl+c] - Exit
  In [BeforeEach] at: C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54 @ 09/05/23 07:54:40.296
[...]

Summarizing 5 Failures:
  [FAIL] odo dev command tests when a component is bootstrapped when odo dev is executed and Ephemeral is set to false [BeforeEach] should have created resources
  C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54
  [FAIL] odo dev command tests when a component is bootstrapped when odo dev is executed and Ephemeral is set to false [BeforeEach] when stopping odo dev normally should have deleted all resources before returning
  C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54
  [FAIL] odo dev command tests when a component is bootstrapped when odo dev is executed and Ephemeral is set to false [BeforeEach] when killing odo dev and running odo delete component --wait should have deleted all resources before returning
  C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54
  [FAIL] odo dev command tests when running odo dev with devfile containing volume-component - with metadata.name [BeforeEach] should successfully use the volume components in container components
  C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54
  [FAIL] odo dev command tests when running odo dev with devfile containing volume-component - without metadata.name [BeforeEach] should successfully use the volume components in container components
  C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54

Ran 388 of 954 Specs in 1476.036 seconds
FAIL! -- 383 Passed | 5 Failed | 0 Pending | 566 Skipped

Looks like there are some issues with volumes. Let's keep an eye out for those failures to see if they happen again.
Overriding because this is a UI-only PR, so not related to the failures here.

/override windows-integration-test/Windows-test

@openshift-ci
Copy link

openshift-ci bot commented Sep 5, 2023

@rm3l: Overrode contexts on behalf of rm3l: windows-integration-test/Windows-test

In response to this:


 [FAILED] Timed out after 420.012s.
 Expected
     <string>:   __
      /  \__     Developing using the "wmrbzj" Devfile
      \__/  \    Namespace: cmd-dev-test577yhx
      /  \__/    odo version: v3.14.0
      \__/
     
     - Running on the cluster in Dev mode
      -  Waiting for Kubernetes resources  ...
      !  Pod is Pending
      !  0/3 nodes are available: 3 pod has unbound immediate PersistentVolumeClaims. preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling.
      !  0/3 nodes are available: 3 pod has unbound immediate PersistentVolumeClaims. preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling.
      !  AttachVolume.Attach failed for volume "pvc-0e9d83df-b6c5-479c-b131-bbe589324d8a" : timed out waiting for external-attacher of vpc.block.csi.ibm.io CSI driver to attach volume r010-75cd0332-23ad-484f-b11d-7c4827c29cf6
      !  Unable to attach or mount volumes: unmounted volumes=[odo-projects], unattached volumes=[odo-projects odo-shared-data kube-api-access-mphwf]: timed out waiting for the condition
     
 to contain substring
     <string>: [Ctrl+c] - Exit
 In [BeforeEach] at: C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54 @ 09/05/23 07:54:40.296
[...]

Summarizing 5 Failures:
 [FAIL] odo dev command tests when a component is bootstrapped when odo dev is executed and Ephemeral is set to false [BeforeEach] should have created resources
 C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54
 [FAIL] odo dev command tests when a component is bootstrapped when odo dev is executed and Ephemeral is set to false [BeforeEach] when stopping odo dev normally should have deleted all resources before returning
 C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54
 [FAIL] odo dev command tests when a component is bootstrapped when odo dev is executed and Ephemeral is set to false [BeforeEach] when killing odo dev and running odo delete component --wait should have deleted all resources before returning
 C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54
 [FAIL] odo dev command tests when running odo dev with devfile containing volume-component - with metadata.name [BeforeEach] should successfully use the volume components in container components
 C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54
 [FAIL] odo dev command tests when running odo dev with devfile containing volume-component - without metadata.name [BeforeEach] should successfully use the volume components in container components
 C:/Users/Administrator.ANSIBLE-TEST-VS/4557/tests/helper/helper_run.go:54

Ran 388 of 954 Specs in 1476.036 seconds
FAIL! -- 383 Passed | 5 Failed | 0 Pending | 566 Skipped

Looks like there are some issues with volumes. Let's keep an eye out for those failures to see if they happen again.
Overriding because this is a UI-only PR, so not related to the failures here.

/override windows-integration-test/Windows-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-merge-robot openshift-merge-robot merged commit adc9699 into redhat-developer:main Sep 5, 2023
@rm3l rm3l deleted the 7054-ui-form-validation-should-display-non-valid-fields-as-red-in-all-forms branch September 5, 2023 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/Dev-UI Issues or PRs related to the odo dev Web UI, a.k.a Devfile Builder kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UI] Form validation should display non-valid fields as Red in all forms
3 participants