PAC-1569-added codes for Packs API fetch #2484

nage1234 · 2024-03-26T18:13:54Z

Describe the Change

Still lots of changes to be added in this PR. please DO NOT MERGE.. API error handling has to be done, so dont merge this..

Changed Pages

💻 Add Preview URL for Page

Jira Tickets

🎫 PAC-1569

Backports

Can this PR be backported?
No

Yes. Remember to add the relevant backport labels to your PR.
No. Please leave a short comment below about why this PR cannot be backported.

karl-cardenas-coding · 2024-03-26T20:02:55Z

@nage1234 I put the PR in draft to prevent accidental merge 😄

…ogy card as per mockup

…readme

codrin-iftimie · 2024-04-12T10:51:31Z

plugins/packs-integrations.js

-      return doc.frontMatter.type === "integration";
+function getReadMeMap(packValues) {
+  const map = new Map();
+  packValues.forEach((packValue) => {


reduce can be used here.
side note: you should use map.set instead of assigning attributes on this map

generateCustomData

codrin-iftimie · 2024-04-12T10:55:03Z

plugins/packs-integrations.js

    });
-  return packsData;
+    //console.log("roots length= ", roots.length);


code cleanup

codrin-iftimie · 2024-04-12T11:00:30Z

package.json

@@ -45,7 +46,7 @@
    "@fortawesome/free-solid-svg-icons": "^6.4.0",
    "@fortawesome/react-fontawesome": "^0.2.0",
    "@mdx-js/react": "^3.0.0",
-    "antd": "^5.6.2",
+    "antd": "^4.22.6",


this has a high chance of breaking things

codrin-iftimie · 2024-04-22T08:10:44Z

src/components/PacksReadme/PacksReadme.tsx

+    setSelectedVersion(version);
+  }
+
+  function getOptions() {


renderVersionOptions

codrin-iftimie · 2024-04-22T08:11:48Z

src/components/PacksReadme/PacksReadme.tsx

+          })}
+        </Tabs>
+      );
+    } else if (Object.keys(packData.packReadme).length) {


I think you can simplify this if else logic

codrin-iftimie · 2024-04-22T08:13:22Z

src/components/PacksReadme/PacksReadme.tsx

+    } else if (md) {
+      return md;
+    } else {
+      return renderEmptyContent();


I'm mostly in favor of creating a function for reusability.
if it just wraps some content and is not called more than once, then you can safely inline that content in the return

else you have to track what a function truly returns

codrin-iftimie · 2024-04-22T08:14:33Z

src/components/PacksReadme/PacksReadme.tsx

+    );
+  }
+  function getProviders() {
+    if(packData.provider.includes("all")) {


if (
isn't there a linter here?

codrin-iftimie · 2024-04-22T08:15:15Z

src/components/PacksReadme/index.ts

+import PacksReadme from "./PacksReadme";
+
+export default PacksReadme;


I don't understand why this file was created

codrin-iftimie · 2024-04-22T08:26:23Z

src/components/Technologies/PackCardIcon.tsx

+      {isError ?(<IconMapper type={type} />) :
+      (<Image
+        preview={false}
+        height={52}
+        src={logoUrl}
+        alt={`${title} logo`}
+        onError={handleImageError}
+      />)
+      }


codrin-iftimie · 2024-04-22T08:41:18Z

src/components/Technologies/Technologies.tsx

-
-      if (category1 < category2) {
-        return -1;
+  const filteredTechCards = useMemo(() => {


this entire block is very hard to read.
My suggestion is to create a list of conditions that each card would have to accommodate.

const conditions = []; // .... if (selectedFilters.provider) { conditions.push(card => card.cloudTypes.includes(selectedFilters.provider)) } // ... // apply the filter only once cards.filter(card => { return conditions.every(condition => condition(card)) })

at the end you can use the fuse search to narrow things down even further and then group them by categories

I'm not sure if your approach here to have the data already grouped by categories is the best.
Flat structures are easier to manipulate.

codrin-iftimie · 2024-04-22T08:45:07Z

plugins/packs-integrations.js

+          versions: getAggregatedVersions(packContent.tags, packValues, packName, layer)
+        }
+      };
+      return packValueMap;


why not just return the object?

codrin-iftimie · 2024-04-22T08:45:45Z

plugins/packs-integrations.js

+      const packValueMap = {
+        fields: {


why nest all of the properties under one parent key?

codrin-iftimie · 2024-04-22T08:47:34Z

plugins/packs-integrations.js

+  counter += 1;
+  if (counter % 10 === 0) {
+    await setTimeout(2000);
+  }


do you need this?

codrin-iftimie · 2024-04-25T12:38:26Z

package.json

@@ -54,10 +55,15 @@
    "docusaurus-plugin-sass": "^0.2.5",
    "docusaurus-theme-openapi-docs": "^3.0.0-beta.3",
    "fuse.js": "^6.6.2",
+    "markdown-to-jsx": "^7.0.0",


is this used?

codrin-iftimie · 2024-04-25T12:39:52Z

plugins/packs-integrations.js

+      const layer = packMDValue.spec.layer === "addon" ? packMDValue.spec.addonType : packTypeNames[packMDValue.spec.layer];
+      const packValues = packContent.packValues;
+      return {
+        fields: {


do we need this wraping object?

codrin-iftimie · 2024-04-25T12:40:16Z

plugins/packs-integrations.js

+}
+
+function getAggregatedVersions(tags, packValues, packName, layer) {
+  const _sortedVersions = sortVersions(tags);


what does _ mean?

codrin-iftimie · 2024-04-25T12:42:04Z

src/components/PacksReadme/PacksReadme.module.scss

+      background-color: #1c202b;
+    }
+    padding: 10px;
+    .packdescfirstcol {


while some classes are camel case this one is hard to read.
please make it consistent.

codrin-iftimie · 2024-04-25T12:53:00Z

src/components/PacksReadme/PacksReadme.tsx

+    });
+  }
+
+  function renderTabs() {


a lot of duplicated code that is hard to read.
side note: refactoring if else if statements in ternary operations makes it even worse.
Take a step back. What are you trying to render here?
How do you simplify it, but how I make it scalable?
You have 3 cases:
tabs, readme only, author-provided content only.
I think you boxed yourself in with thinking only at these cases.
Lets think about the happy path. You'd render tabs if the API returned readme file and the author has provided content.

const tabs = [ readme && {label: "Readme", key: "readme", content: <Markdown children={readme} /> }, md && {label: "Additional Details" // .... etc } ].filter(Boolean)

From here you have edge cases. Only one or no tabs might render:
Single tab would not make sense so add a check for it

if (tabs.length === 1) { return <><h2>{tabs[0].label}</h2>{tabs[0].content}</> }

no tabs... no problem

if (tabs.length === 0) { return <div className={styles.emptyContent}> // etc }

Why?
Well it's scalable. If in the future you add another tab this will still render perfectly.

codrin-iftimie · 2024-04-25T13:25:28Z

src/components/PacksReadme/PacksReadme.tsx

+
+export default function PacksReadme() {
+  const [selectedVersion, setSelectedVersion] = useState<string>("");
+  const [md, setMd] = useState<ReactElement<any, any> | null>(null);


what does md mean?

please use a better name for this

codrin-iftimie · 2024-04-25T13:26:25Z

plugins/packs-integrations.js

+      let promises = new Array();
+      for (let i = 0; i < packDataArr.length; i++) {
+        const packData = packDataArr[i];
+        if (packData.spec.registries.length && isSelectedRegistry(packData.spec.registries, mappedRepos)) {
+          packMDMap[packData.spec.name] = packData;
+          const cloudType = packData.spec.cloudTypes.includes("all") ? "aws" : packData.spec.cloudTypes[0];
+          promises.push(`${packUrl}${packData.spec.name}/registries/${packData.spec.registries[0].uid}?cloudType=${cloudType}&layer=${packData.spec.layer}`);
+        }
+      }


refactor using map

codrin-iftimie · 2024-04-25T13:28:42Z

src/components/Technologies/PacksFilters.tsx

+          <Select
+            className={filterStyles.selectbox}
+            mode="multiple"
+            allowClear
+            placeholder="Search"
+            onChange={(item) => setSelectedSearchFilters({ category: item as string[] })}
+          >
+            {categories.map((category) => {
+              return (
+                <Select.Option key={category}>
+                  {packTypeNames[category as keyof typeof packTypeNames]}
+                </Select.Option>
+              );
+            })}
+          </Select>


create a component for this and reuse it bellow as well. remove unnecessary code duplication

codrin-iftimie · 2024-04-25T13:30:34Z

src/components/Technologies/Technologies.tsx

+        const additionalFilters = Array.isArray(_values) ? _values.reduce((acc, _value) => {
+          acc.push({
+            name: _value,
+            values: true
+          });
+          return acc;
+        }, [] as SearchFilter[]) : [];
+        _selectedFilters = _selectedFilters.concat(additionalFilters);


I don't understand why this has to be so different then the normal data structure.
Please try to use typescript to have a consistent data structure / interface for all filters

codrin-iftimie · 2024-04-25T13:33:59Z

src/components/Technologies/TechnologyCard.tsx

+  const handleClick = () => {
+    history.push({
+      pathname: `/integrations/packs/${name}`,
+    });
+  };


so basically a Link

what you created here makes the component less accessible.

codrin-iftimie · 2024-04-29T13:10:28Z

plugins/packs-integrations.js

+      const urls = packDataArr.map((packData) => {
+        packMDMap[packData.spec.name] = packData;
+        const cloudType = packData.spec.cloudTypes.includes("all") ? "aws" : packData.spec.cloudTypes[0];
+        return (`${packUrl}${packData.spec.name}/registries/${packData.spec.registries[0].uid}?cloudType=${cloudType}&layer=${packData.spec.layer}`);
+      })


this map should return functions that return promises directly.
we thunk the promises to avoid triggering them.

codrin-iftimie · 2024-04-29T13:11:30Z

src/components/PacksReadme/PacksReadme.tsx

      }
    };
    importComponent();
  }, []);

  const packData = useMemo(() => {
-    const { packs, repositories } = usePluginData("plugin-packs-integrations") as PacksIntegrationsPluginData;
-    const _packData = packs.filter((pack) => pack.fields.name === packName)[0];
+    const _packData = packs.find((pack) => pack.name === packName);


const pack =

codrin-iftimie · 2024-04-29T13:12:57Z

src/components/Technologies/CategorySelector/FilterSelect.tsx

+  key: string,
+  name: string
+}
+export default function FilterSelect({ selectMode, options, setSelectedSearchFilters }: FilterSelectProps) {


value and onChange as props

codrin-iftimie · 2024-04-30T08:43:46Z

src/services/api/index.js

-  return Promise.allSettled(
-    urls.map(url => limit(() => (api[method](url, payload)).catch(e => e)))
-  );
+function callRateLimitAPI(url, method, payload) {


callRateLimitAPI(delayedApiCall) { return limit(delayedApiCall) }

codrin-iftimie · 2024-04-30T08:45:00Z

plugins/packs-integrations.js

-  const tempPackArr = packDataArr.concat(response[0]?.value.data.items);
-  if (response[0]?.value.data.listmeta.continue) {
-    return fetchPackListItems("?limit=100&continue=" + response[0]?.value.data.listmeta.continue, tempPackArr, counter);
+  const response = await callRateLimitAPI("/v1/packs/search" + queryParams, 'post', payload);


await callRateLimitAPI(() => api.post(`/v1/packs/search${queryParams}, payload));

codrin-iftimie · 2024-04-30T08:45:42Z

plugins/packs-integrations.js

-      })
-      const results = await callRateLimitAPI(urls, 'get', {});
+        const url = `${packUrl}${packData.spec.name}/registries/${packData.spec.registries[0].uid}?cloudType=${cloudType}&layer=${packData.spec.layer}`;
+        return callRateLimitAPI(url, 'get', {});


callRateLimitAPI(() => api.get(url));

codrin-iftimie · 2024-04-30T08:49:12Z

src/components/Technologies/CategorySelector/FilterSelect.tsx

  return (
    <div className={filterStyles.wrapper}>
      <Select
        className={filterStyles.selectBox}
        mode={selectMode as "tags" | "multiple" | undefined}
        allowClear={true}
        placeholder="Search"
-        onChange={(item: any) => setSelectedSearchFilters(item)}
+        onChange={(item: any) => onChange(item)}


onChange={onChange}

codrin-iftimie · 2024-04-30T08:54:59Z

src/components/Technologies/CategorySelector/FilterSelect.tsx

@@ -3,27 +3,24 @@ import { Select } from "antd";
 import filterStyles from "./CategorySelector.module.scss";
 interface FilterSelectProps {
  selectMode?: string;


selectMode?: SelectProps["mode"]

import type {SelectProps} from "antd"

codrin-iftimie · 2024-04-30T08:58:15Z

src/components/Technologies/PacksFilters.tsx

+      <div className={styles.filterItems}>
+        <CustomLabel label="Type" />
+        <FilterSelect
+          selectMode="multiple"


value={selectedFilters.type}

…omments

PAC-1569-added codes for Packs API fetch

3abb7f2

nage1234 added the do-not-merge/work-in-progress Work in-progress. DO NOT MERGE label Mar 26, 2024

nage1234 requested a review from a team as a code owner March 26, 2024 18:13

nage1234 requested review from karl-cardenas-coding, lennessyy, caroldelwing and codrin-iftimie and removed request for a team March 26, 2024 18:13

karl-cardenas-coding marked this pull request as draft March 26, 2024 20:02

nage1234 added 5 commits March 31, 2024 23:21

PAC-1569-added few components for filtering, changed exisitng Technol…

5d5b209

…ogy card as per mockup

filters logic added for various pack filters

af3e9aa

added some more changes for routing, integrated the readme and custom…

87a48d8

…readme

fixed the filter issue, handled api failure, eslint error fixed

2a78b10

removed the unused component

c3c662c

codrin-iftimie reviewed Apr 12, 2024

View reviewed changes

nage1234 added 10 commits April 13, 2024 00:14

addressed the review comments

cb515d8

reverted the antd version

00d1514

search support added

e222d9d

addressed few review comments

93355b8

fixed broken search, moved the filtering logic to memo

369e37e

formatted the doc

71ad8d6

fixed dark mode Ant components, added empty content packs

0243674

cleanup, renamed pack details to Readme in one of the tab

1960a63

registry changes added as per the new mockup

ad196a8

added config file passed registries param

26ed76c

codrin-iftimie requested changes Apr 22, 2024

View reviewed changes

nage1234 added 2 commits April 24, 2024 13:24

addressed the review comments

a780ec9

a small fix - version wise readme

cc6d2b2

nage1234 added 2 commits April 24, 2024 17:14

removed unwanted file and correct variable name

9ed38ab

fixed a small issue with search

cfb6c05

codrin-iftimie requested changes Apr 25, 2024

View reviewed changes

Addressed the comments and restructured the codes

dca42b8

codrin-iftimie requested changes Apr 29, 2024

View reviewed changes

addressed few more review comments

954d6f5

codrin-iftimie reviewed Apr 30, 2024

View reviewed changes

changed API rate limit to function to return promise, addressed the c…

ee78203

…omments

nage1234 removed the do-not-merge/work-in-progress Work in-progress. DO NOT MERGE label May 3, 2024

codrin-iftimie approved these changes May 8, 2024

View reviewed changes

codrin-iftimie marked this pull request as ready for review May 8, 2024 07:20

codrin-iftimie merged commit 4f0a818 into PAC-938_nageswari May 8, 2024
9 of 12 checks passed

codrin-iftimie deleted the PAC-1569_nageswari branch May 8, 2024 07:21

		import PacksReadme from "./PacksReadme";

		export default PacksReadme;

PAC-1569-added codes for Packs API fetch #2484

PAC-1569-added codes for Packs API fetch #2484

Conversation

nage1234 commented Mar 26, 2024 • edited Loading

Describe the Change

Changed Pages

Jira Tickets

Backports

karl-cardenas-coding commented Mar 26, 2024

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codrin-iftimie Apr 22, 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

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

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

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

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

Choose a reason for hiding this comment

nage1234 commented Mar 26, 2024 •

edited

Loading

codrin-iftimie Apr 22, 2024 •

edited

Loading