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.

Sign up for GitHub

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

Jump to bottom

[UI] Allow single item deletion from multi-value fields #7084

Merged
merged 14 commits into from
Sep 18, 2023
Merged

[UI] Allow single item deletion from multi-value fields #7084

merged 14 commits into from
Sep 18, 2023

Conversation

rm3l
Copy link
Member

@rm3l rm3l commented Sep 12, 2023 •
edited
Loading

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

What does this PR do / why we need it:

This adds the ability in the odo dev UI to delete single items from multi-value fields:

  • container endpoints
  • volume mounts
  • multi-text fields, like Container Command and Args, or Image Build Args
  • multi-key-value fields, like Container Env Vars, Deployment Annotations and Service Annotations
  • multi-commands fields, in use in Composite Commands
Screenshots

image

image

image

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

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation labels Sep 12, 2023
@openshift-ci
Copy link

openshift-ci bot commented Sep 12, 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

@openshift-ci openshift-ci bot added the area/Dev-UI Issues or PRs related to the odo dev Web UI, a.k.a Devfile Builder label Sep 12, 2023
@netlify
Copy link

netlify bot commented Sep 12, 2023 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit b0412b0
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/6504800ef0f346000899efa1

@odo-robot
Copy link

odo-robot bot commented Sep 12, 2023 •
edited
Loading

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

@rm3l rm3l force-pushed the 7055-ui-multi-value-fields-should-allow-to-delete-single-item branch from 2bbc1ce to 0a0ca54 Compare September 13, 2023 09:30
@odo-robot
Copy link

odo-robot bot commented Sep 13, 2023 •
edited
Loading

Windows Tests (OCP) on commit 83ea480 finished successfully.
View logs: TXT HTML

@rm3l rm3l changed the title [WIP] [UI] Allow single item deletion from multi-value fields [UI] Allow single item deletion from multi-value fields Sep 13, 2023
@odo-robot
Copy link

odo-robot bot commented Sep 13, 2023 •
edited
Loading

OpenShift Unauthenticated Tests on commit 83ea480 finished successfully.
View logs: TXT HTML

@rm3l rm3l marked this pull request as ready for review September 13, 2023 09:38
@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 13, 2023
@openshift-ci openshift-ci bot requested review from feloy and kadel September 13, 2023 09:39
@odo-robot
Copy link

odo-robot bot commented Sep 13, 2023 •
edited
Loading

NoCluster Tests on commit 83ea480 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 13, 2023 •
edited
Loading

Unit Tests on commit 83ea480 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 13, 2023 •
edited
Loading

Validate Tests on commit 83ea480 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 13, 2023 •
edited
Loading

Kubernetes Tests on commit 83ea480 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Sep 13, 2023 •
edited
Loading

OpenShift Tests on commit 83ea480 finished with errors.
View logs: TXT HTML

rm3l and others added 2 commits September 15, 2023 14:47
@rm3l @feloy
Update Delete icon and add tooltip to it
d588446 
Co-authored-by: Philippe Martin <phmartin@redhat.com>
@rm3l @feloy
Move the "delete endpoint" button closer to the element it is attache…
4bf895f 
…d to

Co-authored-by: Philippe Martin <phmartin@redhat.com>
@sonarqubecloud
Copy link

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

@rm3l rm3l requested a review from feloy September 15, 2023 16:03
feloy
feloy approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@feloy feloy left a comment

Choose a reason for hiding this comment

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

Works great, thanks

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

rm3l commented Sep 18, 2023 

  [FAILED] Expected
      <string>:   __
       /  \__     Developing using the "lwvxbb" Devfile
       \__/  \    Namespace: cmd-logs-test249kbi
       /  \__/    odo version: v3.14.0 (b0412b0ee)
       \__/
      
      ↪ Running on the cluster in Dev mode
       •  Waiting for Kubernetes resources  ...
       âš   Pod is Pending
       ✓  Pod is Running
      --> Logs for lwvxbb-app-845986b854-nl689 / runtime
       •  Syncing files into the container  ...
      
 ✓  Syncing files into the container [457ms]
       •  Building your application in container (command: install)  ...
      runtime: npm notice created a lockfile as package-lock.json. You should commit this file.
      runtime: npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@~2.3.2 (node_modules/chokidar/node_modules/fsevents):
      runtime: npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.3.3: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})
      runtime: npm WARN node-echo@0.0.1 No repository field.
      runtime: npm WARN node-echo@0.0.1 No license field.
      runtime: 
      runtime: added 33 packages from 29 contributors and audited 34 packages in 1.481s
      runtime: 
      runtime: 3 packages are looking for funding
      runtime:   run `npm fund` for details
      runtime: 
      runtime: found 0 vulnerabilities
      runtime: 
      
 ✓  Building your application in container (command: install) [2s]
      
      ↪ Deploying Kubernetes Component: myapp-inner
       •  Creating resource Pod/myapp-inner  ...
      
 ✓  Creating resource Pod/myapp-inner 
       •  Executing the application (command: start)  ...
      runtime: 
      runtime: > node-echo@0.0.1 start /projects
      runtime: > node server.js
      runtime: 
      runtime: Server running on 0.0.0.0:8080
       •  Waiting for the application to be ready  ...
      
 ✓  Waiting for the application to be ready [1s]
       -  Forwarding from 127.0.0.1:34059 -> 8080
      
      
      ↪ Dev mode
       Status:
       Watching for changes in the current directory /tmp/2912389663
      
      Keyboard Commands:
      [Ctrl+c] - Exit and delete resources from the cluster
           [p] - Manually apply local changes to the application on the cluster
      
  to contain substring
      <string>: main: 

