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

Minor inventory collection enhancements #15108

Merged
merged 2 commits into from
May 16, 2017

Conversation

Ladas
Copy link
Contributor

@Ladas Ladas commented May 16, 2017

Add a helper for determining if IC model uses STI. Add checks to build method, build should return nil if we cannot get the unique uuid of the InventoryObject and it should return exiting object if we have it.

Ladas added 2 commits May 16, 2017 18:22
Add a helper for determining if IC model uses STI
Add checks to build method, build should return nil if we
cannot get the unique uuid of the InventoryObject and it
should return exiting object if we have it.
@Ladas
Copy link
Contributor Author

Ladas commented May 16, 2017

@miq-bot assign @agrare

@Ladas
Copy link
Contributor Author

Ladas commented May 16, 2017

@miq-bot add_label enhancement

@miq-bot
Copy link
Member

miq-bot commented May 16, 2017

Checked commits Ladas/manageiq@63d81f5~...acaa987 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

@agrare agrare merged commit edb1082 into ManageIQ:master May 16, 2017
@@ -378,6 +378,10 @@ def data_collection_finalized?
data_collection_finalized
end

def supports_sti?
@supports_sti_cache ||= model_class.column_names.include?("type")
Copy link
Member

Choose a reason for hiding this comment

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

@Ladas this is the true/false/nil pattern, so you probably need something like the following so you are memoizing and not recalculating when value is false

def supports_sti?
  if @supports_sti_cache.nil?
    @supports_sti_cache = model_class.column_names.include?("type")
  end
  @supports_sti_cache
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ha, nice catch, you are right of course :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agrare agrare added this to the Sprint 61 Ending May 22, 2017 milestone May 17, 2017
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