Add a new event group level

[CharlleDaniel]

This PR is able to :

• Add support to add new group levels in the EventStream class.
• Add the warning group_level, that the user can filter warning events on the API.

[CharlleDaniel]

@miq-bot add_reviewer @agrare

[CharlleDaniel]

@miq-bot assign @agrare

[agrare]

I'm not sure who best to review this, maybe @gmcculloug or @lfu ?
I don't know where the event groups are used.

[CharlleDaniel]

@agrare no problem. Thank you 👍

[lfu]

Is & a typo here?
Where are the warning events defined? Otherwise value[:warning] returns nil.

[CharlleDaniel]

hey & not is a typo, this handle new values and avoid the flow be stopped when occurs nil.include?. Also we can define warning events in the settings.yml for each provider and I will open a PR to add warning events in the provider lenovo after this PR be merged.

[CharlleDaniel]

@lfu

[lfu]

@CharlleDaniel Could you please write some test cases to ensure the format of the return value is the same as before?

[CharlleDaniel]

@lfu ok, no problem.

[CharlleDaniel]

@lfu done, the tests were created.

[lfu]

How about this?

  def self.group_and_level(event_type)
    group, value = event_groups.detect { |_k, v| GROUP_LEVELS.any? { |g| v[g]&.include?(event_type) } }
    if group.nil?
      group, level = :other, :detail
    else
      level = GROUP_LEVELS.detect { |g| value[g].include?(event_type) }
    end
    return group, level
  end

[CharlleDaniel]

@lfu This works, but I think that is more complex, cause you need to do one more iteration. So I prefer the another form, but thank you.

[lfu]

It can be simplified as:

  def self.group_and_level(event_type)
    level = :detail # the level is detail as default
    group, _ = event_groups.find do |_k, value|
      GROUP_LEVELS.detect { |lvl| value[lvl]&.include?(event_type) }.tap do |level_found|
        level = level_found if level_found
      end
    end

    group ||= :other
    return group, level
  end

[CharlleDaniel]

I liked this and changed the code to your suggestion, thank you 😄 .

[lfu]

This method used to support passed in group as a string and it returns nil if group is nil.
Is it intentional to change the behavior?

[lfu]

Can we also add test cases here to ensure the behavior is not changed accidentally?

[CharlleDaniel]

This method used to support passed in group as a string and it returns nil if group is nil.
Is it intentional to change the behavior?

@lfu Good point, it is a symbol, but I will add the group.sym again because the group column have a type as string( Line 31 ) and when persisted in the virtual column it is converted as string. Someone can do EventStream.group, receive a string and try get the group_name passing the string as param.

[CharlleDaniel]

Can we also add test cases here to ensure the behavior is not changed accidentally?

I think the tests already cover this case.

[lfu]

@dclarizio @h-kataria Wondering if this PR would have any impact on the timeline UI.

[CharlleDaniel]

@lfu I tested the timeline and everything looks good.

[h-kataria]

@CharlleDaniel @lfu the new Group level "Warning" being added in this PR will not show up in UI unless changes are made in UI code to show/support the new level. Currently timeline options screen only shows Critical and Detail group level events only. This will require some UI work to show events from "Warning" group level.
As a side note, Is this new group level supported by all provider types, some providers might not have any events under the new group.

[miq-bot]

Checked commit CharlleDaniel@be7834d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

[CharlleDaniel]

@lfu Morning, if you not have more suggestion could you to approve this PR 😅 ? I will open a PR adding support to filter warning events on the UI today or tomorrow.

[gmcculloug]

@CharlleDaniel Can you comment on the questions raise here #17611 (comment) ?

[CharlleDaniel]

@gmcculloug and @h-kataria about this comment, all providers are able to use the new group level if they want. Otherwise there are providers that not have events_groups or not need to use this new group level, but the group levels (new and old) not break the provider flow. Also, I will open a PR adding support to filter warning events on the UI today or tomorrow.

[CharlleDaniel]

@gmcculloug morning, if you not have more discussions, could you approve this PR? 😅

[h-kataria]

@CharlleDaniel if a provider does not support the new group level, will the new level still show up in UI, it will not yield any timeline results in that case since there wont be any event groups under that, correct?

[CharlleDaniel]

@h-kataria hey morning,

if a provider does not support the new group level, will the new level still show up in UI, it will not yield any timeline results in that case since there wont be any event groups under that, correct?

Yes, but we can see the same case in categories field, always showing all categories even if provider do not have any event in it.

[h-kataria]

👍