Report both includes #13639
Merged
Report both includes #13639
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kbrock
force-pushed
the
report_both_includes
branch
3 times, most recently
from
59bcd4a
to
7e398df
Compare
Going forward, will be using only 1 set of includes per report. At one time, I split these out into 2 fields to reduce sql munging, but really no need for this change anymore
view "reports" had 2 fields: includes and includes_for_find reports only had 1 field: includes before: get_include_for_find produced includes() from includes field some code manually merged includes_for_find field after: renamed old get_include_for_find to include_as_hash It still converts the includes field into an ar includes() hash syntax new get_include_for_find merges includes_for_find field (hash) so now both reports and views use both fields. but the nuances of includes vs include_for_field is in a single method
kbrock
force-pushed
the
report_both_includes
branch
from
7e398df
to
5ff3b98
Compare
|
Checked commits kbrock/manageiq@6fb7f21~...5ff3b98 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 app/models/miq_report/generator.rb
|
kbrock
commented
Jan 26, 2017
| @@ -82,12 +82,10 @@ def paged_view_search(options = {}) | |||
| self.display_filter = options.delete(:display_filter_hash) if options[:display_filter_hash] | |||
| self.display_filter = options.delete(:display_filter_block) if options[:display_filter_block] | |||
|
|
|||
| includes1 = get_include_for_find(include) | |||
| includes = MiqExpression.merge_includes(includes1, include_for_find) | |||
There was a problem hiding this comment.
This is basically the whole PR:
# before
includes = MiqExpression.merge_includes(get_include_for_find, include_for_find)
# after
includes = get_include_for_find
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MiqReport(definitions inproduct/*/*.yaml) are used for our table views and reports.In reports we define associated tables using the
:includesvalue.In table views, we define associated tables with both
:includesand:includes_for_find.This PR removes that distinction. So both reports and views use both fields.
https://www.pivotaltracker.com/story/show/137377249