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

Only show visible custom attributes #881

Closed

Conversation

enoodle
Copy link

@enoodle enoodle commented Apr 3, 2017

Based on ManageIQ/manageiq#14611, This will only show custom attributes where visible => true.

Due to CodeClimate's insistence I had to also create a mixin for visible custom attributes and I also included the redact function in it.

@enoodle
Copy link
Author

enoodle commented Apr 3, 2017

@enoodle enoodle force-pushed the only_show_visible_custom_attributes branch 2 times, most recently from 07ae9e8 to 6287e0a Compare April 3, 2017 10:09
end

def textual_miq_custom_attributes
attrs = @record.custom_attributes.where(:visible => true)
Copy link
Contributor

Choose a reason for hiding this comment

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

@record.miq_custom_attributes, not custom_attributes

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, the filtering may be better done in custom_attribute_mixin.rb

@miq-bot
Copy link
Member

miq-bot commented Apr 7, 2017

This pull request is not mergeable. Please rebase and repush.

@enoodle enoodle force-pushed the only_show_visible_custom_attributes branch from 943c3ea to 4293f5e Compare April 9, 2017 12:41
@enoodle enoodle force-pushed the only_show_visible_custom_attributes branch from 4293f5e to afdccc3 Compare April 25, 2017 10:56
@miq-bot
Copy link
Member

miq-bot commented Apr 25, 2017

Checked commit enoodle@afdccc3 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
7 files checked, 0 offenses detected
Everything looks good. ⭐

@simon3z
Copy link
Contributor

simon3z commented May 12, 2017

@enoodle if relevant can you fix travis? Thanks.

@enoodle
Copy link
Author

enoodle commented May 14, 2017

@simon3z This needs the base PR (ManageIQ/manageiq#14611) to be merged for the tests to be green.

@enoodle
Copy link
Author

enoodle commented Jul 17, 2017

With the move to use options per provider, this is not needed anymore.

@enoodle enoodle closed this Jul 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.

5 participants