Use type names directly from GenericObjectDefinition object

[mzazrivec]

This change is to make sure API uses pretty (i.e. human readable) type names directly from the GenericObjectDefinition object, rather than from the locale/en.yml file.

This PR depends on ManageIQ/manageiq#16563

[mzazrivec]

The core PR has been merged, this one is ready for review.

[imtayadeway]

Does GenericObjectDefinition::TYPE_NAMES.stringify_keys.invert.to_a.sort suffice here?

Was this constant extracted for this use only? These transformations seem to suggest that its data structure is a poor fit 😉

[mzazrivec]

Yes, right now this is the only place where GenericObjectDefinition::TYPE_NAMES is being used.

What you're suggesting definitely looks better.

[miq-bot]

Checked commit mzazrivec@050d107 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

[imtayadeway]

This LGTM. Perhaps we can look into making this constant something more like:

TYPE_NAMES = [
    [N_('Boolean'), "boolean"],
    [N_('Date/Time'), "datetime"],
    [N_('Float'), "float"],
    [N_('Integer'), "integer"],
    [N_('String'), "string"],
    [N_('Time'), "time"]
  ].freeze

in the future, which would simplify this to:

GenericObjectDefinition::TYPE_NAMES.sort

[abellotti]

LGTM!! Thanks @mzazrivec for the enhancement.

[mzazrivec]

@imtayadeway Actually, I'd rather keep the model structures as they are. I think the hash used there is just fine. Yes, in the API there's this data transformation being done, which is not exactly pretty.

But if you look at the UI end consuming this data, the author of the code, for reasons I don't quite comprehend, decided to do the same transformation backwards: invert the fields + back to hash.

So I think the true fix is to avoid this data transformation back and forth, just send the hash over API, consume the same hash on the javascript end and that's the end of it. I'd like to do that in separate PRs though.

[mzazrivec]

@imtayadeway #238

[AparnaKarve]

the author of the code, for reasons I don't quite comprehend, decided to do the same transformation backwards: invert the fields + back to hash.

Could it be that the author of the code initially intended to have an array across the board (API + UI), and later had to change it to a hash in the UI due to some reason?

[mzazrivec]

@AparnaKarve Are you telling it as a matter of fact, or are you just guessing?

[AparnaKarve]

@mzazrivec I was alluding to the fact that I could have answered whatever questions you had and you didn't have to speculate what the author of the code was trying to do.

In retrospect, and after a second look at the JS code, I think the hash conversion PR #238 wasn't really necessary - a few adjustments in the JS was what was required.

Hashes/Arrays - they both are fine, so I think we are good.

[mzazrivec]

I wasn't really speculating. I simply stated that I don't understand the reasons for doing things this way.

You're welcome to explain your point on #238 in that PR.