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.

Sign up for GitHub

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

Jump to bottom

Extend Catalog filtering #2009

Merged
merged 6 commits into from
Nov 24, 2023
Merged

Extend Catalog filtering #2009

merged 6 commits into from
Nov 24, 2023

Conversation

nelsonkopliku
Copy link
Member

@nelsonkopliku nelsonkopliku commented Nov 17, 2023 •
edited

Description

This PR adds extra filters to the catalog. Besides the already present provider, the following have been added:

  • target type
  • cluster type
  • ensa version

catalog-filtering-no-ensa

Notes:

  • first two commits are mainly about making some useful functions/constants/components a bit more reusable
  • logic about filter dependencies (ie enable cluster type only if target type was selected) will follow up, with more extensive e2e testing

How was this tested?

Automated tests and storybook.
https://2009.prenv.trento.suse.com/catalog

@nelsonkopliku nelsonkopliku force-pushed the catalog-filtering branch 2 times, most recently from e157505 to 02fd741 Compare November 20, 2023 15:11
@nelsonkopliku nelsonkopliku changed the base branch from main to generic-selection-component November 20, 2023 15:12
@nelsonkopliku nelsonkopliku changed the title wip Extend Catalog filtering Nov 20, 2023
@nelsonkopliku nelsonkopliku self-assigned this Nov 20, 2023
@nelsonkopliku nelsonkopliku added enhancement New feature or request user story ux javascript Pull requests that update Javascript code labels Nov 20, 2023
@nelsonkopliku nelsonkopliku force-pushed the catalog-filtering branch 2 times, most recently from 03d99dc to 7470720 Compare November 20, 2023 16:21
Base automatically changed from generic-selection-component to main November 21, 2023 15:06
@nelsonkopliku nelsonkopliku force-pushed the catalog-filtering branch 5 times, most recently from 023dba9 to abd06a0 Compare November 22, 2023 13:09
@nelsonkopliku nelsonkopliku added the env Create an ephimeral environment for the pr branch label Nov 22, 2023
@nelsonkopliku nelsonkopliku marked this pull request as ready for review November 22, 2023 13:09
abravosuse
abravosuse suggested changes Nov 22, 2023
View reviewed changes
Copy link
Contributor

@abravosuse abravosuse left a comment •
edited

Choose a reason for hiding this comment

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

Feeding the ensa_version to the check from the web component won't work for multi-sid clusters. The Checks Team (Eike) is working right now on doing it from the agent instead, with the sapcontrol gatherer. Which means that, when it comes to ASCS/ERS clusters, we will no longer have ENSA2 and ENSA1 checks. We will have checks, valid for both ENSA versions, where the logic will perform one evaluation of the expectations or another -when applicable- based on the ENSA version of a managed system. Because of this, the need for a ENSA filter in the UI decays.

Therefore the proposed changes in this PR are OK for me, but we need to leave the ENSA filter out. Everything else is OK.

abravosuse
abravosuse approved these changes Nov 23, 2023
View reviewed changes
Copy link
Contributor

@abravosuse abravosuse left a comment

Choose a reason for hiding this comment

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

LGTM Nelson!

jagabomb
jagabomb approved these changes Nov 23, 2023
View reviewed changes
Copy link
Contributor

@jagabomb jagabomb left a comment

Choose a reason for hiding this comment

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

LGTM!

dottorblaster
dottorblaster requested changes Nov 23, 2023
View reviewed changes
Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

Some things to adjust and then we can merge 👍

assets/js/components/TargetIcon/TargetIcon.jsx Outdated Show resolved Hide resolved
assets/js/components/TargetIcon/TargetIcon.jsx Outdated Show resolved Hide resolved
assets/js/components/ChecksCatalog/ChecksCatalogPage.jsx Outdated Show resolved Hide resolved
@nelsonkopliku
Copy link
Member Author

@dottorblaster thanks! here we go 54d3e02 and 9059064

dottorblaster
dottorblaster approved these changes Nov 24, 2023
View reviewed changes
Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

LGTM

@nelsonkopliku nelsonkopliku merged commit 8315019 into main Nov 24, 2023
26 checks passed
@nelsonkopliku nelsonkopliku deleted the catalog-filtering branch November 24, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dottorblaster dottorblaster dottorblaster approved these changes

@jagabomb jagabomb jagabomb approved these changes

@abravosuse abravosuse abravosuse approved these changes

@jamie-suse jamie-suse Awaiting requested review from jamie-suse

Assignees

@nelsonkopliku nelsonkopliku

Labels
enhancement New feature or request env Create an ephimeral environment for the pr branch javascript Pull requests that update Javascript code user story ux
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nelsonkopliku @dottorblaster @jagabomb @abravosuse