upcoming: [DI-20935] - Support for tags filter in CloudPulse #11457

ankita-akamai · 2024-12-23T13:56:52Z

Description 📝

Added tags as an optional filter under service type - linode.

Changes 🔄

List any change(s) relevant to the reviewer.

Added tag selection component which is a multi-select.
Added subtitle as 'optional' for tags.
Made tag filter as an optional dependency for resources.
Passed tag as an x-filter to filter resources.
Handled preferences.
Added unit test cases.
Updated mocks for handling tags.

Target release date 🗓️

14-Jan-2025

Preview 📷

Screen.Recording.2024-12-23.at.7.08.29.PM.mov

How to test 🧪

Verification steps

Navigate to monitor tab.
Select a dashboard under service type - linode.
You will be able to see tags filter listed as part of filters.
Select any number of tags, select a region, you will see resources filtered out based on both the filters.
Now, unselect all tags, choose region and resources, you will still be able to observe metrics as tags is an optional filter.

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 unit tests are passing
TypeScript compilation succeeded without errors
Code passes all linting rules

- Added tag filter selection component - Made tag filter as an optional dependency - Passed tag as an x-filter to filter resources - Included exhaustive unit test cases - Updated mocks for handling tags

… from other filters

github-actions · 2024-12-23T14:33:54Z

Coverage Report: ❌
Base Coverage: 86.96%
Current Coverage: 86.94%

jaalah-akamai · 2024-12-27T19:53:50Z

packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.tsx

+  label: string;
+  optional?: boolean;
+  placeholder?: string;
+  resourceType: string | undefined;


Note: For resourceType I would love to type this a little better even if for now we only support linode, volumes, dbaas but that's a separate effort.

jaalah-akamai · 2024-12-27T19:57:13Z

packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.tsx

+      return Array.from(
+        new Set(linodes.flatMap((linode: Linode) => linode.tags))
+      )


Nit: A little cleaner

Suggested change

return Array.from(

new Set(linodes.flatMap((linode: Linode) => linode.tags))

)

return [...new Set(linodes.flatMap((linode: Linode) => linode.tags))]

fyi, this throws error Type 'Set<string>' can only be iterated through when using the '--downlevelIteration' flag or with a '--target' of 'es2015' or higher.

jaalah-akamai · 2024-12-27T19:59:10Z

packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.tsx

+      if (tags && savePreferences) {
+        const defaultTags =
+          defaultValue && Array.isArray(defaultValue)
+            ? defaultValue.map((tag) => String(tag))
+            : [];
+        const tag = tags.filter((tag) =>
+          defaultTags.includes(String(tag.label))
+        );
+        handleTagsChange(tag);
+        setSelectedTags(tag);
+      } else {
+        setSelectedTags([]);
+        handleTagsChange([]);
+      }


Nit: I think it's always best to define the base case first like:

if (!tags || !savePreferences) { setSelectedTags([]); handleTagsChange([]); return; }

Then everything else doesn't need to be wrapped in any if/else block

jaalah-akamai · 2024-12-27T19:59:40Z

packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.tsx

