fix: [M3-7083 & M3-7175] MarketPlace regex fix + end to end coverage #9704

abailly-akamai · 2023-09-20T15:42:40Z

Description 📝

This PR achieves a few things:

fix the overly permissive regex to map stackcripts labels to OCA labels.
implements an e2e suite for the marketplace, which doubles as validating the fix above (there was no e2e coverage before and this will be quite helpful when migrating those class components into functional ones and avoid state issues such as M3-7045.
Improve code structure & quality, and utility coverage of the feature
Remove reliance on hard coded OCAs for the "New apps" section

Preview 📷

This PR aims to introduce no visual change or regression

How to test 🧪

Pull code locally and make sure no functional or visual regression was introduced by comparing local and production for the OCA create flow (/linodes/create?type=One-Click)
Test info drawers for some apps and confirm they are populated
run yarn cy:debug and run one-click-apps.spec.ts

abailly-akamai · 2023-09-26T16:39:28Z

packages/manager/cypress/tsconfig.json

@@ -9,6 +9,7 @@
    "moduleResolution": "node",
    "lib": ["es6", "dom"],
    "baseUrl": "..",
+    "jsx": "react",


This was necessary to remove an import warning when importing a utility from a .tsx extension

abailly-akamai · 2023-09-26T16:41:23Z

packages/manager/src/features/Linodes/LinodesCreate/AppPanelSection.tsx

-            const label = isCluster
-              ? decodedLabel.split(' Cluster')[0]
-              : decodedLabel;
+            const { decodedLabel, isCluster, label } = handleAppLabel(eachApp);


Moved this to a util so it's reusable and testable in isolation.

abailly-akamai · 2023-09-26T16:44:58Z

packages/manager/src/features/Linodes/LinodesCreate/LinodeCreateContainer.tsx

-        const trimmedApps = filteredApps.map((stackscript) =>
-          trimOneClickFromLabel(stackscript)
-        );
+


Same here. Moved to a util. This helps testing in isolation and re-use as an assertion in the e2e test.

abailly-akamai · 2023-09-26T16:48:47Z

packages/manager/src/features/Linodes/LinodesCreate/SelectAppPanel.tsx

-        'hashicorp nomad clients cluster',
-        'hashicorp nomad cluster',
-      ].includes(app.label.toLowerCase().trim());
+      return newAppsIds.includes(app.id.toString());
    });


I don't understand why we were hardcoding those since they are already returning from the oneClickApps feature flag. This fix also makes the New Apps selection more robust since it references the Id rather than a label. This is the reason for passing the flags from the Class Component to the selection panel. (also a class component so no useFlag hook there...)

Confirmed with @HanaXu this was the right thing to do.

abailly-akamai · 2023-09-26T17:05:50Z

packages/manager/src/features/OneClickApps/AppDetailDrawer.tsx

-
-      const cleanedAppName = app.name.replace('&reg;', '');
-
-      return cleanedStackScriptLabel === cleanedAppName;


This is moved to https://github.com/linode/manager/pull/9704/files#diff-218af8db6da1dc4532edcb6cab99724a4363a16c0fdbbf78557ab9d3acf23e54R14 below, where the regex has been fixed to remove the codeQL warning

packages/manager/src/features/OneClickApps/utils.ts

jdamore-linode · 2023-09-28T15:01:22Z

packages/manager/src/features/OneClickApps/AppDetailDrawer.tsx

+      data-qa-drawer
+      data-testid="drawer"


Suggested change

data-qa-drawer

data-testid="drawer"

data-qa-drawer

data-testid="drawer"

Nice catch. Curious what's special about this drawer that we need to use the MUI component and re-implement these attributes and the close button instead of using the component in src/components/Drawer.tsx

Edit: Oh never mind, I forgot that this drawer has the really nice styling

Yeah i thought about moving it to our drawer but it would have expanded the scope too much cause I would have had to add props to our Drawer etc. for another time 😄

jdamore-linode

Looks great @abailly-akamai! Tests look good and are passing for me, and manually testing OCA I wasn't able to notice any regressions or differences between this and Prod.

jdamore-linode · 2023-09-28T15:20:13Z

packages/manager/cypress/e2e/core/oneClickApps/one-click-apps.spec.ts

+      cy.scrollTo(0, 0);
+      const initialNumberOfApps = trimmedApps.length;
+      cy.findByPlaceholderText('Search for app name')
+        .should('exist')
+        .type(candidate);


Suggested change

cy.scrollTo(0, 0);

const initialNumberOfApps = trimmedApps.length;

cy.findByPlaceholderText('Search for app name')

.should('exist')

.type(candidate);

const initialNumberOfApps = trimmedApps.length;

