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

Generic object enhancement. #15893

Merged
merged 4 commits into from
Sep 8, 2017
Merged

Conversation

lfu
Copy link
Member

@lfu lfu commented Aug 25, 2017

Some changes to generic object.

  • Use save! over save to be API call ready.
  • Convert input to string format as keywords in properties hash are all in string format.
  • Enhance GenericObject#delete_property to work for both property attributes and associations.
  • Add GenericObject#property_associations.

@miq-bot assign @gmcculloug
@miq-bot add_label enhancement

cc @jntullo

@jntullo
Copy link

jntullo commented Aug 25, 2017

Thanks for the update @lfu! 👍

@lfu lfu force-pushed the generic_object_enhancement branch 3 times, most recently from 0c9113a to 97d2786 Compare August 28, 2017 21:15
@lfu lfu changed the title [WIP] Generic object enhancement. Generic object enhancement. Aug 28, 2017
@lfu
Copy link
Member Author

lfu commented Aug 28, 2017

cc @bzwei

@bzwei
Copy link
Contributor

bzwei commented Sep 6, 2017

Instead of converting all attribute names to string to_s, how about all to symbols?

@bzwei
Copy link
Contributor

bzwei commented Sep 6, 2017

Instead of inserting require_generic_object_definition for almost every method we can have a before_save hook to ensure the existence of definition.

@lfu
Copy link
Member Author

lfu commented Sep 6, 2017

@bzwei Jsonb column has all keywords stored in String format. So I think String format may be more appropriate.
It is possible that the generic object has the definition in DB but somehow the definition object does not exist. I would like each method checks the existence of the definition before it does the work which may make the debugging easier in the future.

@bzwei
Copy link
Contributor

bzwei commented Sep 6, 2017

Is it a valid use case that "It is possible that the generic object has the definition in DB but somehow the definition object does not exist"? In a production it is a penalty to validate for every method call. If it is for the debugging purpose then the code should exist only in debug mode (although I don't know how)

@lfu lfu force-pushed the generic_object_enhancement branch from 61e0fbe to ad8a8ee Compare September 8, 2017 14:04
properties.delete(name.to_s)
save!

val
Copy link
Member

Choose a reason for hiding this comment

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

Instead of creating a local variable you can use tap here to return the result of _property_getter(name)

_property_getter(name).tap do
  properties.delete(name.to_s)
  save!
end

@gmcculloug
Copy link
Member

@lfu Please review this PR description to make sure it is still accurate.

@lfu lfu force-pushed the generic_object_enhancement branch from ad8a8ee to af30158 Compare September 8, 2017 14:50
@gmcculloug gmcculloug requested a review from bzwei September 8, 2017 15:08
@gmcculloug
Copy link
Member

@bzwei PTAL

selected = objs.select { |obj| obj.kind_of?(klass) }
properties[name] = (properties[name] + selected.pluck(:id)).uniq if selected
save
return unless properties_changed?
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to explicitly check for properties_changed??. I think save! internally does the check.

@lfu lfu force-pushed the generic_object_enhancement branch from af30158 to 2009663 Compare September 8, 2017 20:11
@miq-bot
Copy link
Member

miq-bot commented Sep 8, 2017

Checked commits lfu/manageiq@71794cf~...2009663 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 👍

@gmcculloug gmcculloug merged commit 3636064 into ManageIQ:master Sep 8, 2017
@gmcculloug gmcculloug added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 8, 2017
@lfu lfu deleted the generic_object_enhancement branch October 16, 2017 20:15
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.

5 participants