+          defaultValue && Array.isArray(defaultValue)
+            ? defaultValue.map((tag) => String(tag))
+            : [];
+        const tag = tags.filter((tag) =>


Should we renamed tag to filteredTags?

linode-gh-bot · 2024-12-30T12:46:12Z

Cloud Manager UI test results

🔺 1 failing test on test run #3 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
`1 Failing`	`468 Passing`	`2 Skipped`	`108m 7s`

Details

Failing Tests
	Spec	Test
❌	`clone-linode.spec.ts`	clone linode » can clone a Linode from Linode details page

Troubleshooting

Use this command to re-run the failing tests:

yarn cy:run -s "cypress/e2e/core/linodes/clone-linode.spec.ts"

coliu-akamai

✅ confirmed tags select + filtering of the mock data when selecting tags

thank you! Have a few optional comments below

coliu-akamai · 2025-01-02T18:58:14Z

packages/manager/src/mocks/serverHandlers.ts

-          const filteredById = orFilters.some(
+      let filteredLinodes = linodes; // Default to the original linodes in case no filters are applied
+
+      // filter the linodes based on id or region


note - I see this changing how mock data is handled for placement groups (ex no linodes appearing despite the placement group having 9/10 linodes), but probably not too big an issue / a non issue since it's mock data

coliu-akamai · 2025-01-02T19:03:30Z

packages/manager/src/features/CloudPulse/shared/CloudPulseTagsFilter.tsx

@@ -0,0 +1,120 @@
+import { Autocomplete } from '@linode/ui';


should this file be renamed to match the CloudPulseTagsSelect component name?

coliu-akamai · 2025-01-02T19:22:41Z

packages/manager/src/features/CloudPulse/shared/CloudPulseComponentRenderer.test.tsx

+    });
+
+    if (tagProps === undefined) {
+      expect(true).toEqual(false); // fail test


do we need this line / do we want this test to ever fail? (I also wasn't able to get this test to fail unless I purposely set tagProps to undefined)

note - the way our github checks are currently set up, if a unit test fails, that would prevent us from merging a PR, so I'm hesitant about having this here

@coliu-akamai , previously we were checking for undefined like below

expect(tagProps).toBeUndefined();

if we do that proceed to validation below it will introduce a non null assertion, so we have validated explicitly and proceeded with early return and failure

const { getByPlaceholderText } = renderWithTheme( <Grid item sx={{ marginLeft: 2 }} xs> {RenderComponent({ componentKey: 'tags', componentProps: { ...getTagsProperties( { config: tagProps!, // we need to introduce a non null assertion here, so we are dashboard: mockDashboard, isServiceAnalyticsIntegration: false, }, vi.fn() ), }, key: 'tags', })} </Grid> );

Also we need to fail here because we need tagFilter for linode case

cypress · 2025-01-03T17:32:55Z

Cloud Manager E2E Run #7035

Run Properties: Failed #7035 • 47758c6476: upcoming: [DI-20935] - Support for tags filter in CloudPulse (#11457)

Project	`Cloud Manager E2E`
Branch Review	`develop`
Run status	`Failed #7035`
Run duration	`29m 01s`
Commit	`47758c6476: upcoming: [DI-20935] - Support for tags filter in CloudPulse (#11457)`
Committer	`Ankita`
View all properties for this run ↗︎

Test results
Failures	`1`
Flaky	`2`
Pending	`2`
Skipped	`0`
Passing	`470`
View all changes introduced in this branch ↗︎

Tests for review

cypress/e2e/core/linodes/linode-config.spec.ts • 1 failed test

View Output Video

Test		Artifacts
Linode Config management > End-to-End > Clones a config		`Screenshots` `Video`

linodes/resize-linode.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
resize linode > resizes a linode by decreasing size		`Screenshots` `Video`

parentChild/account-switching.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
Parent/Child account switching > From Parent to Child > can switch from Parent account user to Proxy account user from Billing page		`Screenshots` `Video`

…11457) * upcoming: [DI-20935] - Add support for tags filter - Added tag filter selection component - Made tag filter as an optional dependency - Passed tag as an x-filter to filter resources - Included exhaustive unit test cases - Updated mocks for handling tags * upcoming: [DI-20935] - updated mocks * upcoming: [DI-20935] - updated UTs * upcoming: [DI-20935] - small fix * upcoming: [DI-20935] - small fix * upcoming: [DI-20935] - add required field for custom select, update UT * upcoming: [DI-20935] - PR comments * upcoming: [DI-20935] - PR comments * upcoming: [DI-20935] - small fix * upcoming: [DI-20935] - test case fix * upcoming: [DI-20935] - Add optional subtitle on tags, remove required from other filters * upcoming: [DI-20935] - PR comments * upcoming: [DI-20935] - small fix * upcoming: [DI-20935] - small fix * upcoming: [DI-20935] - small fix * upcoming: [DI-20935] - PR Comments * upcoming: [DI-20935] - prop drill optional * upcoming: [DI-20935] - linting fix * upcoming: [DI-20935] - mock enhancement * upcoming: [DI-20935] - small preferences fix * upcoming: [DI-20935] - Add changeset * upcoming: [DI-20935] - resolve pr comments

ankita-akamai added 20 commits December 23, 2024 18:21

upcoming: [DI-20935] - Add support for tags filter

6504b68

- Added tag filter selection component - Made tag filter as an optional dependency - Passed tag as an x-filter to filter resources - Included exhaustive unit test cases - Updated mocks for handling tags

upcoming: [DI-20935] - updated mocks

d915cef

upcoming: [DI-20935] - updated UTs

eba5695

upcoming: [DI-20935] - small fix

e6b69fd

upcoming: [DI-20935] - small fix

4676cb1

upcoming: [DI-20935] - add required field for custom select, update UT

48f14bd

upcoming: [DI-20935] - PR comments

a7c2053

upcoming: [DI-20935] - PR comments

52102db

upcoming: [DI-20935] - small fix

45e76d1

upcoming: [DI-20935] - test case fix

52e5da4

upcoming: [DI-20935] - Add optional subtitle on tags, remove required…

eecfd7d

… from other filters

upcoming: [DI-20935] - PR comments

1d6f6f4

upcoming: [DI-20935] - small fix

94bfb45

upcoming: [DI-20935] - small fix

f14c968

upcoming: [DI-20935] - small fix

9e28a0c

upcoming: [DI-20935] - PR Comments

d553c60

upcoming: [DI-20935] - prop drill optional

1839b8e

upcoming: [DI-20935] - linting fix

694b422

upcoming: [DI-20935] - mock enhancement

56fa766

upcoming: [DI-20935] - small preferences fix

eb4d05d

ankita-akamai requested a review from a team as a code owner December 23, 2024 13:56

ankita-akamai requested review from carrillo-erik and cpathipa and removed request for a team December 23, 2024 13:56

upcoming: [DI-20935] - Add changeset

76172ea

ankita-akamai changed the title ~~upcoming: [DI-20935] - Support for tags filter~~ upcoming: [DI-20935] - Support for tags filter in CloudPulse Dec 23, 2024

ankita-akamai self-assigned this Dec 23, 2024

jaalah-akamai self-requested a review December 26, 2024 13:43

jaalah-akamai approved these changes Dec 27, 2024

View reviewed changes

jaalah-akamai added Add'tl Approval Needed Waiting on another approval! Cloud Pulse labels Dec 27, 2024

upcoming: [DI-20935] - resolve pr comments

1332dd1

coliu-akamai approved these changes Jan 2, 2025

View reviewed changes

cpathipa approved these changes Jan 2, 2025

View reviewed changes

jaalah-akamai added Approved Multiple approvals and ready to merge! Question and removed Add'tl Approval Needed Waiting on another approval! labels Jan 3, 2025

jaalah-akamai merged commit 47758c6 into linode:develop Jan 3, 2025
21 of 23 checks passed

This was referenced Jan 8, 2025

Release v1.134.0 - release → staging #11494

Closed

Release v1.134.0 - release → staging #11497

Merged

hana-akamai mentioned this pull request Jan 9, 2025

Release v1.134.0 - release → staging #11503

Merged

jaalah-akamai mentioned this pull request Jan 14, 2025

Release v1.134.0 - staging → master #11519

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upcoming: [DI-20935] - Support for tags filter in CloudPulse #11457

upcoming: [DI-20935] - Support for tags filter in CloudPulse #11457

ankita-akamai commented Dec 23, 2024

github-actions bot commented Dec 23, 2024 •

edited

Loading

jaalah-akamai Dec 27, 2024

jaalah-akamai Dec 27, 2024

ankita-akamai Dec 30, 2024

jaalah-akamai Dec 27, 2024

jaalah-akamai Dec 27, 2024

linode-gh-bot commented Dec 30, 2024

coliu-akamai left a comment

coliu-akamai Jan 2, 2025

coliu-akamai Jan 2, 2025

coliu-akamai Jan 2, 2025

venkymano-akamai Jan 3, 2025 •

edited

Loading

cypress bot commented Jan 3, 2025

upcoming: [DI-20935] - Support for tags filter in CloudPulse #11457

upcoming: [DI-20935] - Support for tags filter in CloudPulse #11457

Conversation

ankita-akamai commented Dec 23, 2024

Description 📝

Changes 🔄

Target release date 🗓️

Preview 📷

How to test 🧪

Verification steps

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

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

github-actions bot commented Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linode-gh-bot commented Dec 30, 2024

Cloud Manager UI test results

🔺 1 failing test on test run #3 ↗︎

Details

Troubleshooting

coliu-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

venkymano-akamai Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

cypress bot commented Jan 3, 2025

Tests for review

github-actions bot commented Dec 23, 2024 •

edited

Loading

venkymano-akamai Jan 3, 2025 •

edited

Loading