Skip to content

Conversation

@nikhagra-akamai
Copy link
Contributor

Description 📝

Dashboard filters now will be rendered based on dashboard id instead of service type

Changes 🔄

List any change(s) relevant to the reviewer.

  1. Updated filter config to map based on id number instead of service type
  2. All the consumer components using the config will now use dashboard id instead of service type

Scope 🚢

Upon production release, changes in this PR will be visible to:

  • All customers
  • Some customers (e.g. in Beta or Limited Availability)
  • No customers / Not applicable

Target release date 🗓️

26th August

Preview 📷

Note

No difference in the UI after this change.

How to test 🧪

  1. Switch to mock and go to metrics tab
  2. Select dashboard from dashboard dropdown.
  3. You'll notice no difference in the UI compared to this branch and current production branch.
Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 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


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All tests and CI checks are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@nikhagra-akamai nikhagra-akamai requested review from a team as code owners August 5, 2025 12:32
@nikhagra-akamai nikhagra-akamai requested review from cliu-akamai and removed request for a team August 5, 2025 12:32
@nikhagra-akamai nikhagra-akamai requested review from coliu-akamai and mjac0bs and removed request for a team August 5, 2025 12:32
@nikhagra-akamai nikhagra-akamai self-assigned this Aug 5, 2025
>
{showAppliedFilters && (
<CloudPulseAppliedFilterRenderer
dashboardId={dashboard.id}
Copy link
Contributor

@jaalah-akamai jaalah-akamai Aug 6, 2025

Choose a reason for hiding this comment

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

question(non-blocking): Is it not confusing that we have both a dashboard.id and dashboardId? How do we know which to pass into CloudPulseAppliedFilterRenderer?

Copy link
Contributor Author

@nikhagra-akamai nikhagra-akamai Aug 6, 2025

Choose a reason for hiding this comment

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

both are same. If you see here CloudPulseDashboardWithFilters, we are fetching dashboard object for that dashboardId, so at the end both ids will be same only

@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Aug 6, 2025
Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

thank you!


export const FILTER_CONFIG: Readonly<
Map<string, CloudPulseServiceTypeFilterMap>
Map<number, CloudPulseServiceTypeFilterMap>
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (optional, nonblocking): for more type safety, perhaps we could do a union type like type FilterConfigId = 1 | 2 | 3 | 4 and then use it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this. value of Id is coming from API response and based on that filter will be rendered, that's why kept as number to have a fallback logic

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 3 failing tests on test run #9 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
3 Failing702 Passing4 Skipped153m 6s

Details

Failing Tests
SpecTest
qemu-reboot-upgrade-notice.spec.tsCloud Manager Cypress Tests→QEMU reboot upgrade notification » should display maintenance banner in 'Linode' landing page when one or more Linodes get impacted.
restricted-user-details-pages.spec.tsCloud Manager Cypress Tests→restricted user details pages » should disable action elements and buttons in the 'Linodes' details page
linode-storage.spec.tsCloud Manager Cypress Tests→linode storage tab » delete disk

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/notificationsAndEvents/qemu-reboot-upgrade-notice.spec.ts,cypress/e2e/core/account/restricted-user-details-pages.spec.ts,cypress/e2e/core/linodes/linode-storage.spec.ts"

@nikhagra-akamai nikhagra-akamai added Approved Multiple approvals and ready to merge! and removed Ready for Review labels Aug 6, 2025
@jaalah-akamai jaalah-akamai merged commit 787acaf into linode:develop Aug 6, 2025
34 of 35 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Cloud Manager Aug 6, 2025
abailly-akamai pushed a commit to abailly-akamai/manager that referenced this pull request Aug 7, 2025
…rd id instead of service type (linode#12637)

* change: [DI-26319] - Updated logic to render filters based on dashboard id instead of service type

* Removed unused code

* change: [DI-26319] - Updated mock data

* change: [DI-26319] - Updated dashboard id for firewall and nodebalancer

* change: [DI-26319] - Updated logic to render filter

* upcoming: [DI-26319] - Updated mock data

* change: [DI-26319] - Fixed failing test cases

* added changeset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Cloud Pulse

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants