Improve marketplace vendors behavior (#2168)

* Improve the marketplace vendors behavior * Use approprieted function name * Push the vendor list like it comes * Replace key by identifier (more explicit) * Improve variable name to selectedVendorIdentifiers * Update src/components/VendorSelector/__tests__/VendorSelector.test.tsx Co-authored-by: George Adams <[email protected]> * Programmatically generate vendor identifiers /util/vendors.tsx view getVendorIdentifier() * Add the expected method * Add test for getVendorIdentifier() --------- Co-authored-by: George Adams <[email protected]>
adoptium · Aug 29, 2023 · 970d170 · 970d170
1 parent c82adf2
commit 970d170
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 77 deletions.
diff --git a/src/components/DownloadDropdowns/index.tsx b/src/components/DownloadDropdowns/index.tsx
@@ -1,4 +1,4 @@
-import React, { useRef, useState, useCallback, useEffect } from 'react';
+import React, { useState, useCallback, useEffect } from 'react';
 import { useStaticQuery, graphql } from 'gatsby';
 import { useLocation } from '@gatsbyjs/reach-router';
 import queryString from 'query-string';
@@ -139,16 +139,15 @@ const DownloadDropdowns = ({updaterAction, marketplace, Table}) => {
     const [version, udateVersion] = useState(defaultSelectedVersion);
 
     // Marketplace vendor selector only
-    const checkboxRef = useRef({});
-    const [checkbox, updateCheckbox] = useState({checkboxRef});
+    const [selectedVendorIdentifiers, updateSelectedVendorIdentifiers] = useState<string[]>([]);
 
     const [releases, setReleases] = useState(null);
 
     useEffect(() => {
         (async () => {
-          setReleases(await updaterAction(version, os, arch, packageType, checkboxRef));
+          setReleases(await updaterAction(version, os, arch, packageType, selectedVendorIdentifiers));
         })();
-    }, [version, os, arch, packageType, checkbox]);
+    }, [version, os, arch, packageType, selectedVendorIdentifiers]);
 
     const setOS = useCallback((os) => {
         setURLParam('os', os)
@@ -170,13 +169,14 @@ const DownloadDropdowns = ({updaterAction, marketplace, Table}) => {
         udateVersion(version);
     }, []);
 
-    const setCheckbox= useCallback(() => {
-        updateCheckbox({checkboxRef});
+    const setSelectedVendorIdentifiers= useCallback((newSelectedVendorIdentifiers) => {
+        // do not change the URL
+        updateSelectedVendorIdentifiers(newSelectedVendorIdentifiers);
     }, []);
 
     return (
         <>
-            {marketplace && <VendorSelector checkboxRef={checkboxRef} setCheckbox={setCheckbox} />}
+            {marketplace && <VendorSelector selectedVendorIdentifiers={selectedVendorIdentifiers} setSelectedVendorIdentifiers={setSelectedVendorIdentifiers} />}
             <div className="input-group mb-5 row g-2">
                 <div className="input-group-prepend flex-colunm col-12 col-md-3">
                     <label className="px-2 fw-bold" htmlFor="os"><Trans>Operating System</Trans></label>

diff --git a/src/components/VendorSelector/__tests__/VendorSelector.test.tsx b/src/components/VendorSelector/__tests__/VendorSelector.test.tsx
@@ -3,13 +3,18 @@ import { render, fireEvent, screen } from '@testing-library/react';
 import { vi } from 'vitest';
 import VendorSelector from '../index';
 import vendors from '../../../json/marketplace.json';
+import getVendorIdentifier from '../../../util/vendors';
 
 describe('VendorSelector', () => {
-  const mockSetCheckbox = vi.fn();
-  const checkboxRef = { current: {} };
+  const mockSetSelectedVendorIdentifiers = vi.fn();
+  const mockSelectedVendorIdentifiers = vendors.map(v => getVendorIdentifier(v));
 
   beforeEach(() => {
-    render(<VendorSelector checkboxRef={checkboxRef} setCheckbox={mockSetCheckbox} />);
+    render(<VendorSelector selectedVendorIdentifiers={mockSelectedVendorIdentifiers} setSelectedVendorIdentifiers={mockSetSelectedVendorIdentifiers} />);
+  });
+
+  afterEach(() => {
+    vi.clearAllMocks();
   });
 
   test('renders component correctly', () => {
@@ -22,15 +27,18 @@ describe('VendorSelector', () => {
 
   vendors.forEach((vendor, i) => {
     test(`renders the input checkbox with correct attributes for vendor ${i}`, () => {
-      const checkbox = screen.getByTestId(vendor.name);
-      expect(checkbox).toHaveAttribute('id', `vendor${vendor.name}`);
+      const identifier = getVendorIdentifier(vendor);
+      const checkbox = screen.getByTestId(`checkbox-${identifier}`);
+      expect(checkbox).toHaveAttribute('id', `vendor-${identifier}`);
       expect(checkbox).toHaveAttribute('type', 'checkbox');
-      expect(checkbox).toHaveProperty('defaultChecked', true);
+      expect(checkbox).toHaveProperty('readOnly', true);
+      expect(checkbox).toHaveProperty('checked', true);
     });
 
     test(`renders the label element with correct attributes for vendor ${i}`, () => {
+      const identifier = getVendorIdentifier(vendor);
       const label = screen.getByTitle(vendor.name);
-      expect(label).toHaveAttribute('for', `vendor${vendor.name}`);
+      expect(label).toHaveAttribute('for', `vendor-${identifier}`);
     });
 
     test(`renders the img element with correct attributes for vendor ${i}`, () => {
@@ -41,8 +49,8 @@ describe('VendorSelector', () => {
   });
 
   test('calls handleChange function when a checkbox is toggled', () => {
-    const checkbox = screen.getByTitle(vendors[0].name);
-    fireEvent.click(checkbox);
-    expect(mockSetCheckbox).toHaveBeenCalledTimes(1);
+    const li = screen.getByTestId(`li-${mockSelectedVendorIdentifiers[0]}`);
+    fireEvent.click(li);
+    expect(mockSetSelectedVendorIdentifiers).toHaveBeenCalledTimes(2 /* 1 by the useEffec() and 1 by the onclick() */);
   });
 });
diff --git a/src/components/VendorSelector/index.tsx b/src/components/VendorSelector/index.tsx
@@ -2,35 +2,50 @@ import React, { useState, useEffect } from "react"
 import { shuffle } from '../../util/shuffle'
 import vendors from '../../json/marketplace.json';
 import './VendorSelector.scss';
+import getVendorIdentifier from '../../util/vendors';
 
-const VendorSelector = ({checkboxRef, setCheckbox}) => {
+const VendorSelector = ({selectedVendorIdentifiers, setSelectedVendorIdentifiers}) => {
 
-    const [randomizedVendors, setRandomizedVendors] = useState(vendors);
+    const [randomizedVendors, setRandomizedVendors] = useState<VendorProps[]>([]);
 
     useEffect(() => {
         let vendorsCpy = [...vendors];
         setRandomizedVendors(shuffle(vendorsCpy));
+        setSelectedVendorIdentifiers(vendorsCpy.map(vendor => getVendorIdentifier(vendor)))
     }, [])
 
-    const handleChange = () => {
-        setCheckbox(checkboxRef.current.checked)
+    const handleChange = (e, identifier) => {
+        e.preventDefault();
+        let newselectedVendorIdentifiers = [...selectedVendorIdentifiers];
+        let idx = newselectedVendorIdentifiers.indexOf(identifier);
+        if(idx >= 0) newselectedVendorIdentifiers.splice(idx, 1);
+        else newselectedVendorIdentifiers.push(identifier);
+        setSelectedVendorIdentifiers(newselectedVendorIdentifiers)
     };
 
     return (
         <ul className="vendor-list pt-5">
-            {randomizedVendors.map(
-                (vendor, i): string | JSX.Element =>
-                    vendor && (
-                        <li key={vendor.name} className="vendor-li">
-                            <input data-testid={vendor.name} id={`vendor${vendor.name}`} ref={el => checkboxRef.current[`vendor${vendor.name.replace(/\s+/g, '')}`] = el} type="checkbox" defaultChecked={true} onChange={handleChange} />
-                            <label className="vendor-label" htmlFor={`vendor${vendor.name}`} title={vendor.name}>
+            {randomizedVendors.map((vendor, i): string | JSX.Element => {
+                let identifier = getVendorIdentifier(vendor);
+                return (
+                        <li key={`vendor-${i}`} data-testid={`li-${identifier}`} className="vendor-li" onClick={(e) => handleChange(e, identifier)}>
+                            <input id={`vendor-${identifier}`} data-testid={`checkbox-${identifier}`} readOnly className="vendor-name" type="checkbox" checked={selectedVendorIdentifiers.indexOf(identifier) >= 0}/>
+                            <label className="vendor-label" htmlFor={`vendor-${identifier}`} title={vendor.name}>
                                 <img src={`/images/vendors/${vendor.icon}`} alt={`${vendor.name} icon`} style={ vendor.iconPadding ? { padding:vendor.iconPadding} : {}}/>
                             </label>
                         </li>
-                )
+                )}
             )}
         </ul>
     );
 };
 
+export interface VendorProps {
+    name: string;
+    icon: string;
+    iconPadding: string;
+    postDownload: string;
+    selected: boolean
+};
+
 export default VendorSelector;
diff --git a/src/hooks/__tests__/fetchMarketplace.test.tsx b/src/hooks/__tests__/fetchMarketplace.test.tsx
@@ -3,15 +3,10 @@ import { describe, expect, it, vi } from 'vitest'
 import { getAllPkgsForVersion, getImageForDistribution } from '../fetchMarketplace';
 import { createMockTemurinFeatureReleaseAPI  } from '../../__fixtures__/hooks';
 import vendors from '../../json/marketplace.json';
+import getVendorIdentifier from '../../util/vendors';
 
 let mockResponse = [createMockTemurinFeatureReleaseAPI(false)];
-let ref = { current: {} };
-
-// Mock checkbox values
-for (let vendor of vendors) {
-  const checked = true;
-  ref.current[`vendor${vendor.name.replace(/\s+/g, '')}`] = { checked };
-}
+let selectedVendorIdentifiers = vendors.map(v => getVendorIdentifier(v));
 
 global.fetch = vi.fn(() => Promise.resolve({
   json: () => Promise.resolve(mockResponse)
@@ -24,15 +19,15 @@ afterEach(() => {
 describe('getAllPkgsForVersion', () => {
   it('returns valid JSON', async() => {
     renderHook(async() => {
-      await getAllPkgsForVersion(8, 'linux', 'x64', 'jdk', ref).then((data) => {
+      await getAllPkgsForVersion(8, 'linux', 'x64', 'jdk', selectedVendorIdentifiers).then((data) => {
         expect(data).toMatchSnapshot()
       })
     });
   });
 
   it('returns valid JSON - Alpine Linux', async() => {
     renderHook(async() => {
-      await getAllPkgsForVersion(8, 'alpine-linux', 'x64', 'any', ref).then((data) => {
+      await getAllPkgsForVersion(8, 'alpine-linux', 'x64', 'any', selectedVendorIdentifiers).then((data) => {
         expect(data).toMatchSnapshot()
       })
     });
@@ -41,7 +36,7 @@ describe('getAllPkgsForVersion', () => {
   it('returns valid JSON - installer', async() => {
     mockResponse = [createMockTemurinFeatureReleaseAPI(true)];
     renderHook(async() => {
-      await getAllPkgsForVersion(8, 'linux', 'x64', 'jdk', ref).then((data) => {
+      await getAllPkgsForVersion(8, 'linux', 'x64', 'jdk', selectedVendorIdentifiers).then((data) => {
         expect(data).toMatchSnapshot()
       })
     });

diff --git a/src/hooks/fetchMarketplace.tsx b/src/hooks/fetchMarketplace.tsx
@@ -6,16 +6,8 @@ export async function getAllPkgsForVersion(
     os: string,
     architecture: string,
     package_type: string,
-    checkboxRef: any,
+    vendors: string[]= [],
 ): Promise<MarketplaceRelease[] | null> {
-    let microsoftSelected = checkboxRef.current.vendorMicrosoft && checkboxRef.current.vendorMicrosoft.checked;
-    let temurinSelected = checkboxRef.current.vendorAdoptium && checkboxRef.current.vendorAdoptium.checked;
-    let redhatSelected = checkboxRef.current.vendorRedHat && checkboxRef.current.vendorRedHat.checked;
-    let huaweiSelected = checkboxRef.current.vendorHuawei && checkboxRef.current.vendorHuawei.checked;
-    let zuluSelected = checkboxRef.current.vendorAzul && checkboxRef.current.vendorAzul.checked;
-    let ibmSelected = checkboxRef.current.vendorIBM && checkboxRef.current.vendorIBM.checked;
-    let alibabaSelected = checkboxRef.current.vendorAlibaba && checkboxRef.current.vendorAlibaba.checked;
-
     let params = '?'
     params += 'feature_version=' + version;
 
@@ -37,32 +29,8 @@ export async function getAllPkgsForVersion(
         params += ('&image_type=' + package_type)
     }
 
-    if (temurinSelected) {
-        params += '&vendor=adoptium'
-    }
-
-    if (redhatSelected) {
-        params += '&vendor=redhat'
-    }
-
-    if (huaweiSelected) {
-        params += '&vendor=huawei'
-    }
-
-    if (microsoftSelected) {
-        params += ('&vendor=microsoft')
-    }
-
-    if (zuluSelected) {
-        params += ('&vendor=azul')
-    }
-
-    if (ibmSelected) {
-        params += ('&vendor=ibm')
-    }
-
-    if (alibabaSelected) {
-        params += ('&vendor=alibaba')
+    for(const vendor of vendors) {
+        params += ('&vendor=' + vendor)
     }
 
     const url = new URL(baseUrl + '/v1/assets/latestForVendors' + params);

diff --git a/src/pages/__tests__/__snapshots__/marketplace.test.tsx.snap b/src/pages/__tests__/__snapshots__/marketplace.test.tsx.snap
@@ -51,16 +51,19 @@ exports[`Marketplace page > renders correctly 1`] = `
     >
       <li
         class="vendor-li"
+        data-testid="li-adoptium"
       >
         <input
           checked=""
-          data-testid="Adoptium"
-          id="vendorAdoptium"
+          class="vendor-name"
+          data-testid="checkbox-adoptium"
+          id="vendor-adoptium"
+          readonly=""
           type="checkbox"
         />
         <label
           class="vendor-label"
-          for="vendorAdoptium"
+          for="vendor-adoptium"
           title="Adoptium"
         >
           <img

diff --git a/src/util/__tests__/vendors.test.tsx b/src/util/__tests__/vendors.test.tsx
@@ -0,0 +1,17 @@
+import getVendorIdentifier from "../vendors"
+import vendors from '../../json/marketplace.json';
+
+describe("vendors", () => {
+    it("should be lowercase", () => {
+        const adoptium = vendors.find(v => v.name === "Adoptium");
+        expect(getVendorIdentifier(adoptium)).toBe("adoptium");
+
+        const ibm = vendors.find(v => v.name === "IBM");
+        expect(getVendorIdentifier(ibm)).toBe("ibm");
+    });
+
+    it("should be with no space", () => {
+        const redhat = vendors.find(v => v.name === "Red Hat");
+        expect(getVendorIdentifier(redhat)).toBe("redhat");
+    });
+});
diff --git a/src/util/vendors.tsx b/src/util/vendors.tsx
@@ -0,0 +1,5 @@
+import VendorProps from '../components/VendorSelector'
+
+export default function getVendorIdentifier (vendor: VendorProps) {
+    return vendor.name.toLocaleLowerCase().replace(/\s+/g, '');
+}