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

Remove generic object from all related services before destroy #17679

Merged
merged 2 commits into from
Jul 10, 2018
Merged

Remove generic object from all related services before destroy #17679

merged 2 commits into from
Jul 10, 2018

Conversation

pkomanek
Copy link
Contributor

@pkomanek pkomanek commented Jul 9, 2018

Calling the generic_objects method from the service model returns array of GenericObjects and nils(for allready destroyed objects), which makes some UI crashes. So we decided to remove a GO from all related services in the #before_destroy callback. This PR is based on the BZ bellow.

Links [Optional]

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

@pkomanek
Copy link
Contributor Author

pkomanek commented Jul 9, 2018

@miq-bot add_label bug

@miq-bot miq-bot added the bug label Jul 9, 2018
@@ -200,4 +201,10 @@ def _call_automate(method_name, *args)
ws = MiqAeEngine.deliver(options)
ws.root['method_result']
end

def remove_go_from_all_related_services
ServiceResource.where(:resource_id => id).each do |resource|
Copy link
Member

Choose a reason for hiding this comment

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

Use :resource => self. The resource association is polymorphic, if any other object type happens to share the same ID as this generic object you would delete it. 😱

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gmcculloug changed. Thanks

@gmcculloug gmcculloug self-assigned this Jul 9, 2018
@gmcculloug gmcculloug requested a review from bzwei July 9, 2018 18:06

def remove_go_from_all_related_services
ServiceResource.where(:resource => self).each do |resource|
remove_from_service(Service.find_by(:id => resource.service_id))
Copy link
Member

@gmcculloug gmcculloug Jul 9, 2018

Choose a reason for hiding this comment

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

You do not have to lookup the service as it is an association of the resource object. Also, I am not sure if we need to protect if resource.service is nil but in case:

remove_from_service(resource.service)) if resource.service

@bzwei
Copy link
Contributor

bzwei commented Jul 9, 2018

LTGM other than the only comment GM made.

@miq-bot
Copy link
Member

miq-bot commented Jul 10, 2018

Checked commits pkomanek/manageiq@06cb29e~...9e1380e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@gmcculloug gmcculloug merged commit 6bf5eca into ManageIQ:master Jul 10, 2018
@gmcculloug gmcculloug added this to the Sprint 90 Ending Jul 16, 2018 milestone Jul 10, 2018
@pkomanek pkomanek deleted the remove_generic_object_from_all_related_services_before_destroy branch July 10, 2018 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants