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

Features to support Event explorer De-explorization #20947

Merged
merged 2 commits into from
Feb 1, 2021

Conversation

h-kataria
Copy link
Contributor

Added miq_event_show_list and miq_event_show product features for Events

ManageIQ/manageiq-ui-classic#6819

Describe the rationale and use case for this pull request. Provide any background, examples, and images that provide further information to accurately describe what it is that you are adding to the repo. Add subsections as necessary to organize and feel free to link and reference other PRs as necessary, but also include them in the links section below as a quick reference.

Guidelines:

  • Keep Pull Request titles short and to the point, ideally under 72 characters
  • Provide as much context/info in the description as necessary to get the reviewer up to the same domain knowledge level as yourself
  • Keep code changes as short as possible and implementing a single feature/fix/refactoring, when possible
  • Please, make reviewers work easier by checking, if you are not introducing any technical debt: https://github.com/ManageIQ/guides/blob/master/reviewers_guidelines.md

Links [Optional]

Steps for Testing/QA [Optional]

If there are any manual steps that you would like the reviewer(s) to take to verify your changes, please describe in detail the steps to reproduce the features added by the pull request, or the bug before and after the change.

@h-kataria h-kataria requested a review from skateman January 11, 2021 23:42
h-kataria added a commit to h-kataria/manageiq-schema that referenced this pull request Jan 12, 2021
If any of the users had their startpage set to Policy Events explorer screen, this migration sets it to non-explorer version screen of Policy Events list view.

UI PR ManageIQ/manageiq-ui-classic#7569
Core PR ManageIQ/manageiq#20947
Added `miq_event_show_list` and `miq_event_show` product features for Events

ManageIQ/manageiq-ui-classic#6819
@h-kataria h-kataria force-pushed the event_de-explorization branch from 93825dd to dc47fda Compare January 17, 2021 04:02
@miq-bot
Copy link
Member

miq-bot commented Jan 18, 2021

Checked commits h-kataria/manageiq@dc47fda~...8c3ab24 with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint
2 files checked, 7 offenses detected

app/models/miq_policy.rb

@h-kataria
Copy link
Contributor Author

@gtanzillo please review, applied suggested changes to get filtered Events list.

@@ -355,6 +355,22 @@ def self.all_policy_events
MiqEventDefinition.all_events.select { |e| !e.memberof.empty? && !EVENT_GROUPS_EXCLUDED.include?(e.memberof.first.name) }
end

def self.all_policy_events_filter
# Todo Convert to SQL if possible
Copy link
Member

Choose a reason for hiding this comment

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

@kbrock Can you take a look at this expression we came up with to get the list of seeded policy event definitions and see if it can be done in pure SQL? This new code creates a filter expression that can be used in a view, driven by reporting, get the same output this method returns.

Copy link
Member

Choose a reason for hiding this comment

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

@kbrock Can you look/comment on ^^ when you get a chance, thanks

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

this is expensive to fix.
merging as is should work better if we fixed the relationships sets

@@ -14,6 +14,11 @@ class MiqEventDefinition < ApplicationRecord
has_many :miq_policy_contents
has_many :policy_events

virtual_column :event_group_name, :type => :string
Copy link
Member

Choose a reason for hiding this comment

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

fyi for those following
acts_as_miq_set_member introduced :memberof

  1. make memberof an association
  2. introduce has_one for the association (basically same as Move ftp upload methods to file depot #1 but with a has_one instead)
  3. turn into a delegate
has_one :event_group ... memberof
  viritual_delegate :name, :prefix_true, :allow_nil => true, :to => "event_group"

@gtanzillo gtanzillo merged commit cf7f6e4 into ManageIQ:master Feb 1, 2021
@chessbyte
Copy link
Member

why did we merge a PR prefixed with [WIP]?

@h-kataria h-kataria removed the wip label Feb 1, 2021
@h-kataria h-kataria changed the title [WIP] Features to support Event explorer De-explorization Features to support Event explorer De-explorization Feb 1, 2021
h-kataria added a commit to h-kataria/manageiq-schema that referenced this pull request Feb 5, 2021
If any of the users had their startpage set to Policy Events explorer screen, this migration sets it to non-explorer version screen of Policy Events list view.

UI PR ManageIQ/manageiq-ui-classic#7569
Core PR ManageIQ/manageiq#20947
h-kataria added a commit to h-kataria/manageiq-schema that referenced this pull request Feb 8, 2021
If any of the users had their startpage set to Policy Events explorer screen, this migration sets it to non-explorer version screen of Policy Events list view.

UI PR ManageIQ/manageiq-ui-classic#7569
Core PR ManageIQ/manageiq#20947
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