cy.findByPlaceholderText('Search for app name')

.scrollIntoView()

.should('be.visible')

.type(candidate);

This might be slightly clearer about what the test is trying to do but very minor difference

carrillo-erik · 2023-09-28T16:03:27Z

Looks good to me. After running the e2e tests in debug mode I was able to verify that they passed.

abailly-akamai added 10 commits September 26, 2023 12:06

fix: [fix-M3-7083] initial coomit: save work

3e6175f

Make utils and introduce new e2e

9b7d47c

Asserting apps

e3699d0

Refactor hard coded new apps

68a889f

Assert matching drawers

a4d75c7

Assert drawer content

fd058c0

Save work

99b44a1

Assert filtering

30ae9f6

Cleanup

7539107

Create linode from OCA

b1cfef5

abailly-akamai force-pushed the fix-M3-7083 branch from 1411e0c to b1cfef5 Compare September 26, 2023 16:06

abailly-akamai changed the title ~~fix: [M3-7083] draft~~ fix: [M3-7083 & M3-7175] MarketPlace regex fix + end to end coverage Sep 26, 2023

abailly-akamai self-assigned this Sep 26, 2023

abailly-akamai added 2 commits September 26, 2023 13:26

Cleanup

97e6665

Moar Cleanup

c6e428c

abailly-akamai commented Sep 26, 2023

View reviewed changes

abailly-akamai added 2 commits September 26, 2023 13:40

Moar Cleanup

ba729cc

Fix unit test

b27898d

abailly-akamai marked this pull request as ready for review September 26, 2023 18:13

abailly-akamai requested a review from a team as a code owner September 26, 2023 18:13

abailly-akamai requested review from jdamore-linode and carrillo-erik and removed request for a team September 26, 2023 18:13

abailly-akamai added 2 commits September 26, 2023 14:14

Added changeset: Overly permissive regex in One Click Apps flow

dce1afa

Fix import

cb8ed19

abailly-akamai requested a review from coliu-akamai September 27, 2023 13:17

abailly-akamai mentioned this pull request Sep 27, 2023

Feat: [M3-7195] - Marketplace OCA loading pattern POC #9724

Closed

jdamore-linode reviewed Sep 28, 2023

View reviewed changes

jdamore-linode approved these changes Sep 28, 2023

View reviewed changes

carrillo-erik approved these changes Sep 28, 2023

View reviewed changes

abailly-akamai merged commit 77db948 into linode:develop Sep 28, 2023
11 checks passed

cpathipa mentioned this pull request Sep 29, 2023

Release v1.104.0 - release → staging #9740

Merged

mjac0bs mentioned this pull request Oct 2, 2023

Release v1.104.0 - staging → master #9744

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: [M3-7083 & M3-7175] MarketPlace regex fix + end to end coverage #9704

fix: [M3-7083 & M3-7175] MarketPlace regex fix + end to end coverage #9704

abailly-akamai commented Sep 20, 2023 •

edited

Loading

abailly-akamai Sep 26, 2023

abailly-akamai Sep 26, 2023

abailly-akamai Sep 26, 2023

abailly-akamai Sep 26, 2023

abailly-akamai Sep 26, 2023

jdamore-linode Sep 28, 2023 •

edited

Loading

abailly-akamai Sep 28, 2023

jdamore-linode left a comment

jdamore-linode Sep 28, 2023

carrillo-erik commented Sep 28, 2023


		const cleanedAppName = app.name.replace('®', '');

		return cleanedStackScriptLabel === cleanedAppName;

fix: [M3-7083 & M3-7175] MarketPlace regex fix + end to end coverage #9704

fix: [M3-7083 & M3-7175] MarketPlace regex fix + end to end coverage #9704

Conversation

abailly-akamai commented Sep 20, 2023 • edited Loading

Description 📝

Preview 📷

How to test 🧪

abailly-akamai Sep 26, 2023

Choose a reason for hiding this comment

abailly-akamai Sep 26, 2023

Choose a reason for hiding this comment

abailly-akamai Sep 26, 2023

Choose a reason for hiding this comment

abailly-akamai Sep 26, 2023

Choose a reason for hiding this comment

abailly-akamai Sep 26, 2023

Choose a reason for hiding this comment

jdamore-linode Sep 28, 2023 • edited Loading

Choose a reason for hiding this comment

abailly-akamai Sep 28, 2023

Choose a reason for hiding this comment

jdamore-linode left a comment

Choose a reason for hiding this comment

jdamore-linode Sep 28, 2023

Choose a reason for hiding this comment

carrillo-erik commented Sep 28, 2023

abailly-akamai commented Sep 20, 2023 •

edited

Loading

jdamore-linode Sep 28, 2023 •

edited

Loading