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

Move PPCHOICES(2) out of UiConstants #1680

Merged
merged 2 commits into from
Jul 14, 2017

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented Jul 12, 2017

Welcome to the next installment of "Killing UiConstants softly" (with @NickLaMuro): Part Deux, the default pagination epic!

Removes both PPCHOICES and PPCHOICES2 from the UiConstants lib, and moves them into properly scoped places. For PPCHOICES2, this was relatively simple since it is only used in the MiqTaskController.

For PPCHOICES, though, it was not only used to populate the @pp_choices variable in the controller, but also called from a view directly (not using the instance variable), so this either needed to be exposed in the view, or a helper method to expose this variable. I went with the latter to avoid the practice of polluting the global namespace, even in the view, but the way I did it might be controversial.

Links

Steps for Testing/QA

PPCHOICES that was called in the view should only be called from the /configuration/index page, so you can confirm that works by heading to that page with this change applied. The page would break trying to populate the "Default Items Per Page" section if things didn't work properly.

This is currently used in only one view, and in the before_action that
is included in ApplicationController, so let ApplicationController own
this constant by adding it to the ControllerConstants mixin.

For the view that used it, expose a helper that creates a method to
access that constant.
This is only used in the MiqTaskController, so define that in a constant
there instead of the UiConstants.
@miq-bot
Copy link
Member

miq-bot commented Jul 12, 2017

Checked commits NickLaMuro/manageiq-ui-classic@20ac0fb~...3a3e68c with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks fine. 🍪

@martinpovolny martinpovolny merged commit 1528241 into ManageIQ:master Jul 14, 2017
@martinpovolny martinpovolny added this to the Sprint 65 Ending Jul 24, 2017 milestone Jul 14, 2017
@martinpovolny martinpovolny self-assigned this Jul 14, 2017
@martinpovolny
Copy link
Member

ref #1661

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.

3 participants