From d495fd7edbf96f262aaf950fa9fe914cd2db0967 Mon Sep 17 00:00:00 2001 From: Connie Liu <139280159+coliu-akamai@users.noreply.github.com> Date: Wed, 27 Nov 2024 10:00:10 -0500 Subject: [PATCH 1/5] test: [M3-8836] - Add test to create a mock accelerated Linode (#11327) * accelerated linode * update comment explanation * remove .only oops * fix comment typo * Added changeset: Add test to create a mock accelerated Linode * update comments --- .../pr-11327-tests-1732571554878.md | 5 + .../e2e/core/linodes/create-linode.spec.ts | 122 ++++++++++++++++-- 2 files changed, 117 insertions(+), 10 deletions(-) create mode 100644 packages/manager/.changeset/pr-11327-tests-1732571554878.md diff --git a/packages/manager/.changeset/pr-11327-tests-1732571554878.md b/packages/manager/.changeset/pr-11327-tests-1732571554878.md new file mode 100644 index 00000000000..d9a98df67a1 --- /dev/null +++ b/packages/manager/.changeset/pr-11327-tests-1732571554878.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Tests +--- + +Add test to create a mock accelerated Linode ([#11327](https://github.com/linode/manager/pull/11327)) diff --git a/packages/manager/cypress/e2e/core/linodes/create-linode.spec.ts b/packages/manager/cypress/e2e/core/linodes/create-linode.spec.ts index f57807153d6..fbe81f6b9c6 100644 --- a/packages/manager/cypress/e2e/core/linodes/create-linode.spec.ts +++ b/packages/manager/cypress/e2e/core/linodes/create-linode.spec.ts @@ -12,13 +12,20 @@ import { authenticate } from 'support/api/authentication'; import { interceptCreateLinode, mockCreateLinodeError, + mockCreateLinode, + mockGetLinodeDisks, + mockGetLinodeType, + mockGetLinodeTypes, + mockGetLinodeVolumes, } from 'support/intercepts/linodes'; import { interceptGetProfile } from 'support/intercepts/profile'; import { Region, VLAN, Config, Disk } from '@linode/api-v4'; import { getRegionById } from 'support/util/regions'; import { + accountFactory, linodeFactory, linodeConfigFactory, + linodeTypeFactory, VLANFactory, vpcFactory, subnetFactory, @@ -27,18 +34,11 @@ import { LinodeConfigInterfaceFactoryWithVPC, } from 'src/factories'; import { dcPricingMockLinodeTypes } from 'support/constants/dc-specific-pricing'; -import { - mockGetLinodeType, - mockGetLinodeTypes, -} from 'support/intercepts/linodes'; +import { mockGetAccount } from 'support/intercepts/account'; +import { mockAppendFeatureFlags } from 'support/intercepts/feature-flags'; import { mockGetRegions } from 'support/intercepts/regions'; import { mockGetVLANs } from 'support/intercepts/vlans'; import { mockGetVPC, mockGetVPCs } from 'support/intercepts/vpc'; -import { - mockCreateLinode, - mockGetLinodeDisks, - mockGetLinodeVolumes, -} from 'support/intercepts/linodes'; import { mockGetLinodeConfigs } from 'support/intercepts/configs'; import { fbtClick, @@ -47,7 +47,7 @@ import { getVisible, containsVisible, } from 'support/helpers'; -import {} from 'support/helpers'; + let username: string; authenticate(); @@ -85,6 +85,7 @@ describe('Create Linode', () => { planId: 'g7-premium-2', }, // TODO Include GPU plan types. + // TODO Include Accelerated plan types (when they're no longer as restricted) ].forEach((planConfig) => { /* * - Parameterized end-to-end test to create a Linode for each plan type. @@ -170,6 +171,107 @@ describe('Create Linode', () => { }); }); + // Mocks creating an accelerated Linode due to accelerated linodes currently having limited deployment availability + // TODO: eventually transition this to an e2e test (in the above test) + it('creates a mock accelerated Linode and confirms response', () => { + // Create mocks + const linodeLabel = randomLabel(); + const mockLinode = linodeFactory.build({ + label: linodeLabel, + specs: { + accelerated_devices: 2, + disk: 51200, + gpus: 0, + memory: 2048, + transfer: 2000, + vcpus: 1, + }, + type: 'accelerated-1', + }); + const mockAcceleratedType = [ + linodeTypeFactory.build({ + id: 'accelerated-1', + label: 'accelerated-1', + class: 'accelerated', + }), + ]; + const mockRegions = [ + regionFactory.build({ + capabilities: ['Linodes', 'Kubernetes', 'NETINT Quadra T1U'], + id: 'us-east', + label: 'Newark, NJ', + }), + ]; + const linodeRegion = mockRegions[0]; + + // Create request intercepts + mockGetAccount( + accountFactory.build({ + capabilities: ['NETINT Quadra T1U'], + }) + ).as('getAccount'); + mockAppendFeatureFlags({ + acceleratedPlans: { + linodePlans: true, + lkePlans: false, + }, + }).as('getFeatureFlags'); + mockGetRegions(mockRegions).as('getRegions'); + mockGetLinodeTypes([...mockAcceleratedType]).as('getLinodeTypes'); + mockCreateLinode(mockLinode).as('createLinode'); + + cy.visitWithLogin('/linodes/create'); + cy.wait([ + '@getRegions', + '@getLinodeTypes', + '@getAccount', + '@getFeatureFlags', + ]); + + // Set Linode label, OS, plan type, password, etc. + linodeCreatePage.setLabel(linodeLabel); + linodeCreatePage.selectImage('Debian 11'); + linodeCreatePage.selectRegionById(linodeRegion.id); + linodeCreatePage.selectPlan('Accelerated', mockAcceleratedType[0].label); + linodeCreatePage.setRootPassword(randomString(32)); + + // Confirm information in summary is shown as expected. + cy.get('[data-qa-linode-create-summary]') + .scrollIntoView() + .within(() => { + cy.findByText('Debian 11').should('be.visible'); + cy.findByText(`US, ${linodeRegion.label}`).should('be.visible'); + cy.findByText(mockAcceleratedType[0].label).should('be.visible'); + }); + + // Create Linode and confirm it's provisioned as expected. + ui.button + .findByTitle('Create Linode') + .should('be.visible') + .should('be.enabled') + .click(); + + cy.wait('@createLinode').then((xhr) => { + const requestPayload = xhr.request.body; + const responsePayload = xhr.response?.body; + + // Confirm that API request and response contain expected data + expect(requestPayload['label']).to.equal(linodeLabel); + expect(requestPayload['region']).to.equal(linodeRegion.id); + expect(requestPayload['type']).to.equal(mockAcceleratedType[0].id); + + expect(responsePayload['label']).to.equal(linodeLabel); + expect(responsePayload['region']).to.equal(linodeRegion.id); + expect(responsePayload['type']).to.equal(mockAcceleratedType[0].id); + + // Accelerated linodes: Confirm accelerated_devices value is returned as expected + expect(responsePayload['specs']).has.property('accelerated_devices', 2); + + // Confirm that Cloud redirects to details page + cy.url().should('endWith', `/linodes/${responsePayload['id']}`); + }); + }); + it('adds an SSH key to the linode during create flow', () => { const rootpass = randomString(32); const sshPublicKeyLabel = randomLabel(); From f6906336ae22252ae93cda37a1105b173cf1ded3 Mon Sep 17 00:00:00 2001 From: Alban Bailly <130582365+abailly-akamai@users.noreply.github.com> Date: Wed, 27 Nov 2024 11:13:42 -0500 Subject: [PATCH 2/5] feat: [M3-8338] - Placement Groups UI updates during migrations (#11261) * Initial commit: new types * Introducing logic to the PG linode row * update linode status * invalidation and improved UI * Added changeset: Improve Placement Groups UI during Linode Migrations * Added changeset: Placement Groups migrations Types * Update packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx Co-authored-by: Connie Liu <139280159+coliu-akamai@users.noreply.github.com> * Update packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx Co-authored-by: Connie Liu <139280159+coliu-akamai@users.noreply.github.com> --------- Co-authored-by: Connie Liu <139280159+coliu-akamai@users.noreply.github.com> --- .../pr-11261-added-1732225555236.md | 5 ++ packages/api-v4/src/linodes/types.ts | 4 +- packages/api-v4/src/placement-groups/types.ts | 17 ++-- .../pr-11261-changed-1732225439368.md | 5 ++ packages/manager/src/__data__/linodes.ts | 4 + packages/manager/src/factories/linodes.ts | 20 +++-- .../manager/src/factories/placementGroups.ts | 10 +-- .../Linodes/MigrateLinode/ConfigureForm.tsx | 2 + .../PlacementGroupsLinodesTableRow.tsx | 81 +++++++++++++++++-- .../src/features/PlacementGroups/constants.ts | 7 ++ .../manager/src/hooks/useEventHandlers.ts | 5 ++ .../presets/crud/handlers/placementGroups.ts | 1 + .../manager/src/queries/placementGroups.ts | 61 ++++++++++++++ 13 files changed, 197 insertions(+), 25 deletions(-) create mode 100644 packages/api-v4/.changeset/pr-11261-added-1732225555236.md create mode 100644 packages/manager/.changeset/pr-11261-changed-1732225439368.md diff --git a/packages/api-v4/.changeset/pr-11261-added-1732225555236.md b/packages/api-v4/.changeset/pr-11261-added-1732225555236.md new file mode 100644 index 00000000000..0ec0038ff54 --- /dev/null +++ b/packages/api-v4/.changeset/pr-11261-added-1732225555236.md @@ -0,0 +1,5 @@ +--- +"@linode/api-v4": Added +--- + +Placement Groups migrations Types ([#11261](https://github.com/linode/manager/pull/11261)) diff --git a/packages/api-v4/src/linodes/types.ts b/packages/api-v4/src/linodes/types.ts index 7c70fcb4d07..6f3d94caad3 100644 --- a/packages/api-v4/src/linodes/types.ts +++ b/packages/api-v4/src/linodes/types.ts @@ -1,7 +1,7 @@ import type { Region, RegionSite } from '../regions'; import type { IPAddress, IPRange } from '../networking/types'; import type { SSHKey } from '../profile/types'; -import type { PlacementGroupPayload } from '../placement-groups/types'; +import type { LinodePlacementGroupPayload } from '../placement-groups/types'; export type Hypervisor = 'kvm' | 'zen'; @@ -30,7 +30,7 @@ export interface Linode { ipv6: string | null; label: string; lke_cluster_id: number | null; - placement_group?: PlacementGroupPayload; // If not in a placement group, this will be excluded from the response. + placement_group?: LinodePlacementGroupPayload; // If not in a placement group, this will be excluded from the response. type: string | null; status: LinodeStatus; updated: string; diff --git a/packages/api-v4/src/placement-groups/types.ts b/packages/api-v4/src/placement-groups/types.ts index 4b79bba15fe..197a28fe44d 100644 --- a/packages/api-v4/src/placement-groups/types.ts +++ b/packages/api-v4/src/placement-groups/types.ts @@ -24,15 +24,22 @@ export interface PlacementGroup { is_compliant: boolean; }[]; placement_group_policy: PlacementGroupPolicy; + migrations: { + inbound?: Array<{ linode_id: number }>; + outbound?: Array<{ linode_id: number }>; + } | null; } -export type PlacementGroupPayload = Pick< - PlacementGroup, - 'id' | 'label' | 'placement_group_type' | 'placement_group_policy' ->; +export interface LinodePlacementGroupPayload + extends Pick< + PlacementGroup, + 'id' | 'label' | 'placement_group_type' | 'placement_group_policy' + > { + migrating_to: number | null; +} export interface CreatePlacementGroupPayload - extends Omit { + extends Omit { region: Region['id']; } diff --git a/packages/manager/.changeset/pr-11261-changed-1732225439368.md b/packages/manager/.changeset/pr-11261-changed-1732225439368.md new file mode 100644 index 00000000000..c2f4450cb07 --- /dev/null +++ b/packages/manager/.changeset/pr-11261-changed-1732225439368.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Changed +--- + +Improve Placement Groups UI during Linode Migrations ([#11261](https://github.com/linode/manager/pull/11261)) diff --git a/packages/manager/src/__data__/linodes.ts b/packages/manager/src/__data__/linodes.ts index b3fa200b66a..cf156461fa9 100644 --- a/packages/manager/src/__data__/linodes.ts +++ b/packages/manager/src/__data__/linodes.ts @@ -28,6 +28,7 @@ export const linode1: Linode = { placement_group: { id: 1, label: 'pg-1', + migrating_to: null, placement_group_policy: 'strict', placement_group_type: 'anti_affinity:local', }, @@ -76,6 +77,7 @@ export const linode2: Linode = { placement_group: { id: 1, label: 'pg-1', + migrating_to: null, placement_group_policy: 'strict', placement_group_type: 'anti_affinity:local', }, @@ -124,6 +126,7 @@ export const linode3: Linode = { placement_group: { id: 1, label: 'pg-1', + migrating_to: null, placement_group_policy: 'strict', placement_group_type: 'anti_affinity:local', }, @@ -172,6 +175,7 @@ export const linode4: Linode = { placement_group: { id: 1, label: 'pg-1', + migrating_to: null, placement_group_policy: 'strict', placement_group_type: 'anti_affinity:local', }, diff --git a/packages/manager/src/factories/linodes.ts b/packages/manager/src/factories/linodes.ts index b5063025665..b2e14153965 100644 --- a/packages/manager/src/factories/linodes.ts +++ b/packages/manager/src/factories/linodes.ts @@ -1,8 +1,5 @@ import Factory from 'src/factories/factoryProxy'; -import { placementGroupFactory } from './placementGroups'; - -import type { RegionalNetworkUtilization } from '@linode/api-v4/lib/account'; import type { CreateLinodeRequest, Linode, @@ -10,12 +7,14 @@ import type { LinodeBackup, LinodeBackups, LinodeIPsResponse, + LinodePlacementGroupPayload, LinodeSpecs, LinodeType, NetStats, + RegionalNetworkUtilization, Stats, StatsData, -} from '@linode/api-v4/lib/linodes/types'; +} from '@linode/api-v4'; export const linodeAlertsFactory = Factory.Sync.makeFactory({ cpu: 10, @@ -269,6 +268,16 @@ export const proDedicatedTypeFactory = Factory.Sync.makeFactory({ vcpus: 56, }); +export const linodePlacementGroupPayloadFactory = Factory.Sync.makeFactory( + { + id: Factory.each((i) => i), + label: Factory.each((i) => `pg-${i}`), + migrating_to: null, + placement_group_policy: 'strict', + placement_group_type: 'anti_affinity:local', + } +); + export const linodeFactory = Factory.Sync.makeFactory({ alerts: linodeAlertsFactory.build(), backups: linodeBackupsFactory.build(), @@ -283,10 +292,9 @@ export const linodeFactory = Factory.Sync.makeFactory({ ipv6: '2600:3c00::f03c:92ff:fee2:6c40/64', label: Factory.each((i) => `linode-${i}`), lke_cluster_id: null, - placement_group: placementGroupFactory.build({ + placement_group: linodePlacementGroupPayloadFactory.build({ id: 1, label: 'pg-1', - placement_group_type: 'anti_affinity:local', }), region: 'us-east', site_type: 'core', diff --git a/packages/manager/src/factories/placementGroups.ts b/packages/manager/src/factories/placementGroups.ts index dd13d13f6a4..6e8d95c051c 100644 --- a/packages/manager/src/factories/placementGroups.ts +++ b/packages/manager/src/factories/placementGroups.ts @@ -1,5 +1,4 @@ import Factory from 'src/factories/factoryProxy'; - import { pickRandom } from 'src/utilities/random'; import type { @@ -8,10 +7,8 @@ import type { } from '@linode/api-v4'; export const placementGroupFactory = Factory.Sync.makeFactory({ - placement_group_type: 'anti_affinity:local', id: Factory.each((id) => id), is_compliant: Factory.each(() => pickRandom([true, false])), - placement_group_policy: 'strict', label: Factory.each((id) => `pg-${id}`), members: [ { @@ -51,14 +48,17 @@ export const placementGroupFactory = Factory.Sync.makeFactory({ linode_id: 43, }, ], + migrations: null, + placement_group_policy: 'strict', + placement_group_type: 'anti_affinity:local', region: 'us-east', }); export const createPlacementGroupPayloadFactory = Factory.Sync.makeFactory( { - placement_group_type: 'anti_affinity:local', - placement_group_policy: 'strict', label: Factory.each((id) => `mock-pg-${id}`), + placement_group_policy: 'strict', + placement_group_type: 'anti_affinity:local', region: pickRandom(['us-east', 'us-southeast', 'ca-central']), } ); diff --git a/packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx b/packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx index e0ddb0fc00f..c423512c291 100644 --- a/packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx +++ b/packages/manager/src/features/Linodes/MigrateLinode/ConfigureForm.tsx @@ -194,6 +194,8 @@ export const ConfigureForm = React.memo((props: Props) => { handlePlacementGroupSelection(placementGroup); }} textFieldProps={{ + helperText: + 'If your Linode already belongs to a placement group, it will be automatically unassigned during the migration. You can choose to move it to a new placement group in the same region here.', tooltipText: hasRegionPlacementGroupCapability ? '' : 'Placement Groups are not available in this region.', diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.tsx index 9c8b4d1cbe3..5b53be099bc 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.tsx @@ -1,12 +1,27 @@ import * as React from 'react'; +import { useParams } from 'react-router-dom'; import { InlineMenuAction } from 'src/components/InlineMenuAction/InlineMenuAction'; import { Link } from 'src/components/Link'; import { StatusIcon } from 'src/components/StatusIcon/StatusIcon'; import { TableCell } from 'src/components/TableCell'; import { TableRow } from 'src/components/TableRow'; +import { ProgressDisplay } from 'src/features/Linodes/LinodesLanding/LinodeRow/LinodeRow'; +import { StyledButton } from 'src/features/Linodes/LinodesLanding/LinodeRow/LinodeRow.styles'; import { getLinodeIconStatus } from 'src/features/Linodes/LinodesLanding/utils'; +import { + getProgressOrDefault, + linodeInTransition, + transitionText, +} from 'src/features/Linodes/transitions'; +import { notificationCenterContext } from 'src/features/NotificationCenter/NotificationCenterContext'; +import { + PLACEMENT_GROUP_MIGRATION_INBOUND_MESSAGE, + PLACEMENT_GROUP_MIGRATION_OUTBOUND_MESSAGE, +} from 'src/features/PlacementGroups/constants'; import { useIsResourceRestricted } from 'src/hooks/useIsResourceRestricted'; +import { useInProgressEvents } from 'src/queries/events/events'; +import { usePlacementGroupQuery } from 'src/queries/placementGroups'; import { capitalizeAllWords } from 'src/utilities/capitalize'; import type { Linode } from '@linode/api-v4'; @@ -16,33 +31,85 @@ interface Props { linode: Linode; } +type MigrationType = 'inbound' | 'outbound' | null; + export const PlacementGroupsLinodesTableRow = React.memo((props: Props) => { const { handleUnassignLinodeModal, linode } = props; const { label, status } = linode; + const { id: placementGroupId } = useParams<{ id: string }>(); + const notificationContext = React.useContext(notificationCenterContext); + const isLinodeMigrating = Boolean(linode.placement_group?.migrating_to); + const { data: placementGroup } = usePlacementGroupQuery( + Number(placementGroupId), + isLinodeMigrating // we only really need to fetch the placement group if the linode is migrating + ); + const { data: events } = useInProgressEvents(); + const recentEvent = events?.find( + (e) => e.entity?.type === 'linode' && e.entity.id === linode.id + ); const iconStatus = getLinodeIconStatus(status); - + const isMigrationInProgress = linodeInTransition(status, recentEvent); const isLinodeReadOnly = useIsResourceRestricted({ grantLevel: 'read_write', grantType: 'linode', id: linode.id, }); + const getMigrationType = React.useCallback((): MigrationType => { + if (!placementGroup?.migrations) { + return null; + } + + if ( + placementGroup.migrations.inbound?.some((m) => m.linode_id === linode.id) + ) { + return 'inbound'; + } + + if ( + placementGroup.migrations.outbound?.some((m) => m.linode_id === linode.id) + ) { + return 'outbound'; + } + + return null; + }, [placementGroup, linode.id]); + return ( {label} - - {capitalizeAllWords(linode.status.replace('_', ' '))} + {isMigrationInProgress ? ( + <> + + + + + + ) : ( + <> + + {capitalizeAllWords(status.replace('_', ' '))} + + )} handleUnassignLinodeModal(linode)} /> diff --git a/packages/manager/src/features/PlacementGroups/constants.ts b/packages/manager/src/features/PlacementGroups/constants.ts index 0d8de076906..506556f8120 100644 --- a/packages/manager/src/features/PlacementGroups/constants.ts +++ b/packages/manager/src/features/PlacementGroups/constants.ts @@ -38,3 +38,10 @@ export const PLACEMENT_GROUP_POLICY_STRICT = export const PLACEMENT_GROUP_POLICY_FLEXIBLE = "Allows the addition of more compute instances to the group even if it breaks the placement group's compliance."; + +// Migrations +export const PLACEMENT_GROUP_MIGRATION_INBOUND_MESSAGE = + 'This Linode is migrating into this placement group. It will be available after the migration is complete.'; + +export const PLACEMENT_GROUP_MIGRATION_OUTBOUND_MESSAGE = + 'This Linode is being migrated. It will be removed from this placement group after the migration completes.'; diff --git a/packages/manager/src/hooks/useEventHandlers.ts b/packages/manager/src/hooks/useEventHandlers.ts index 9f6312c36ea..f12d5c3345a 100644 --- a/packages/manager/src/hooks/useEventHandlers.ts +++ b/packages/manager/src/hooks/useEventHandlers.ts @@ -9,6 +9,7 @@ import { imageEventsHandler } from 'src/queries/images'; import { linodeEventsHandler } from 'src/queries/linodes/events'; import { diskEventHandler } from 'src/queries/linodes/events'; import { nodebalancerEventHandler } from 'src/queries/nodebalancers'; +import { placementGroupEventHandler } from 'src/queries/placementGroups'; import { sshKeyEventHandler } from 'src/queries/profile/profile'; import { tokenEventHandler } from 'src/queries/profile/tokens'; import { stackScriptEventHandler } from 'src/queries/stackscripts'; @@ -69,6 +70,10 @@ export const eventHandlers: { filter: (event) => event.action.startsWith('oauth_client'), handler: oauthClientsEventHandler, }, + { + filter: (event) => event.action.startsWith('placement_group'), + handler: placementGroupEventHandler, + }, { filter: (event) => event.action.startsWith('linode') || event.action.startsWith('backups'), diff --git a/packages/manager/src/mocks/presets/crud/handlers/placementGroups.ts b/packages/manager/src/mocks/presets/crud/handlers/placementGroups.ts index 0095fe2cd63..b97f3a536d4 100644 --- a/packages/manager/src/mocks/presets/crud/handlers/placementGroups.ts +++ b/packages/manager/src/mocks/presets/crud/handlers/placementGroups.ts @@ -204,6 +204,7 @@ export const placementGroupLinodeAssignment = (mockState: MockState) => [ placement_group: { id: placementGroup.id, label: placementGroup.label, + migrating_to: null, placement_group_policy: placementGroup.placement_group_policy, placement_group_type: placementGroup.placement_group_type, }, diff --git a/packages/manager/src/queries/placementGroups.ts b/packages/manager/src/queries/placementGroups.ts index f63f80ea927..efc4313c285 100644 --- a/packages/manager/src/queries/placementGroups.ts +++ b/packages/manager/src/queries/placementGroups.ts @@ -31,6 +31,7 @@ import type { UnassignLinodesFromPlacementGroupPayload, UpdatePlacementGroupPayload, } from '@linode/api-v4'; +import type { EventHandlerData } from 'src/hooks/useEventHandlers'; const getAllPlacementGroupsRequest = ( _params: Params = {}, @@ -223,3 +224,63 @@ export const useUnassignLinodesFromPlacementGroup = ( }, }); }; + +export const placementGroupEventHandler = ({ + event, + invalidateQueries, +}: EventHandlerData) => { + const { action, entity, secondary_entity } = event; + // for assignment/unassignment events + // in the case of a migration, the assignment/unassignment events are happening asynchronously, + // without using the hook. We need to invalidate the placement group queries here. + // invalidateQueries({ queryKey: placementGroupQueries._def }); + // event looks as follow: + // { + // "id": {id}, + // "created": {created}, + // "seen": false, + // "read": false, + // "percent_complete": null, + // "time_remaining": null, + // "rate": null, + // "duration": null, + // "action": "placement_group_unassign", + // "username": {username}, + // "entity": { + // "label": {label}, + // "id": {id}, + // "type": "placement_group", + // "url": "/v4/placement/groups/{id}" + // }, + // "status": "notification", + // "secondary_entity": { + // "id": {id}, + // "type": "linode", + // "label": {label}, + // "url": "/v4/linode/instances/{id}" + // }, + // "message": "" + // } + if ( + action !== 'placement_group_unassign' && + action !== 'placement_group_assign' + ) { + return; + } + + if (entity && secondary_entity) { + invalidateQueries({ + queryKey: placementGroupQueries.placementGroup(entity.id).queryKey, + }); + invalidateQueries({ + queryKey: linodeQueries.linode(secondary_entity.id).queryKey, + }); + } + + invalidateQueries({ + queryKey: placementGroupQueries.paginated._def, + }); + invalidateQueries({ + queryKey: linodeQueries.linodes._ctx.all._def, + }); +}; From 0285c7a67faa32031d10a0f2245aafadc15ff39e Mon Sep 17 00:00:00 2001 From: santoshp210-akamai <159890961+santoshp210-akamai@users.noreply.github.com> Date: Thu, 28 Nov 2024 00:59:10 +0530 Subject: [PATCH 3/5] upcoming: [DI-21694] - Added Resource Select component to the Create-alert form (#11331) * upcoming: [DI-21694] - Added the Resources Select component, UT for that, converted few variables from snake case to camel case * upcoming: [DI-21694] - Added memoization to the resources fetching method * upcoming: [DI-21694] - Rendering resources component before Severity * Added changeset: ResourceMultiSelect component, along with UT. Changed case for few variables and properties * upcoming: [DI-21694] - Removed the styling for the Autocomplete in ResourceMultiSelect and modified and added few properties for the Alert interfaces as per the latest API-spec * upcoming: [DI-21694] - Addressed the review changes: Made the values dynamic in the Unit Test, replaced the variable name from serviceWatcher to serviceTypeWatcher, added comment addressing need for useEffect() * upcoming: [DI-21694] - Addressed the review changes: Changed the Label to Clusters in case of Database service type --- packages/api-v4/src/cloudpulse/alerts.ts | 4 +- packages/api-v4/src/cloudpulse/types.ts | 5 +- .../pr-11331-added-1732627930598.md | 5 + .../src/factories/cloudpulse/alerts.ts | 3 +- .../AlertsLanding/AlertsDefinitionLanding.tsx | 2 +- .../Alerts/AlertsLanding/AlertsLanding.tsx | 1 - .../CreateAlertDefinition.test.tsx | 1 + .../CreateAlert/CreateAlertDefinition.tsx | 28 ++- .../GeneralInformation/EngineOption.test.tsx | 6 +- .../ResourceMultiSelect.test.tsx | 234 ++++++++++++++++++ .../ResourceMultiSelect.tsx | 98 ++++++++ .../ServiceTypeSelect.test.tsx | 8 +- .../CloudPulse/Alerts/CreateAlert/schemas.ts | 4 +- .../CloudPulse/Alerts/CreateAlert/types.ts | 5 +- .../Alerts/CreateAlert/utilities.ts | 11 +- .../manager/src/queries/cloudpulse/alerts.ts | 4 +- packages/validation/src/cloudpulse.schema.ts | 2 +- 17 files changed, 384 insertions(+), 37 deletions(-) create mode 100644 packages/manager/.changeset/pr-11331-added-1732627930598.md create mode 100644 packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx create mode 100644 packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx diff --git a/packages/api-v4/src/cloudpulse/alerts.ts b/packages/api-v4/src/cloudpulse/alerts.ts index f7c4b732043..3c6f909b9db 100644 --- a/packages/api-v4/src/cloudpulse/alerts.ts +++ b/packages/api-v4/src/cloudpulse/alerts.ts @@ -5,12 +5,12 @@ import { BETA_API_ROOT as API_ROOT } from 'src/constants'; export const createAlertDefinition = ( data: CreateAlertDefinitionPayload, - service_type: AlertServiceType + serviceType: AlertServiceType ) => Request( setURL( `${API_ROOT}/monitor/services/${encodeURIComponent( - service_type! + serviceType! )}/alert-definitions` ), setMethod('POST'), diff --git a/packages/api-v4/src/cloudpulse/types.ts b/packages/api-v4/src/cloudpulse/types.ts index 6a863076a73..4b64bf16c30 100644 --- a/packages/api-v4/src/cloudpulse/types.ts +++ b/packages/api-v4/src/cloudpulse/types.ts @@ -143,7 +143,7 @@ export interface ServiceTypesList { export interface CreateAlertDefinitionPayload { label: string; description?: string; - resource_ids?: string[]; + entity_ids?: string[]; severity: AlertSeverityType; rule_criteria: { rules: MetricCriteria[]; @@ -174,11 +174,12 @@ export interface Alert { id: number; label: string; description: string; + has_more_resources: boolean; status: AlertStatusType; type: AlertDefinitionType; severity: AlertSeverityType; service_type: AlertServiceType; - resource_ids: string[]; + entity_ids: string[]; rule_criteria: { rules: MetricCriteria[]; }; diff --git a/packages/manager/.changeset/pr-11331-added-1732627930598.md b/packages/manager/.changeset/pr-11331-added-1732627930598.md new file mode 100644 index 00000000000..6a2f1d24a13 --- /dev/null +++ b/packages/manager/.changeset/pr-11331-added-1732627930598.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Added +--- + +ResourceMultiSelect component, along with UT. Changed case for few variables and properties ([#11331](https://github.com/linode/manager/pull/11331)) diff --git a/packages/manager/src/factories/cloudpulse/alerts.ts b/packages/manager/src/factories/cloudpulse/alerts.ts index 4fbf4e0e222..a0bc2b6edf7 100644 --- a/packages/manager/src/factories/cloudpulse/alerts.ts +++ b/packages/manager/src/factories/cloudpulse/alerts.ts @@ -7,9 +7,10 @@ export const alertFactory = Factory.Sync.makeFactory({ created: new Date().toISOString(), created_by: 'user1', description: '', + entity_ids: ['0', '1', '2', '3'], + has_more_resources: true, id: Factory.each((i) => i), label: Factory.each((id) => `Alert-${id}`), - resource_ids: ['0', '1', '2', '3'], rule_criteria: { rules: [], }, diff --git a/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsDefinitionLanding.tsx b/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsDefinitionLanding.tsx index f6be4ae8b84..352ded4e3e2 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsDefinitionLanding.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsDefinitionLanding.tsx @@ -14,7 +14,7 @@ export const AlertDefinitionLanding = () => { /> } - path="/monitor/cloudpulse/alerts/definitions/create" + path="/monitor/alerts/definitions/create" /> ); diff --git a/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsLanding.tsx b/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsLanding.tsx index 50aaa6fa994..718050113c1 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsLanding.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsLanding.tsx @@ -84,7 +84,6 @@ export const AlertsLanding = React.memo(() => { diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx index 35d4e4ad328..c8342ee6293 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx @@ -34,5 +34,6 @@ describe('AlertDefinition Create', () => { expect(getByText('Severity is required.')).toBeVisible(); expect(getByText('Service is required.')).toBeVisible(); expect(getByText('Region is required.')).toBeVisible(); + expect(getByText('At least one resource is needed.')).toBeVisible(); }); }); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx index 4b7cb07bd0f..61a6822075e 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx @@ -12,6 +12,7 @@ import { useCreateAlertDefinition } from 'src/queries/cloudpulse/alerts'; import { CloudPulseAlertSeveritySelect } from './GeneralInformation/AlertSeveritySelect'; import { EngineOption } from './GeneralInformation/EngineOption'; import { CloudPulseRegionSelect } from './GeneralInformation/RegionSelect'; +import { CloudPulseMultiResourceSelect } from './GeneralInformation/ResourceMultiSelect'; import { CloudPulseServiceSelect } from './GeneralInformation/ServiceTypeSelect'; import { CreateAlertDefinitionFormSchema } from './schemas'; import { filterFormValues, filterMetricCriteriaFormValues } from './utilities'; @@ -33,14 +34,14 @@ const criteriaInitialValues: MetricCriteriaForm = { }; const initialValues: CreateAlertDefinitionForm = { channel_ids: [], - engine_type: null, + engineType: null, + entity_ids: [], label: '', region: '', - resource_ids: [], rule_criteria: { rules: filterMetricCriteriaFormValues(criteriaInitialValues), }, - service_type: null, + serviceType: null, severity: null, triggerCondition: triggerConditionInitialValues, }; @@ -48,19 +49,18 @@ const initialValues: CreateAlertDefinitionForm = { const overrides = [ { label: 'Definitions', - linkTo: '/monitor/cloudpulse/alerts/definitions', + linkTo: '/monitor/alerts/definitions', position: 1, }, { label: 'Details', - linkTo: `/monitor/cloudpulse/alerts/definitions/create`, + linkTo: `/monitor/alerts/definitions/create`, position: 2, }, ]; export const CreateAlertDefinition = () => { const history = useHistory(); - const alertCreateExit = () => - history.push('/monitor/cloudpulse/alerts/definitions'); + const alertCreateExit = () => history.push('/monitor/alerts/definitions'); const formMethods = useForm({ defaultValues: initialValues, @@ -78,10 +78,10 @@ export const CreateAlertDefinition = () => { } = formMethods; const { enqueueSnackbar } = useSnackbar(); const { mutateAsync: createAlert } = useCreateAlertDefinition( - getValues('service_type')! + getValues('serviceType')! ); - const serviceWatcher = watch('service_type'); + const serviceTypeWatcher = watch('serviceType'); const onSubmit = handleSubmit(async (values) => { try { await createAlert(filterFormValues(values)); @@ -140,9 +140,15 @@ export const CreateAlertDefinition = () => { control={control} name="description" /> - - {serviceWatcher === 'dbaas' && } + + {serviceTypeWatcher === 'dbaas' && } + { it('should render the component when resource type is dbaas', () => { const { getByLabelText, getByTestId } = renderWithThemeAndHookFormContext({ - component: , + component: , }); expect(getByLabelText('Engine Option')).toBeInTheDocument(); expect(getByTestId('engine-option')).toBeInTheDocument(); @@ -17,7 +17,7 @@ describe('EngineOption component tests', () => { it('should render the options happy path', async () => { const user = userEvent.setup(); renderWithThemeAndHookFormContext({ - component: , + component: , }); user.click(screen.getByRole('button', { name: 'Open' })); expect(await screen.findByRole('option', { name: 'MySQL' })); @@ -26,7 +26,7 @@ describe('EngineOption component tests', () => { it('should be able to select an option', async () => { const user = userEvent.setup(); renderWithThemeAndHookFormContext({ - component: , + component: , }); user.click(screen.getByRole('button', { name: 'Open' })); await user.click(await screen.findByRole('option', { name: 'MySQL' })); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx new file mode 100644 index 00000000000..d89fe9d3a24 --- /dev/null +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx @@ -0,0 +1,234 @@ +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import * as React from 'react'; + +import { linodeFactory } from 'src/factories'; +import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers'; + +import { CloudPulseMultiResourceSelect } from './ResourceMultiSelect'; + +const queryMocks = vi.hoisted(() => ({ + useResourcesQuery: vi.fn().mockReturnValue({}), +})); + +vi.mock('src/queries/cloudpulse/resources', async () => { + const actual = await vi.importActual('src/queries/cloudpulse/resources'); + return { + ...actual, + useResourcesQuery: queryMocks.useResourcesQuery, + }; +}); +const SELECT_ALL = 'Select All'; +const ARIA_SELECTED = 'aria-selected'; +describe('ResourceMultiSelect component tests', () => { + it('should render disabled component if the props are undefined or regions and service type does not have any values', () => { + const mockLinodes = linodeFactory.buildList(2); + queryMocks.useResourcesQuery.mockReturnValue({ + data: mockLinodes, + isError: false, + isLoading: false, + status: 'success', + }); + const { + getByPlaceholderText, + getByTestId, + } = renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + expect(getByTestId('resource-select')).toBeInTheDocument(); + expect(getByPlaceholderText('Select Resources')).toBeInTheDocument(); + }); + + it('should render resources happy path', async () => { + const user = userEvent.setup(); + const mockLinodes = linodeFactory.buildList(2); + queryMocks.useResourcesQuery.mockReturnValue({ + data: mockLinodes, + isError: false, + isLoading: false, + status: 'success', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + user.click(screen.getByRole('button', { name: 'Open' })); + expect( + await screen.findByRole('option', { + name: mockLinodes[0].label, + }) + ).toBeInTheDocument(); + expect( + screen.getByRole('option', { + name: mockLinodes[1].label, + }) + ).toBeInTheDocument(); + }); + + it('should be able to select all resources', async () => { + const user = userEvent.setup(); + const mockLinodes = linodeFactory.buildList(2); + queryMocks.useResourcesQuery.mockReturnValue({ + data: mockLinodes, + isError: false, + isLoading: false, + status: 'success', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + user.click(await screen.findByRole('button', { name: 'Open' })); + await user.click(await screen.findByRole('option', { name: SELECT_ALL })); + expect( + await screen.findByRole('option', { + name: mockLinodes[0].label, + }) + ).toHaveAttribute(ARIA_SELECTED, 'true'); + expect( + screen.getByRole('option', { + name: mockLinodes[0].label, + }) + ).toHaveAttribute(ARIA_SELECTED, 'true'); + }); + + it('should be able to deselect the selected resources', async () => { + const user = userEvent.setup(); + const mockLinodes = linodeFactory.buildList(2); + queryMocks.useResourcesQuery.mockReturnValue({ + data: mockLinodes, + isError: false, + isLoading: false, + status: 'success', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + user.click(screen.getByRole('button', { name: 'Open' })); + await user.click(await screen.findByRole('option', { name: SELECT_ALL })); + await user.click( + await screen.findByRole('option', { name: 'Deselect All' }) + ); + expect( + await screen.findByRole('option', { + name: mockLinodes[0].label, + }) + ).toHaveAttribute(ARIA_SELECTED, 'false'); + expect( + screen.getByRole('option', { + name: mockLinodes[1].label, + }) + ).toHaveAttribute(ARIA_SELECTED, 'false'); + }); + + it('should select multiple resources', async () => { + const user = userEvent.setup(); + const mockLinodes = linodeFactory.buildList(3); + queryMocks.useResourcesQuery.mockReturnValue({ + data: mockLinodes, + isError: false, + isLoading: false, + status: 'success', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + user.click(screen.getByRole('button', { name: 'Open' })); + await user.click( + await screen.findByRole('option', { name: mockLinodes[0].label }) + ); + await user.click( + await screen.findByRole('option', { name: mockLinodes[1].label }) + ); + + expect( + await screen.findByRole('option', { + name: mockLinodes[0].label, + }) + ).toHaveAttribute(ARIA_SELECTED, 'true'); + expect( + screen.getByRole('option', { + name: mockLinodes[1].label, + }) + ).toHaveAttribute(ARIA_SELECTED, 'true'); + expect( + screen.getByRole('option', { + name: mockLinodes[2].label, + }) + ).toHaveAttribute(ARIA_SELECTED, 'false'); + expect( + screen.getByRole('option', { + name: 'Select All', + }) + ).toHaveAttribute(ARIA_SELECTED, 'false'); + }); + + it('should render the label as cluster when resource is of dbaas type', () => { + const { getByLabelText } = renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + expect(getByLabelText('Clusters')); + }); + + it('should render error messages when there is an API call failure', () => { + queryMocks.useResourcesQuery.mockReturnValue({ + data: undefined, + isError: true, + isLoading: false, + status: 'error', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + expect( + screen.getByText('Failed to fetch the resources.') + ).toBeInTheDocument(); + }); +}); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx new file mode 100644 index 00000000000..fc19b4335f9 --- /dev/null +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx @@ -0,0 +1,98 @@ +import * as React from 'react'; +import { Controller, useFormContext } from 'react-hook-form'; + +import { Autocomplete } from 'src/components/Autocomplete/Autocomplete'; +import { useResourcesQuery } from 'src/queries/cloudpulse/resources'; + +import type { Item } from '../../constants'; +import type { CreateAlertDefinitionForm } from '../types'; +import type { AlertServiceType } from '@linode/api-v4'; +import type { FieldPathByValue } from 'react-hook-form'; + +interface CloudPulseResourceSelectProps { + /** + * engine option type selected by the user + */ + engine: null | string; + /** + * name used for the component to set in the form + */ + name: FieldPathByValue; + /** + * region selected by the user + */ + region: string | undefined; + /** + * service type selected by the user + */ + serviceType: AlertServiceType | null; +} + +export const CloudPulseMultiResourceSelect = ( + props: CloudPulseResourceSelectProps +) => { + const { engine, name, region, serviceType } = { ...props }; + const { control, setValue } = useFormContext(); + + const { data: resources, isError, isLoading } = useResourcesQuery( + Boolean(region && serviceType), + serviceType?.toString(), + {}, + engine !== null ? { engine, region } : { region } + ); + + const getResourcesList = React.useMemo((): Item[] => { + return resources && resources.length > 0 + ? resources.map((resource) => ({ + label: resource.label, + value: resource.id, + })) + : []; + }, [resources]); + + /* useEffect is used here to reset the value of entity_ids back to [] when the region, engine, serviceType props are changed , + as the options to the Autocomplete component are dependent on those props , the values of the Autocomplete won't match with the given options that are passed + and this may raise a warning or error with the isOptionEqualToValue prop in the Autocomplete. + */ + React.useEffect(() => { + setValue(name, []); + }, [region, serviceType, engine, setValue, name]); + + return ( + ( + { + const resourceIds = resources.map((resource) => resource.value); + field.onChange(resourceIds); + }} + value={ + field.value + ? getResourcesList.filter((resource) => + field.value.includes(resource.value) + ) + : [] + } + autoHighlight + clearOnBlur + data-testid="resource-select" + disabled={!Boolean(region && serviceType)} + isOptionEqualToValue={(option, value) => option.value === value.value} + label={serviceType === 'dbaas' ? 'Clusters' : 'Resources'} + limitTags={2} + loading={isLoading && Boolean(region && serviceType)} + multiple + onBlur={field.onBlur} + options={getResourcesList} + placeholder="Select Resources" + /> + )} + control={control} + name={name} + /> + ); +}; diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.test.tsx index 5e14b886375..42e3f9e1e68 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.test.tsx @@ -41,7 +41,7 @@ queryMocks.useCloudPulseServiceTypes.mockReturnValue({ describe('ServiceTypeSelect component tests', () => { it('should render the Autocomplete component', () => { const { getAllByText, getByTestId } = renderWithThemeAndHookFormContext({ - component: , + component: , }); expect(getByTestId('servicetype-select')).toBeInTheDocument(); getAllByText('Service'); @@ -49,7 +49,7 @@ describe('ServiceTypeSelect component tests', () => { it('should render service types happy path', async () => { renderWithThemeAndHookFormContext({ - component: , + component: , }); userEvent.click(screen.getByRole('button', { name: 'Open' })); expect( @@ -66,7 +66,7 @@ describe('ServiceTypeSelect component tests', () => { it('should be able to select a service type', async () => { renderWithThemeAndHookFormContext({ - component: , + component: , }); userEvent.click(screen.getByRole('button', { name: 'Open' })); await userEvent.click( @@ -81,7 +81,7 @@ describe('ServiceTypeSelect component tests', () => { isLoading: false, }); renderWithThemeAndHookFormContext({ - component: , + component: , }); expect( screen.getByText('Failed to fetch the service types.') diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/schemas.ts b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/schemas.ts index 2e8fee3e200..5bbe4289000 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/schemas.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/schemas.ts @@ -8,8 +8,8 @@ const engineOptionValidation = string().when('service_type', { }); export const CreateAlertDefinitionFormSchema = createAlertDefinitionSchema.concat( object({ - engine_type: engineOptionValidation, + engineType: engineOptionValidation, region: string().required('Region is required.'), - service_type: string().required('Service is required.').nullable(), + serviceType: string().required('Service is required.').nullable(), }) ); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts index c7c58fc1dcb..844b47639a0 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts @@ -9,9 +9,10 @@ import type { export interface CreateAlertDefinitionForm extends Omit { - engine_type: null | string; + engineType: null | string; + entity_ids: string[]; region: string; - service_type: AlertServiceType | null; + serviceType: AlertServiceType | null; severity: AlertSeverityType | null; } diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts index 3db313f7b1f..7459ca1c5da 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts @@ -11,21 +11,22 @@ export const filterFormValues = ( formValues: CreateAlertDefinitionForm ): CreateAlertDefinitionPayload => { const values = omitProps(formValues, [ - 'service_type', + 'serviceType', 'region', - 'engine_type', + 'engineType', 'severity', ]); // severity has a need for null in the form for edge-cases, so null-checking and returning it as an appropriate type const severity = formValues.severity!; - return { ...values, severity }; + const entityIds = formValues.entity_ids; + return { ...values, entity_ids: entityIds, severity }; }; export const filterMetricCriteriaFormValues = ( formValues: MetricCriteriaForm ): MetricCriteria[] => { - const aggregation_type = formValues.aggregation_type!; + const aggregationType = formValues.aggregation_type!; const operator = formValues.operator!; const values = omitProps(formValues, ['aggregation_type', 'operator']); - return [{ ...values, aggregation_type, operator }]; + return [{ ...values, aggregation_type: aggregationType, operator }]; }; diff --git a/packages/manager/src/queries/cloudpulse/alerts.ts b/packages/manager/src/queries/cloudpulse/alerts.ts index 4406a90e0c2..0da27a07093 100644 --- a/packages/manager/src/queries/cloudpulse/alerts.ts +++ b/packages/manager/src/queries/cloudpulse/alerts.ts @@ -10,10 +10,10 @@ import type { } from '@linode/api-v4/lib/cloudpulse'; import type { APIError } from '@linode/api-v4/lib/types'; -export const useCreateAlertDefinition = (service_type: AlertServiceType) => { +export const useCreateAlertDefinition = (serviceType: AlertServiceType) => { const queryClient = useQueryClient(); return useMutation({ - mutationFn: (data) => createAlertDefinition(data, service_type), + mutationFn: (data) => createAlertDefinition(data, serviceType), onSuccess() { queryClient.invalidateQueries(queryFactory.alerts); }, diff --git a/packages/validation/src/cloudpulse.schema.ts b/packages/validation/src/cloudpulse.schema.ts index e8b7e63c414..18c5b59886b 100644 --- a/packages/validation/src/cloudpulse.schema.ts +++ b/packages/validation/src/cloudpulse.schema.ts @@ -30,7 +30,7 @@ const triggerCondition = object({ export const createAlertDefinitionSchema = object({ label: string().required('Name is required.'), description: string().optional(), - resource_ids: array().of(string()).min(1, 'At least one resource is needed.'), + entity_ids: array().of(string()).min(1, 'At least one resource is needed.'), severity: string().required('Severity is required.').nullable(), criteria: array() .of(metricCriteria) From 4f777b9a444eaa5a6c3ca9f1a61d80b9ac04d88e Mon Sep 17 00:00:00 2001 From: hasyed-akamai Date: Thu, 28 Nov 2024 11:05:12 +0530 Subject: [PATCH 4/5] test: [M3-8547] - Add unit tests for `DocsLink` component (#11336) * test: [M3-8547] - Add unit tests for `DocsLink` component * Added changeset: unit test cases for `DocsLink` component --- .../pr-11336-added-1732711112187.md | 5 +++ .../src/components/DocsLink/DocsLink.test.tsx | 45 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 packages/manager/.changeset/pr-11336-added-1732711112187.md create mode 100644 packages/manager/src/components/DocsLink/DocsLink.test.tsx diff --git a/packages/manager/.changeset/pr-11336-added-1732711112187.md b/packages/manager/.changeset/pr-11336-added-1732711112187.md new file mode 100644 index 00000000000..363797bb07a --- /dev/null +++ b/packages/manager/.changeset/pr-11336-added-1732711112187.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Added +--- + +unit test cases for `DocsLink` component ([#11336](https://github.com/linode/manager/pull/11336)) diff --git a/packages/manager/src/components/DocsLink/DocsLink.test.tsx b/packages/manager/src/components/DocsLink/DocsLink.test.tsx new file mode 100644 index 00000000000..d8ddf2bee3f --- /dev/null +++ b/packages/manager/src/components/DocsLink/DocsLink.test.tsx @@ -0,0 +1,45 @@ +import userEvent from '@testing-library/user-event'; +import React from 'react'; + +import { sendHelpButtonClickEvent } from 'src/utilities/analytics/customEventAnalytics'; +import { renderWithTheme } from 'src/utilities/testHelpers'; + +import { DocsLink } from './DocsLink'; + +import type { DocsLinkProps } from './DocsLink'; + +vi.mock('src/utilities/analytics/customEventAnalytics', () => ({ + sendHelpButtonClickEvent: vi.fn(), +})); + +const mockLabel = 'Custom Doc Link Label'; +const mockHref = + 'https://techdocs.akamai.com/cloud-computing/docs/faqs-for-compute-instances'; +const mockAnalyticsLabel = 'Label'; + +const defaultProps: DocsLinkProps = { + analyticsLabel: mockAnalyticsLabel, + href: mockHref, + label: mockLabel, +}; + +describe('DocsLink', () => { + it('should render the label', () => { + const { getByText } = renderWithTheme(); + expect(getByText(mockLabel)).toBeVisible(); + }); + + it('should allow user to click the label and redirect to the url', async () => { + const { getByRole } = renderWithTheme(); + const link = getByRole('link', { + name: 'Custom Doc Link Label - link opens in a new tab', + }); + expect(link).toBeInTheDocument(); + await userEvent.click(link); + expect(sendHelpButtonClickEvent).toHaveBeenCalledTimes(1); + expect(sendHelpButtonClickEvent).toHaveBeenCalledWith( + mockHref, + mockAnalyticsLabel + ); + }); +}); From 8611b577d00f11936d8f7693282665758f091985 Mon Sep 17 00:00:00 2001 From: Purvesh Makode Date: Mon, 2 Dec 2024 16:47:37 +0530 Subject: [PATCH 5/5] feat: [M3-8947] - Improve linter rules for naming convention (#11337) * Add linter rules for naming convention * Add rules for typeLike * Add rules for destrctured variables * Added changeset: Linter rules for naming convention * Added changeset: Linter rules for naming convention --- .../pr-11337-added-1732714186488.md | 5 +++ packages/api-v4/.eslintrc.json | 45 +++++++++++++++---- .../pr-11337-added-1732714227095.md | 5 +++ packages/manager/.eslintrc.cjs | 27 +++++++++++ 4 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 packages/api-v4/.changeset/pr-11337-added-1732714186488.md create mode 100644 packages/manager/.changeset/pr-11337-added-1732714227095.md diff --git a/packages/api-v4/.changeset/pr-11337-added-1732714186488.md b/packages/api-v4/.changeset/pr-11337-added-1732714186488.md new file mode 100644 index 00000000000..b261294d469 --- /dev/null +++ b/packages/api-v4/.changeset/pr-11337-added-1732714186488.md @@ -0,0 +1,5 @@ +--- +"@linode/api-v4": Added +--- + +Linter rules for naming convention ([#11337](https://github.com/linode/manager/pull/11337)) diff --git a/packages/api-v4/.eslintrc.json b/packages/api-v4/.eslintrc.json index 039685b3d24..f551347ff3f 100644 --- a/packages/api-v4/.eslintrc.json +++ b/packages/api-v4/.eslintrc.json @@ -13,6 +13,42 @@ "plugin:prettier/recommended" ], "rules": { + "@typescript-eslint/naming-convention": [ + "warn", + { + "format": ["camelCase", "UPPER_CASE", "PascalCase"], + "leadingUnderscore": "allow", + "selector": "variable", + "trailingUnderscore": "allow" + }, + { + "format": null, + "modifiers": ["destructured"], + "selector": "variable" + }, + { + "format": ["camelCase", "PascalCase"], + "selector": "function" + }, + { + "format": ["camelCase"], + "leadingUnderscore": "allow", + "selector": "parameter" + }, + { + "format": ["PascalCase"], + "selector": "typeLike" + } + ], + "@typescript-eslint/no-unused-vars": "off", + "@typescript-eslint/no-inferrable-types": "off", + "@typescript-eslint/no-namespace": "warn", + "@typescript-eslint/explicit-function-return-type": "off", + "@typescript-eslint/no-empty-interface": "warn", + "@typescript-eslint/no-non-null-assertion": "off", + "@typescript-eslint/no-explicit-any": "warn", + "@typescript-eslint/no-use-before-define": "off", + "@typescript-eslint/interface-name-prefix": "off", "no-unused-vars": [ "warn", { @@ -33,15 +69,6 @@ "no-console": "error", "no-undef-init": "off", "radix": "error", - "@typescript-eslint/no-unused-vars": "off", - "@typescript-eslint/no-inferrable-types": "off", - "@typescript-eslint/no-namespace": "warn", - "@typescript-eslint/explicit-function-return-type": "off", - "@typescript-eslint/no-empty-interface": "warn", - "@typescript-eslint/no-non-null-assertion": "off", - "@typescript-eslint/no-explicit-any": "warn", - "@typescript-eslint/no-use-before-define": "off", - "@typescript-eslint/interface-name-prefix": "off", "sonarjs/cognitive-complexity": "warn", "sonarjs/no-duplicate-string": "warn", "sonarjs/prefer-immediate-return": "warn", diff --git a/packages/manager/.changeset/pr-11337-added-1732714227095.md b/packages/manager/.changeset/pr-11337-added-1732714227095.md new file mode 100644 index 00000000000..9282e940f63 --- /dev/null +++ b/packages/manager/.changeset/pr-11337-added-1732714227095.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Added +--- + +Linter rules for naming convention ([#11337](https://github.com/linode/manager/pull/11337)) diff --git a/packages/manager/.eslintrc.cjs b/packages/manager/.eslintrc.cjs index 37a8ee55d6c..99870119ff7 100644 --- a/packages/manager/.eslintrc.cjs +++ b/packages/manager/.eslintrc.cjs @@ -164,6 +164,33 @@ module.exports = { '@typescript-eslint/explicit-function-return-type': 'off', '@typescript-eslint/explicit-module-boundary-types': 'off', '@typescript-eslint/interface-name-prefix': 'off', + '@typescript-eslint/naming-convention': [ + 'warn', + { + format: ['camelCase', 'UPPER_CASE', 'PascalCase'], + leadingUnderscore: 'allow', + selector: 'variable', + trailingUnderscore: 'allow', + }, + { + format: null, + modifiers: ['destructured'], + selector: 'variable', + }, + { + format: ['camelCase', 'PascalCase'], + selector: 'function', + }, + { + format: ['camelCase'], + leadingUnderscore: 'allow', + selector: 'parameter', + }, + { + format: ['PascalCase'], + selector: 'typeLike', + }, + ], '@typescript-eslint/no-empty-interface': 'warn', '@typescript-eslint/no-explicit-any': 'warn', '@typescript-eslint/no-inferrable-types': 'off',