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

Fix policy_events relationship on VmOrTemplate #17036

Merged
merged 1 commit into from
Mar 16, 2018

Conversation

jntullo
Copy link

@jntullo jntullo commented Feb 22, 2018

Currently (back to Fine), the policy_events relationship on VmOrTemplate is not working, and even if "fixed" to resolve the current internal server error, it does not appear to do what I think it was intended to be doing, which is simply returning all policy events with a target_type of VmOrTemplate

Here's the step by step (although most of it is probably obvious):

:policy_events, -> { where(["target_id = ? OR target_class = 'VmOrTemplate'", id]).order(:timestamp) }, :class_name => "PolicyEvent"

does not work because it is referencing "id" which is an invalid reference, resulting in:

NameError: undefined local variable or method `id' for #<PolicyEvent::ActiveRecord_Relation:0x007fe7489066b8>

Adjusting the query to reference the VmOrTemplate id:

:policy_events, ->(vmt) { where(["target_id = ? OR target_class = 'VmOrTemplate'", vmt.id]).order(:timestamp) }, :class_name => "PolicyEvent"

Still does not work, because it is expecting the foreign_key to be vm_or_template_id. Even if that is fixed, it is doing an AND on the foreign_key and this where clause, which would not return what this where is expecting.

I got to this point and realized I think that this query is simply attempting to return all policy_events where the target_class is VmOrTemplate? But it is difficult to tell when the query was not working/does not make much sense in the first place

Sanity check for what this query was supposed to be doing?

@miq-bot assign @gtanzillo
@miq-bot add_label bug, gaprindashvili/yes, fine/yes

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

@@ -122,7 +122,7 @@ class VmOrTemplate < ApplicationRecord
has_many :ems_events_src, :class_name => "EmsEvent"
has_many :ems_events_dest, :class_name => "EmsEvent", :foreign_key => :dest_vm_or_template_id

has_many :policy_events, -> { where(["target_id = ? OR target_class = 'VmOrTemplate'", id]).order(:timestamp) }, :class_name => "PolicyEvent"
has_many :policy_events, :class_name => "PolicyEvent"
Copy link
Member

Choose a reason for hiding this comment

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

Try has_many :policy_events, ->(vm) { where(["target_id = ? AND target_class = 'VmOrTemplate'", vm.id]).order(:timestamp) }, :foreign_key => "target_id"

Copy link
Author

Choose a reason for hiding this comment

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

@lfu yes, that works 👍 I wasn't sure that's what the query was supposed to be doing, since there was an "OR" in the original query

The policy_events relationship currently results in an internal server error because it is referencing "id" which is not valid

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1546995
@miq-bot
Copy link
Member

miq-bot commented Feb 26, 2018

Checked commit jntullo@adc676c with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

app/models/vm_or_template.rb

@kbrock kbrock merged commit 9472102 into ManageIQ:master Mar 16, 2018
@kbrock kbrock added this to the Sprint 82 Ending Mar 26, 2018 milestone Mar 16, 2018
@kbrock kbrock assigned kbrock and unassigned gtanzillo Mar 16, 2018
@kbrock kbrock added the core label Mar 16, 2018
simaishi pushed a commit that referenced this pull request Mar 19, 2018
Fix policy_events relationship on VmOrTemplate
(cherry picked from commit 9472102)

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

Gaprindashvili backport details:

$ git log -1
commit 835b24bbe0fb63407390aae1ba52097bb851a07c
Author: Keenan Brock <keenan@thebrocks.net>
Date:   Fri Mar 16 09:52:16 2018 -0400

    Merge pull request #17036 from jntullo/bz_1546995
    
    Fix policy_events relationship on VmOrTemplate
    (cherry picked from commit 94721027a9e8ed44bb6905d99bb86e66a78a11cf)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1558030

simaishi pushed a commit that referenced this pull request Apr 9, 2018
Fix policy_events relationship on VmOrTemplate
(cherry picked from commit 9472102)

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

simaishi commented Apr 9, 2018

Fine backport details:

$ git log -1
commit 8fa1eb7009cbdd000313322de9a38c293c16bfe6
Author: Keenan Brock <keenan@thebrocks.net>
Date:   Fri Mar 16 09:52:16 2018 -0400

    Merge pull request #17036 from jntullo/bz_1546995
    
    Fix policy_events relationship on VmOrTemplate
    (cherry picked from commit 94721027a9e8ed44bb6905d99bb86e66a78a11cf)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1558032

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
Fix policy_events relationship on VmOrTemplate
(cherry picked from commit 9472102)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1558032
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.

6 participants