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

Reset selected snapshot in session when deleting the snapshot #183

Merged
merged 2 commits into from
Jan 27, 2017

Conversation

bmclaughlin
Copy link
Contributor

@bmclaughlin bmclaughlin commented Jan 18, 2017

Fixes an error in the logs and the page not loading when the currently selected snapshot is deleted and attempting to view the Snapshot Summary page.

Closes #209

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

@bmclaughlin
Copy link
Contributor Author

@miq-bot add_label bug

@miq-bot miq-bot added the bug label Jan 18, 2017
@bmclaughlin
Copy link
Contributor Author

@h-kataria please review.

@@ -1874,6 +1874,12 @@ def vm_button_operation(method, display_name, partial_after_single_selection = n
@single_delete = true unless flash_errors?
end

# Reset session[:snap_selected] if that snapshot is being deleted
if method == 'remove_snapshot' && session[:snap_selected].present? &&
Snapshot.find(session[:snap_selected]).vm_or_template_id.to_s == params[:id]
Copy link
Contributor

Choose a reason for hiding this comment

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

@bmclaughlin can't we just check for existence of Snapshot record here like this: Snapshot.find_by_id(session[:snap_selected]).nil?

@bmclaughlin
Copy link
Contributor Author

@h-kataria, ready for another review.

@ZitaNemeckova
Copy link
Contributor

@bmclaughlin your PR solves #209 as well :)

@h-kataria
Copy link
Contributor

changes looks good.

@miq-bot
Copy link
Member

miq-bot commented Jan 27, 2017

Checked commits bmclaughlin/manageiq-ui-classic@722bd2c~...7962ae0 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🏆

@h-kataria h-kataria added this to the Sprint 53 Ending Jan 30, 2017 milestone Jan 27, 2017
@h-kataria h-kataria merged commit a9c11d9 into ManageIQ:master Jan 27, 2017
@bmclaughlin bmclaughlin deleted the reset-snapshot-in-session branch January 30, 2017 14:49
@simaishi
Copy link
Contributor

@bmclaughlin There is a conflict in backporting to Euwe for the spec file. Can you create an Euwe PR?

@simaishi
Copy link
Contributor

Backported to Euwe via ManageIQ/manageiq#14361

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

Successfully merging this pull request may close these issues.

None yet

5 participants