Skip to content
This repository has been archived by the owner on May 7, 2021. It is now read-only.

fix(dynamic-component): Remove None from Enum field values #2798

Merged

Conversation

jarifibrahim
Copy link
Member

Fixes openshiftio/openshift.io#3831 and openshiftio/openshift.io#3832 (comment)

This PR removes the unnecessary None field in the dropdown for enum types. The backend would always send a default value for enum fields.

Before

Now

screenshot from 2018-11-01 17-26-20

@alien-ike
Copy link
Collaborator

alien-ike commented Nov 1, 2018

Ike Plugins (test-keeper)

Thank you @jarifibrahim for this contribution!

It appears that no tests have been added or updated in this PR.

Automated tests give us confidence in shipping reliable software. Please add some as part of this change.

If you are an admin or the reviewer of this PR and you are sure that no test is needed then you can use the command /ok-without-tests as a comment to make the status green.

Your plugin configuration is stored in the file.

@jarifibrahim jarifibrahim changed the title fix(dynamic-component): Remove None from Enum and Boolean field values fix(dynamic-component): Remove None from Enum field values Nov 1, 2018
@centos-ci
Copy link
Collaborator

@jarifibrahim Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2798 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2798 and visit http://localhost:5000 to access it.

@jarifibrahim
Copy link
Member Author

[test]

@centos-ci
Copy link
Collaborator

@jarifibrahim Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2798 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2798 and visit http://localhost:5000 to access it.

@sahil143
Copy link
Contributor

sahil143 commented Nov 2, 2018

/ok-without-tests

@jarifibrahim jarifibrahim merged commit 8cc7af7 into fabric8-ui:master Nov 2, 2018
@jarifibrahim jarifibrahim deleted the remove-none-from-enum-input branch November 2, 2018 11:02
@kwk
Copy link
Collaborator

kwk commented Nov 2, 2018

/ok-without-tests

@sahil143 how can this be okay without a test?

@jarifibrahim
Copy link
Member Author

jarifibrahim commented Nov 2, 2018

/ok-without-tests

@sahil143 how can this be okay without a test?

@kwk We don't have the tests for the dropdowns. We test user interactions via e2e tests and we test services via unit test. The piece of code I modified doesn't have any test (and it's not not easy to test it because it changes some HTML. You can't easily test rendered HTML)

@kwk
Copy link
Collaborator

kwk commented Nov 2, 2018

The piece of code I modified doesn't have any test (and it's not not easy to test it because it changes some HTML. You can't easily test rendered HTML

I guess we have to find a way to test it though. Relying on tests that run in a different repo is not very cool because you cannot verify this. If everything was in monorepo things looks different I guess.

@jarifibrahim
Copy link
Member Author

jarifibrahim commented Nov 2, 2018

ind a way to test it though. Relying on tests that run in a different repo is not very cool because you cannot verify this. If everything was in monorepo things looks different I guess.

@kwk No no. I can assure you this PR doesn't break anything 😉 . We have our own e2e tests. The PR build executed the unit tests and about 68 tests functional Tests (aka e2e tests) on each SDD and AGILE template. We cover most of the user interaction scenario via these e2e-tests.

The QE team has their own e2e tests which cover the entire osio workflow (build, launcher, etc) and we have our own e2e tests that cover common planner workflows (only planner).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants