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

Create custom button events for each object in target list #18042

Merged

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Oct 1, 2018

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

When we originally added CustomButtonEvents we didn't account for CustomButtons being run on multiple objects at once. I think we should be creating events for all the objects a custom button runs on.

@d-m-u
Copy link
Contributor Author

d-m-u commented Oct 1, 2018

@miq-bot add_label bug
@miq-bot add_label hammer/yes
@miq-bot assign @gmcculloug
@miq-bot add_reviewer @tinaafitz

@@ -131,7 +135,6 @@ def invoke_async(target, source = nil)
:action => "Calling automate for user #{userid}",
:userid => User.current_user
}

Copy link
Member

Choose a reason for hiding this comment

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

Why did this change?

@d-m-u d-m-u force-pushed the custom_button_event_fix_for_bz1628224 branch from e9b586a to 458c268 Compare October 1, 2018 15:43
:event_type => 'button.trigger.start',
:user_id => user.id
)
expect(CustomButtonEvent.first[:full_data]).to include(
Copy link
Member

Choose a reason for hiding this comment

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

Can this be merged into the expectation above?
:full_data => a_hash_including(:automate_entry_point => "/SYSTEM/PROCESS/Request")

%i(invoke invoke_async).each do |method|
describe "##{method}" do
it "publishes CustomButtonEvents with multiple vms" do
Timecop.freeze(Time.now.utc) do
Copy link
Member

Choose a reason for hiding this comment

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

How is time related to this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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


def create_event(source, target, args)
CustomButtonEvent.create!(
:event_type => 'button.trigger.start',
Copy link
Member

Choose a reason for hiding this comment

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

This appears to be indented more than 2 spaces

@d-m-u d-m-u force-pushed the custom_button_event_fix_for_bz1628224 branch from 458c268 to f305d9a Compare October 1, 2018 15:54
@@ -326,5 +328,29 @@
end
end
end

%i(invoke invoke_async).each do |method|
describe "##{method}" do
Copy link
Member

Choose a reason for hiding this comment

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

Also, shouldn't this be merged into the describe above?

@d-m-u d-m-u force-pushed the custom_button_event_fix_for_bz1628224 branch 2 times, most recently from debb360 to 783f8b1 Compare October 1, 2018 16:08
@@ -303,8 +305,8 @@
EvmSpecHelper.local_miq_server(:is_master => true, :zone => Zone.seed)
end

%i(invoke invoke_async).each do |method|
describe "##{method}" do
context "with only one VM" do
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of this?

%i(invoke invoke_async).each do |method|
  describe "##{method}", "publishes CustomButtonEvent(s)" do
    it "with a single VM" do
      Timecop.freeze(Time.now.utc) do
        User.with_user(user) { custom_button.send(method, vm, 'UI') }
        expect(CustomButtonEvent.first.timestamp).to be_within(0.01).of(Time.now.utc)
      end

      expect(CustomButtonEvent.count).to eq(1)
      expect(CustomButtonEvent.first).to have_attributes(
        :source      => 'UI',
        :target_id   => vm.id,
        :target_type => 'VmOrTemplate',
        :type        => 'CustomButtonEvent',
        :event_type  => 'button.trigger.start',
        :user_id     => user.id,
        :full_data   => a_hash_including(:automate_entry_point => "/SYSTEM/PROCESS/Request")
      )
    end

    it "with multiple vms" do
      Timecop.freeze(Time.now.utc) do
        User.with_user(user) { custom_button.send(method, [vm, vm2, vm3], 'UI') }
        expect(CustomButtonEvent.first.timestamp).to be_within(0.01).of(Time.now.utc)
      end

      expect(CustomButtonEvent.count).to eq(3)
      expect(CustomButtonEvent.first).to have_attributes(
        :source      => 'UI',
        :target_id   => vm.id,
        :target_type => 'VmOrTemplate',
        :type        => 'CustomButtonEvent',
        :event_type  => 'button.trigger.start',
        :user_id     => user.id,
        :full_data   => a_hash_including(:automate_entry_point => "/SYSTEM/PROCESS/Request")
      )
    end
  end
end

@d-m-u d-m-u force-pushed the custom_button_event_fix_for_bz1628224 branch from 783f8b1 to f5dcc96 Compare October 1, 2018 18:14
@miq-bot
Copy link
Member

miq-bot commented Oct 1, 2018

Checked commit d-m-u@f5dcc96 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. 🏆

@bdunne bdunne merged commit 4b78ead into ManageIQ:master Oct 1, 2018
@bdunne bdunne assigned bdunne and unassigned gmcculloug Oct 1, 2018
@bdunne bdunne added this to the Sprint 96 Ending Oct 8, 2018 milestone Oct 1, 2018
@d-m-u d-m-u deleted the custom_button_event_fix_for_bz1628224 branch October 1, 2018 18:36
simaishi pushed a commit that referenced this pull request Oct 2, 2018
…28224

Create custom button events for each object in target list

(cherry picked from commit 4b78ead)

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

simaishi commented Oct 2, 2018

Hammer backport details:

$ git log -1
commit 9a9f529a3be9c31132cc9d57a809faeb7e23661f
Author: Brandon Dunne <[email protected]>
Date:   Mon Oct 1 14:35:47 2018 -0400

    Merge pull request #18042 from d-m-u/custom_button_event_fix_for_bz1628224
    
    Create custom button events for each object in target list
    
    (cherry picked from commit 4b78ead7c15a39ac01fb17b9678cd64b7378007a)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1628224

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.

5 participants