[...]
Summarizing 1 Failure:
  [FAIL] odo logs command tests when component is created and odo logs is executed when running in Dev mode with --logs [It] 1. should successfully show logs of the running component
  /go/odo_1/tests/integration/cmd_logs_test.go:253

Ran 404 of 954 Specs in 1409.505 seconds
FAIL! -- 403 Passed | 1 Failed | 0 Pending | 550 Skipped

Flaky issue with odo logs - reported in #6619 (will reopen if the issue persists on other PRs)..

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

@openshift-ci
Copy link

openshift-ci bot commented Sep 18, 2023

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

In response to this:

 [FAILED] Expected
     <string>:   __
      /  \__     Developing using the "lwvxbb" Devfile
      \__/  \    Namespace: cmd-logs-test249kbi
      /  \__/    odo version: v3.14.0 (b0412b0ee)
      \__/
     
     ↪ Running on the cluster in Dev mode
      •  Waiting for Kubernetes resources  ...
      âš   Pod is Pending
      ✓  Pod is Running
     --> Logs for lwvxbb-app-845986b854-nl689 / runtime
      •  Syncing files into the container  ...
     
✓  Syncing files into the container [457ms]
      •  Building your application in container (command: install)  ...
     runtime: npm notice created a lockfile as package-lock.json. You should commit this file.
     runtime: npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@~2.3.2 (node_modules/chokidar/node_modules/fsevents):
     runtime: npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.3.3: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})
     runtime: npm WARN node-echo@0.0.1 No repository field.
     runtime: npm WARN node-echo@0.0.1 No license field.
     runtime: 
     runtime: added 33 packages from 29 contributors and audited 34 packages in 1.481s
     runtime: 
     runtime: 3 packages are looking for funding
     runtime:   run `npm fund` for details
     runtime: 
     runtime: found 0 vulnerabilities
     runtime: 
     
✓  Building your application in container (command: install) [2s]
     
     ↪ Deploying Kubernetes Component: myapp-inner
      •  Creating resource Pod/myapp-inner  ...
     
✓  Creating resource Pod/myapp-inner 
      •  Executing the application (command: start)  ...
     runtime: 
     runtime: > node-echo@0.0.1 start /projects
     runtime: > node server.js
     runtime: 
     runtime: Server running on 0.0.0.0:8080
      •  Waiting for the application to be ready  ...
     
✓  Waiting for the application to be ready [1s]
      -  Forwarding from 127.0.0.1:34059 -> 8080
     
     
     ↪ Dev mode
      Status:
      Watching for changes in the current directory /tmp/2912389663
     
     Keyboard Commands:
     [Ctrl+c] - Exit and delete resources from the cluster
          [p] - Manually apply local changes to the application on the cluster
     
 to contain substring
     <string>: main: 

[...]
Summarizing 1 Failure:
 [FAIL] odo logs command tests when component is created and odo logs is executed when running in Dev mode with --logs [It] 1. should successfully show logs of the running component
 /go/odo_1/tests/integration/cmd_logs_test.go:253

Ran 404 of 954 Specs in 1409.505 seconds
FAIL! -- 403 Passed | 1 Failed | 0 Pending | 550 Skipped

Flaky issue with odo logs - reported in #6619 (will reopen if the issue persists on other PRs)..

/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.

@openshift-merge-robot openshift-merge-robot merged commit 9724292 into redhat-developer:main Sep 18, 2023
@rm3l rm3l deleted the 7055-ui-multi-value-fields-should-allow-to-delete-single-item branch September 18, 2023 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feloy feloy feloy approved these changes

@kadel kadel Awaiting requested review from kadel

Assignees

@feloy feloy

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[UI] Multi-value fields should allow to delete single item
3 participants
@rm3l @feloy @openshift-merge-robot