Impl/cluster status #107
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,13 @@ import useForm, { dummyEvent } from '~/root/lib/client/hooks/use-form'; | |||||||||||||
| import Yup from '~/root/lib/server/helpers/yup'; | ||||||||||||||
| import { handleError } from '~/root/lib/utils/common'; | ||||||||||||||
| import Select from '~/components/atoms/select'; | ||||||||||||||
| import { listStatus, parseStatus } from '~/console/components/sync-status'; | ||||||||||||||
| import { IClusters } from '~/console/server/gql/queries/cluster-queries'; | ||||||||||||||
| import { | ||||||||||||||
| ExtractNodeType, | ||||||||||||||
| parseName, | ||||||||||||||
| parseNodes, | ||||||||||||||
| } from '../server/r-utils/common'; | ||||||||||||||
| import { ensureAccountClientSide } from '../server/utils/auth-utils'; | ||||||||||||||
| import { INewProjectFromAccountLoader } from '../routes/_a+/$a+/new-project'; | ||||||||||||||
| import { useConsoleApi } from '../server/gql/api-provider'; | ||||||||||||||
|
|
@@ -19,6 +25,14 @@ import MultiStepProgress, { | |||||||||||||
| import MultiStepProgressWrapper from '../components/multi-step-progress-wrapper'; | ||||||||||||||
| import { TitleBox } from '../components/raw-wrapper'; | ||||||||||||||
|
|
||||||||||||||
| const statusRender = (item: ExtractNodeType<IClusters>) => { | ||||||||||||||
| return listStatus({ | ||||||||||||||
| key: parseName(item), | ||||||||||||||
| item, | ||||||||||||||
| className: 'text-center', | ||||||||||||||
| }); | ||||||||||||||
| }; | ||||||||||||||
|
|
||||||||||||||
| const NewProject = () => { | ||||||||||||||
| const { clustersData } = useLoaderData<INewProjectFromAccountLoader>(); | ||||||||||||||
| const clusters = parseNodes(clustersData); | ||||||||||||||
|
|
@@ -125,22 +139,49 @@ const NewProject = () => { | |||||||||||||
| placeholder="Select a cluster" | ||||||||||||||
| value={values.clusterName} | ||||||||||||||
| options={async () => [ | ||||||||||||||
| ...clusters | ||||||||||||||
| .filter( | ||||||||||||||
|
There was a problem hiding this comment. suggestion (llm): It seems like the filtering logic for clusters based on their status ('deleting') has been introduced. I recommend adding unit tests to verify that clusters with a status of 'deleting' are indeed filtered out and not displayed as options. This is crucial to ensure the UI behaves as expected and only presents valid clusters to the user. There was a problem hiding this comment. suggestion (llm): Considering the addition of conditional rendering based on the cluster status ('ready', 'syncing', 'notready'), it would be beneficial to include integration or end-to-end tests. These tests should verify that the UI correctly displays the cluster information and status indicators according to the cluster's current state. This ensures that users receive accurate feedback on the cluster status. |
||||||||||||||
| (clster) => parseStatus({ item: clster }) !== 'deleting' | ||||||||||||||
| ) | ||||||||||||||
| .map((clster) => ({ | ||||||||||||||
| label: clster.displayName, | ||||||||||||||
| value: parseName(clster), | ||||||||||||||
| cluster: clster, | ||||||||||||||
| render: () => ( | ||||||||||||||
| <div> | ||||||||||||||
| {parseStatus({ item: clster }) === 'ready' ? ( | ||||||||||||||
| <div className="flex flex-col"> | ||||||||||||||
| <div>{clster.displayName}</div> | ||||||||||||||
| <div className="bodySm text-text-soft"> | ||||||||||||||
| {parseName(clster)} | ||||||||||||||
| </div> | ||||||||||||||
| </div> | ||||||||||||||
| ) : ( | ||||||||||||||
| // parseStatus({ item: clster }) === 'syncing' || | ||||||||||||||
| // parseStatus({ item: clster }) === 'notready' ? | ||||||||||||||
| <div className="flex text-text-disabled"> | ||||||||||||||
| <div className="flex-grow"> | ||||||||||||||
| <div className="flex flex-col"> | ||||||||||||||
| <div>{clster.displayName}</div> | ||||||||||||||
| <div className="bodySm"> | ||||||||||||||
| {parseName(clster)} | ||||||||||||||
| </div> | ||||||||||||||
| </div> | ||||||||||||||
| </div> | ||||||||||||||
| <div className="flex flex-grow-0 items-center"> | ||||||||||||||
| {statusRender( | ||||||||||||||
| clster as ExtractNodeType<IClusters> | ||||||||||||||
| ).render()} | ||||||||||||||
| </div> | ||||||||||||||
| </div> | ||||||||||||||
| )} | ||||||||||||||
| </div> | ||||||||||||||
| ), | ||||||||||||||
| })), | ||||||||||||||
|
Comment on lines
+143
to
+180
There was a problem hiding this comment. suggestion (llm): The filtering and mapping logic for clusters based on their status introduces a significant amount of complexity within the JSX. Consider abstracting this logic into a separate function or utilizing useMemo to avoid recalculating this on every render, especially since this operation depends on the clusters' data which might not change on every render. This could improve readability and performance. |
||||||||||||||
| ]} | ||||||||||||||
| onChange={(v) => { | ||||||||||||||
| if (parseStatus({ item: v.cluster }) === 'ready') | ||||||||||||||
| handleChange('clusterName')(dummyEvent(v.value)); | ||||||||||||||
|
Comment on lines
+183
to
+184
There was a problem hiding this comment. suggestion (rule): Sourcery has identified the following issue:
Suggested change
ExplanationIt is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing |
||||||||||||||
| }} | ||||||||||||||
| /> | ||||||||||||||
| <div className="flex flex-row justify-start"> | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (llm): The modification to the
IStatustype to include 'ready' is a significant change. It's important to ensure that all components consumingIStatusproperly handle the new 'ready' status. This might include rendering logic, styling, or even business logic changes. If not already covered, adding tests to verify these components react appropriately to the 'ready' status would mitigate potential issues.