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

upcoming: [DI-21814] - ACLP UI - DBaaS instances order by label #11224

Closed

Conversation

ankita-akamai
Copy link
Contributor

@ankita-akamai ankita-akamai commented Nov 7, 2024

Description 📝

Added one new filter - orderFilter for fetching instances in dbass servictype.

Changes 🔄

  • In our resource selection component, OrderFilter is added into the request for fetching databases instances in dbass serviceType.

Target release date 🗓️

12-11-2024

Preview 📷

Before After
image image

How to test 🧪

Verification steps

  1. Login as a mock user.
  2. Navigate to monitor tab.
  3. Select a DBasS dashboard.
  4. Open inspect, later select all the filters.
  5. In networks tab, go to database instance api call, you should be able to see order_by filter in headers.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@ankita-akamai ankita-akamai requested a review from a team as a code owner November 7, 2024 12:23
@ankita-akamai ankita-akamai requested review from bnussman-akamai and abailly-akamai and removed request for a team November 7, 2024 12:23
@ankita-akamai ankita-akamai changed the title upcoming: [DI-21814] - ACLP UI - Resources components reset fix through filter upcoming: [DI-21814] - ACLP UI - DBaaS instances order by label Nov 7, 2024
@ankita-akamai ankita-akamai changed the base branch from develop to release-v1.132.0 November 7, 2024 13:50
@ankita-akamai ankita-akamai changed the base branch from release-v1.132.0 to develop November 7, 2024 13:50
@ankita-akamai ankita-akamai changed the base branch from develop to release-v1.132.0 November 7, 2024 13:57
@ankita-akamai ankita-akamai changed the base branch from release-v1.132.0 to develop November 7, 2024 13:58
Comment on lines 46 to +51
const platformFilter =
resourceType === 'dbaas' ? { platform: 'rdbms-default' } : {};

const orderFilter: Partial<Filter> =
resourceType === 'dbaas' ? { '+order': 'asc', '+order_by': 'label' } : {};

Copy link
Member

Choose a reason for hiding this comment

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

Optional: we may want to consider something like the following so that it will be easier to apply different filters to different entities once more are added

const resourseFilterMap: Record<string, Filter> = {
  dbaas: { '+order': 'asc', '+order_by': 'label', platform: 'rdbms-default' },
};

Copy link
Contributor

Choose a reason for hiding this comment

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

I like that too ++

Comment on lines 46 to +51
const platformFilter =
resourceType === 'dbaas' ? { platform: 'rdbms-default' } : {};

const orderFilter: Partial<Filter> =
resourceType === 'dbaas' ? { '+order': 'asc', '+order_by': 'label' } : {};

Copy link
Contributor

Choose a reason for hiding this comment

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

I like that too ++

@@ -46,16 +46,21 @@ export const CloudPulseResourcesSelect = React.memo(
const platformFilter =
resourceType === 'dbaas' ? { platform: 'rdbms-default' } : {};

const orderFilter: Partial<Filter> =
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need Partial

Suggested change
const orderFilter: Partial<Filter> =
const orderFilter: Filter =

@jaalah-akamai
Copy link
Contributor

@ankitaakamai let's make sure we make the changes requested in #11226

@venkymano-akamai
Copy link
Contributor

@jaalah-akamai , added the suggestions from @bnussman-akamai in the release PR https://github.com/linode/manager/pull/11226/commits

Now it is ready for merging

We can close this PR, anyway the release PR gets merged to develop as well.

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.

6 participants