Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

CloneDialog: support non-admin users #521

Merged
merged 2 commits into from
Jul 22, 2019

Conversation

atiratree
Copy link
Contributor

@atiratree
Copy link
Contributor Author

I tried not to interfiere with CloneDialog component much, because it it targeted against stable v2.0.0

@coveralls
Copy link

coveralls commented Jul 19, 2019

Pull Request Test Coverage Report for Build 2170

  • 9 of 22 (40.91%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.3%) to 81.108%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/Dialog/CloneDialog/CloneDialog.js 9 22 40.91%
Totals Coverage Status
Change from base Build 2070: -0.3%
Covered Lines: 3888
Relevant Lines: 4578

💛 - Coveralls

<Button
bsStyle="primary"
onClick={this.cloneVm}
disabled={!(this.state.valid && dataVolumesValid && pvcsValid)}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should handle error state more explicitly - the user should be informed about transition from loading to error state.
I believe rendering info about just the most serious issue (as filtered by Firehose) is enough.

Copy link
Contributor

@mareklibra mareklibra left a comment

Choose a reason for hiding this comment

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

Better error handling from user perspective should be added, otherwise lgtm

@atiratree
Copy link
Contributor Author

added error handling

@mareklibra mareklibra merged commit 195dfbc into kubevirt:web-ui-v2.0.0 Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants