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

Disable delete button for the active snapshot on oVirt #1628

Merged
merged 1 commit into from
Jul 14, 2017
Merged

Disable delete button for the active snapshot on oVirt #1628

merged 1 commit into from
Jul 14, 2017

Conversation

borod108
Copy link

Disable the delete button when the active snapshot is selected
for a vm on oVirt.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1443411

Depends on: ManageIQ/manageiq-providers-ovirt#54

@borod108 borod108 mentioned this pull request Jun 29, 2017
Merged
@borod108
Copy link
Author

@martinpovolny can you please review?

@borod108
Copy link
Author

screenshot from 2017-06-29 13-44-50

@borod108
Copy link
Author

please note specs are failing because the PR this one depends on is not merged yet.

@borod108 borod108 closed this Jul 2, 2017
@borod108 borod108 reopened this Jul 2, 2017
@borod108
Copy link
Author

borod108 commented Jul 2, 2017

@mzazrivec can you please review?

@borod108
Copy link
Author

borod108 commented Jul 3, 2017

VM snapshot: delete option is enabled, for Active VM

@borod108
Copy link
Author

borod108 commented Jul 3, 2017

@miq-bot remove_label "pending core"

@miq-bot
Copy link
Member

miq-bot commented Jul 3, 2017

@borod108 Cannot remove the following label because they are not recognized: "pending core"

@martinpovolny
Copy link
Member

@romanblanco, @PanSpagetka : please, review and ping me if all is ok

mzazrivec
mzazrivec reviewed Jul 10, 2017
View reviewed changes
app/helpers/application_helper/toolbar/x_vm_center.rb Outdated
@@ -310,7 +310,7 @@ class ApplicationHelper::Toolbar::XVmCenter < ApplicationHelper::Toolbar::Basic
:confirm => N_("The selected snapshot will be permanently deleted. Are you sure you want to delete the selected snapshot?"),
:url_parms => "main_div",
:onwhen => "1",
:klass => ApplicationHelper::Button::GenericFeatureButtonWithDisable,
:klass => ApplicationHelper::Button::VmSnapshotRemoveOne,
:options => {:feature => :remove_snapshot}),
Copy link
Contributor

Choose a reason for hiding this comment

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

@romanblanco Is :options here needed when we use a specific button class rather than GenericFeatureButtonWithDisable ?

Copy link
Member

Choose a reason for hiding this comment

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

@mzazrivec @borod108 the :option is in this case useless. The feature is hardcoded in the button code (https://github.com/ManageIQ/manageiq-ui-classic/pull/1628/files#diff-533cf50bb9cc5f712939b84bbbc9a0a1R7)

Copy link
Contributor

Choose a reason for hiding this comment

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

@borod108 Can you please remove the :options parameter here?

@romanblanco
Copy link
Member

I'm not able to get to the snapshots, is this working for you? @mzazrivec @borod108

screencast from 2017-07-11 15-11-33

@borod108 borod108 force-pushed the bugs/delete_active_snapshot_button branch from abdaf28 to 0b3b082 Compare July 11, 2017 13:13
@borod108
Copy link
Author

borod108 commented Jul 11, 2017 •
edited
Loading

@romanblanco snapshots work for me. you need to be connected to RHV 4 and above to use snapshots, could that be the problem?
Anyway after rebasing and applying the chnage that was suggested the fix does not work anymore so I am moving this back to wip to investigate.

@miq-bot add_label wip

@miq-bot miq-bot changed the title Disable delete button for the active snapshot on oVirt [WIP] Disable delete button for the active snapshot on oVirt Jul 11, 2017
@miq-bot miq-bot added the wip label Jul 11, 2017
Disable delete button for the active snapshot on oVirt
fc818e9 
Disable the delete button when the active snapshot is selected
for a vm on oVirt.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1443411
@borod108 borod108 force-pushed the bugs/delete_active_snapshot_button branch from 0b3b082 to fc818e9 Compare July 12, 2017 05:40
@borod108
Copy link
Author

@miq-bot remove-label wip

Validated, it works.

@miq-bot
Copy link
Member

miq-bot commented Jul 12, 2017

Checked commit borod108@fc818e9 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 🍰

@miq-bot miq-bot changed the title [WIP] Disable delete button for the active snapshot on oVirt Disable delete button for the active snapshot on oVirt Jul 12, 2017
@miq-bot miq-bot removed the wip label Jul 12, 2017
@borod108
Copy link
Author

@mzazrivec removed the options, tnx.
Is it ok for merge?

@h-kataria
Copy link
Contributor

looks good.

@h-kataria h-kataria added this to the Sprint 65 Ending Jul 24, 2017 milestone Jul 14, 2017
@h-kataria h-kataria merged commit 2811730 into ManageIQ:master Jul 14, 2017
simaishi pushed a commit that referenced this pull request Aug 10, 2017
@h-kataria @simaishi
Merge pull request #1628 from borod108/bugs/delete_active_snapshot_bu…
0725049 
…tton

Disable delete button for the active snapshot on oVirt
(cherry picked from commit 2811730)

https://bugzilla.redhat.com/show_bug.cgi?id=1480377
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 0725049156070280fd0d8929c5bf20138b3f4d7d
Author: Harpreet Kataria <hkataria@redhat.com>
Date:   Fri Jul 14 11:14:50 2017 -0400

    Merge pull request #1628 from borod108/bugs/delete_active_snapshot_button
    
    Disable delete button for the active snapshot on oVirt
    (cherry picked from commit 28117304848cbd2565b35a743c1bfba43e269fde)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1480377

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romanblanco romanblanco romanblanco left review comments

@mzazrivec mzazrivec mzazrivec left review comments

Assignees

@romanblanco romanblanco

@PanSpagetka PanSpagetka

Labels
bug compute/infrastructure fine/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@borod108 @miq-bot @martinpovolny @romanblanco @h-kataria @simaishi @mzazrivec @PanSpagetka