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

Towards provider plugins: dialog definition action #4377

Merged
merged 2 commits into from
Aug 3, 2018

Conversation

martinpovolny
Copy link
Member

Increase version of react-ui-components (needed for demo).

Implement /dashboars/dialog_definition with tests.

Extracted from #4315 with added specs.

@mzazrivec mzazrivec changed the title Towars provider plugings: dialog definition action Towards provider plugins: dialog definition action Jul 31, 2018
allow(controller).to receive(:check_privileges).and_return(true)
end

let(:klass) { 'sorryjako' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my.

end

let(:klass) { 'sorryjako' }
let(:name) { 'kalousek' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh boy!


context 'existing dialog' do
it 'returns json with data' do
data = 'je to kampaň!'
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep.

end

context 'directory traversal' do
it 'ignores anything but [a-z_]'do
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing whitespace before do

@martinpovolny martinpovolny force-pushed the dialog_definition branch 2 times, most recently from 7eb0988 to 9e178e6 Compare August 2, 2018 17:52
@@ -421,6 +421,77 @@
end
end

context '#dialog_definition' do
Copy link
Member

Choose a reason for hiding this comment

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

describe

end
end

context 'private #load_dialog_definition' do
Copy link
Member

Choose a reason for hiding this comment

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

describe and you don't need the private

Copy link
Member Author

Choose a reason for hiding this comment

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

The "private" is part of the description, surely I don't >>need<< it. But I want it.

let (:name) { 'existing_dialog' }
let (:dialog_path) { Pathname.new("/dialogs/#{name}.json") }

before(:each) do
Copy link
Member

Choose a reason for hiding this comment

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

each is redundant

@@ -421,6 +421,77 @@
end
end

context '#dialog_definition' do
before(:each) do
Copy link
Member

Choose a reason for hiding this comment

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

each is redundant

@miq-bot
Copy link
Member

miq-bot commented Aug 3, 2018

Some comments on commits martinpovolny/manageiq-ui-classic@1a1ee31~...33fc5e1

spec/controllers/dashboard_controller_spec.rb

  • ⚠️ - 426 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

@miq-bot
Copy link
Member

miq-bot commented Aug 3, 2018

Checked commits martinpovolny/manageiq-ui-classic@1a1ee31~...33fc5e1 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 3 offenses detected

spec/controllers/dashboard_controller_spec.rb

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

@mzazrivec mzazrivec self-assigned this Aug 3, 2018
@mzazrivec mzazrivec added this to the Sprint 92 Ending Aug 13, 2018 milestone Aug 3, 2018
@mzazrivec mzazrivec merged commit 496028f into ManageIQ:master Aug 3, 2018
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.

4 participants