Extract Expression Substitution code

[mkanoor]

Extracted substitution code from UI specific code so that
it can be used in other parts of the application. We would like
to use expressions with substitutions as Automate Method.

[mkanoor]

@gtanzillo @dclarizio
Please review

[miq-bot]

Checked commit mkanoor@96845b3 with ruby 2.2.3, rubocop 0.34.2, and haml-lint 0.13.0
2 files checked, 24 offenses detected

app/helpers/miq_expression/filter_subst.rb

• 🔶 - Line 1, Col 1 - Metrics/ModuleLength - Module has too many lines. [104/100]
• 🔶 - Line 24, Col 65 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 25, Col 34 - Style/WordArray - Use %w or %W for array of words.
• 🔶 - Line 27, Col 34 - Style/WordArray - Use %w or %W for array of words.
• 🔶 - Line 32, Col 20 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 34, Col 48 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 57, Col 8 - Style/WordArray - Use %w or %W for array of words.
• 🔶 - Line 66, Col 59 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 68, Col 34 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 75, Col 27 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 78, Col 43 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 104, Col 33 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔹 - Line 4, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for exp_build_table is too high. [56.44/15]
• 🔹 - Line 4, Col 3 - Metrics/CyclomaticComplexity - Cyclomatic complexity for exp_build_table is too high. [12/6]
• 🔹 - Line 4, Col 3 - Metrics/MethodLength - Method has too many lines. [47/25]
• 🔹 - Line 4, Col 3 - Metrics/PerceivedComplexity - Perceived complexity for exp_build_table is too high. [16/7]
• 🔹 - Line 40, Col 11 - Metrics/BlockNesting - Avoid more than 3 levels of block nesting.
• 🔹 - Line 55, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for exp_replace_qs_tokens is too high. [24.56/15]
• 🔹 - Line 55, Col 3 - Metrics/CyclomaticComplexity - Cyclomatic complexity for exp_replace_qs_tokens is too high. [8/6]
• 🔹 - Line 55, Col 3 - Metrics/PerceivedComplexity - Perceived complexity for exp_replace_qs_tokens is too high. [9/7]
• 🔹 - Line 74, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for exp_find_by_token is too high. [18/15]
• 🔹 - Line 74, Col 3 - Metrics/CyclomaticComplexity - Cyclomatic complexity for exp_find_by_token is too high. [9/6]
• 🔹 - Line 74, Col 3 - Metrics/PerceivedComplexity - Perceived complexity for exp_find_by_token is too high. [10/7]
• 🔹 - Line 97, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for create_tokens is too high. [28.44/15]

[Fryguy]

This should not be be necessary if the file is named properly and the class matches. On that note, I am just confused about what the new module class is (since there's no other usage of it), and thus am not sure where it should live. The helpers are actually "view helpers", so if this is not used in a view, then it doesn't belong here. If it's a controller mixin, then it should live in controllers/mixins, however the OP states that you want to use it in automate.

To me this just feels like it should be a model class that you just instantiate and use or perhaps as a part of MiqExpression the model. Can you elaborate a bit on how this helps you?

[mkanoor]

@Fryguy
I have a separate WIP PR for Automate Expression Method #6655
where I am going to use these functions.

The 4 methods that I included in the helper file were extracted from the app/controllers/application_controller/filter.rb where it was only available to the UI and the controllers.

I would need these functions to do substitution from the automate expression methods. These instance methods which are in a module can be included by the app/controllers/application_controller/filter.rb as well as the lib/miq_automation_engine/engine/miq_ae_expression_method.rb in my WIP PR.

[Fryguy]

Are these methods UI specific though, or are they related to any MiqExpression? Can they just be a part of the MiqExpression class (even as class methods)?

[mkanoor]

@Fryguy
These methods do the substitution into an expression given a set of input parameters. Till now that feature was UI specific, since UI was the only place you could supply user inputs. Maybe @gtanzillo @dclarizio might have more ideas on it.

[Fryguy]

Based on that it really feels like it belongs in MiqExpression directly.

[gtanzillo]

Looking at this and reading Jason's comments I agree that this code would be better suited as a module that gets mixed in to MiqExpression as class methods. But it could also be taken further by making them instance methods too. Maybe set the substituted expression an a new accessor or something like that.

[mkanoor]

@gtanzillo @Fryguy
Please Review