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

Update ::Button::OrchestrationTemplateEditRemove methods #11078

Merged

Conversation

jzigmund
Copy link

@jzigmund jzigmund commented Sep 7, 2016

Purpose or Intent

Updated already defined button class, used in Services/Catalogs/listnav 'Orchestration Templates'.

1st commit - Method calculate_properties in ::Button::Basic class is calling the disabled? so let the setting of the enable flag there, just call disabled? defined in the class. Also remove the code from build_toolbar_disable_button in toolbar_builder.rb (that's the main goal of the refactoring).
if @view_context.x_active_tree == :ot_tree && @record is not needed as I found the buttons of the class generated in ot_tree only. @record is checked before the calculate_properties method call.

2nd commit - role_allows? will be solved in #10942

Marking the PR as WIP as it needs to have solved PR mentioned above.

Links

Parent issue: #6259

def disabled?
@record.in_use?
end

Copy link
Member

Choose a reason for hiding this comment

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

should have

needs_record

?

@jzigmund jzigmund force-pushed the tb_refactoring-orchestration_templates branch from 806da26 to 274634c Compare September 8, 2016 10:39
end
end
end
self[:title] = if self[:id] =~ /_edit$/
Copy link
Member

Choose a reason for hiding this comment

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

@jzigmund I think this should be assigned to self[:title] only if disabled? returns true. It seems to me that this could lead to a situation where you would have enabled button with label saying you can not do the operation.

Copy link
Author

Choose a reason for hiding this comment

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

yep, good catch ;-)

@jzigmund jzigmund force-pushed the tb_refactoring-orchestration_templates branch from 274634c to 8fbe42f Compare September 13, 2016 15:25
@jzigmund
Copy link
Author

@miq-bot remove_label wip

@jzigmund jzigmund changed the title [WIP] Update ::Button::OrchestrationTemplateEditRemove methods Update ::Button::OrchestrationTemplateEditRemove methods Sep 13, 2016
@jzigmund
Copy link
Author

@martinpovolny @romanblanco please review

@miq-bot miq-bot removed the wip label Sep 13, 2016
@romanblanco
Copy link
Member

@jzigmund can you address rubocop's issue?

@jzigmund jzigmund force-pushed the tb_refactoring-orchestration_templates branch from 8fbe42f to 070d601 Compare September 13, 2016 15:42
@romanblanco
Copy link
Member

@jzigmund @martinpovolny otherwise, code looks good to me 👍

@miq-bot
Copy link
Member

miq-bot commented Sep 13, 2016

Checked commits jzigmund/manageiq@c2ea53f~...070d601 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 0 offenses detected
Everything looks good. 🏆

@mzazrivec mzazrivec merged commit 8b44197 into ManageIQ:master Sep 14, 2016
@mzazrivec mzazrivec added this to the Sprint 47 Ending Oct 3, 2016 milestone Sep 14, 2016
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