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

[WIP] Topology - unify and deduplicate entity_status #2355

Closed
wants to merge 4 commits into from
Closed

[WIP] Topology - unify and deduplicate entity_status #2355

wants to merge 4 commits into from

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Oct 11, 2017

wip

(should be done - todo extract desc from commits & test)

…ass in addition to text

Right now, the visual class is being computed by TopologyService.getItemStatusClass in js, based on the string from the server.

This does not work if the string is translated - no i18n for statuses or no visual class.

This makes all topology services use entity_status (Middleware didn't), and set it via set_entity_status, providing the class when available.

It does not remove the JS logic yet, since these values can also come from the DB.
   * always use case
   * don't repeat "Unknown" for every other condition - settle on `nil` meaning Unknown
   * use default authentication status for all managers, not the first authentication (and use ExtManagementSystem instead of listing each kind)
   * prefer `x.try(:capitalize)` over `x ? x.capitalize : "Unknown"` and `x.capitalize if x`
   * prefer `x.foo if x` over `x ? x.foo : "Unknown"`
   * move all entity types mentioned in multiple topologies to TopologyService
this makes us set `:status => _('OK'), :status_class => 'success'` instead of just `:status => 'OK'` for all the statuses which were a string constant

this does not attempt to read DB data
@himdel himdel added the wip label Oct 11, 2017
so we can actually try to translate the strings after determining the class
@miq-bot
Copy link
Member

miq-bot commented Oct 12, 2017

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/bff73544fa3100da9fabd9039d6484c5e1c8845e~...942c2230fab1525df7abeb702ac226dc7042c031 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
7 files checked, 1 offense detected

app/services/topology_service.rb

@miq-bot
Copy link
Member

miq-bot commented Oct 13, 2017

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

@miq-bot
Copy link
Member

miq-bot commented Apr 23, 2018

This pull request has been automatically closed because it has not been updated for at least 6 months.

Feel free to reopen this pull request if these changes are still valid.

Thank you for all your contributions!

@miq-bot miq-bot closed this Apr 23, 2018
@himdel himdel deleted the topology-status branch April 23, 2018 12:05
@himdel himdel restored the topology-status branch April 23, 2018 12:05
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.

2 participants